Thanks for the very thorough write-up — the self-review section made this much faster to audit, and every hardening claim in it held up under execution (details below).

Reviewed at 0948572a (merge-base b6a0520b). Web-only: no Python file changes.

Author claims I reproduced

Truncation contract, end-to-end

Rather than trust the copied constant, I drove the real validator to truncation:

300 non-string capabilities: total=100 markers=1 marker_idx=[99] last_sev=error
   last_message='Additional validation findings omitted.'
   EXACT-MATCH-UI-CONSTANT=True

100 including the marker, exactly one marker, last position, error severity because errors were dropped, and byte-identical to ValidationFindings.OMISSION_MESSAGE. isOmissionMarker requires index === all.length - 1, so the marker text in a non-final position does render as an ordinary finding, as documented.

The secret-leak path is really pinned

I mutated getProfileSource to drop /source (so the editor would read the resolved profile):

Tests  9 failed | 6 passed (15)

Nine editor tests go red. That behaviour is enforced, not just asserted in prose.

Things I suspected and cleared

• Portaled CustomSelect menu breaking existing callers. z-[80] clears the z-[60] maximum elsewhere, and the two backdrop closers (AgentPanel:518, FlowsPanel:300) are element-level onClicks, not document-level handlers, so a click on the portaled menu never reaches them. The pre-existing memory-graph test opens the menu and clicks an option, so this is covered by tests that predate the PR.
• Mixed line endings from a CRLF clone. rewriteFrontmatterName does emit a mixed-ending document (---\n ... \r\n ... ---\r\n). I fed it to the real backend parser and it round-trips cleanly (meta={'name': 'newname', 'description': 'd'}), as does ${VAR} inside a JSON-valued field. Not a defect — noting it only so it doesn't get re-litigated.
• Route ordering. All 14 URLs match api/main.py, and every literal path is declared before /agents/profiles/{name}.
• deleteProfile against a 204. fetchJSON's empty-body branch already covers it.

What blocks

Two defects, same root cause: a debounced effect whose early return does not invalidate the in-flight request token. The monotonic tokens are correct for the case they were written for (a newer request superseding an older one) but not for "the reason to want this response disappeared".

The first one silently persists the wrong document, so I'm marking this changes-requested — everything else here is in good shape, and both fixes are one-liners I verified.

Both fixes together keep 240 passed and tsc/build clean.

PR Review: #692 — feat(web): Profiles tab — browse, search, create, edit, clone, delete profiles

Summary

Adds the Profiles tab to the CAO Web UI (browse/search/create/edit/clone/delete over the #575/#585 APIs) — the final piece of #510. The implementation and its 63 new tests are high quality: we independently reproduced the 240/240 suite, clean tsc/build, and every hardening claim in the PR body. However, maintainer @haofeif has an open CHANGES_REQUESTED at this exact head for a data-integrity race (stale template preview silently persisted), and our review found the root cause is systemic: previewSeq is bumped only when a request is issued, never when the preview is invalidated, so the flagged one-liner alone leaves two more paths to the same silent persistence. Recommendation: fix the token-invalidation class as a whole (all clear sites), not just the two flagged lines, and land regression tests with it.

Blocking (must fix before merge)

• [correctness][verification] web/src/components/ProfileCreateModal.tsx:286-306 (open-reset effect) and :311-314 (template-deselect branch) — 🆕 Both setPreview(null) without bumping previewSeq.current (the reset also leaves previewTimer and previewLoading untouched). The modal stays mounted across close (open prop), so: preview in flight → close → reopen → the stale response still satisfies seq === previewSeq.current, re-lands setPreview(staleContent) + setProfileName(extractFrontmatterName(stale)), and canCreate (which checks only preview !== null && !previewLoading, not template) enables Create — persisting a body from a template that isn't even selected, with the preview pane not rendered. This is the same silent-persistence class as the maintainer's P1 but a distinct trigger his line-333 fix does not cover. Fix: invalidate the token at every clear site — bump previewSeq.current, clear previewTimer, reset previewLoading in the reset effect and the deselect branch (or bump the token on every preview-effect run/teardown).

Important (should fix)

• [consistency][correctness] ProfileCreateModal.tsx:309-327 — 🆕 getTemplateSchema is the one fetch in the modal with no staleness guard (no seq token, no cancelled flag, no abort) — unlike the search, preview, and ProfileDetail.getProfile effects beside it. Fast A→B template switch with out-of-order resolution leaves templateSchema = A's schema and config = A's seeded defaults while template = B: the form renders A's fields under a B selection and the debounced preview renders B with A's config. Same fix norm as the blocking item: token or cancelled flag keyed on the selected template.
• [tests] web/src/test/ — 🆕 The monotonic seq-guard machinery the PR headlines has zero test coverage: no test creates two overlapping in-flight requests with reordered resolution, so the seq !== ...current discard branch is never taken (all search/preview mocks resolve synchronously). The P1/P2 fixes should ship with gate-promise regression tests — (a) search: type, let the request go in flight, clear the box, then resolve → assert results stay null; (b) preview: template A render in flight, switch to B, resolve A → assert no A content in pane and Create does not POST A's body. Otherwise the one-liner fixes land undefended.
• [tests] web/src/components/CustomSelect.tsx — 🆕 The shared-component rewrite (portal to document.body, fixed positioning, flip-up, scroll/resize-to-close, portal-aware outside-click, new invalid prop) is essentially untested — tests only pick options by testid, and the invalid red-boundary path on a select is never asserted (the two red-boundary tests hit text/number inputs). This component backs Flows and Agents too; a regression in scroll-close/flip would be invisible to the suite.
• [correctness] ProfileCreateModal.tsx (SchemaField object branch) — 🆕 Object/JSON textareas are uncontrolled (defaultValue) and always receive value=undefined (object fields render only in Advanced, which passes undefined for object types). Collapse and reopen Advanced (or trigger the error-driven auto-expand) and the textarea remounts empty while jsonDrafts still holds — and persists — the typed JSON. Display/state desync: user sees an empty field, the value saves anyway; retyping clobbers the draft. Make the textarea controlled from the draft string.
• [conventions] web/README.md — 🆕 The canonical frontend doc (linked from docs/web-ui.md and CODEBASE.md) enumerates every page, supporting component, and file in three places; this PR adds a page (ProfilesPanel) + three components (ProfileCreateModal, ProfileEditorModal, ValidationFindings) and updates none of them. Per the repo's documentation-maintenance rule, update all three enumerations in this PR.
• [conventions] CHANGELOG.md — 🆕 No [Unreleased] → Added entry for the new Profiles tab. The #510 arc's API work is already recorded in the changelog; this user-facing UI piece should be too.

Nits (optional)

• [correctness] ProfileCreateModal/ProfileEditorModal save paths — a transport error/timeout on the pre-save validateProfile (a UX gate; the write route re-validates authoritatively) hard-blocks the save behind a generic "Validation failed". Consider letting transport failures fall through to the server-side validation while still blocking on real 400s.
• [consistency] web/src/api.ts:174, 484-495 — TemplateConfigValidation + validateTemplateConfig() are defined but never referenced anywhere in web/src (the UI uses previewTemplate instead). Wire it or drop it.
• [consistency] web/src/api.ts:474-476 — the "category/name travels as two path segments, slash must NOT be encoded" comment sits above getProfileSchema (fixed path, no template arg) but describes getTemplateSchema below it. Move it.
• [tests] App.tsx Alt+N renumbering — the flagged-as-risky shortcut shift has no keyboard test (only DOM order is asserted); one test that Alt+2 opens Profiles and Alt+3 opens Agents would pin it.
• [tests] ConfirmModal typed reset-on-open and the delete/detail error paths (DELETE rejection → snackbar + row retained; detail-pane getProfile failure) are untested.
• [conventions] docs/web-ui.md — the Features paragraph doesn't mention profile management; one clause would do (component detail stays in web/README.md).
• [conventions] commit subject — 77 chars, over the ~72 norm; move the verb enumeration to the body.
• [conventions] ProfilesPanel.tsx — the {/* Master list */} comment: "master-detail" is a standard pattern name, but the standalone "Master" could read as "List pane" per the project's inclusive-language convention.

Tests

63 new tests across 4 files, counts exact, and assertion quality is deliberately anti-tautological (ordering fixtures defeat both alphabetical and catalog-order coincidences; $&/$'/CRLF adversarial cases assert the actual old bugs; the byte round-trip pins ${VAR} through edit→PUT; the clone test asserts validated bytes equal the POST body; optimistic delete is proven independent of the refetch). The systematic gap is concurrency: the stale-response discard branch is never exercised anywhere, which is precisely where all the known and new race findings live — see the Important items for the two regression tests that should accompany the fixes. Secondary gaps: the CustomSelect rewrite, Alt+N mapping, ConfirmModal reset, and a few error paths.

Verification

Independent dynamic verification at this head (clean checkout):

• ✓ VERIFIED — web suite: 16 files, 240/240 passed (after working around the known npm optional-deps issue for rolldown's native binding, unrelated to the PR).
• ✓ VERIFIED — npm run build (tsc + vite) exit 0; standalone tsc exit 0; only the advisory 823 kB chunk-size notice.
• ✓ VERIFIED — maintainer P1 defect chain end-to-end by code reading (early return at :333 leaves token valid → stale setPreview → canCreate enables → createProfile persists the stale body), plus the two additional un-flagged instances of the same root cause (open-reset effect, template-deselect branch) now in Blocking.
• ✓ VERIFIED — maintainer P2 at ProfilesPanel.tsx:270-276 as described.
• ✓ VERIFIED — the extractFrontmatterName nit reproduced empirically (body name: returned when frontmatter lacks one).
• ✓ Audited the remaining async paths in the diff: ProfileEditorModal's unguarded load is not a practical defect (conditional mount discards stale state); ProfilesPanel detail effect, MemoryGraphView, and useEventFollow use correct guard patterns.

Verdict

Request changes — maintainer @haofeif's CHANGES_REQUESTED stands at this exact head with the P1 data-integrity blocker unaddressed, and the fix should cover the whole token-invalidation class (the two additional preview-clear sites and the unguarded getTemplateSchema fetch), with regression tests, not just the two flagged lines. Everything else about the PR is in strong shape; once the race class is closed and the doc/CHANGELOG drift is patched, this is a clean approve.

Summary

Reviewed PR #692 (feat/510-profiles-ui @ 33d6477) against main @ b6a0520: 18 files, +3422/−24, no Python or CI changes. Every backend route the UI calls exists at HEAD, and the UI's reading of the server contracts is accurate — /source (not the resolved GET /{name}) backs the editor, the ranked search order is never re-sorted, duplicated_in is surfaced, the {message, errors} write-rejection shape is rendered through the shared findings panel, and the _OMISSION_MESSAGE truncation contract (last-position, severity-of-omitted) is implemented correctly. Validation, staleness tokens, and the type-to-confirm delete gate are all real, not decorative. tsc --noEmit, npm test (260 tests), npm run build, and the four backend profile test modules (192 tests) are green locally.

Six findings, four of them demonstrated with runtime probes against the PR's own components: the detail pane never refreshes after an in-place edit, the header X re-opens the mid-save dismissal hole that Cancel and the backdrop were deliberately closed against, the scratch-mode document embeds an untrimmed name while the POST sends the trimmed one, and a failed profile-schema fetch leaves From-scratch mode on a permanent spinner. None is a security, data-loss, or build failure, so nothing here is P1.

Two design calls I looked at and am not flagging: the fall-through-to-write on a 5xx/transport pre-save validate failure is safe (the write route re-validates and returns a renderable 400) and is documented at both call sites; and the absence of an Authorization header is repo-wide and pre-existing, not introduced here — worth a separate issue now that write operations are on the ungated client.

Findings

P2 — The detail pane keeps showing pre-edit values after a successful save

web/src/components/ProfilesPanel.tsx:94-104 (the fetch effect), :403 (the render site), :221-229 (handleSaved)

ProfileDetail's fetch effect is keyed on [row.name]. After an in-place edit, handleSaved calls setSelected(name) (same name) and refreshCatalog(). The catalog refresh produces a new row object but an unchanged row.name, so the effect never re-runs and detail — the source of Provider, Model, Role, Tags and Capabilities — is never refetched.

Triggering scenario: select a local profile whose model is claude-sonnet-4, click Edit, change the frontmatter to model: claude-opus-5, save. The snackbar says Profile 'developer' saved, the PUT succeeded, and the detail pane still reads claude-sonnet-4 until the user selects a different profile and comes back. Verified by probe: detailFetches stays at 1 across the whole save, and the rendered pane is …Providerkiro_cliModelclaude-sonnet-4. description and source do update (they come from the refreshed catalog row), which makes the stale half more convincing, not less. Clone is unaffected — the new name changes the effect key.

Correction: give the detail fetch an explicit reload token, e.g. a reloadNonce counter bumped in handleSaved and passed as a prop into the effect's dependency array (or key={${selectedRow.name}:${reloadNonce}} on <ProfileDetail>). No existing test covers detail state after a save; profile-editor.test.tsx:84 asserts only the PUT body.

P2 — The header X is not disabled during a save, and closing mid-flight discards the write outcome silently

web/src/components/ProfileEditorModal.tsx:126, web/src/components/ProfileCreateModal.tsx:589

Both modals gate the backdrop on !saving (ProfileEditorModal.tsx:116, ProfileCreateModal.tsx:573) and disable Cancel while saving (:193, :755), with a comment at each site explaining why. The aria-label="Close" X calls onClose unconditionally, so it reopens exactly that hole — and for the editor it is worse than the comment anticipates, because ProfilesPanel.tsx:419 renders the editor as {editor && <ProfileEditorModal …>}, so onClose unmounts it.

Triggering scenario: click Save changes, then click X while the PUT is in flight. The PUT returns 400 {message: "Profile failed validation and was not written.", errors: […]}. setSaveError/setFindings land on an unmounted tree, and nothing else surfaces the failure: no snackbar, no alert. Verified by probe — with Cancel confirmed disabled === true at the same instant, clicking X unmounted the editor (Profile source textarea gone) and after the 400 resolved screen.queryAllByRole('alert') was []. The user is left believing an edit was saved that was rejected. The create modal has the same outcome by a different route: it stays mounted but renders null, so saveError is invisible and is then wiped by the reset effect on the next open.

Correction: onClick={saving ? undefined : onClose} (or disabled={saving}) on both X buttons, matching Cancel. profile-editor.test.tsx:370 already pins the backdrop path — parameterising it over backdrop / Cancel / X would pin all three.

P2 — Scratch-mode frontmatter carries the untrimmed name while the POST carries the trimmed one

web/src/components/ProfileCreateModal.tsx:496 vs :515 (and canCreate at :503-504)

handleCreate computes const name = profileName.trim() for the POST, and template mode rewrites the document with that trimmed value. buildScratchContent instead does { name: profileName, … } — untrimmed. canCreate only requires profileName.trim() !== '', so surrounding whitespace passes the client gate.

Triggering scenario: in From scratch, paste my-agent (trailing space — routine when copying a name out of a doc or terminal) and click Create. Verified by probe: the validated/POSTed document is ---\nname: "my-agent "\n---\n\n while the POST name is "my-agent". Confirmed against the real backend, validate_profile_text on that document returns exactly one finding: error | name | 'my-agent ' does not match '^[A-Za-z0-9_-]{1,64}$'. So the pre-save gate blocks the create and paints the Profile-name box red while quoting a value whose only defect is invisible — and the identical input succeeds in template mode. Were the pattern ever relaxed, the same skew would hit _validate_profile_for_write's name-identity check (api/main.py:2489-2495) instead.

Correction: use the trimmed name in buildScratchContent, e.g. { name: profileName.trim(), …scratchValues }, or hoist const name = profileName.trim() and pass it in.

P2 — A failed profile-schema fetch leaves From-scratch mode on a permanent spinner

web/src/components/ProfileCreateModal.tsx:336, rendered at :674-677

api.getProfileSchema().then(setProfileSchema).catch(() => setProfileSchema(null)) maps failure onto the same null that means "still loading", and the render is {!profileSchema ? <Loader2 …/> Loading profile schema… : …}. There is no error state and no retry. This is the opposite shape from the sibling template-schema fetch, which commit 2/4 specifically hardened to surface its error (:362-365).

Triggering scenario: GET /agents/profiles/schema returns 500 (or the request times out). Verified by probe: after switching to From scratch, profile-schema-loading is still present and screen.queryAllByRole('alert') is [] — the mode is permanently unusable with no indication of why. api.listProfileTemplates().catch(() => setTemplates([])) at :335 degrades more visibly (the select reads "No options available") but also swallows the cause. Neither path is tested.

Correction: add a schemaError state set in the catch and render it through the same amber/red banner the template path uses, keeping the spinner for the genuinely-in-flight case.

P3 — Object-typed primary fields would render blank while still saving their JSON (latent)

web/src/components/ProfileCreateModal.tsx:687 vs :726

handleScratchChange (:487-493) routes any field whose schema type === 'object' into jsonDrafts, never into scratchValues. The ADVANCED call site accounts for that (value={scratchProps[k].type === 'object' ? (jsonDrafts[k] ?? '') : scratchValues[k]}); the PRIMARY call site passes value={scratchValues[k]}. Commit e070c3b fixed exactly this bug — a blank textarea that still POSTs the typed JSON — but only at the advanced site.

Not reachable today: the six PRIMARY_FIELDS resolve against agent_profile.schema.json to name/description/provider/model (string) and tags/capabilities (array), none of them object. It returns silently the moment an object-typed field is added to PRIMARY_FIELDS or an existing primary field's schema type changes. Correction: use the advanced site's expression at both call sites.

P3 — The portaled menu declares role="listbox" but contains no options

web/src/components/CustomSelect.tsx:121 (container), :134 (items)

The portal wrapper gained role="listbox", but its children are <button> elements inside plain <div> group wrappers — no role="option", no aria-selected, and the trigger has aria-expanded without role="combobox", aria-haspopup, or aria-controls. Assistive technology announces a list box with zero items, which is worse than the previous role-less <div>. This affects every consumer of the shared component (Agents, Flows, Memory, workflow run comparison), not just the new modals.

Correction: either drop role="listbox" and keep the menu a plain container of buttons (matching the pre-PR behaviour), or complete the pattern — role="option" + aria-selected on each item, role="presentation" on the group wrappers, and role="combobox"/aria-controls on the trigger.

Validation / tests

All commands run from /tmp/cao-pr-review-awslabs-cli-agent-orchestrator-pr-692/checkout at 33d6477, darwin/arm64. The checkout was not modified (git status --porcelain empty before and after; the Vite output directory is gitignored).

Backend contract spot-checks (this PR changes no Python; these confirm the assumptions the UI is built on):

• validate_profile_text('---\nname: "my-agent "\n---\n\n') → error | name | 'my-agent ' does not match '^[A-Za-z0-9_-]{1,64}$' (finding 3).
• render_template('aws/sqs-monitor', {…defaults only}) → Config validation failed …\n - (root): 'profile' is a required property, which the templateErrorFields parser at ProfileCreateModal.tsx:423-438 matches exactly, (root) branch included.
• All seven shipped templates/aws/*/template.md.j2 files carry a frontmatter name: line, so rewriteFrontmatterName's no-name: no-op path is not reachable from the UI.

Findings 1–4 were each demonstrated by running the PR's own components under vitest with assertions written for the correct behaviour, so a failure means the defect is present. Probes live in a scratch copy of web/ at /tmp/probe692-web (rsync excluding node_modules, which is symlinked back) — the checkout itself was never written to. Probe outputs quoted inline above: detailFetches after save = 1; cancel disabled during save = true / editor still mounted after X = false / alerts visible anywhere = []; validated content = "---\nname: \"my-agent \"\n---\n\n" with POST name = "my-agent"; spinner still shown = true / alerts = [].

Open questions / residual risk

1. CustomSelect in a real browser. The portal + position: fixed rewrite is the widest-blast-radius change and jsdom cannot lay out, so the 9 new tests pin behaviour (portal placement, flip via mocked getBoundingClientRect, portal-aware outside-click, scroll/resize close, Escape) but not geometry. Worth one manual pass through the Flows and Agents creation modals. Related nit, not filed as a finding: when the menu flips up, maxHeight stays at MENU_MAX_H rather than being clamped to the space above, so on a very short viewport (< ~500 px) the top of the list would sit above the viewport edge and be unreachable.
2. OMISSION_MESSAGE drift. ValidationFindings.tsx:13 is a hand-copied profile_validator._OMISSION_MESSAGE. If the Python string changes, the truncation notice silently degrades into an ordinary 100th finding row; no test crosses the language boundary.
3. Refresh. The catalog is fetched once on mount plus after this panel's own writes, with no manual refresh control — a cao profile install while the tab is open is invisible until remount. Deliberate and commented; flagging because it compounds finding 1.

Thanks for the thorough turnaround on this — the blockers were fixed as a class rather than patched at the one site I happened to reproduce, and every fix landed with a regression test that actually fails without it.

Reviewed at 33d6477eee97383dd3f93eb4399e91f8ebce15ec (merge-base b6a0520bd2cc3e1c2e7756197c84710afdc8bbb3). Three additive commits since my last review, no force-push, so the earlier history is intact.

Claims verified by execution

Mutation results

Reverting each fix and re-running only its guard test:

M1 preview-effect early-return bump        -> FAILS (guards)
M3 templateSeq staleness token             -> FAILS (guards)
M4 searchSeq bump on clear                 -> FAILS (guards)
M5 bounded extractFrontmatterName          -> FAILS (guards)
M6 JSON draft controlled value             -> FAILS (guards)
resize `instanceof Node` guard             -> FAILS (guards, with the TypeError above)

A correction to my own method, so the above is not over-read. My first pass mutated the open-reset invalidatePreview() alone and reported the reopen test as vacuous. That was my error: the three invalidation sites are mutually redundant by design, so removing one leaves the template-deselect branch covering the path. Removing all three together fails both your test and a stronger probe I wrote:

all 3 guards present   -> create disabled: true,  POSTed stale A body: no POST
remove all 3 sites     -> create disabled: false, POSTed stale A body: true

So the reopen test does guard real behaviour, and the redundancy is a property of the fix rather than a gap in the test. Worth stating explicitly because a single-site mutation is a misleading way to grade this fix — as I demonstrated on myself.

On the two unrequested changes in these commits

Both were outside my original findings, so I reviewed them fresh rather than re-verifying:

• 5xx/transport fall-through on the pre-save validate (ProfileCreateModal.tsx, ProfileEditorModal.tsx). Safe, and the justification in the comment holds: POST /agents/profiles and PUT /agents/profiles/{name} both call _validate_profile_for_write before write_profile (api/main.py:2571, :2605), whose docstring is explicit that "an invalid profile never reaches disk". A 4xx still blocks — ApiError.status is set on every non-OK response in fetchJSON — and a write rejection is surfaced through the same findings renderer via detailMeta.errors, so nothing fails silently. Net effect is a flaky-validate call no longer hard-blocks behind a phantom "Validation failed".
• JSON draft display desync. The defaultValue → controlled value change is the right fix; an uncontrolled textarea remounted empty on Advanced collapse/reopen while the draft that actually saves persisted, which is the same silent-divergence shape as the original P1.

Verdict

Approving. All three findings from my previous review are fixed at the root, the fixes are pinned by tests that fail without them, the full suite is green on a pristine tree, and I found no new defects.

Note for the author: @gutosantos82's CHANGES_REQUESTED at the old head is a separate outstanding block and will need their re-review independently of this approval.

PR Review: #692 — feat(web): Profiles tab — browse, search, create, edit, clone, delete profiles

Summary

Adds the Profiles tab to the CAO Web UI (browse/search/create-from-template/create-from-scratch/edit/clone/delete over the #575/#585 APIs) — the final piece of #510. The implementation quality is high: no XSS sinks, well-hardened frontmatter helpers, genuinely discriminating race tests, and docs/CHANGELOG in sync. The earlier review blockers (stale-preview token class, search-clear restore) are fixed as a class at this head and were mutation-verified by @haofeif, who has approved. However, @fanhongy's four demonstrated P2 defects remain present and unanswered at the current head, and our review adds one new defect of the same family (cross-mode error-state bleed in the create modal). Recommendation: one more fix pass for the open P2s + the mode-switch bleed, then this is merge-ready.

Important (should fix)

• [correctness] web/src/components/ProfileCreateModal.tsx (mode-switch handlers) — 🆕 Switching between the From-template and From-scratch tabs clears none of the shared error state (findings, saveError, previewError, and the derived errorFields). A validation failure in template mode leaves the findings panel and red save-error box rendered on the scratch form, paints red boundaries on scratch controls whose field names collide (name, provider, …), and can auto-expand the scratch form's Advanced section for a finding that came from the template attempt. Reverse direction bleeds the same way; reset only happens on modal open. Fix: clear findings/saveError/previewError on mode change — same one-effect shape as the open-reset.
• [correctness] web/src/components/ProfilesPanel.tsx (handleCreated, clone handleSaved) — 🆕 setSelected(newName) runs before the async refreshCatalog() lands, so the just-created/cloned profile renders "Select a profile…" until the refetch resolves — and if refreshCatalog() fails, the new profile is selected but never appears (permanent blank detail with no path out except reselecting). Consider selecting after the refresh resolves, or tolerating a selected name not yet in the catalog.
• [tests] web/src/test/ — 🆕 The create path lacks the end-to-end panel coverage the edit path has: every create test renders ProfileCreateModal in isolation, so the New-profile button → handleCreated → warning/success snackbar → post-create selection → refreshCatalog wiring is untested (the analogous handleSaved path IS covered through the panel). Adding it would also pin whichever behavior is chosen for the select-before-refresh item above.

Nits (optional)

• [correctness] both modals — after a blocked save, findings and the red field outlines persist while the user edits and only clear at the start of the next save attempt — misleading feedback; clearing (or greying) on edit would be kinder.
• [correctness] ProfilesPanel.tsx (ProfileDetail) — the duplicated_in shadow warning renders only for catalog rows; ProfileSearchResult carries no duplicated_in, so the same shadowed profile shows the amber "Also defined in …" banner via the catalog but not when reached through search.
• [correctness] ProfileCreateModal.tsx (open-reset effect) — the initial listProfileTemplates/getProfileSchema/listProviders fetches are plain .then(setX) with no staleness guard — the one async class in this PR without the seq-token discipline applied everywhere else. Low impact (idempotent endpoints), but a rapid close→reopen can repopulate from the earlier open's response.
• [correctness] both modals — the Create/Save buttons gate on !saving, but two synchronous clicks before re-render both observe saving === false, opening a double-submit window; a ref-based guard closes it.
• [consistency] PR body — the Tests paragraph is stale after the three fix commits: counts are now 15/30/17/11 per file (not 12/25/15/11), the suite is 260 not 240, and custom-select.test.tsx is an entire fifth new test file the body's "four files" never mentions. Worth refreshing before merge so the record matches what ships.
• [conventions] docusaurus/docs/features/web-ui.md — the published-site features list did not get the Profiles bullet that docs/web-ui.md got; per the documentation-maintenance rule, add it for parity.
• [consistency] docs/web-ui.md, web/README.md — the detail pane is described as showing "source, provider, model, tags, capabilities" but also renders a Role field; trivial omission.
• [conventions] CHANGELOG.md — the new ### Added block is packed tight (no blank lines between bullets) unlike the rest of the section, and three adjacent provider entries (#624, #559, grok_cli) ride in via the merge of main; a glance post-squash that only the #510 entry lands as this PR's net change (it currently is — no duplication observed).

Tests

Strong, behavior-focused suite (~83 new tests; 260/260 claimed after the fix commits). The async/race tests are genuinely high quality: gated promises released after the reordering event, asserting the discard branch is actually taken; the search-order fixture defeats both alphabetical and catalog-order coincidences; debounce is asserted by call count; the truncation contract is tested with full 100-finding payloads including marker-severity and marker-mid-list cases; adversarial frontmatter regressions ($-patterns, CRLF, body decoy) are excellent. Two gaps mask the real defects noted above: nothing exercises the header X in either modal, and nothing inspects the detail pane after a save (a test that edits a parsed field and re-checks the pane would fail today). Minor: some tests couple to Tailwind class strings and layout selectors rather than testids/aria; detail-loading/search-spinner testids are never asserted; delete-success snackbar unasserted.

Verification

Dynamic verification did not return by synthesis time. Independent maintainer evidence on record: @haofeif ran npm ci && npm test && npm run build at 33d6477 (260 passed, build clean) and mutation-tested every race-fix guard; @fanhongy reproduced the suite, tsc, build, and the four backend profile test modules green at the same commit. The current head adds only a merge of main on top.

Verdict

Request changes — the code is close, but four demonstrated P2 defects from earlier review remain unaddressed at this head (stale detail pane after save, ungated header X in both modals, untrimmed scratch-mode name, permanent spinner on a failed schema fetch), our review adds a cross-mode error-state bleed of the same family, and an earlier CHANGES_REQUESTED review remains open pending its author's re-check. One focused fix pass (plus tests for the X and detail-pane paths) should make this merge-ready; the underlying feature and its test suite are in very good shape.

Re-reviewed exact head cad050a947635f45cbdaa9503dd2bf2147fd063a. The feature and fixes approved at 33d6477eee97383dd3f93eb4399e91f8ebce15ec are unchanged; the only newer commit merges main at a5ccbe2624aabadfbdf64642c5f1e364db299ec3. git show --remerge-diff confirms the sole conflict was CHANGELOG.md, resolved additively by retaining both the Profiles entry and every upstream release entry. The web track remains at 260/260 tests with type-check/build successful, and all current checks are green. No findings.

Re-reviewed exact head 98f6b996. The round-three fixes close the previously reported synchronous state issues, and the focused 71-test web slice plus production build pass. Four async ordering gaps remain: one can persist an obsolete template render, while the others leave mutation/search state stale or reintroduce errors across create modes. These were independently exercised at this head; details are inline.

Re-reviewed exact head 42b7df72. The four previously reported async defects are addressed, but three adjacent generation/ownership races remain: one P1 can save a settled preview from the previous template, and two P2s can display results for the wrong query or overwrite navigation performed after a save. The focused frontend suite passes (55 tests) and the production build succeeds; the passing tests do not exercise these windows.