Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/admin/dashboard/templates/index.html`:
- Around line 384-388: The User Path input (x-model="guardrailForm.user_path")
only affects auxiliary rewrite guardrails, so either conditionally render it
only when guardrailForm.type === 'llm_based_altering' or update the hint to
state it only applies to rewrite-type guardrails; locate the input block around
the label with class "alias-form-field" and either wrap it in a conditional
(e.g., x-show/x-if using guardrailForm.type === 'llm_based_altering') or change
the small.alias-form-hint text to explicitly say "Only used for auxiliary
rewrite (llm_based_altering) guardrails; ignored for other guardrail types."
Ensure the visible behavior and aria-label remain unchanged.
- Around line 411-423: The checkbox collection currently rendered when
field.input === 'checkboxes' lacks a semantic group; replace the outer div with
a fieldset and render field.label inside a legend (keeping the same visual
styling) so screen readers announce a shared group label, and keep the template
x-for loop using the same input handling functions guardrailArrayFieldSelected
and toggleGuardrailArrayFieldValue; ensure the hint (field.help) remains
associated (e.g., aria-describedby on inputs or the fieldset) and verify the
rendered editor with accessibility tooling after the change.

In `@internal/admin/handler_guardrails_test.go`:
- Around line 296-316: Update the TestUpsertGuardrailRejectsSlashInName test to
assert the full OpenAI-compatible error envelope instead of only the HTTP 400
code: after calling h.UpsertGuardrail(c) and checking rec.Code, parse/inspect
rec.Body and reuse the same envelope helper used in the delete tests (the helper
used there to validate the `error` object shape) to assert the response contains
an `error` object with `type`, `message`, `param`, and `code` fields and
appropriate values for the invalid name case; this ensures the handler method
UpsertGuardrail returns the required contract.

In `@internal/auditlog/entry_capture.go`:
- Around line 203-218: In boundedAuditBody, the nested nil check inside the
len(body) <= MaxBodyCapture branch is redundant and unclear; explicitly handle a
nil body first (e.g., if body == nil { return []byte{}, false }) then proceed to
the length-based logic so callers get a consistent empty slice vs nil result,
and remove the unreachable inner nil check in boundedAuditBody to simplify the
control flow.

In `@internal/guardrails/llm_based_altering_test.go`:
- Around line 198-217: Add a second test that cancels the context during
processing to exercise mid-flight cancellation: create a cancellable ctx, pass a
mockChatCompletionExecutor into NewLLMBasedAlteringGuardrail whose chatFn blocks
on a channel or sync point (e.g., make chatFn wait on a started channel), then
start Process in a goroutine and trigger cancel() after the chatFn has begun
(unblock/notify started channel or sleep briefly then call cancel), and assert
Process returns a context cancellation error; reference
TestLLMBasedAltering_Process_PropagatesContextCancellation,
NewLLMBasedAlteringGuardrail, g.Process, mockChatCompletionExecutor, and chatFn
to locate where to implement the blocking/unblocking and cancellation logic.

In `@internal/guardrails/llm_based_altering.go`:
- Around line 22-228: The large hard-coded prompt DefaultLLMBasedAlteringPrompt
should be externalized; create an external template file (e.g.,
llm_based_altering_prompt.txt) and replace the long const with a var (e.g.,
DefaultLLMBasedAlteringPrompt string) that is populated at init by reading the
file (use go:embed or a config loader) with a fallback to the original string if
the file is missing; update any code that references
DefaultLLMBasedAlteringPrompt to use the var so deployments can override the
prompt without recompiling.
- Around line 438-440: The loop over texts currently spawns detached goroutines
via group.Go() and uses i := i; text := text to avoid captures; instead, process
each text synchronously within the current context (respecting ctx) and remove
the inner reassignments; also change the function's return type from error to
core.GatewayError and convert any returned errors into that typed value before
returning; locate usages of group.Go, the loop over texts, and the function
signature in llm_based_altering.go to implement these changes.

In `@internal/guardrails/provider.go`:
- Around line 1423-1432: The cloneStringAnyMap function currently performs a
shallow copy and will share nested references (maps, slices, etc.) between src
and the clone; update the code by adding a clear comment above cloneStringAnyMap
describing this shallow-copy semantics and that nested structures are not
deep-copied (and mark intended usage assumptions: values are primitives or fully
replaced), and if any caller (e.g., response input patching code) might mutate
nested structures instead of replacing them, replace cloneStringAnyMap with a
deep-cloning implementation or add a helper deepCloneStringAnyMap to avoid
accidental shared mutations.

In `@internal/responsecache/responsecache.go`:
- Line 177: The code creates a new Echo instance for every internal request (c
:= echo.New().NewContext(req, rec)), causing allocations; modify the middleware
to accept or cache a shared Echo instance in the middleware struct (e.g., set a
field in NewResponseCacheMiddleware) and use that instance to create contexts in
HandleInternalRequest (replace echo.New() with the cached/shared echo instance),
or add an optional constructor parameter to pass an existing *echo.Echo so the
context creation reuses the same router/middleware chain.
- Around line 168-170: The code currently silently replaces a nil ctx with
context.Background(), which masks caller bugs; instead remove the fallback and
fail fast: in the function(s) where you see the snippet that sets "if ctx == nil
{ ctx = context.Background() }" (any method in the responsecache package that
accepts a ctx parameter), change it to validate ctx != nil and return an error
(or propagate an error type) immediately when ctx is nil; update the function
signature to return an error if it doesn't already (or document panics if you
prefer), and update all callers to pass a non-nil context or handle the returned
error so request-scoped values are preserved.

In `@internal/server/translated_inference_service.go`:
- Around line 183-199: The current withCacheRequestContext uses s.guardrailsHash
as a fallback and only writes a guardrails hash into the context when
guardrailsHash != "", which lets an incoming context keep a stale caller hash
when plan.Policy is present but plan.GuardrailsHash() returns "". Update
withCacheRequestContext so that when a plan exists and plan.Policy != nil you
always call core.WithGuardrailsHash(ctx, plan.GuardrailsHash()) to explicitly
overwrite/clear any inherited hash (even if empty); otherwise, if no plan
policy, fall back to applying s.guardrailsHash only when non-empty (using
core.WithGuardrailsHash). This ensures guardrail-origin requests can clear the
caller’s cache namespace.
- Around line 164-167: Remove the redundant conditional around setting the
request context: always call c.SetRequest(c.Request().WithContext(ctx)) after
obtaining ctx from withCacheRequestContext(c.Request().Context(), plan). The
withCacheRequestContext function always returns a wrapped context (it uses
core.WithExecutionPlan and may add core.WithGuardrailsHash), so drop the if ctx
!= c.Request().Context() check and unconditionally update the request context
via c.SetRequest; this matches the pattern used elsewhere in the file.

---

Outside diff comments:
In `@internal/guardrails/factory.go`:
- Around line 60-90: The constructors accept variadic ChatCompletionExecutor but
the implementation (NewService) only uses executors[0], so callers can silently
pass extra executors; add a fast-fail check to reject multiple executors. In
newResult (and optionally in New and NewWithSharedStorage) check if
len(executors) > 1 and return a clear error like "only one
ChatCompletionExecutor is supported" before any work is done; this enforces the
single-executor contract and avoids silently dropping extra executors (refer to
the functions New, NewWithSharedStorage, newResult and the
ChatCompletionExecutor parameter and NewService usage).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/auditlog/entry_capture.go`:
- Around line 96-101: CaptureInternalJSONExchange currently calls
internalJSONAuditRequest and internalJSONAuditResponse unconditionally which
causes headers-only auditing to still attempt body marshaling and lose synthetic
header metadata when bodies are nil/unserializable; update
internalJSONAuditRequest and internalJSONAuditResponse to always construct and
return header/request metadata (including synthetic trace/user-path/X-Request-ID
via requestIDForEntry) and add a logBodies boolean parameter (pass cfg.LogBodies
from CaptureInternalJSONExchange) so the helpers only perform JSON marshaling of
bodies when logBodies is true, then call PopulateRequestData and
PopulateResponseData with the metadata and marshaled body only when available;
apply the same change to the other call sites referenced (the other
CaptureInternalJSONExchange usages).

In `@internal/guardrails/llm_based_altering_test.go`:
- Around line 154-174: The test TestLLMBasedAltering_Process_SkipsPrefix can
return a false positive because Process is fail-open on executor errors; update
the mockChatCompletionExecutor chatFn used in NewLLMBasedAlteringGuardrail to
explicitly fail the test if invoked (e.g., call t.Fatalf or set a called flag
and assert it is false after Process), so you verify the executor is never
called when SkipContentPrefix matches; reference
mockChatCompletionExecutor.chatFn, NewLLMBasedAlteringGuardrail and Process to
locate where to inject the fail-fast assertion.

In `@internal/guardrails/provider.go`:
- Around line 923-932: The function_call_output branch currently flattens
item.Output to text via stringifyResponsesValue and then writes that text back
into the message, losing original types; change the assignment so we preserve
typed values: call stringifyResponsesValue(item.Output) as before, then try to
json.Unmarshal the resulting content string into a temporary interface{}; if
unmarshalling succeeds use that decoded value for preserved.Output (or
Message.Content/Output field as appropriate), otherwise fall back to the
original item.Output (so arrays/objects/numbers/null remain typed) — update the
logic around stringifyResponsesValue, item.Output, preserved.Output (and the
case "function_call_output" handling) to implement this.

In `@internal/responsecache/responsecache.go`:
- Around line 242-252: The mapping in internalCacheType is fragile because it
checks hardcoded strings ("HIT (exact)" / "HIT (semantic)") that are composed
elsewhere in writeCachedResponse; define canonical header-value constants (e.g.,
CacheHeaderExact, CacheHeaderSemantic) next to CacheTypeExact and
CacheTypeSemantic in semantic.go and update both writeCachedResponse to use
those constants when writing the header and internalCacheType to compare against
them (instead of the current literal strings) so there is a single source of
truth for the header formats.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/auditlog/entry_capture.go`:
- Around line 117-145: internalJSONAuditRequest currently only attaches a
RequestSnapshot to the context when logBodies is true and json.Marshal succeeds,
which can leave an ingress snapshot in place and leak outer request data; change
internalJSONAuditRequest to always create and attach a synthetic
core.RequestSnapshot whenever logBodies is true (use core.NewRequestSnapshot and
core.WithRequestSnapshot), setting capturedBody to the marshaled bytes on
success and to an empty or nil captured body with bodyTooBig/body-not-captured
flags set appropriately on nil body or marshal failure; update the function to
compute capturedBody/bodyTooBig deterministically and attach the snapshot to
reqCtx regardless of marshal outcome, and add a regression test that pre-seeds
an ingress snapshot and exercises an unmarshalable internal body to assert the
internal snapshot is used.

In `@internal/guardrails/llm_based_altering_test.go`:
- Around line 300-347: Replace raw int32 counters with sync/atomic helper types
inside TestLLMBasedAltering_Process_LimitsConcurrentRewrites: change inFlight,
maxInFlight, and requestsSeen to atomic.Int32, update increments/decrements to
inFlight.Add(1)/inFlight.Add(-1) and requestsSeen.Add(1), replace
atomic.LoadInt32(&maxInFlight)/atomic.CompareAndSwapInt32(&maxInFlight,
previous, current) in the mockChatCompletionExecutor.chatFn CAS loop with
maxInFlight.Load() and maxInFlight.CompareAndSwap(previous, current) (or use
maxInFlight.Swap/Store as appropriate), and update the final checks to use
requestsSeen.Load() and maxInFlight.Load(); keep all logic identical but use the
atomic.Int32 methods for cleaner code.

In `@internal/guardrails/provider.go`:
- Around line 873-900: In coerceResponsesInputElements, remove the intermediate
[]any allocation in the case []map[string]any branch and instead directly
convert each map to a core.ResponsesInputElement: allocate elements :=
make([]core.ResponsesInputElement, len(typed)) and for each map in the input
marshal the map to JSON and unmarshal into elements[i], returning elements, nil;
update the case []map[string]any in function coerceResponsesInputElements
accordingly and preserve error handling that returns core.NewInvalidRequestError
on marshal/unmarshal failures.

In `@internal/responsecache/responsecache.go`:
- Around line 179-185: The current fallback that silently calls echo.New() when
m.echo is nil should be removed because it masks constructor bugs; instead,
detect when m != nil && m.echo == nil and log a warning or error (using the
package logger e.g., response cache's logger or the standard log) indicating the
unexpected nil m.echo (include context like the ResponseCache instance ID or
constructor used), and then fail-fast by returning an error or nil to the caller
rather than creating a new Echo instance; update the calling code or the
constructor (e.g., NewResponseCache / any New* functions that initialize m.echo)
to ensure m.echo is always initialized so this branch never executes.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/auditlog/entry_capture_test.go`:
- Around line 57-84: The test only covers the marshal-failure/no-body path for
CaptureInternalJSONExchange; add two more subtests in
TestCaptureInternalJSONExchange_PreservesHeadersWhenBodyMarshalFails (or new
tests) that exercise the responseErr branch and the oversized/truncation branch:
1) call CaptureInternalJSONExchange with a non-nil responseErr value and assert
that entry.Data.ResponseError (or the field your helper sets for synthesized
errors) is populated and headers are still preserved; 2) simulate an oversized
payload/truncation by using the helper path that marks payloads as truncated (or
set the config/inputs that trigger the oversized marker) and assert that
RequestBody/ResponseBody are replaced with the truncation marker while headers
remain intact. Reference CaptureInternalJSONExchange, Config, and the
responseErr/synthesized-error/truncation logic when adding assertions so the new
tests cover both branches.

In `@internal/guardrails/provider.go`:
- Around line 1017-1019: Remove the no-op assignment "_ = typed" in the switch's
default branch and instead ignore the value at the switch declaration or case
pattern so the compiler doesn't require silencing; specifically locate the
switch that declares the variable "typed" and change the case that falls to
default to use the blank identifier (or bind to _ in the case pattern) rather
than assigning "_ = typed" in the default branch, then delete the "_ = typed"
line.

In `@internal/responsecache/responsecache.go`:
- Around line 180-188: The nil-receiver branch currently creates a new Echo
instance which hides a call-site bug; change HandleInternalRequest to fail fast
when m == nil by logging an error and returning a provider/internal server error
instead of allocating echo.New(); keep the existing behavior for the case where
m != nil but m.echo == nil (log and return provider error as done now). Update
the switch handling in HandleInternalRequest (the method on the response cache
type) to detect m == nil first and return an explicit error with a clear log
message rather than creating a new Echo.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/app/app.go`:
- Around line 411-423: The failure branches after calling
guardrailResult.Service.SetExecutor and executionPlanResult.Service.Refresh
forget to close the response cache middleware (rcm) opened earlier by
responsecache.NewResponseCacheMiddleware; update both error cleanup sequences to
include rcm.Close() in the errors.Join(...) calls (and any subsequent closeErr
handling) so rcm is always closed when SetExecutor or Refresh fails.

In `@internal/auditlog/entry_capture_test.go`:
- Around line 105-121: The test currently only checks X-Request-ID and
error.message; extend it to assert the full synthesized-error contract by
verifying that entry.Data.RequestHeaders and entry.Data.ResponseHeaders include
core.UserPathHeader (using http.CanonicalHeaderKey lookup) and that
entry.Data.ResponseBody (the synthesized error envelope) contains an "error"
object with all OpenAI-compatible fields "type", "message", "param", and "code"
(with expected values or non-nil presence as appropriate) so the test fails if
any of those fields or headers are omitted.

In `@internal/executionplans/service_test.go`:
- Around line 625-628: Replace the inline map[string]any payload used in the
test (currently passed to mustMarshalJSON) with a small strongly-typed struct
(e.g., ModelConfig or TestModelConfig) that has fields matching the shape: Model
string and Roles []string; instantiate that struct with Model: "gpt-4o-mini" and
Roles: []string{"user"} and pass it to mustMarshalJSON instead of the map to
ensure compile-time safety and easier evolution of the payload shape.

In `@internal/responsecache/responsecache.go`:
- Around line 246-254: internalCacheType currently only matches exact header
values and returns empty for the "HIT (<cacheType>)" shape emitted by
stream_cache; update internalCacheType to parse that generic form: trim the
header, if it equals CacheHeaderExact or CacheHeaderSemantic return the
corresponding CacheType; else if it matches the pattern "HIT (<inner>)" (use
strings.HasPrefix/HasSuffix to detect and extract the inner value), trim the
inner value and run the same switch on it to return
CacheTypeExact/CacheTypeSemantic; otherwise return "" — make changes inside
internalCacheType to keep all other call sites unchanged.

---

Duplicate comments:
In `@internal/auditlog/entry_capture_test.go`:
- Around line 145-162: The test currently only checks ResponseBody != nil and
ResponseBodyTooBigToHandle, which doesn’t prove the oversized response was
truncated or that the user-path header was preserved; update the assertions
around entry.Data.ResponseBody and entry.Data.ResponseHeaders to (a) assert the
ResponseBody shows the expected truncation shape/marker or is shorter than the
original oversized payload (so the full payload wasn’t retained) and (b) assert
entry.Data.ResponseHeaders[http.CanonicalHeaderKey(core.UserPathHeader)] equals
the preserved user-path value; keep the existing checks for
ResponseBodyTooBigToHandle and RequestBodyTooBigToHandle and use the
entry.Data.* symbols to locate the checks.

In `@internal/guardrails/provider.go`:
- Around line 1055-1060: In the "function_call_output" branch the code assigns
preserved.Output = modified.Content, which collapses typed JSON outputs into
strings on the typed-envelope path; change this to restore the original typed
value (assign preserved.Output from the field that holds the reconstructed/typed
output rather than modified.Content) so that object/array/number/null values
survive no-op guardrail passes—update the same pattern at the other occurrence
referenced (the similar assignment at the later branch around lines 1156-1157)
and ensure you use the typed output field (the value used by the map-backed
patch path) when setting preserved.Output.

In `@internal/responsecache/responsecache.go`:
- Around line 179-199: The second nil-receiver branch is unreachable and
triggers SA5011; remove the later "if m == nil { err = next(c) } else { err =
m.HandleRequest(...)" block and always call m.HandleRequest(c, body, func()
error { return next(c) }) after you create the Echo context (since the function
already returned when m == nil), using the previously assigned e and c; this
ensures HandleInternalRequest, m.HandleRequest, next and e.NewContext are used
consistently and eliminates the dead m==nil path.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/auditlog/entry_capture.go`:
- Around line 169-180: The current switch in CaptureInternalJSONExchange
prioritizes bodyValue over responseErr causing partial successful audit entries;
update the logic in CaptureInternalJSONExchange to check responseErr first and,
if non-nil, marshal the error envelope (using the same errors.As gatewayErr
handling and core.NewProviderError(...).ToJSON() fallback) into body, otherwise
fall back to marshalling bodyValue; keep the existing default return behavior
unchanged.

In `@internal/executionplans/service_test.go`:
- Around line 769-785: The test helper assertPipelineRewrite currently only
checks the rewritten message content; add an assertion to validate the message
role wasn't mutated. After calling pipeline.Process and before checking content,
assert that msgs[0].Role equals the original role ("user") (or change the helper
signature to accept a wantRole and assert msgs[0].Role == wantRole) so the test
fails if the rewrite changes role semantics; reference assertPipelineRewrite and
the guardrails.Message produced by pipeline.Process when adding this additional
check.

In `@internal/guardrails/llm_based_altering.go`:
- Around line 484-495: The current acceptance of the first choice in
llm_based_altering.go can let truncated or non-terminal auxiliary completions
(e.g., finish_reason != "stop") replace the original text; update the validation
to reject choices whose FinishReason is not the terminal value (e.g., not
"stop") by checking resp.Choices[0].FinishReason (and ideally any other selected
choice) and returning an error (so the caller falls back to fail-open) before
extracting content or returning unwrapAlteredText(content); keep the existing
checks for nil resp, tool calls, and empty content.

In `@internal/responsecache/responsecache.go`:
- Around line 244-248: The switch on headerValue contains dead comparisons to
CacheHeaderExact and CacheHeaderSemantic because headerValue has already been
stripped of the "HIT (...)" wrapper; remove the CacheHeaderExact and
CacheHeaderSemantic cases and only compare against CacheTypeExact and
CacheTypeSemantic (referencing the headerValue variable and the constants
CacheTypeExact/CacheTypeSemantic) so the switch is simplified to the effective,
reachable branches.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/admin/dashboard/templates/execution-plan-chart.html`:
- Line 65: The connector for async audit flows (the div with class "ep-conn
ep-conn-async") is currently hidden by x-show="{{.}}.showUsage &&
{{.}}.showAudit", which makes audit-only flows invisible when showAudit is true
but showUsage is false; change the x-show to rely on audit visibility (e.g.,
x-show="{{.}}.showAudit") or to show when either flag is set (e.g.,
x-show="{{.}}.showUsage || {{.}}.showAudit") so the Audit node remains
connected; update the attribute on the element with class ep-conn-async and
ensure this aligns with the flags set in execution-plans.js (showUsage and
showAudit).

In `@internal/auditlog/entry_capture_test.go`:
- Around line 48-50: The test inconsistently accesses the Traceparent header
directly; change the lookup to use http.CanonicalHeaderKey("Traceparent") like
the other assertions so it reads
entry.Data.RequestHeaders[http.CanonicalHeaderKey("Traceparent")] and keep the
same t.Fatal message; this keeps the pattern consistent with the other checks
that use http.CanonicalHeaderKey.

In `@internal/responsecache/responsecache.go`:
- Around line 221-237: internalRequestHeaders currently copies all headers from
snapshot.GetHeaders(), which can leak sensitive credentials (Authorization,
X-Api-Key, Cookie, etc.); change it to filter using an explicit allowlist (e.g.,
Content-Type, Accept, User-Agent, standard tracing headers like
Traceparent/Request-Id and X-Request-ID) instead of copying everything from
snapshot.GetHeaders(), and preserve the existing behavior that sets Content-Type
and X-Request-ID when absent; update the logic in internalRequestHeaders to
iterate snapshot.GetHeaders() but only Add values for keys present in the
allowlist and ignore/omit sensitive names such as Authorization, Cookie, and
X-Api-Key.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/responsecache/responsecache.go`:
- Around line 23-36: The internalRequestHeaderAllowlist currently omits the
standard "Cache-Control" header, so HandleInternalRequest can forward requests
that then bypass the same cache-bypass behavior used by HandleRequest; add
http.CanonicalHeaderKey("Cache-Control") to internalRequestHeaderAllowlist in
internal/responsecache/responsecache.go so internal requests preserve the
standard Cache-Control semantics, and update the new allowlist unit test (the
test that asserts forwarded headers for internal requests) to include a case
asserting "Cache-Control" is allowed and preserved.
- Around line 205-208: m.HandleRequest's returned error must be normalized to a
core.GatewayError before returning: after calling err := m.HandleRequest(...),
if err != nil use errors.As to check whether err is already a *core.GatewayError
and if so return it unchanged, otherwise wrap it in a new core.GatewayError
(preserving the original message/cause) and return that; add errors to the
imports. Ensure you reference m.HandleRequest and core.GatewayError in the
change.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@internal/responsecache/responsecache.go`:
- Around line 208-214: The code currently returns core.NewProviderError with
err.Error(), leaking internal cache/backend details; change the return to use a
stable public message (e.g. "internal server error") instead of err.Error(), and
log the original err server-side before returning (use the existing logger in
this package or the surrounding context). Update the branch that checks
errors.As(err, &core.GatewayError) to keep returning gatewayErr unchanged, but
for the generic path replace core.NewProviderError("",
http.StatusInternalServerError, err.Error(), err) with core.NewProviderError("",
http.StatusInternalServerError, "internal server error", err) and add a
server-side log of err; ensure
TestHandleInternalRequest_NormalizesNonGatewayErrors still uses errors.Is(...)
and does not depend on the public message.