Reapply #167 for follow-up fixes

[bbsngg]

Summary

• reapply the reverted changes from Topic/upstream session lifecycle 20260412 #167
• use this PR as the new collaboration thread for follow-up fixes and review
• supersedes the reverted Topic/upstream session lifecycle 20260412 #167 after Revert "Topic/upstream session lifecycle 20260412" #169 rolled it back

Context

#167 was merged by mistake and then reverted in #169. This PR reintroduces that work on a fresh branch so collaborators can continue fixing and refining it safely.

Testing

• not run in this update

[bbsngg]

Code review finding:

Blocker: the new temporary-session replacement flow only recognizes new-session-*, but some providers still bootstrap with temp-*. isTemporarySessionId() now returns true only for new-session-* (src/utils/sessionScope.ts:56-58), and both replaceTemporarySession() (src/hooks/useSessionProtection.ts:138-146) and the optimistic session swap in useProjectsState (src/hooks/useProjectsState.ts:485-492, 560-564) rely on that check. Gemini still creates an initial temp-* key before the real session id arrives (server/gemini-cli.js:705, 897-899), so those sessions will not be replaced anymore. In practice that leaves the stale placeholder marked active/processing and adds a second real session row instead of reconciling the optimistic one.

Please make the temporary-session helpers and replacement paths treat temp-* the same as new-session-*, and add test coverage for that case.

[bbsngg]

Another review finding:

High risk: resolveValidProjectOwnerUserId() now falls back to userDb.getFirstUser() whenever the stored owner is missing or invalid (server/projects.js:213-235). That value is then used during the bootstrap/sync paths (server/projects.js:335-337, 513-514) that run inside getProjects() (server/projects.js:1524-1533). The problem is that getProjects() is still called without an authenticated userId from background/broadcast code (server/index.js:204) and from TaskMaster detection (server/routes/taskmaster.js:1271-1275). Because projectDb.upsertProject() writes excluded.user_id whenever the existing row is NULL (server/database/db.js:1451-1457), an anonymous/background scan can now permanently assign previously unowned/orphaned projects to the first account in the database.

That is a real multi-user permission regression: projects can silently become visible only to user 1, or appear under the wrong owner after a filesystem refresh. I think the fallback here needs to stay null when there is no authenticated user context, and this should get a regression test that covers getProjects() / bootstrap with no userId in a multi-user DB.

[bbsngg]

One more review finding:

Medium/high risk: the new provider-scoped session identity work is not carried through to the local message-cache lookup, so sessions with the same projectName + sessionId can still read each other's cached transcript across providers. buildSessionMessageCacheCandidateKeys() explicitly falls back from the scoped key to both the default claude key and the legacy provider-less key (src/components/chat/utils/sessionMessageCache.ts:7-23), and readStoredChatMessages() returns the first match (src/components/chat/hooks/useChatSessionState.ts:67-95). That lookup is used during initial session load and external reloads (src/components/chat/hooks/useChatSessionState.ts:178-188, 726-729, 982-986).

The issue is that this PR already codifies that the same sessionId under different providers must be treated as distinct identities (src/hooks/__tests__/projectsSessionSync.test.ts:132-145, src/components/sidebar/utils/__tests__/sessionListBehavior.test.ts:104-136), but the cache layer still allows cross-provider fallback. In practice, if codex and gemini both have sess-1 under the same project, opening one can hydrate the other provider's locally cached transcript before the network request finishes, or keep showing it if the backend temporarily returns no messages. I think the fallback to claude / provider-less cache keys should only be used for a one-time migration path when provider metadata is genuinely unavailable, not as a normal read path after this PR.

[Zhang-Henry]

Review — conditional LGTM (depends on follow-up commits)

Read through the diff end-to-end with a focus on the three areas that caused #167 to be reverted (per this PR's discussion thread):

1. Temp session replacement only recognized new-session-* and broke for temp-* IDs
2. Multi-user permission regression where anonymous bootstrap could assign unowned projects to the first DB user
3. Message cache cross-contamination across providers (default fallback allowed legacy key reuse)

All three are fixed in this branch by e0b424e (+ further hardening in 8dd702b):

• Temp sessions — src/utils/sessionScope.ts:56-63 now matches both new-session-* and temp-*. Covered by src/utils/__tests__/sessionScope.test.ts:92-100.
• Owner fallback — server/projects.js:232 no longer falls back to getFirstUser() and returns null for unowned bootstrap. Covered by server/__tests__/project-sync-dedup.test.mjs:235-272.
• Cache scoping — src/components/chat/utils/sessionMessageCache.ts:23-24 makes the legacy fallback opt-in via allowLegacyFallback. Covered by src/components/chat/utils/__tests__/sessionMessageCache.test.ts:6-24.

Residual risks worth another pair of eyes

1. Blast radius of the client state refactor — useChatComposerState.ts (+1617/-492), useChatRealtimeHandlers.ts (+1468/-390), and useChatSessionState.ts (+677/-70) are a substantial rewrite of session/queue/message plumbing. Unit tests exist for the new sub-utilities (codexQueue, sessionLoadGuards, sessionSnapshotCache, projectsSessionSync), but there is no end-to-end test covering multi-provider concurrent sessions on the same project. Recommend manual smoke testing that scenario before release.
2. Lifecycle event enrichment in server/index.js (+335 lines, isKnownLifecycleProjectPath around index.js:1375-1387) validates project existence via fs.existsSync + encodeProjectPath, but has no direct unit test. Edge cases around symlinks, relative paths, or encoding could slip through. A small unit test for enrichSessionEventPayload() / buildLifecycleMessageFromPayload() would prevent future drift.
3. codexQueue.ts reconcile logic (codexQueue.ts:50-54 prepend, 94-109 reconcile) is complex. Tests cover promotion and dispatch, but there is no explicit test for order preservation under concurrent temp→settled promotions.
4. Provider arrays are scattered across 7 providers (claude, cursor, codex, gemini, openrouter, local, nano). Adding a new provider in the future will require touching multiple locations (resolveProjectSessionArrayKey, session routing, etc.) — consider a follow-up to consolidate.
5. Message cache migration — legacy fallback is opt-in, which is the right default, but users with existing localStorage entries keyed the old way may silently lose transcript history on first load. This is migration debt rather than a regression, but it's worth documenting in the changelog.

Test coverage

Strong on unit level — all three regression scenarios have targeted tests. The new sessionScope, project-sync-dedup, sessionMessageCache, projectsSessionSync, and codexQueue test files are meaningful and well-scoped.

Weak on integration level — no test for simultaneous cross-provider requests on the same session, no test for message cache migration under real cache hits, and no regression test pinned to the original #167 bug.

Recommendation

Approve if and only if e0b424e and 8dd702b are part of the merge (they are, on this branch). Before merging, please manually verify:

• Multi-provider concurrent sessions on the same project
• Message cache behavior after upgrading from an older build with persisted localStorage
• Rapid user promotion of codex queue turns

Since this is a reapply of previously-reverted code, I'd err toward extra caution and ask whoever merges to watch error telemetry for ~24h post-merge.

[everett4320]

Henry提出的问题已经都修复了，把main最新的几个commit也涵盖了进来。ops 4.6又review了一遍，又根据那个改了一下。现在应该可以了。

[everett4320]

copilot 最后提到的两个小点都优化了

[bbsngg]

Manual Testing Findings (2026-04-15)

@everett4320

1. Gemini session shows no output and displays incorrect name

• Harness: Gemini CLI 0.37.1
• Observed: After creating a session with the Gemini provider, no output is displayed. The session name stays as "Untitled Session".
• Expected: Gemini responses should render normally. Session name should be auto-generated from conversation content or default to "Gemini Session".

2. Codex queue system not activated — send button disabled during session processing

• Harness: Codex CLI 0.120.0
• Observed: When a Codex session is actively processing, the send button becomes disabled, preventing new messages from being sent. As a result, the queued turn mechanism (codexQueue) is never triggered.
• Expected: The send button should remain enabled during processing so that new messages enter the queue and are dispatched automatically after the current turn completes.

3. Processing/resuming state lost after page refresh (e.g. Codex)

• Harness: Codex CLI 0.120.0
• Observed: Refreshing the page while a Codex session is processing results in the UI showing an idle state — no processing spinner or "Resuming..." indicator.
• Expected: After refresh, the UI should restore the "Resuming..." state. Once the WebSocket reconnects and confirms the session is still running via check-session-status, the loading spinner should persist until the session actually completes or times out.

[everett4320]

Latest version merged, e975aa7. All issues fixed as Dingjie mentioned. Except the second one: queue and steer are in a separate branch and I will submit another pr once done.

[bbsngg]

PR Split Notice

This PR has been split into 6 smaller, independently reviewable PRs to reduce review complexity (55 files, +7800/-1967 → 6 focused PRs).

Split PRs

Order	PR	Scope	Base	Status
1	#183	File preview refactor (4 files)	main	Ready to merge
2	#184	Foundation types & utilities (14 files)	main	Ready to merge
3	#185	Server config migration (6 files)	main	Ready to merge
4	#186	Server session lifecycle protocol (12 files)	#185	Blocked on #185
5	#187	Frontend session scope integration (5 files)	#184	Blocked on #184
6	#188	Chat hooks session lifecycle (17 files)	#187	Blocked on #187

Merge order

#183, #184, #185  (parallel, no dependencies)
  → #186 (after #185)
  → #187 (after #184)
    → #188 (after #187, final)

Once all 6 split PRs are merged, this PR can be closed.