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

Inline comments:
In `@internal/admin/dashboard/static/css/dashboard.css`:
- Line 2487: The CSS declaration uses the incorrect casing for the keyword
value: change the stroke property value in the rule that currently reads
"stroke: currentColor;" to use the lowercase keyword "currentcolor" to satisfy
the stylelint `value-keyword-case` rule; locate the CSS rule containing the
stroke property (the line with stroke: currentColor) and update the value casing
accordingly.

In `@internal/admin/dashboard/templates/index.html`:
- Line 638: The filter input bound to x-model="executionPlanFilter" (class
"filter-input") and the guardrail inputs (the two inputs added on lines
referenced) rely solely on placeholders and therefore lose accessible names once
populated; fix this by adding stable accessible names—either add visible <label
for="..."> elements tied to unique id attributes on each input or add clear
aria-label attributes (e.g., aria-label="Filter workflows by scope, name, hash,
or guardrail" for the executionPlanFilter input and aria-labels like "Guardrail
name" and "Guardrail value" for the guardrail inputs) so screen readers always
have a persistent name; ensure ids used in labels are unique per repeated
guardrail row (generate row-specific ids) or use aria-labelledby pointing to a
stable label element for each row.

In `@internal/admin/handler_executionplans_test.go`:
- Around line 384-390: Replace the loose map unmarshalling in the test
(currently decoding into body map[string]map[string]any in
handler_executionplans_test.go) with a typed struct that models the
OpenAI-compatible error envelope (fields: type, message, param *string or
interface{} nullable, code *string or interface{} nullable) and assert against
those typed fields (e.g., check errEnvelope.Error.Message equals "unknown
guardrail ref: missing-guardrail" and that param/code are nil or expected).
Update the same pattern in the other failing assertions referenced (around the
blocks at the other occurrences you noted) so tests validate the full error
shape rather than a generic map.

In `@internal/admin/handler.go`:
- Around line 681-699: The request handler currently skips validating
scope_provider/scope_model against the server-side registry before creating
plans; update the handler (the block that binds createExecutionPlanRequest and
calls h.plans.Create) to validate req.ScopeProvider and req.ScopeModel against
h.registry (use whatever registry lookup method exists on h.registry) and return
a clear invalid-request error via handleError if the provider or model is
unknown, before calling h.validateExecutionPlanGuardrails and h.plans.Create;
ensure the check references the same request fields (req.ScopeProvider,
req.ScopeModel) and short-circuits with an error response when validation fails
to prevent creating plans with dead/unknown scopes.

In `@internal/executionplans/service.go`:
- Around line 176-182: Concurrent writes can cause an older Refresh() to
overwrite a newer persisted snapshot because Create()/Deactivate() call
s.store.Create(...) (and s.store.Deactivate(...)) then call s.Refresh() with no
ordering guard; serialize the mutation+snapshot rebuild by adding a mutex or
generation/CAS around the write+refresh sequence so snapshots only advance:
acquire the lock (or read current generation), perform s.store.Create(...) (or
s.store.Deactivate(...)), then call s.Refresh() while still holding the lock (or
verifying/incrementing the generation/CAS) and release the lock only after
successful refresh; apply the same protection to the other write path mentioned
(the block around lines 218-225) and ensure StartBackgroundRefresh() respects
the same lock/generation so background refreshes cannot interleave and roll back
newer snapshots.

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

Inline comments:
In `@internal/admin/dashboard/static/js/modules/execution-plans.js`:
- Around line 201-217: The form hydration manually rebuilds features and
guardrails instead of using the shared normalizers; replace the inline parsing
of plan.plan_payload.features and the map over this.planGuardrails(plan) when
setting this.executionPlanForm.features and this.executionPlanForm.guardrails by
calling the existing helpers executionPlanSourceFeatures(plan) and
executionPlanSourceGuardrails(plan) (or the appropriate exported names) so the
form uses the same normalization path as the editor and renderer; keep the other
fields (scope_provider, scope_model, name, description) as-is and populate
features/guardrails from those helper return values.
- Around line 385-396: Add a reentrancy guard to submitExecutionPlanForm: at the
top of the method check the boolean executionPlanSubmitting and immediately
return if true to short-circuit duplicate clicks, then set
executionPlanSubmitting = true only when proceeding; ensure the create
POST/async flow in submitExecutionPlanForm is wrapped with try/catch/finally (or
equivalent) so executionPlanSubmitting is cleared in the finally block on both
success and error to avoid getting stuck in a submitting state.
- Around line 255-265: The guardrail mapping in buildExecutionPlanRequest
currently coerces blank/whitespace step inputs to 0 via Number(), letting them
pass validateExecutionPlanRequest; change the mapping so you first read the raw
step value (e.g., const raw = step && step.step), treat empty string / null /
whitespace as invalid (set step to NaN or undefined) and only convert to a
Number when raw is non-empty and numeric (use a strict parse check like
String(raw).trim() !== '' && Number.isFinite(Number(raw))). This ensures
validateExecutionPlanRequest (which checks Number.isInteger(step.step)) will
reject blank steps; update buildExecutionPlanRequest (and optionally
validateExecutionPlanRequest) to rely on that and fail submissions with blank
guardrail steps.

In `@internal/executionplans/service.go`:
- Around line 396-399: The validation currently only checks the error from
s.compiler.Compile(version) but must also reject empty compiled plans; update
validateCreateCandidate (the block calling s.compiler.Compile) to capture the
compiled result (e.g., compiled, err := s.compiler.Compile(version)), then if
err != nil return newValidationError(err.Error(), err), and additionally if
compiled == nil || compiled.Policy == nil return newValidationError("compiled
plan is empty or missing policy", nil) (or similar descriptive message) so empty
compiled results are rejected during preflight validation.

---

Duplicate comments:
In `@internal/admin/dashboard/templates/index.html`:
- Around line 730-733: The repeated guardrail inputs inside the template x-for
(executionPlanForm.guardrails) need unique accessible names per row; update the
input attributes bound to x-model="step.ref" and x-model.number="step.step" to
include row-specific labels (for example set :aria-label="'Guardrail reference '
+ (index + 1)" and :aria-label="'Guardrail step ' + (index + 1)" or generate a
unique id per row and reference it via aria-labelledby on each input) so screen
readers can distinguish each guardrail's reference and step by index or by a
corresponding hidden label element.

In `@internal/admin/handler_executionplans_test.go`:
- Around line 212-245: The tests currently only assert HTTP 503 but must also
decode and assert the OpenAI-compatible error envelope; after calling
ListExecutionPlans, CreateExecutionPlan, and DeactivateExecutionPlan, unmarshal
the response body into the existing executionPlanErrorEnvelope type and assert
its fields (Type, Message, Param == nil, Code == "feature_unavailable" or
expected code) to ensure the public error shape is correct; update the
assertions for listRec, create rec, and deactivate rec to validate
envelope.type/envelope.message/envelope.param/envelope.code alongside the status
checks.

In `@internal/executionplans/service.go`:
- Around line 183-193: The post-commit refresh is being invoked with the
incoming request context (see s.store.Create and the call to
s.refreshLocked(ctx)), which can be canceled after the write succeeds and cause
the API to return an error while leaving s.current stale; change the refresh
calls to use a non-request context (e.g., context.Background() or a
background-derived timeout) instead of the caller-provided ctx when calling
refreshLocked (and similarly for the other occurrence around the other write
path), ensuring you still hold s.refreshMu while invoking s.refreshLocked but do
not propagate the request-scoped context to the refresh.

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

Inline comments:
In `@internal/admin/dashboard/static/js/modules/execution-plans.js`:
- Around line 320-325: In validateExecutionPlanRequest(), the guardrail
validation currently only checks Number.isInteger(step.step) which allows
negative values like -1; update the guardrails loop (the code that iterates the
guardrails array and checks step.ref and step.step) to also require step.step >=
0 by changing the validation to reject non-integers or negative integers and
return an error message such as "Each guardrail step must use a non-negative
integer step number." Ensure the check references the guardrails loop and the
step.step field so negative step numbers are rejected.

In `@internal/admin/dashboard/templates/index.html`:
- Around line 870-876: Add unique, plan-specific accessible names to the two
action buttons so screen readers and keyboard users can distinguish them: update
the Deactivate button (where executionPlanDeactivatingID,
canDeactivateExecutionPlan(plan), and deactivateExecutionPlan(plan) are used) to
include an aria-label that incorporates plan identifier or name (e.g.,
"Deactivate workflow {plan.name || plan.id}") and update the Edit button
(openExecutionPlanCreate(plan)) to include an aria-label like "Edit workflow
{plan.name || plan.id}"; keep the existing title/disabled logic and dynamic text
but ensure the aria-labels are set per-plan.

---

Duplicate comments:
In `@internal/admin/dashboard/static/js/modules/execution-plans.js`:
- Around line 525-539: The deactivateExecutionPlan function sets
this.executionPlanDeactivatingID but never uses it to prevent reentrant
requests; add a guard at the top of deactivateExecutionPlan(plan) that returns
early if this.executionPlanDeactivatingID is truthy (or equals the same planID)
to short-circuit duplicate clicks, then set this.executionPlanDeactivatingID =
planID before sending the POST and clear it (set to '') in both the success and
error paths (use a finally block or ensure both then/catch clear it) so only one
/deactivate request per plan runs at a time.

In `@internal/executionplans/service.go`:
- Around line 186-194: The current code writes to the persistent store via
s.store.Create (and similarly in the deactivate path) and then calls
s.refreshLocked; if refreshLocked fails we must not surface a failure because
the store commit already succeeded. Fix by either (preferred) acquiring
refreshMu and update s.current incrementally with the committed change (e.g.,
insert the new execution plan/version or apply the deactivation) under the same
lock before returning, so the in-memory snapshot matches the persisted state, or
(alternative) perform a compensating rollback to the store (undo the
Create/deactivate) before returning an error. Locate the flows around
s.store.Create / s.store.Deactivate and s.refreshLocked and ensure updates to
s.current occur while holding refreshMu (or roll back the store write) so
callers don’t see a false failure after persistence succeeded.

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

Inline comments:
In `@internal/admin/dashboard/static/js/modules/execution-plans.js`:
- Around line 74-98: The option builders executionPlanProviderOptions and
executionPlanModelOptions (and the similar blocks around lines 189-219 and
294-308) currently only include providers/models present in this.models, which
drops a plan's stored scope if that provider/model is retired; modify these
functions to also include the plan's current scoped value when building the
options: accept (or read) the preserved provider and model IDs from the hydrated
plan (e.g., a parameter like currentProvider/currentModel or
this.hydratedScope), add those strings into the options Set before returning,
and update the form validator to allow the preserved provider/model values
(i.e., treat them as valid if they match the hydrated plan scope) so cloned
workflows remain valid even when the provider/model is no longer in this.models.

In `@internal/executionplans/service.go`:
- Around line 255-259: ListViews currently aborts the entire listing when
service.viewForVersion(version) returns an error; change ListViews to treat
compile/validation failures as row-level errors instead of returning nil for the
whole call: iterate versions, call s.viewForVersion(version), and on error
capture the persisted row metadata (the version/ID and any stored fields) and
attach a per-row compile error field or status rather than returning the error;
return the full slice of rows where successful rows contain their view and
failed rows contain their persisted metadata plus the compile error string so
the caller can display/inspect bad workflows (apply same change for the similar
loop around the 415-421 block).
- Around line 179-193: ListViews currently aborts the entire listing when
compiling an active execution plan fails; change ListViews to skip any versions
that fail compilation instead of returning early: when iterating versions call
the compilation step (the code path that uses compiledPlanForVersion/compile
logic) inside a try/if err != nil block, append the error to a warnings slice or
log it, and continue to the next version so the rest of the plans are returned;
alternatively return the successful results plus a non-fatal aggregated warning
(e.g., as part of the response or metadata) rather than failing the whole
request.

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

Inline comments:
In `@internal/admin/dashboard/static/js/modules/execution-plans.js`:
- Around line 379-403: fetchExecutionPlans currently awaits fetch with no
timeout so a hung request can leave executionPlansLoading true; wrap the fetch
in an AbortController with a timeout (e.g. setTimeout -> controller.abort()) and
pass signal to fetch (use this.headers() as before), handle the abort exception
in the existing catch to set executionPlans = [] and this.executionPlanError to
a descriptive message, clear the timeout on success/failure, and ensure finally
still sets this.executionPlansLoading = false; reference fetchExecutionPlans,
this.headers(), and this.handleFetchResponse when making the changes.

In `@internal/executionplans/service_test.go`:
- Around line 27-35: The variable name `copy` in staticStore.Get and
concurrentStore.Get shadows Go's built-in copy function; rename that local
variable (e.g., to `vCopy`, `versionCopy`, or `ver`) in both methods so you
return &versionCopy instead of &copy, updating both occurrences in
staticStore.Get and concurrentStore.Get to avoid shadowing the built-in.

In `@internal/executionplans/service.go`:
- Around line 498-507: Remove the dead function scopeSpecificity: delete the
unused function declaration for scopeSpecificity (which returns int based on
scopeType) since viewScopeSpecificity is the actual implementation used by
ListViews; ensure no other references to scopeSpecificity remain and run
tests/static analysis to confirm removal is safe.