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

Inline comments:
In `@internal/admin/dashboard/static/js/dashboard.js`:
- Around line 206-209: The guardrailsPageVisible() helper currently falls back
to false when executionPlanRuntimeBooleanFlag isn't available; change the
fallback to true so it matches the execution-plans module behavior — update
guardrailsPageVisible() to return
this.executionPlanRuntimeBooleanFlag('GUARDRAILS_ENABLED', true) when the
function exists (and otherwise default to true) so the top-level Guardrails nav
uses the same GUARDRAILS_ENABLED fallback as
internal/admin/dashboard/static/js/modules/execution-plans.js.

In `@internal/admin/dashboard/static/js/modules/guardrails.test.js`:
- Around line 40-53: Add an edge-case unit test for normalizeGuardrailConfig to
ensure it behaves defensively when the requested type isn't present in
guardrailTypes: in a new test (e.g., "normalizeGuardrailConfig returns empty
config for unknown type") create a module via createGuardrailsModule(), set
guardrailTypes to include only 'system_prompt', call
module.normalizeGuardrailConfig({ content: 'test' }, 'unknown_type'), and assert
the returned config equals the input ({ content: 'test' }) rather than
attempting to merge non-existent defaults.

In `@internal/admin/dashboard/templates/index.html`:
- Around line 309-312: The new guardrail filter input (the element with
x-model="guardrailFilter" and class "filter-input") lacks an accessible name;
add one by giving the input a unique id (e.g., guardrail-filter) and either (a)
add a visible or screen-reader-only <label for="guardrail-filter">Guardrail
filter</label> placed before the input, or (b) add an aria-label="Guardrail
filter" (or aria-labelledby pointing to a label element) so screen readers have
a stable, descriptive name for the control.
- Around line 263-350: The page hides all status alerts when authError is true,
causing expired sessions to show the empty-state instead of prompting
re-authentication; update the guardrails template to surface auth errors by
either rendering the shared auth-banner component on this page or removing the
"!authError" suppression on the alert blocks that use authError (those
referencing authError, guardrailError, guardrailNotice, guardrailsAvailable).
Specifically, ensure the auth-banner (or an equivalent alert) is rendered when
authError is truthy and that the existing alerts (x-show expressions around
guardrailsRuntimeEnabled/guardrailsAvailable/guardrailError/guardrailNotice) do
not blanket-suppress messages by checking authError; adjust the x-show
conditions so authError triggers the auth-banner instead of hiding all alerts.

In `@internal/admin/handler_guardrails_test.go`:
- Around line 17-59: The guardrailTestStore (and newGuardrailTestStore) in this
file duplicates test storage logic already present as testStore in
internal/guardrails/service_test.go; extract the shared in-memory store into a
single test helper (e.g., internal/guardrails/testutil_test.go) that exposes a
reusable constructor and methods matching List, Get, Upsert, Delete, Close, then
replace the local guardrailTestStore/newGuardrailTestStore usages with the
shared helper to avoid duplication and keep behavior consistent across tests.

In `@internal/admin/handler.go`:
- Around line 1025-1043: The handler currently does a racy check-then-delete
(activeWorkflowGuardrailReferences(...) then guardrailDefs.Delete(...)); move
the consistency boundary so the check-and-delete is atomic by implementing the
reference check inside the guardrail deletion logic (e.g., add an atomic
DeleteIfNotReferenced or make guardrailDefs.Delete perform the active-workflow
reference query within the same storage transaction/lock and return a clear
ErrGuardrailInUse/ErrReferenced if found), then simplify the handler to call
guardrailDefs.Delete and handle ErrGuardrailInUse (or existing guardrail write
errors) and continue to call refreshExecutionPlansAfterGuardrailChange on
successful delete. Ensure the unique symbols touched are
activeWorkflowGuardrailReferences, guardrailDefs.Delete, and
refreshExecutionPlansAfterGuardrailChange so callers and error handling are
updated accordingly.
- Around line 1175-1185: The code currently treats any view.Payload.Guardrails
entry as active; update the loop to skip checking guardrail refs for a view when
its feature flag is disabled by first checking view.Payload.Features.Guardrails
(or equivalent Features struct) and continuing if it's false. In other words,
inside the outer loop over views, add a guard like "if
!view.Payload.Features.Guardrails { continue }" before iterating
view.Payload.Guardrails so stale refs are ignored (consistent with
validateExecutionPlanGuardrails), then proceed to append view.ScopeDisplay and
sort as before.

In `@internal/app/app.go`:
- Around line 190-204: EnsureSeedDefinitions currently only seeds when store is
empty so config guardrail updates are ignored after first run; change startup
logic to persist the config-managed subset as an upsert instead of one-time
seed. Replace the call to guardrailResult.Service.EnsureSeedDefinitions(ctx,
seedGuardrails) with an upsert operation (e.g., add/implement
Service.UpsertDefinitions(ctx, seedGuardrails) or change EnsureSeedDefinitions
to perform upserts) so configGuardrailDefinitions are merged/updated into the DB
on every startup; additionally adjust the default-plan bootstrap logic that
reads Service.Names() (the code deriving the global workflow from runtime
config) to include names from the current configGuardrailDefinitions when
computing defaults so config-backed guardrails are not silently dropped once the
DB becomes authoritative.

In `@internal/guardrails/catalog.go`:
- Around line 1-8: The BuildPipeline method's second return value (the string)
is undocumented; update the Catalog interface comment to document
BuildPipeline's return values by describing the string as a configuration/hash
used for cache validation or change detection, and mention semantics for
empty/unchanged values so implementers of BuildPipeline (which accepts steps
[]StepReference and returns (*Pipeline, string, error)) know how to compute and
return the config hash alongside the Pipeline and error.

In `@internal/guardrails/definitions.go`:
- Around line 151-156: The normalization currently calls
effectiveSystemPromptMode(cfg.Mode) but does not reject unknown values, allowing
invalid modes to be persisted; update the normalization in the system_prompt
path (e.g., where systemPromptDefinitionConfig is created) to validate the mode
after calling effectiveSystemPromptMode and return a validation error for
unsupported modes instead of silently defaulting—ensure Service.Upsert paths
that call buildDefinition/NewSystemPromptGuardrail cannot persist unknown modes
by performing this check in the same normalization/validation function,
returning newValidationError("system_prompt mode is invalid", ...) when the mode
is not one of the allowed enums.

In `@internal/guardrails/factory.go`:
- Around line 103-109: createStore currently passes ctx only to
NewPostgreSQLStore causing SQLite and MongoDB initialization to ignore caller
cancellation; update createStore to pass the incoming ctx into NewSQLiteStore
and NewMongoDBStore calls (i.e., call NewSQLiteStore(ctx, db) and
NewMongoDBStore(ctx, db) or whatever constructor signature you create), then
modify the SQLite constructor/function (NewSQLiteStore) to use
ExecContext/QueryContext for schema/setup instead of Exec, and change the
MongoDB constructor (NewMongoDBStore) to derive its timeout/operations from the
provided ctx (do not use context.Background()) and avoid launching detached
goroutines; also ensure error returns are converted to the typed
core.GatewayError where appropriate so startup failures propagate synchronously.

In `@internal/guardrails/service.go`:
- Around line 205-235: StartBackgroundRefresh currently spawns a detached
goroutine rooted in context.Background() and swallows Refresh errors; change its
signature to accept a parent context (e.g., ctx context.Context) and stop using
context.Background(), run the ticker loop tied to that parent context, and
surface Refresh failures instead of dropping them — for example return a cancel
function plus an error channel or return typed core.GatewayError values from
StartBackgroundRefresh so callers can observe failures; ensure you call
s.Refresh with a derived context (e.g., context.WithTimeout(ctx,
30*time.Second)) and propagate any error from Refresh rather than ignoring it,
and keep the once/done semantics but driven by the provided context.

---

Outside diff comments:
In `@internal/executionplans/compiler.go`:
- Around line 67-74: In compileGuardrails, replace the fmt.Errorf call that
returns a plain error when c==nil || c.registry==nil with a typed
core.GatewayError (use core.NewProviderError) so callers receive a
core.GatewayError; specifically change the error return in function
compileGuardrails to return core.NewProviderError("guardrails are enabled but no
guardrail registry is configured") (keeping the same nil, "", error return
shape) and ensure imports/reference to core.NewProviderError are used instead of
fmt.Errorf.

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

Inline comments:
In `@internal/executionplans/compiler.go`:
- Around line 11-22: The compiler currently only nil-checks the catalog but does
not handle an empty catalog: when registry.Len() == 0 BuildPipeline() can return
a plain error instead of a core.GatewayError. Update NewCompilerWithFeatureCaps
(and any call-sites that build the pipeline) to check registry.Len() == 0 and
return a core.GatewayError (wrap the underlying error or create a new
core.GatewayError with a clear message) instead of returning fmt.Errorf; also
ensure any error returned from BuildPipeline() is wrapped with core.GatewayError
before propagating.

In `@internal/executionplans/service.go`:
- Around line 226-229: isManagedDefaultGlobal currently infers "managed" status
from user-editable Version fields (version.Name/version.Description), which lets
users accidentally create workflows that will be treated as managed by
EnsureDefaultGlobal; change this by adding an explicit persisted marker (e.g.,
Version.Managed bool or Version.Metadata["managed"]="true") or by enforcing
reservation of ManagedDefaultGlobalName/ManagedDefaultGlobalDescription at
creation time; update isManagedDefaultGlobal to check the new persisted marker
(not Name/Description) and adjust creation/validation paths so user-created
Version objects cannot use the reserved name/description.

In `@internal/guardrails/service.go`:
- Around line 75-90: The current UpsertDefinitions (and similar Upsert/Delete)
persist changes to s.store before rebuilding s.snapshot, leaving in-memory state
stale if Refresh fails; fix this by making the operation atomic: prepare and
normalize all changes first, build the new in-memory snapshot (or compute the
snapshot delta) and apply it only if persistence succeeds, or run persistence
inside a store transaction and only swap s.snapshot after a successful commit;
specifically update Service.UpsertDefinitions (and the corresponding
Upsert/Delete methods) to either (a) use a transactional store API so you call
s.store.Upsert/Delete inside a transaction and call s.Refresh/snapshot swap only
after commit, or (b) compute the new snapshot in-memory, attempt persistence,
and on success assign s.snapshot = newSnapshot (and on failure leave snapshot
unchanged and return the error).

---

Outside diff comments:
In `@internal/app/app.go`:
- Around line 562-584: The shutdown comments in the Close sequence are
misnumbered (two "8." comments). Update the inline step comments so they are
sequential (e.g., keep "// 7. Close batch store...", "// 8. Close usage
tracking..." and change "// 8. Close audit logging..." to "// 9. Close audit
logging...") near the code that calls a.batch.Close(), a.usage.Close(), and
a.audit.Close() so the shutdown steps read in order.

---

Duplicate comments:
In `@internal/admin/dashboard/templates/index.html`:
- Around line 350-352: The empty-state paragraph (class "empty-state" with the
x-show expression) should also check authentication by adding a "!authError"
clause to its condition so the message only shows when there is no
authentication error; update the x-show expression on the <p
class="empty-state"> element to include "&& !authError" alongside the existing
checks (filteredGuardrails.length === 0 && !guardrailsLoading &&
guardrailsAvailable && !guardrailError).

In `@internal/admin/handler.go`:
- Around line 1025-1037: The current flow does a race-prone check-then-delete by
calling activeWorkflowGuardrailReferences(...) before guardrailDefs.Delete;
instead move that in-use check into the delete write path so
guardrailDefs.Delete performs the reference lookup and returns a specific in-use
error (e.g. define and return guardrails.ErrInUse or similar) when active
workflows reference the guardrail; update the Delete implementation to call
activeWorkflowGuardrailReferences or equivalent within the same
transactional/write operation and have this handler only call
guardrailDefs.Delete and handle errors by mapping guardrails.ErrNotFound to
core.NewNotFoundError and the new guardrails.ErrInUse to
core.NewInvalidRequestError (using guardrailWriteError for other write errors).

In `@internal/guardrails/service.go`:
- Around line 196-240: Remove the internal background worker from Service:
delete or deprecate StartBackgroundRefresh so Service no longer starts a
detached goroutine or owns a ticker; instead keep only the synchronous
Refresh(ctx) method on Service and ensure Refresh returns the typed
core.GatewayError on failure. Move the scheduling/loop (ticker, context
cancellation, and error propagation) into the application/factory lifecycle code
where callers can synchronously call s.Refresh(ctx) on each tick and
handle/aggregate errors; do not spawn detached goroutines inside Service and
ensure callers own the retry/scheduling logic and error handling.

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

Inline comments:
In `@internal/admin/dashboard/static/css/dashboard.css`:
- Around line 2930-3011: The CSS is missing rules for the classes
ep-node-async-usage and ep-node-async-audit which the template
execution-plan-chart.html applies (classes on nodes: "ep-node-async
ep-node-async-usage" and "ep-node-async ep-node-async-audit"), causing styling
regression; add definitions for .ep-node-async-usage and .ep-node-async-audit
(and their child selectors .ep-node-icon, .ep-node-label, .ep-node-sub,
.ep-node-badge as needed) following the pattern used for
.ep-node-feature/.ep-node-success/.ep-node-error so Usage and Audit Log get
their distinct color mixes and border/background rules and restore the test
expectations in execution-plans-layout.test.js.

In `@internal/admin/dashboard/templates/execution-plan-chart.html`:
- Around line 65-66: The connector spacer currently uses x-show="{{.}}.showUsage
&& {{.}}.showAudit" so when executionPlanAuditChart() forces {{.}}.showAudit
true but {{.}}.showUsage is false the spacer disappears and the Audit node
shifts; change the condition on the spacer element with class ep-conn-async to
render whenever the audit node renders (e.g. x-show="{{.}}.showAudit" or the
equivalent OR logic), so the ep-conn ep-conn-async divider remains aligned with
the ep-node-async-audit node; update the template line for the spacer (and any
matching connector) accordingly to reference the same {{.}}.showAudit check used
by the ep-node-async-audit element.

In `@internal/admin/dashboard/templates/index.html`:
- Around line 292-297: The first warning banner is incorrectly bound to
guardrailsRuntimeEnabled() (which returns this.guardrailsAvailable) so it shows
when the runtime is unavailable; change its x-show to use the actual
feature-flag helper (the "GUARDRAILS_ENABLED" helper function used elsewhere,
e.g., guardrailsEnabled() or the helper that checks the feature flag) instead of
guardrailsRuntimeEnabled(), and keep the second banner bound to
guardrailsAvailable/authError; update the banner binding in index.html to
reference the real feature-flag helper (not
guardrailsRuntimeEnabled/guardrailsAvailable) so the message about
GUARDRAILS_ENABLED reflects the flag state only.
- Around line 384-387: The user_path field is rendered but never wired into the
module state or payload; update the guardrails module to persist it by adding
guardrailForm.user_path to the form defaults object, set guardrailForm.user_path
when hydrating the form for editing (e.g., in the function that populates the
form from an existing guardrail — assign guardrailForm.user_path =
guardrail.user_path || ''), and include user_path in the object sent by the
submit/save handler (e.g., add user_path: guardrailForm.user_path to the payload
built in the submitGuardrailForm/saveGuardrail function).

In `@internal/app/app.go`:
- Around line 711-732: The current configGuardrailDefinitions function always
marshals SystemPrompt.Mode and SystemPrompt.Content into the
guardrails.Definition.Config regardless of rule.Type; change
configGuardrailDefinitions to perform type-specific marshaling by switching on
rule.Type (and/or presence of rule.Config) and serializing the appropriate
fields for each type (e.g., for "system_prompt" marshal Mode/Content, for future
types marshal their required fields or forward existing rule.Config), returning
an error for unknown/unsupported rule.Type; update references to rule.Type,
rule.SystemPrompt, cfg.Rules, and guardrails.Definition to locate and implement
the type-aware switch and ensure Config contains the correct JSON payload per
type.

In `@internal/executionplans/compiler_test.go`:
- Around line 159-173: The test
TestCompilerCompile_WrapsBuildPipelineErrorsAsGatewayErrors currently creates an
empty registry so the compiler fails on the empty-catalog path instead of
exercising the missing-ref handling; populate the registry with at least one
dummy guardrail entry (not "missing") before calling
NewCompiler(registry).Compile so the compiler proceeds past the empty-registry
check and triggers the resolution of Ref: "missing" (e.g., register a minimal
guardrail/step into the registry variable created by guardrails.NewRegistry() so
the missing-ref wrapping path is exercised).

In `@internal/executionplans/service.go`:
- Around line 222-223: The seed path currently sets normalized.Managed = true
and calls s.store.Create(ctx, normalized) which bypasses
validateCreateCandidate, refreshMu, and the in-memory snapshot update (causing
Match() to potentially fail); change this to route the seed through the normal
creation flow by calling s.Create(ctx, normalized) (or the internal create entry
point used by s.Create) instead of s.store.Create so that
validateCreateCandidate is executed, refreshMu is updated, and the in-memory
snapshot is refreshed before returning.

In `@internal/guardrails/service.go`:
- Around line 223-236: The error path in Service.BuildPipeline currently returns
a plain fmt.Errorf when snapshot.registry is nil; change this to return a
client-facing core.GatewayError (e.g., use the project's constructor like
core.NewGatewayError or build a core.GatewayError instance) with the same
descriptive message ("guardrail catalog is not loaded"); update the return to
nil, "", core.GatewayError and add the core import if missing so the
BuildPipeline method and its callers return the proper typed gateway error
instead of fmt.Errorf.

In `@internal/guardrails/store_mongodb.go`:
- Around line 200-209: The function mongoConfigFromRaw can receive
empty/whitespace or JSON "null"/"{}" and downstream guardrail builders may not
handle these consistently; update mongoConfigFromRaw and the consumers to
normalize configs to a single representation (e.g., always return a non-nil
empty bson.M for empty/whitespace, for JSON "null" treat as empty, and ensure
any guardrail builder functions that consume the result accept an empty map
rather than nil). Specifically, in mongoConfigFromRaw handle the "null" JSON
case as empty, and audit the guardrail builder functions that read the config
(references to the guardrail builder constructors/usages) to accept and treat an
empty bson.M the same as no config so both "{}" and empty RawMessage behave
identically.

In `@internal/guardrails/store_sqlite.go`:
- Around line 71-77: Replace the direct sentinel comparison "err ==
sql.ErrNoRows" with errors.Is(err, sql.ErrNoRows) in the error handling block
(keep the same return behavior to return ErrNotFound when matched), and add the
"errors" package import if missing; target the block that checks err after
querying the store (the variables/functions involved are err, sql.ErrNoRows,
ErrNotFound and the returned definition).

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

Inline comments:
In `@internal/app/app.go`:
- Around line 752-773: The code builds an availability set using trimmed
guardrail names but then writes the untrimmed rule.Name into the payload.Ref;
update the loop that constructs payload.Guardrails so that it uses the trimmed
name (e.g., assign name := strings.TrimSpace(rule.Name) and use that name for
both the availability check and for GuardrailStep.Ref) when appending
executionplans.GuardrailStep to payload.Guardrails, ensuring consistency between
the filter and the written Ref.

In `@internal/guardrails/store_mongodb.go`:
- Around line 133-183: The UpsertMany method currently issues many UpdateOne
calls inside a transaction (using StartSession and WithTransaction) which is
inefficient for large batches; replace the per-definition UpdateOne loop with a
single BulkWrite call: build a slice of mongo.WriteModel (e.g., UpdateOneModel
or ReplaceOneModel) for each normalized definition using
mongoDefinitionIDFilter{ID: normalized.Name} as the filter and the same update
document (including $set and $setOnInsert), set each model to Upsert true, then
call s.collection.BulkWrite(sessionCtx, models,
options.BulkWrite().SetOrdered(true)) inside the existing session transaction;
keep normalization, mongoConfigFromRaw, created_at/updated_at handling and
preserve error wrapping around the BulkWrite call.

In `@internal/guardrails/store_sqlite.go`:
- Around line 197-203: The current isSQLiteDuplicateColumnError uses loose
substring checks which can false-positive on generic "already exists" messages;
update the function to normalize the error string (strings.ToLower(err.Error()))
and then return true only when it matches more specific SQLite patterns such as
"duplicate column", "duplicate column name", or an "already exists" phrase that
also contains "column" (e.g., strings.Contains(msg,"already exists") &&
strings.Contains(msg,"column")); reference the isSQLiteDuplicateColumnError
function and replace the two simple contains checks with these stricter combined
checks to reduce false positives while keeping pragmatic string-based detection.

---

Outside diff comments:
In `@internal/executionplans/service.go`:
- Around line 190-223: The current reconciliation reads active plans via
s.store.ListActive then later calls s.Create with modified normalized, which is
racy; replace this read-then-write with an atomic store-side check-and-set
(e.g., add and call a transactional method on the store such as
CreateIfNoActiveGlobal/CreateOrUpdateManagedDefault or
UpdateManagedDefaultWithCompare) that performs the recheck of active global
execution plans and only inserts/updates the managed default when the checked
state still permits it; ensure the method invoked from service.go (instead of
s.Create) accepts the normalized execution plan and the planHash/identity and
returns a clear error if a competing active global exists so callers (the loop
using s.store.ListActive, normalizeScope, isManagedDefaultGlobal, normalized) do
not overwrite newer active plans.

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

Inline comments:
In `@internal/admin/handler_executionplans_test.go`:
- Around line 101-114: The reconciliation check in EnsureManagedDefaultGlobal
currently short-circuits creation when a managed global version matches only
Name and Description; update the condition to also compare PlanHash so the fake
store only no-ops when version.Name == input.Name && version.Description ==
input.Description && version.PlanHash == input.PlanHash (using the existing
version.PlanHash and input.PlanHash fields), otherwise proceed to call
s.Create(ctx, input) to mirror real-store behavior.

In `@internal/app/app.go`:
- Around line 716-736: Trim and validate rule.Name and rule.Type before building
the guardrails.Definition: canonicalize rule.Type and rule.Name with
strings.TrimSpace (and e.g. strings.ToLower if you want canonical casing) and
reject empty/blank names or types with a clear error; use the normalized values
when creating rawConfig and when appending guardrails.Definition (set
Definition.Name and Definition.Type to the trimmed/normalized strings) so
downstream dispatch sees the same canonical type as workflow refs rather than
the original raw input.

In `@internal/executionplans/service.go`:
- Around line 221-223: The helper function isManagedDefaultGlobal (taking
Version) is unused and causing a lint failure; remove the entire function
declaration (the isManagedDefaultGlobal function) from the file and any related
unused imports or references to Version that only existed for this helper so the
linter stops flagging it.

In `@internal/guardrails/store_sqlite_test.go`:
- Around line 15-19: The tests open an in-memory SQLite DB with
sql.Open("sqlite", ":memory:") but rely on a single connection; make this
explicit by calling db.SetMaxOpenConns(1) immediately after the sql.Open(...)
success check (i.e., after the db variable is created and before any setup or
assertions) so the connection pool cannot create multiple connections and split
the in-memory database; apply the same change wherever sql.Open("sqlite",
":memory:") and db are used in this test file.

---

Outside diff comments:
In `@internal/app/app.go`:
- Around line 294-317: The server Config literal removed populating
server.Config.GuardrailsHash leaving handler.guardrailsHash empty; restore
wiring by assigning cfg.GuardrailsHash into the Config (set
Server.Config.GuardrailsHash = cfg.GuardrailsHash) so that
internal/server/http.go's handler.guardrailsHash continues to receive the
guardrails hash, or alternatively remove the stale handler.guardrailsHash usage
in internal/server/http.go alongside this change—update the code paths
referencing GuardrailsHash (server.Config.GuardrailsHash,
handler.guardrailsHash, cfg.GuardrailsHash) to be consistent.

---

Duplicate comments:
In `@internal/executionplans/service.go`:
- Around line 197-217: The current flow calls s.store.EnsureManagedDefaultGlobal
(which mutates storage) before validating the candidate plan; move the
compile/validation step so it happens prior to any store mutation. Concretely:
obtain the candidate (normalized, planHash) and call s.compiler.Compile on the
candidate version (or change EnsureManagedDefaultGlobal to provide a
non-mutating "preview" version) and verify compiled != nil and compiled.Policy
!= nil before invoking s.store.EnsureManagedDefaultGlobal; after successful
compile/validation call
s.storeActivatedCompiledLocked(compiledPlanForVersion(compiled, *version)) as
before, and keep the refreshLocked/snapshot logic unchanged for the nil-version
path. Ensure changes reference EnsureManagedDefaultGlobal, s.compiler.Compile,
compiledPlanForVersion, and s.storeActivatedCompiledLocked.