refactor: route-scoped registry and live custom mode - #5472
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (25 files)
Previous Review Summaries (5 snapshots, latest commit 634d4ca)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 634d4ca)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous review (commit 02a5d4f)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 files)
Previous review (commit 54d7050)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (21 files)
Fix these issues in Kilo Cloud Previous review (commit a9b4003)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (21 files)
Fix these issues in Kilo Cloud Previous review (commit 361350e)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (21 files)
Reviewed by grok-4.6 · Input: 60.5K · Output: 11.9K · Cached: 565.9K Review guidance: REVIEW.md from base branch |
|
|
||
| export function clearDiffSelection() { | ||
| selection = null; | ||
| prDiffSelectionSlot.clearAll(); |
There was a problem hiding this comment.
bot: Clearing a selection for one PR deletes selections for every PR.
Suggested fix: Make the clear operation PR-scoped: change clearDiffSelection to accept PrIdentity (or a route key) and call prDiffSelectionSlot.clear(prRouteKey(pr)). Pass the current { owner, repo, number } from PrDiffFileList, useDiffSelection, and PrDiffFloatingActions. Add a regression test that creates selections for two PRs, clears one, and asserts the other remains available.
Map build/architect aliases to code/plan, widen isValidSessionConfig to accept any non-empty slug after normalization, and derive the mode picker from runtime and profile agents. The empty control shows Select mode instead of Code. The web Jest test runs in CI (workerSetup provisions Postgres).
The live picker surfaced current-profile agents even when they were not in the session's frozen runtimeAgents; the worker rejects such a slug on send. Filter the profile list to runtimeAgents slugs so the picker never offers an agent the session cannot run.
clearDiffSelection wiped every stored selection, so leaving one PR discarded the selection a second PR entry on the stack was still holding. Take the PR identity and clear that route key.
0b0b6d9 to
8c10932
Compare
Summary
The live cloud chat mode picker lists only the custom agents the session can run. An agent added to the profile after the session was created is not offered in that session.
The mode control shows Select mode when no mode is stored, instead of defaulting to Code. A session stored with a custom agent mode keeps that mode on live resume.
Legacy Build and Architect modes show as Code and Plan.
The live chat page composes the mode picker's custom options from the session's frozen runtime agents plus the current default profile's agents, filtered to the slugs the session can still run. The profile list enriches those runtime agents with current descriptions, but a profile agent whose slug is not in the frozen
runtimeAgentslist is excluded because the worker'svalidateModeAgainstRuntimeAgentsrejects it on send. The send path normalizes the mode, and the provider normalizes the fetched session mode so an absent mode resolves to an empty string instead of null.Files
apps/web/src/components/cloud-agent-next/CloudChatPage.tsx— builds custom mode options from runtime agents plus profile agents filtered toruntimeAgentSlugs; usesmodeControlValuefor the control andnormalizeAlias(mode) || 'code'on send.apps/web/src/components/cloud-agent-next/CloudAgentProvider.tsx— normalizes the fetched session's mode withnormalizeAlias, so an absent mode is now''instead ofnull.Session mode validation now accepts any non-empty slug after alias normalization, matching the already-widened
AgentModetype. New pure helpers normalize legacy aliases (build→code,architect→plan), resolve an empty value to the picker's Select mode placeholder, identify built-in slugs, and dedupe custom options. The runtime validity gate no longer rejects unknown slugs, so a customSessionConfig.modeoutside the five built-ins is now valid.Files
apps/web/src/components/cloud-agent-next/session-config.ts— addsnormalizeAlias,modeControlValue,isBuiltinAgentMode,dedupeCustomModeOptions,ensureSelectedCustomOption, andCustomModeOption; rewritesisValidSessionConfigto accept any non-empty normalized slug with a model.The mobile picker bridges move into a route-scoped registry keyed by a route key. A route key is the session id for the agent-chat pickers, the lowercased owner/repo/#number triple for pull-request flows, or the fixed keys
unscopedandsecurity-filter. TheuseRouteRegistryhook clears every slot under a key when the owning route unmounts, so a stale value can no longer leak between flows, at the cost that every consumer must name its route key.Files
apps/mobile/src/lib/route-registry.ts— adds the typed seven-slot registry,UNFENCED_ROUTE_KEY,SECURITY_FILTER_ROUTE_KEY,prRouteKey, and theuseRouteRegistrycleanup hook.The process-global model, mode, repo, and instance bridges are removed, and each selector now writes its value into a registry slot before navigating. Each picker route reads its slot once on focus and clears it on blur, selection, or unmount. The four bridge types are exported,
RepoOptionnow requires a platform, and the old set/get/clear exports are gone, so a repo list built directly must supply a platform or default it to GitHub.Files
apps/mobile/src/lib/picker-bridge.ts— exports the four bridge types; removes the module-level storage and its set/get/clear functions; adds requiredplatformand optionalworkspaceUuid/repositoryUuidtoRepoOption.apps/mobile/src/components/agents/model-selector.tsx— writesmodelPickerSlotunder the selection scope's session id and pushes the route with that route key.apps/mobile/src/components/agents/mode-selector.tsx— writesmodePickerSlotunder the session-id param orunscoped, and pushes the route key.apps/mobile/src/components/agents/repo-selector.tsx— maps local repo rows to bridgeRepoOptions (defaultingplatformtogithub) and writesrepoPickerSlotunderunscoped.apps/mobile/src/components/agents/instance-selector.tsx— writesinstancePickerSlotunderunscoped.apps/mobile/src/components/agents/model-picker-content.tsx— reads and clearsmodelPickerSlotby route key; commits the selection on blur.apps/mobile/src/app/(app)/agent-chat/mode-picker.tsx— readsmodePickerSlotby route key and clears it on select.apps/mobile/src/app/(app)/agent-chat/repo-picker.tsx— reads and clearsrepoPickerSlotunderunscoped, and keys rows byplatform:fullName.apps/mobile/src/app/(app)/agent-chat/instance-picker.tsx— reads and clearsinstancePickerSlotunderunscoped.The pull-request review bridges keep their public signatures but store state in the registry under each PR's route key.
setDiffSelection,getDiffSelection, andsubscribeFileNavigatorRequest/requestScrollToFilenow resolve per PR via the lowercasedprRouteKey, andclearDiffSelectionclears the whole diff-selection slot. Existing callers compile unchanged, and a request for one PR can no longer reach another PR's mounted listener.Files
apps/mobile/src/lib/pr-review/diff-selection-bridge.ts— delegates toprDiffSelectionSlotkeyed byprRouteKey;clearDiffSelectionclears the entire slot.apps/mobile/src/lib/pr-review/file-navigator-bridge.ts— keeps listener sets per route key; the unsubscribe cleans the key once its set is empty.The security finding filter bridge stores its draft in the registry under the fixed
security-filterkey.setSecurityFindingFilterBridgekeeps its signature so the producer screen compiles unchanged, while the filter route readssecurityFilterSlotand clears it on blur and unmount.Files
apps/mobile/src/lib/security-finding-filter-bridge.ts— replaces the module-level variable withsecurityFilterSlot.setunderSECURITY_FILTER_ROUTE_KEY; removes the get/clear exports.apps/mobile/src/app/(app)/(tabs)/(3_profile)/security-agent/[scope]/filter.tsx— reads the slot by the fixed key, registersuseRouteRegistry, and clears the slot on blur.Tests: 4 test files changed (route-registry.test.ts added; session-config.test.ts grown; picker-bridge.test.ts and repo-picker-filter.test.ts updated).
Generated: none.
Verification
No E2E report is attached for this level. Full E2E verification runs on the tip PR #5479.
Visual Changes
Visual Changes: N/A
Reviewer Notes
Human steps: none known.
Notes: none.
Stacked PRs — merge bottom to top. Each level shows only its own diff.
Full verification (E2E, user advocacy, simplify, bot review) runs on the tip PR over every level.
A finding on a level is repaired on that level, then carried upward with stack.sh forward.
audit-w7b-data-contracts-dffd— refactor(mobile): parse raw HTTP and infer review contracts #5460audit-w7b-data-contracts-dffd-s2— feat(web): add org credit and invoice page procedures #5462audit-w7b-data-contracts-dffd-s3— feat(mobile): owner-keyed credits, ledger paging, list freshness #5468audit-w7b-data-contracts-dffd-s4— refactor: route-scoped registry and live custom mode #5472 ← this PRaudit-w7b-data-contracts-dffd-s5— feat(mobile): provider-aware new-session and layout extract #5479 (tip)