feat(mobile): rebuild the chat tab on the harness SDK - #5895
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (45 files)
Fix these issues in Kilo Cloud Previous Review Summaries (10 snapshots, latest commit b374fb3)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit b374fb3)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous review (commit e585a7e)Status: No Issues Found | Recommendation: Merge Files Reviewed (31 files)
Previous review (commit 326f2c3)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit a7e6c90)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit 02d4575)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit fee11c2)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit 5c686fd)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit 737d3e6)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit a5fa1e0)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit 3f6eee7)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (38 files)
Plus locale JSON, drizzle/db snapshots, lockfile, and deleted quick-chat sources. Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
ef4251c to
326f2c3
Compare
326f2c3 to
2bf4d8d
Compare
62dae71 to
0eb98d1
Compare
0eb98d1 to
5c5a68c
Compare
5c5a68c to
84e71c4
Compare
84e71c4 to
4ae54a0
Compare
4ae54a0 to
8f2d387
Compare
| if (place !== null) { | ||
| void enterChat(place, sessionId); | ||
| } | ||
| }, [place, sessionId]); |
There was a problem hiding this comment.
WARNING: enterChat re-runs on every render because place is a new object
place comes from chatPlaceOf(...) called inline in ChatScreen (chat-screen.tsx:62), so it has a new identity every render and this effect fires after every render, not only when the scope or session changes. During a model switch the registry deletes the old chat and repoints its state (registry.ts:369-376); the render that delivers that change still has the old sessionId, so the effect calls enterChat(place, oldId). chats no longer holds oldId, so it takes the reopen path and calls continueSession(oldId) on the session that was just closed — opening an orphan session that is never added to a screen and never closed, or an unhandled SessionNotFoundError. Depend on a stable value (or memoize place).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| useEffect(() => { | ||
| if (place !== null) { | ||
| void enterChat(place, sessionId); |
There was a problem hiding this comment.
WARNING: A failed open leaves the screen stuck on opening
enterChat is invoked with void, and reopen (registry.ts:175-188) publishes status: 'idle' only after continueSession and history succeed. When the open fails — a stale or deleted session id from a deep link or another screen, or a store error — the promise rejects unhandled and snapshotOf(sessionId) keeps status: 'opening' forever, so the transcript renders empty with no error or retry. Catch the rejection and publish a terminal/error state.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| forgotten. Whoever asked for the move is not always the screen — a question | ||
| queued on another model moves the chat from inside the registry — so the | ||
| state is what says where the conversation went. */ | ||
| change(sessionId, { sessionId: handle.id }); |
There was a problem hiding this comment.
WARNING: Model switch leaks the old chat state and its turns
change(sessionId, { sessionId: handle.id }) keeps the old session's ChatState — including its full turns array — in the module-level states map, plus its watcher entry in watchers. Nothing ever calls forgetState(sessionId): releaseChat(sessionId) early-returns because chats no longer holds the old id (deleted at line 369). Every model switch permanently retains the previous conversation, so repeated switches grow both maps without bound. Drop the old entry once it has been repointed.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| */ | ||
| export async function releaseChat(sessionId: string): Promise<void> { | ||
| const chat = chats.get(sessionId); | ||
| if (chat === undefined) { |
There was a problem hiding this comment.
WARNING: Deleting an unopened chat leaks its state
releaseChat returns early when the chat is not currently open, so forgetState is never called. snapshotOf creates a ChatState for every id it is asked about (state.ts:97-104), and every row subscribes via useChatStatus (chat-row.tsx:34), so each listed chat has one. Deleting a chat from the list without opening it therefore leaves its ChatState in states for the life of the process. Clean the state even when there is no live Chat.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } from '@/lib/feature-flags'; | ||
| export { FEATURE_FLAG_PR_REVIEW, type FeatureFlagDefinition } from '@/lib/feature-flags'; | ||
| /* The key is the one PostHog already rolls out; the surface behind it was rebuilt. */ | ||
| export { FEATURE_FLAG_QUICK_CHAT as FEATURE_FLAG_CHAT } from '@/lib/feature-flags'; |
There was a problem hiding this comment.
WARNING: Reusing the quick-chat flag breaks already-shipped builds
This re-exports the existing mobile-quick-chat key as FEATURE_FLAG_CHAT, which keeps minAppVersion: '1.0.6' (feature-flags.ts:36), so shipped builds 1.0.6–1.0.11 still read the same PostHog flag. This PR deletes quickChatRouter (root-router.ts:99-102) and drops quick_chat_threads/quick_chat_messages (0243_drop_quick_chat.sql). The first time the flag is enabled for the rebuilt surface, those older builds will show their compiled old quick-chat tab and call trpc.quickChat.*/read the dropped tables, which now fail. Per the registry contract in feature-flags.ts:12-14, a rebuilt surface needs a new key (or a version-gated rollout) so old builds keep their default.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| run: pnpm --filter @kilocode/trpc run build | ||
|
|
||
| - name: Build harness SDK | ||
| run: pnpm --filter @kilocode/harness-sdk run build |
There was a problem hiding this comment.
WARNING: The check-unused job never builds the harness SDK
The SDK build was added to the typecheck, lint, i18n-leftover and test jobs, but not to check-unused. apps/mobile now depends on @kilocode/harness-sdk, whose exports resolve into dist/, and scripts/prepare.sh is a no-op in CI (if [ -n "${CI:-}" ]; it only builds trpc), so check-unused runs knip with no dist. The new runtime imports in src/lib/chat/{layers,registry,tools}.ts then fail to resolve and knip exits non-zero. Add the same build step before pnpm --filter kilo-app run check:unused.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if (place === null || model === undefined) { | ||
| return; | ||
| } | ||
| void (async () => { |
There was a problem hiding this comment.
SUGGESTION: A failed newChat is swallowed
newChat rejects when the runtime, openSession or rememberChat fails, but this IIFE is fire-and-forget, so the rejection is unhandled and the button silently does nothing — no feedback and no navigation. Catch it and surface the failure the way the app's other mutations do.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
1 participant
Changelog for users
+, reopen one you had, retry a question that never got an answer, and long press a chat to delete it.FEATURE_FLAG_CHAT, off by default.Changelog for maintainers
chatstable and the SDK'ssessions/turns/partson the one encrypted database. The token comes from the app's auth, randomness from expo-crypto, and the data is SQLCipher-encrypted at rest.en.jsondoes not define are removed. A file that also holds an unreviewed value keeps its approved restored values instead of being reverted whole.profile.signOut,securityAgent.settingsSave.keepEditing,agentChat.filePart.imageUnavailableWithAlt,agentChat.modelSelector.thinkingEffortXhigh,agentChat.modelSelector.thinkingEffortMedium,agentChat.composer.messagePlaceholder,kiloPass.unavailableDescription,kiloPass.webManagementDescription, andorganization.invoices.loadMoreFailed.kiloPass.unavailablecopy becomes "Couldn't load Kilo Pass."; the English subscription description is unchanged.E2E proof
E2E proof — log excerpts
/Users/igor/.local/share/kwf/sections/kwf-fix-ci-fix-2ddd/e2e-mobile-app/e3-probe.log/Users/igor/.local/share/kwf/sections/kwf-fix-ci-fix-2ddd/e2e-mobile-app/e3.log/Users/igor/.local/share/kwf/sections/kwf-fix-ci-fix-2ddd/e2e-mobile-app/catalog-verify.log/Users/igor/.local/share/kwf/sections/kwf-fix-ci-fix-2ddd/e2e-mobile-app/i18n-leftover.log/Users/igor/.local/share/kwf/sections/kwf-fix-ci-fix-2ddd/e2e-mobile-app/format-check-mobile.log/Users/igor/.local/share/kwf/sections/kwf-fix-ci-fix-2ddd/e2e-mobile-app/format-check-root.log