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

Inline comments:
In `@internal/app/app.go`:
- Around line 189-192: The hardcoded refresh interval time.Minute used when
calling executionplans.NewWithSharedStorage and executionplans.New should be
made configurable: add a duration field (e.g., ExecutionPlanRefreshInterval
time.Duration) to the application config struct (and load a sane default like 1m
if unspecified), parse it from env/flags/config loader, then replace the literal
time.Minute in the calls to executionplans.NewWithSharedStorage(ctx,
sharedExecutionPlanStorage, executionPlanCompiler, time.Minute) and
executionplans.New(ctx, appCfg, executionPlanCompiler, time.Minute) with the new
config value so operators can tune the execution plan refresh interval.

In `@internal/auditlog/auditlog_test.go`:
- Around line 636-641: The test currently uses core.DefaultExecutionFeatures()
which makes the Audit flag implicit; update the test setup that constructs the
core.ExecutionPlan (the call to core.WithExecutionPlan with a
core.ResolvedExecutionPolicy) to use an explicit Features value with Audit set
to true (instead of core.DefaultExecutionFeatures()) so the test always
exercises version-ID persistence regardless of package defaults; locate the
instantiation of core.ExecutionPlan / core.ResolvedExecutionPolicy in the
auditlog_test and replace the Features argument with an explicit features struct
(with Audit: true).

In `@internal/auditlog/reader_sqlite.go`:
- Around line 98-99: The rows.Scan call that reads execution_plan_version_id
into e.ExecutionPlanVersionID must handle NULLs; change the scan target for
execution_plan_version_id to a nullable type (e.g., sql.NullString or *string)
in the scan call and then set e.ExecutionPlanVersionID accordingly (assign empty
string or nil as your struct expects) both where rows.Scan is used (the scan at
rows.Scan(&e.ID, &ts, ... &e.ExecutionPlanVersionID, ... ) and the later scan
around line ~305); update any variable names (e.g., executionPlanVersionNS or
execPlanPtr) and the post-scan assignment so NULL rows do not cause scan
conversion errors.

In `@internal/executionplans/service.go`:
- Around line 217-221: The context cancel created by refreshCtx, refreshCancel
:= context.WithTimeout(ctx, 30*time.Second) should be deferred immediately after
creation to guarantee cleanup even if s.Refresh(refreshCtx) panics; move the
call to refreshCancel() into a defer (i.e., defer refreshCancel() right after
the WithTimeout line) and remove the later explicit refreshCancel() invocation
so resources are always released regardless of panic in s.Refresh.

In `@internal/executionplans/store_mongodb.go`:
- Around line 106-120: Wrap the deactivation (UpdateMany) and insertion
(InsertOne) in a MongoDB multi-document transaction so both succeed or both are
rolled back: start a session (client.StartSession or
s.collection.Database().Client().StartSession), use session.WithTransaction (or
explicit StartTransaction/CommitTransaction/AbortTransaction) and run the
UpdateMany and InsertOne calls inside that transaction using the session
context; handle mongo.ErrNoDocuments and unique-index errors by
aborting/returning the wrapped error from the transaction, and ensure the same
session context is passed to s.collection.UpdateMany and s.collection.InsertOne
to avoid leaving the scope without an active plan when input.Activate is true
(the unique index on (scope_key, active=true) must be respected).

In `@internal/executionplans/store_postgresql.go`:
- Around line 98-110: The version allocator using `SELECT COALESCE(MAX(version),
0) + 1` is racy for concurrent Create calls; after starting the transaction
(`tx, err := s.pool.Begin(ctx)`) take a per-scope lock before computing
`nextVersion` (e.g. call `pg_advisory_xact_lock(hashtext(scopeKey))` or
`pg_try_advisory_xact_lock` using `scopeKey`’s hash) and then run the `SELECT
... FROM execution_plan_versions` query to guarantee serialization, or
alternatively add retry logic around the insert to catch the unique `(scope_key,
version)` violation and retry the whole transaction a few times; ensure the
lock/acquire or retry is applied in the same code path that computes
`nextVersion` and performs the insert so `scopeKey`, `nextVersion`, `tx`, and
`execution_plan_versions` are the referenced symbols.

In `@internal/executionplans/store_sqlite.go`:
- Around line 94-106: The allocation of nextVersion using tx.QueryRowContext
with `SELECT COALESCE(MAX(version), 0) + 1 FROM execution_plan_versions WHERE
scope_key = ?` is racy and can cause unique constraint violations; change the
transaction start to an immediate/serialized transaction (e.g., use a BEGIN
IMMEDIATE equivalent instead of s.db.BeginTx) so the MAX+1 allocation is
serialized, or implement retry logic around the insert (catch unique constraint
on (scope_key, version) from the insert and retry the read/insert loop). Update
the logic in store_sqlite.go around the BeginTx/tx creation and the
QueryRowContext + insert path (execution_plan_versions / scope_key allocation)
to use one of these approaches so concurrent creates cannot collide.

In `@internal/executionplans/types.go`:
- Around line 64-80: normalizeScope currently allows provider/model to contain
':' which causes scopeKey to collide (both scopeKey and normalizeScope are
involved); fix by rejecting any ':' in Scope.Provider and Scope.Model during
normalizeScope (trimmed values) and return a validation error (e.g., "scope
fields cannot contain ':'") so keys remain unambiguous; alternatively, if you
prefer to preserve colons, update scopeKey to encode each component (e.g.,
URL-encode or base64) before concatenation so provider/model are
distinguished—implement one of these fixes and ensure tests for scopeKey
uniqueness are updated accordingly.

In `@internal/responsecache/simple.go`:
- Around line 196-203: The nil-plan branch was inverted: change the early check
so that when plan == nil the function returns true (skip caching) instead of
false; update the block containing plan == nil so it returns true, keeping the
subsequent checks using plan.CacheEnabled(), plan.Mode,
core.ExecutionModeTranslated and plan.Resolution unchanged to prevent
cross-provider cache contamination.

In `@internal/server/execution_policy.go`:
- Around line 18-20: The call to resolver.Match can return arbitrary
storage/cache errors that must be normalized before returning; update the error
handling around resolver.Match (where policy, err := resolver.Match(selector) is
called) to check whether err is already a core.GatewayError and return it
unchanged if so, otherwise wrap the error in a core.GatewayError using the
appropriate client-facing category (e.g., "provider_error") and include a short
contextual message about policy resolution so downstream code always receives a
GatewayError.

In `@internal/server/native_batch_service.go`:
- Around line 155-179: storeExecutionPlanForBatch currently propagates errors
returned from applyExecutionPolicy (and ultimately resolver.Match) directly to
callers; update the error handling here so any non-core.GatewayError returned by
applyExecutionPolicy is wrapped into a core.GatewayError before being returned.
Concretely, after calling applyExecutionPolicy(plan, s.executionPolicyResolver,
selector) in storeExecutionPlanForBatch, check the returned error and if it is
not already a core.GatewayError, wrap or convert it to one (using your project's
core.GatewayError constructor/wrapper) and return that wrapped error so clients
always receive a core.GatewayError from resolver.Match()/applyExecutionPolicy.

In `@internal/server/native_batch_support.go`:
- Around line 89-129: determineBatchExecutionSelector currently repeats the
BatchItemRequestedModelSelector/Normalize/ResolveModel loop that another helper
uses to pick the batch provider, causing an extra O(n) pass; refactor so
provider selection and execution-plan selection are derived in a single pass
(either by changing determineBatchExecutionSelector to accept/return the
provider choice or by creating a shared helper that walks req.Requests once and
returns both the resolved common model (or "" if mixed/empty) and the
providerType/selection info). Keep references to
BatchItemRequestedModelSelector, Normalize, and resolver.ResolveModel in that
single walk, and return the combined result (core.ExecutionPlanSelector plus the
provider decision) instead of performing the same resolution twice.

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

Inline comments:
In `@internal/executionplans/store_postgresql.go`:
- Around line 113-128: In createVersion on PostgreSQLStore, acquire a
transaction-scoped advisory lock on the scope before computing nextVersion to
serialize allocation for the same scopeKey: call tx.Exec with "SELECT
pg_advisory_xact_lock(hashtext($1))" (passing scopeKey) immediately after
beginning the tx and before the tx.QueryRow that selects
COALESCE(MAX(version),0)+1 from execution_plan_versions; this ensures concurrent
callers for the same scopeKey wait instead of racing and retrying
(alternatively, implement a SELECT ... FOR UPDATE on a dedicated per-scope row
if you maintain a scope table).

In `@internal/responsecache/middleware_test.go`:
- Around line 419-449: The test
TestSimpleCacheMiddleware_BypassesCacheWithoutExecutionPlan hard-codes that a
nil execution plan must bypass caching; update it to not assume that policy. In
the test that constructs mw via NewResponseCacheMiddlewareWithStore and
registers the handler, remove the assertions that X-Cache must be empty and that
callCount == 2; instead assert that the middleware's behavior is consistent
across repeated requests (i.e., the X-Cache header value is the same for both
responses and callCount is either 1 if cached or 2 if not), or parameterize the
test to explicitly exercise both expected runtime contracts; this keeps the test
from codifying a specific nil-plan policy while still validating determinism of
Middleware().

In `@internal/server/execution_policy.go`:
- Around line 39-48: The cloneCurrentExecutionPlan function performs a shallow
copy so cloned.Policy still points to the same *ResolvedExecutionPolicy as the
original; update cloneCurrentExecutionPlan to deep-copy the Policy pointer
(e.g., if existing.Policy != nil then copy the struct value and set
cloned.Policy = &copiedPolicy) so callers cannot accidentally mutate shared
policy state, or alternatively add a clear comment in cloneCurrentExecutionPlan
documenting the invariant that callers (like applyExecutionPolicy in
native_batch_service.go) must always replace the Policy pointer rather than
mutate it.

---

Outside diff comments:
In `@internal/app/app.go`:
- Around line 458-472: Shutdown currently closes a.aliases before
a.executionPlans which can tear down shared storage used by
executionplans.NewWithSharedStorage; reorder the shutdown sequence so that
a.executionPlans is closed before a.aliases (i.e., move the executionPlans close
block above the aliases close block in Shutdown()), ensuring any errors are
still logged/appended to errs exactly as before.

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

Inline comments:
In `@internal/auditlog/middleware.go`:
- Around line 47-49: The early auditEnabledForContext(c.Request().Context())
check in the middleware rarely short-circuits because the execution plan is
typically not yet in the context (planning middleware runs later), so update the
middleware.go code around the if (!auditEnabledForContext...) branch to add a
concise comment stating this check is primarily for the uncommon case where an
upstream component (e.g., semantic enrichers) has already populated a plan with
Audit=false; reference the symbols auditEnabledForContext and next(c) and note
that the real gating occurs after next(c) (lines around the later check that
inspects the plan).

In `@tests/integration/setup_test.go`:
- Around line 144-153: The switch in resetIntegrationStorage silently ignores
unknown dbType values; add a default case to the switch in function
resetIntegrationStorage that calls t.Fatalf (or t.Fatalf-like test failure) with
a clear message including the unknown dbType (e.g., "unknown db type: %s") so
tests fail fast and indicate that storage reset was skipped.