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

Inline comments:
In `@cli/internal/config/state.go`:
- Around line 148-164: IsValidImageTag currently validates characters but not
length; add a check enforcing Docker's 128-character tag limit by returning
false if len(tag) > 128 (place this near the existing len(tag) == 0 check at the
top of IsValidImageTag) so tags exceeding 128 chars fail validation before
character checks run.

In `@web/src/components/setup/SetupAdmin.vue`:
- Around line 127-135: Add an aria-label to the password visibility toggle
buttons in SetupAdmin.vue so screen readers receive a clear accessible name: for
the button that toggles showPassword (the button with `@click`="showPassword =
!showPassword" and :title="showPassword ? 'Hide password' : 'Show password'")
add a bound aria-label that mirrors the title (e.g., :aria-label="showPassword ?
'Hide password' : 'Show password'"); do the same for the confirm-password toggle
(the corresponding button around lines 152-159) so both use aria-label rather
than relying solely on title.

In `@web/src/components/setup/SetupAgent.vue`:
- Around line 125-128: The startEditing() function currently clears savedAgent
(savedAgent.value = null) which prevents form fields from being pre-populated;
change startEditing() to preserve savedAgent and instead set editing.value =
true while leaving savedAgent.value intact (or copy savedAgent into a separate
mutable form model such as agentForm or initialize reactive form fields from
savedAgent when editing starts), and update the form initialization logic to
read from savedAgent (or agentForm) so fields are populated when entering edit
mode (refer to startEditing(), savedAgent.value, editing.value and the
component's form model where SetupCompany implements similar behavior).

In `@web/src/components/setup/SetupCompany.vue`:
- Around line 88-127: The summary can show stale/fallback values because
createdResult is a component-local ref and lost on refresh; update
SetupCompany.vue so that on mounted (or when showSummary is true) if
setup.isStepComplete('company') you fetch the company details from the backend
(or read them from a Pinia store) and populate createdResult before
rendering/allowing the Next action; ensure the emit call in the Next button uses
the fetched createdResult.companyName (not the fallback) and consider persisting
createdResult into the Pinia store when initially created so navigation/refresh
retains the real values.
- Around line 42-44: startEditing() currently only sets editing.value = true so
form fields stay empty; update startEditing() to populate companyName,
companyDescription, and selectedTemplate from createdResult (guarding for
null/undefined) before flipping editing.value to true so the form is pre-filled.
In other words, inside startEditing() read createdResult (or
createdResult.value) and assign its name/description/template into the reactive
variables companyName, companyDescription, selectedTemplate (use default
fallbacks), then set editing.value = true.

In `@web/src/components/setup/SetupProvider.vue`:
- Around line 165-183: The handler handleChangeProvider sets
changingProvider.value = true when starting a provider change but never resets
it, so showSummary (which checks !changingProvider.value) never re-enables;
update the provider-completion flow to set changingProvider.value = false when
the new provider setup completes successfully (e.g., after store.deleteProvider
resolves and after any subsequent successful provider creation/configuration
path), ensuring you clear changingProvider in the success branch alongside
resetting createdProviderName, selectedPreset, testPassed, testResult, and error
so the summary can show again.

In `@web/src/views/SetupPage.vue`:
- Around line 69-74: The click handler currently permits navigating to any index
less than setup.currentStep which can let users jump to earlier but incomplete
steps; update handleStepClick to only allow navigation when the target step is
marked complete or is the current step itself by replacing the condition with a
check that calls setup.setStep(index, steps.value.length) only if
isStepDone(index) || index === setup.currentStep; keep references to
handleStepClick, isStepDone, setup.currentStep, setup.setStep and
steps.value.length so the change is easy to locate.

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

Inline comments:
In `@cli/cmd/update_cleanup.go`:
- Around line 86-99: collectCurrentImageIDs currently treats an empty idOut as
success and returns a partial currentIDs map, causing findOldImages to
misclassify current images as old; update collectCurrentImageIDs to treat an
empty or whitespace idOut as an error for each svc: after running docker.RunCmd
(in collectCurrentImageIDs) check if strings.TrimSpace(idOut) == "" and if so
return an error (e.g., fmt.Errorf("no current image ID found for %s", svc)) so
callers never receive a partial currentIDs set and findOldImages won't mark
current images for removal.

In `@cli/cmd/update_health.go`:
- Around line 57-69: The Docker probe failure from docker.Detect should not be
treated as a definitive "dirty install" issue; remove adding docker.Detect
errors to the issues slice and only report missing images when
detectMissingImages actually returns missing artifacts. In practice, keep the
short healthCtx and call docker.Detect(healthCtx), but if docker.Detect returns
an error do not append fmt.Sprintf("Docker not available: %v", dockerErr) to
issues — instead treat docker as unavailable (allowing updateContainerImages to
handle it) and only append the formatted missing-image message when
detectMissingImages(healthCtx, info, state) returns a non-empty slice; ensure
the logic around state.ImageTag, detectMissingImages, and promptHealthRecover
remains intact so non-interactive runs are only aborted for confirmed missing
artifacts.

In `@cli/cmd/update.go`:
- Around line 54-59: The recovery path still short-circuits recreate/pull logic
when the stored CLI tag equals the current tag; modify the post-recovery flow so
that when checkInstallationHealth indicates a recover-and-continue scenario you
set a "force recovery" flag and pass it into refreshCompose and
updateContainerImages; change refreshCompose to treat a missing compose.yml as
requiring recreation when force is true (instead of "nothing to refresh") and
change updateContainerImages to skip the early-return on matching stored tag
when force is true (force a re-pull/recreate of images); update calling sites
and signatures for refreshCompose and updateContainerImages accordingly so
recovery triggers recreation even if the version did not change.
- Around line 61-63: Call migrateSettingsKey before running the installation
health check so the migration that populates SettingsKey runs prior to
detectInstallationIssues; specifically, move the migrateSettingsKey(cmd, &state)
invocation to execute before detectInstallationIssues(...) (the health-check
call that currently flags an empty SettingsKey) so older installs with no
SettingsKey are repaired before prompts/aborts.

In `@src/synthorg/providers/discovery.py`:
- Around line 328-339: The code currently always emits the
PROVIDER_MODELS_DISCOVERED event even when all raw entries were malformed;
change the final logger.info call in the discovery function to be conditional:
after computing raw_key, raw_entries, skipped and calling _log_skip_counts, only
call logger.info(PROVIDER_MODELS_DISCOVERED, preset=preset_name,
model_count=len(models)) when not (skipped > 0 and len(models) == 0) (i.e.,
suppress the success event if entries were skipped and no models were parsed) so
that an all_entries_malformed outcome does not also emit a success metric.

In `@src/synthorg/providers/management/service.py`:
- Around line 449-455: The current logic sets trust=True whenever preset_hint
resolves, which re-enables SSRF for arbitrary stored config.base_url; change
discover_models_for_provider to only set trust=True when the stored
config.base_url exactly matches the preset's known safe URLs (the preset's
default_base_url or any entry in candidate_urls) — retrieve the preset object
(the one used by _resolve_discovery_trust or similar lookup), compare
config.base_url against preset.default_base_url and preset.candidate_urls, and
only pass trust=True to discover_models when there is a match; otherwise keep
trust=False and let _validate_discovery_url run normally. Apply the same guarded
check for the other occurrence noted (around the 468-486 block).

In `@src/synthorg/providers/probing.py`:
- Around line 195-209: Change probe_preset_urls so it no longer accepts
arbitrary candidate_urls: have it take only preset_name and internally call
get_preset(preset_name) (from the presets module) to resolve the trusted
candidate URLs (and raise/return an error if the preset is missing), or
alternatively make the function controller-private by renaming it to
_probe_preset_urls and keep callers that already resolve presets; update all
callers of probe_preset_urls to pass only preset_name (or use the new private
name) so the SSRF-trust boundary is enforced in code rather than by docstring.

---

Outside diff comments:
In `@web/src/components/setup/SetupCompany.vue`:
- Around line 73-79: When the component mounts, if the backend reports
isStepComplete('company') true but createdResult is null, call the setup method
that fetches the persisted company (e.g., add a call like
setup.fetchCompanyDetails or setup.getCompany / setup.fetchCreatedResult) inside
the existing onMounted block after setup.fetchTemplates; handle errors the same
way you do for setup.fetchTemplates (surface via setup.error -> error.value).
Also apply the same logic in the other mount/initialization code path around the
showSummary logic (the region referenced by lines 91-130) so the summary is
populated after a page refresh when createdResult is lost.

---

Duplicate comments:
In `@web/src/components/setup/SetupAgent.vue`:
- Around line 125-132: startEditing currently restores agentName, selectedRole
and selectedModel from savedAgent but omits selectedPersonality and
selectedProvider, which causes isValid to fail and the model list to be
unfiltered; update startEditing to also set selectedPersonality.value =
savedAgent.value.personality_preset (or personality field returned by the API)
and selectedProvider.value = savedAgent.value.model_provider when savedAgent
exists, and ensure the API/SetupAgentResponse returns personality_preset if not
present so the personality can be fully restored; keep setting editing.value =
true at the end.

In `@web/src/components/setup/SetupCompany.vue`:
- Around line 42-47: startEditing currently only restores companyName from
createdResult, causing companyDescription and selectedTemplate to be lost when
entering edit mode; update startEditing to also read and set
companyDescription.value and selectedTemplate.value from createdResult.value
(e.g., createdResult.value.companyDescription and the template
identifier/property used in the component) before setting editing.value = true
so all form fields are pre-populated when editing.

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

Inline comments:
In `@cli/cmd/update_health.go`:
- Around line 51-56: The code currently treats a missing state.JWTSecret as
recoverable but only offers the existing recovery path which regenerates
images/compose and calls migrateSettingsKey() (which only repairs
state.SettingsKey), leaving the installation unusable; fix by either (preferred)
synthesizing and persisting a new JWT secret before offering recovery (e.g.,
generate a secure secret, assign to state.JWTSecret and persist the state/save
function used elsewhere) so subsequent recovery works, or skip the in-place
"recover" option for the JWT secret and route this case to the interactive
synthorg init flow instead; also ensure similar handling is applied where
JWTSecret is checked in the later block (lines ~86-107) so you don't present an
inappropriate recover option.

In `@cli/cmd/update.go`:
- Around line 445-450: Change restartIfRunning to signal whether the new
revision is actually running: update its signature to return (bool, error) (true
= restarted and passed health checks, false = skipped or failed). Inside
restartIfRunning ensure you return false when the user skips restart or when the
post-restart health check fails; return true only on a successful
restart+healthy result. Update callers (the call near restartIfRunning(...) then
cleanupOldImages(...) and the other similar block around lines 652-674) to check
the bool and only call cleanupOldImages(cmd, info, updatedState) when the bool
is true; propagate errors unchanged.

In `@src/synthorg/providers/probing.py`:
- Around line 47-54: Extract the _redact_url function into a single shared
utility function in synthorg.providers.utils (keep the same signature and
behavior: parsing, hostname/port handling, and replacing query with
"<redacted>"), then replace the local definitions in probing.py and discovery.py
with an import from synthorg.providers.utils; remove the duplicated
implementations, update imports (use from synthorg.providers.utils import
_redact_url or a public name), and run tests/linters to ensure no references
break.

---

Duplicate comments:
In `@cli/cmd/update.go`:
- Around line 60-74: checkInstallationHealth's recovered boolean is currently
used only to clear state.ImageTag, which doesn't force refreshCompose to rebuild
a missing compose.yml; instead add a recovery/force parameter to refreshCompose
(e.g., refreshCompose(ctx, force bool)) and pass the recovered flag into it
wherever refreshCompose is called (including the update flow around
updateContainerImages and the other call sites referenced), so refreshCompose
treats force=true as a hard rebuild of compose.yml rather than a no-op; remove
the implicit reliance on state.ImageTag="" for this behavior and ensure callers
that receive the recovered value from checkInstallationHealth forward it to
refreshCompose.

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

Inline comments:
In `@cli/cmd/update.go`:
- Around line 250-263: When existing == nil and force recovery is requested, the
code assumes fresh contains generated content but loadAndGenerate currently
returns (nil,nil,nil) for a missing compose.yml; update the recovery branch in
the function containing the existing == nil check to explicitly generate the
compose content (call the same generator used by loadAndGenerate or invoke
loadAndGenerate in a mode that forces generation) to populate fresh before
calling atomicWriteFile(composePath, fresh, safeDir), handle and wrap any
generation errors, and then write and log the new compose.yml; ensure you
reference loadAndGenerate (or the underlying generator), composePath, fresh,
atomicWriteFile, and the force flag when implementing the fix so recovery
succeeds when compose.yml is absent.

In `@src/synthorg/providers/url_utils.py`:
- Around line 16-21: The redact_url() implementation accesses parsed.port
directly (can raise ValueError for malformed ports) and rebuilds IPv6 addresses
without brackets, causing exceptions during logging; fix it by guarding
parsed.port in a try/except (treat invalid/ValueError ports as absent), build
safe_netloc from parsed.hostname (or empty) and if hostname contains ':' wrap it
in brackets before appending a valid port, set query to "<redacted>" when
present, and return the rebuilt URL; update the logic inside redact_url() (refer
to parsed, parsed.port, parsed.hostname, safe_netloc and
urlunparse(parsed._replace(...))) so malformed ports are ignored and IPv6
literals are bracketed to avoid the unhandled exception during
_fetch_json()/discover_models() logging.

---

Duplicate comments:
In `@src/synthorg/providers/probing.py`:
- Around line 164-182: The code only treats None as a probe miss but
_parse_ollama_models/_parse_standard_models can return an empty tuple or empty
list for malformed payloads, so change the check from "if models is None" to a
falsy/emptiness check (e.g., "if not models") so that empty/invalid model lists
are treated as probe misses; keep using _log_probe_miss(preset_name,
"unrecognized_schema", url) and return None, and ensure the subsequent
ProbeResult(...) only runs when models is a non-empty sequence (referencing
preset_name, _parse_ollama_models, _parse_standard_models, models,
_log_probe_miss, url, idx, and ProbeResult).

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

Duplicate comments:
In `@cli/cmd/update.go`:
- Around line 54-75: Do not clear state.ImageTag before calling
refreshCompose(cmd, state, recovered); restore the original tag for compose
generation so compose.ParamsFromState(state) can render valid image refs.
Instead, add a boolean parameter (e.g., forceImageRefresh) to
updateContainerImages(...) and, when recovered is true, call
updateContainerImages(..., forceImageRefresh=true). Modify updateContainerImages
to skip its early-return that compares state.ImageTag == tag only when
forceImageRefresh is true; you can still clear or reset state.ImageTag after
refreshCompose if you must preserve the “force” semantics for later steps.