Fix runIntentActive state logic in handleAbortGeneration documentation - #10
Conversation
…l behavior and sensitivity
…bility with visibility-aware reconnection and delta-merging logic.
…y logic on stream reconnection
… with manual stream processing
…ream to EventSource API proper streaming of messages in real-time
…ndant reconnection message loading logic
…bouncing, and tune stream reconnection logic
📝 WalkthroughWalkthroughThis PR refactors message sending from synchronous to asynchronous, adding session reconciliation to handle concurrent session creation, and updates SSE event parsing to use raw event lines with idle debounce for session status transitions. The backend enqueues async sends while the frontend waits for SSE updates instead of processing immediate responses. ChangesAsync Message Send & SSE Event Reconciliation
Sequence DiagramsequenceDiagram
participant Client
participant App
participant API
participant SSEStream
participant OpenCodeBackend
Client->>App: send message
App->>API: POST /projects/:id/messages (async)
API->>OpenCodeBackend: send_message_async to /prompt_async
OpenCodeBackend-->>API: accepted (no message payload)
API-->>App: {sessionId, ok: true}
App->>SSEStream: listen for updates
OpenCodeBackend->>SSEStream: emit part deltas, message updates
SSEStream->>App: eventLines (classification + JSON)
App->>App: parse approvals/questions from eventLines
App->>App: reconstruct messages from part deltas
App-->>Client: render final message from SSE
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/App.tsx (2)
2341-2553:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFilter session-scoped SSE updates before mutating chat state.
This handler is subscribed at the project level, but it applies message, diff, and session-status updates straight into the active chat without checking which session emitted them. If another session in the same project is still streaming, the current view can be overwritten by foreign deltas/status changes. Use
eventMatchesSessionbefore the message/diff/session branches.Suggested fix
-import { parseApprovalFromEventLines, parseQuestionFromEventLines, classifyRawEvent, extractMessageFromEvent, extractMessagePartText, extractPartFromEvent, extractPartRemovalFromEvent, extractDiffFromEvent, extractSessionStatusFromEvent } from "./utils/streamUtils"; +import { parseApprovalFromEventLines, parseQuestionFromEventLines, classifyRawEvent, extractMessageFromEvent, extractMessagePartText, extractPartFromEvent, extractPartRemovalFromEvent, extractDiffFromEvent, extractSessionStatusFromEvent, eventMatchesSession } from "./utils/streamUtils"; ... const classification = classifyRawEvent(eventLines); if (classification.isHeartbeat) return; + const matchesActiveSession = + !activeSessionId || eventMatchesSession(eventLines, activeSessionId); ... - if (classification.hasPartDelta) { + if (matchesActiveSession && classification.hasPartDelta) { ... - if (classification.hasPartUpdate) { + if (matchesActiveSession && classification.hasPartUpdate) { ... - if (classification.hasPartRemove) { + if (matchesActiveSession && classification.hasPartRemove) { ... - if (classification.hasMessageUpdate) { + if (matchesActiveSession && classification.hasMessageUpdate) { ... - if (classification.hasDiffUpdate) { + if (matchesActiveSession && classification.hasDiffUpdate) { ... - if (classification.hasSessionUpdate) { + if (matchesActiveSession && classification.hasSessionUpdate) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/App.tsx` around lines 2341 - 2553, The SSE handler applies message/diff/session updates globally; filter out events not for the active session by calling eventMatchesSession(eventLines, activeSessionId) (or equivalent) before executing the message-related branches (the Text delta block using extractMessagePartText/setMessages, Part update using extractPartFromEvent/setMessages, Part remove using extractPartRemovalFromEvent/setMessages, Message update using extractMessageFromEvent/setMessages, Diff update using extractDiffFromEvent/setDiffEntries, and Session status using extractSessionStatusFromEvent/setRunIntentActive/loadMessages). If eventMatchesSession returns false, skip those branches (but still allow project-scoped things like approvals/questions if intended). Ensure you check both explicit messageID-derived session (e.g., partData.messageID vs lastAssistantMessageIdBySessionRef) and the event’s session metadata when deciding to apply updates.
2428-2450:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRecompute
message.textaftermessage.part.updated.This branch replaces
partsbut leavestextunchanged. When a text part is finalized or normalized bymessage.part.updated, the bubble can keep showing stale delta text until the next full refresh.Suggested fix
if (existingIdx >= 0) { updatedParts[existingIdx] = part as unknown as ChatMessage["parts"][number]; } else { updatedParts.push(part as unknown as ChatMessage["parts"][number]); } + const allText = updatedParts + .filter((p) => typeof p === "object" && p !== null && (p as Record<string, unknown>).type === "text") + .map((p) => (p as Record<string, string>).text || "") + .join("\n") + .trim(); const next = [...current]; - next[msgIndex] = { ...existing, parts: updatedParts }; + next[msgIndex] = { ...existing, parts: updatedParts, text: allText }; return next; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/App.tsx` around lines 2428 - 2450, When handling classification.hasPartUpdate in the extractPartFromEvent branch you update the message parts but do not recompute message.text, leaving the UI showing stale delta text; after you create updatedParts and before returning next from setMessages, recompute the message text (e.g., derive newText from updatedParts using the same logic/utility used elsewhere to build a ChatMessage.text from ChatMessage.parts — if you have a helper like buildMessageText/renderMessageText/normalizeMessageText call it; otherwise join/concat finalized part contents) and assign it on next[msgIndex] (next[msgIndex] = { ...existing, parts: updatedParts, text: newText }) so the bubble reflects the updated/normalized part content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/routes/helpers.py`:
- Around line 751-757: The reconciliation probe calling _latest_project_session
can raise requests.HTTPError and currently causes the outer except to clear
project.last_session_id even when get_session succeeded; isolate the probe by
wrapping the _latest_project_session(...) call in its own try/except that
catches requests.HTTPError (and optionally logs it) and falls back to not
updating the project (i.e., treat latest as None or equal to
candidate_session_id), so that a transient list_sessions failure cannot
overwrite the validated candidate_session_id; update the logic around latest =
_latest_project_session(project, opencode_client) in the same block to use this
guarded call and avoid touching project.last_session_id or last_activity_at on
probe failures.
- Around line 751-757: The _resolve_project_session function currently always
calls _latest_project_session and may override a caller-supplied session_id by
updating project.last_session_id and returning latest; change this so the "newer
session" auto-advance only happens when the caller did NOT explicitly provide
session_id (i.e., when resolving from project.last_session_id/fallback) or add a
boolean parameter (e.g., allow_auto_advance) to _resolve_project_session that
defaults to False for API paths that accept sessionId; ensure the check that
assigns project.last_session_id, updates last_activity_at, and commits only runs
when session_id was not provided (or allow_auto_advance is True), avoiding the
extra _latest_project_session call for explicit session requests.
- Around line 724-729: The helper currently overrides an explicitly requested
session and raises/clears state when listing latest sessions fails; change
_resolve_project_session to always honor a provided session_id (do not return
"latest" when _latest_project_session finds a newer session), only fall back to
latest when no session_id was requested, and avoid calling
opencode_client.list_sessions unless needed to resolve a true "latest" request;
in _latest_project_session, catch requests.HTTPError and return None instead of
propagating (do not clear project.last_session_id or raise ValueError if
get_session(candidate_session_id) already succeeded and _session_matches_project
passed); also make directory handling consistent by using
_normalize_project_path(project.path) in both _list_project_sessions and
_latest_project_session so upstream path logic is uniform.
In `@backend/app/routes/messages.py`:
- Around line 101-118: The code sets project.session_status = "idle" immediately
after calling send_message_async (in messages.py), which incorrectly overrides
the "running" state set earlier; instead remove or avoid resetting
session_status to "idle" after opencode_client.send_message_async so the session
remains "running" and the SSE-driven busy/idle state transitions (and
_project_to_dict) reflect the real agent processing state; keep updating
project.last_message_preview, project.last_activity_at, and commit the DB but do
not set session_status to "idle" here (or only set it conditionally based on
explicit confirmation from the SSE flow).
In `@backend/app/routes/opencode.py`:
- Around line 69-82: The endpoint opencode_sse_info currently returns
opencode_client.auth_header (the upstream Basic {base64(user:pass)}) which leaks
credentials; update the opencode_sse_info handler to stop including auth in the
JSON response (remove the "auth" key) and instead either proxy the SSE through
the backend (use opencode_client.health()/settings.opencode_base_url only to
validate/redirect and stream events server-side) or implement and return a
short-lived scoped token from the server rather than
opencode_client.auth_header; ensure any client-facing payload contains only
non-sensitive fields (e.g., "url" and "healthy") and keep authentication details
strictly server-side in opencode_client or the proxy logic.
In `@frontend/src/App.tsx`:
- Around line 2391-2403: The SSE handler creates or upserts assistant messages
but doesn't update lastAssistantMessageIdBySessionRef, so subsequent streaming
chunks without messageID bind to an older assistant message; update
lastAssistantMessageIdBySessionRef.current[activeSessionId ?? ""] whenever you
create or upsert an assistant message inside the setMessages block (the branch
that constructs a new assistant message and the branch that updates an existing
message), ensuring the ref is set to the new/updated message id immediately
after creating/upserting (so later SSE parts without messageID will resolve to
the just-created assistant message); also apply the same update in the other SSE
handling block around lines 2483-2507 where assistant messages are
created/updated.
- Around line 3566-3577: After the async send returns, immediately switch the UI
to the reconciled session id so subsequent SSE events are interpreted against
it: call the state updater to set the active session to result.sessionId (e.g.
setActiveSessionId(result.sessionId)) right after setting
awaitingFinalReplyNotificationByChatRef.current[...] and before any refresh/load
calls; keep the existing setMessages(...) cleanup but ensure it runs after
setting the active session so UI and SSE handling use the new session id.
---
Outside diff comments:
In `@frontend/src/App.tsx`:
- Around line 2341-2553: The SSE handler applies message/diff/session updates
globally; filter out events not for the active session by calling
eventMatchesSession(eventLines, activeSessionId) (or equivalent) before
executing the message-related branches (the Text delta block using
extractMessagePartText/setMessages, Part update using
extractPartFromEvent/setMessages, Part remove using
extractPartRemovalFromEvent/setMessages, Message update using
extractMessageFromEvent/setMessages, Diff update using
extractDiffFromEvent/setDiffEntries, and Session status using
extractSessionStatusFromEvent/setRunIntentActive/loadMessages). If
eventMatchesSession returns false, skip those branches (but still allow
project-scoped things like approvals/questions if intended). Ensure you check
both explicit messageID-derived session (e.g., partData.messageID vs
lastAssistantMessageIdBySessionRef) and the event’s session metadata when
deciding to apply updates.
- Around line 2428-2450: When handling classification.hasPartUpdate in the
extractPartFromEvent branch you update the message parts but do not recompute
message.text, leaving the UI showing stale delta text; after you create
updatedParts and before returning next from setMessages, recompute the message
text (e.g., derive newText from updatedParts using the same logic/utility used
elsewhere to build a ChatMessage.text from ChatMessage.parts — if you have a
helper like buildMessageText/renderMessageText/normalizeMessageText call it;
otherwise join/concat finalized part contents) and assign it on next[msgIndex]
(next[msgIndex] = { ...existing, parts: updatedParts, text: newText }) so the
bubble reflects the updated/normalized part content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 949c28e0-d1ab-4141-b879-9ea41523aadc
📒 Files selected for processing (8)
backend/app/opencode.pybackend/app/routes/helpers.pybackend/app/routes/messages.pybackend/app/routes/opencode.pyfrontend/src/App.tsxfrontend/src/api.tsfrontend/src/utils/miscUtils.tsfrontend/src/utils/streamUtils.ts
📜 Review details
🧰 Additional context used
🪛 Ruff (0.15.14)
backend/app/routes/opencode.py
[warning] 81-81: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (5)
frontend/src/api.ts (1)
142-145: LGTM!frontend/src/utils/miscUtils.ts (1)
49-52: LGTM!frontend/src/utils/streamUtils.ts (1)
27-206: LGTM!backend/app/opencode.py (1)
17-19: LGTM!Also applies to: 184-212
backend/app/routes/messages.py (1)
361-363: LGTM!
| def _latest_project_session(project: Project, opencode_client) -> str | None: | ||
| sessions = opencode_client.list_sessions(limit=1, directory=_normalize_project_path(project.path)) | ||
| for session in sessions: | ||
| if _session_matches_project(project, session): | ||
| return str(session.get("id") or "").strip() | ||
| return None |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
opencode list sessions API GET /session ordering newest first default sort
💡 Result:
For OpenCode’s session listing endpoint, the “newest first” behavior is achieved by ordering sessions by last-updated timestamp in descending order (time_updated DESC). In OpenCode’s API implementation, the “List sessions” route is introduced for “GET /session” and the route description explicitly states it returns sessions “sorted by most recently updated” (and that archived sessions are excluded by default). [1] Additionally, the project maintainers discussed that the previous GET /session implementation lacked a database ORDER BY clause, which meant SQLite would return sessions in an undefined internal storage order, making results arbitrary when using limit. The requested fix was to return sessions in time_updated DESC order so clients can reliably request “the N most recent sessions” and have deterministic ordering. [2] Answering your query directly: - Endpoint: GET /session (OpenCode sessions listing) - Default ordering: newest first via ORDER BY time_updated DESC (most recently updated first) [1][2] Note: Some OpenCode documentation/third-party API docs refer to GET /api/opencode/:port/sessions, but those are for a wrapper/instance URL and don’t change the underlying “newest first” ordering requirement described above. [3]
Citations:
- 1: hongymagic/opencode@7419ebc
- 2: GET /session: Add ORDER BY time_updated DESC to Session.list() for deterministic ordering and correct limit behavior anomalyco/opencode#13569
- 3: https://hosenur-portal.mintlify.app/api/sessions
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="backend/app/routes/helpers.py"
echo "== file =="
ls -la "$FILE"
echo
echo "== lines 680-820 =="
nl -ba "$FILE" | sed -n '680,820p'
echo
echo "== find _list_project_sessions and _sort_sessions_desc definitions =="
rg -n "_list_project_sessions|_sort_sessions_desc|_latest_project_session|_resolve_project_session" "$FILE"
echo
echo "== show definitions for _list_project_sessions and _session_matches_project (if present) =="
# Print around match occurrences of function defs (small windows)
for pat in "_list_project_sessions" "_sort_sessions_desc" "_session_matches_project" "_resolve_project_session" "_latest_project_session"; do
line=$(rg -n "def ${pat}\b" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${line}" ]; then
start=$((line-40)); end=$((line+80))
echo
echo "== around def $pat (approx $start-$end) =="
nl -ba "$FILE" | sed -n "${start},${end}p"
fi
doneRepository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="backend/app/routes/helpers.py"
echo "== file =="
ls -la "$FILE"
echo
echo "== lines 680-820 =="
cat -n "$FILE" | sed -n '680,820p'
echo
echo "== rg hits =="
rg -n "_list_project_sessions|_sort_sessions_desc|_latest_project_session|_resolve_project_session" "$FILE" || true
echo
echo "== show defs windows =="
for pat in "_list_project_sessions" "_sort_sessions_desc" "_latest_project_session" "_resolve_project_session"; do
line=$(rg -n "def ${pat}\b" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${line}" ]; then
start=$((line-60)); end=$((line+120))
echo
echo "== around $pat ($start-$end) =="
cat -n "$FILE" | sed -n "${start},${end}p"
fi
doneRepository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 41135
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "def _normalize_project_path\b|_normalize_project_path\(" backend/app/routes/helpers.py
rg -n "def _session_matches_project\b|_session_matches_project\(" backend/app/routes/helpers.py
# Locate opencode_client usage/implementation (list_sessions, get_session)
rg -n "def list_sessions\b|list_sessions\(" backend/app | head -n 200
rg -n "def get_session\b|get_session\(" backend/app | head -n 200
# Also find the opencode client module/class if imported
rg -n "opencode_client" backend/app/routes -S | head -n 200Repository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 11987
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE_HELPERS="backend/app/routes/helpers.py"
FILE_OPENCODE="backend/app/opencode.py"
echo "== _normalize_project_path & _session_matches_project =="
# show around normalize
start=$(rg -n "def _normalize_project_path\b" "$FILE_HELPERS" | cut -d: -f1)
sed_start=$((start-10)); sed_end=$((start+80))
sed -n "${sed_start},${sed_end}p" "$FILE_HELPERS" | cat -n
echo
echo "== _session_matches_project =="
match_line=$(rg -n "def _session_matches_project\b" "$FILE_HELPERS" | cut -d: -f1)
sed_start=$((match_line-10)); sed_end=$((match_line+80))
sed -n "${sed_start},${sed_end}p" "$FILE_HELPERS" | cat -n
echo
echo "== backend/app/opencode.py: list_sessions & get_session =="
# show list_sessions
ls_line=$(rg -n "def list_sessions\b" "$FILE_OPENCODE" | cut -d: -f1)
sed_start=$((ls_line-10)); sed_end=$((ls_line+120))
sed -n "${sed_start},${sed_end}p" "$FILE_OPENCODE" | cat -n
echo
echo "== backend/app/opencode.py: get_session =="
gs_line=$(rg -n "def get_session\b" "$FILE_OPENCODE" | cut -d: -f1)
sed_start=$((gs_line-10)); sed_end=$((gs_line+120))
sed -n "${sed_start},${sed_end}p" "$FILE_OPENCODE" | cat -nRepository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 18165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "Project\.(path|last_session_id|path\s*=" backend/app | head -n 200 || true
rg -n "project\.path\s*=|path\s*=\s*.*project" backend/app | head -n 200 || true
rg -n "_normalize_project_path\(" backend/app | head -n 200Repository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 1947
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "project\.path\s*=" backend/app | head -n 200 || true
rg -n "last_session_id\s*=" backend/app | head -n 200 || true
rg -n "path\s*=\s*.*project" backend/app | head -n 200 || true
rg -n "_normalize_project_path\(" backend/app/routes/projects.py backend/app/routes/sessions.py backend/app/routes/helpers.py | head -n 200 || trueRepository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 3018
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="backend/app/routes/sessions.py"
echo "== lines 1-220 =="
cat -n "$FILE" | sed -n '1,220p'
echo
echo "== lines 220-360 =="
cat -n "$FILE" | sed -n '220,360p'
echo
echo "== rg _ensure_project_session / _resolve_project_session usage =="
rg -n "_ensure_project_session|_resolve_project_session" "$FILE"Repository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 15819
🌐 Web query:
OpenCode GET /session directory parameter normalization list_sessions implementation
💡 Result:
OpenCode’s session listing endpoint uses the directory query parameter as a directory-scoping filter, but on Windows it previously failed when the client sent Windows paths with forward slashes (e.g., C:/Users/...) while the server stored/compared them with backslashes (e.g., C:\Users\...)—because the match was effectively an exact string match. This caused /session?directory=... to return an empty array and prevented sessions from showing up in the Web UI sidebar. Evidence of the failure mode (forward slash vs backslash): One bug report describes the web UI JavaScript sending directory with forward slashes to GET /session?directory=...&roots=true, while the server stores directory paths with backslashes, leading to exact-string mismatch and [] results for forward-slash input (but correct results for backslash input). [1] What was implemented/fixed (server-side normalization): OpenCode maintainers introduced fixes specifically described as normalizing Windows path separators for “session queries” (PR #20215), aligning the server-side handling of the directory query parameter so it doesn’t depend on whether callers used / or \ in the URL. [2] Related/companion issues reinforce the general rule: Additional issues report that Windows path variants (separator differences and other mismatches) can be treated as different directories in session/workspace identification, and recommend normalizing Windows paths before using them as directory identifiers. [3][4] So the “directory parameter normalization list” for GET /session (as of the fixes discussed) is: 1) Normalize Windows path separators so forward slashes and backslashes for the same directory compare equivalently (e.g., convert / to \ or vice versa consistently on the server when interpreting directory). [1][2] 2) Ensure the normalized directory identifier is used consistently for session filtering/storage keys so list results include the expected sessions. [1][3] Implementation note: The exact route handler code is present in the OpenCode repo under the session routes (e.g., packages/opencode/src/server/routes/session.ts), and the normalization behavior is what the above PR/issue trail targets. [5][6] If you want, I can also extract the exact normalization function(s) used in the current route implementation by following the relevant commit/PR(s) in that file and reporting the precise transformation steps (slash conversion, drive-letter case handling, trimming, etc.).
Citations:
- 1: Web UI on Windows: sessions not displayed due to path separator mismatch in /session API queries anomalyco/opencode#25565
- 2: fix: normalize windows path separators in session queries anomalyco/opencode#20215
- 3: Sessions missing from sidebar on Windows due to path separator mismatch anomalyco/opencode#23864
- 4: [Bug] Windows paths with different separators (backslash vs forward slash) treated as different directories in Web UI anomalyco/opencode#12940
- 5: https://github.com/sst/opencode/blob/9ad6588f/packages/opencode/src/server/routes/session.ts
- 6: https://github.com/sst/opencode/blob/c7b35342/packages/opencode/src/server/routes/session.ts
🌐 Web query:
site:github.com opencode packages/opencode/src/server/routes/session.ts directory normalize _normalizeSessionDirectory time_updated desc
💡 Result:
In the OpenCode backend, the route that lists sessions is implemented in packages/opencode/src/server/routes/session.ts, and it returns sessions ordered by most recently updated via time_updated (desc) (and typically desc SessionTable.id as a tie-breaker) [1][2]. About _normalizeSessionDirectory: I could not find any occurrence of a function named “_normalizeSessionDirectory” (or similar) in the sources that were surfaced from your specified file path/session code during this research. What I did find is that session directory handling is done via “resolve directory when list sessions” and via query parameter directory matching/filtering, including fixes for Windows path separator differences (E:... vs E:/...) [3][4]. So, for your exact query: 1) packages/opencode/src/server/routes/session.ts includes the “List sessions” endpoint that uses time_updated to sort by most recently updated [2]. 2) The specific helper name “_normalizeSessionDirectory” was not evidenced in the discovered code/PRs; however, directory normalization/resolution for the session list is clearly addressed in recent PRs, including normalizing Windows separators (fix #20215) [4] and resolving directory for /session listing (fix #22835) [3].
Citations:
- 1: https://github.com/sst/opencode/blob/8b2f8355/packages/opencode/src/session/session.ts
- 2: https://github.com/sst/opencode/blob/9ad6588f/packages/opencode/src/server/routes/session.ts
- 3: fix(opencode): resolve directory when list sessions anomalyco/opencode#22835
- 4: fix: normalize windows path separators in session queries anomalyco/opencode#20215
Don’t override the requested sessionId, and don’t fail the switch if the “latest session” lookup fails.
backend/app/routes/helpers.py(747-768):_resolve_project_sessioncalls_latest_project_session()and, when it finds a different newer session, it returnslatestinstead of honoring the requestedsession_id.backend/app/routes/helpers.py(747-768, 724-729): ifopencode_client.list_sessions(...)(inside_latest_project_session) raisesrequests.HTTPError, the outerexceptclearsproject.last_session_idand raisesValueError("Selected session could not be loaded")even though the candidateget_session(candidate_session_id)already succeeded and passed_session_matches_project.backend/app/routes/helpers.py(724-729): this behavior also introduces an extra HTTP round-trip on every session resolution/selection.- Optional:
_list_project_sessionspassesdirectory=project.pathwhile_latest_project_sessionpassesdirectory=_normalize_project_path(project.path)—make directory normalization consistent to avoid relying on upstream path handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/routes/helpers.py` around lines 724 - 729, The helper currently
overrides an explicitly requested session and raises/clears state when listing
latest sessions fails; change _resolve_project_session to always honor a
provided session_id (do not return "latest" when _latest_project_session finds a
newer session), only fall back to latest when no session_id was requested, and
avoid calling opencode_client.list_sessions unless needed to resolve a true
"latest" request; in _latest_project_session, catch requests.HTTPError and
return None instead of propagating (do not clear project.last_session_id or
raise ValueError if get_session(candidate_session_id) already succeeded and
_session_matches_project passed); also make directory handling consistent by
using _normalize_project_path(project.path) in both _list_project_sessions and
_latest_project_session so upstream path logic is uniform.
| # Check if a newer session exists for this directory (e.g., created by TUI) | ||
| latest = _latest_project_session(project, opencode_client) | ||
| if latest and latest != candidate_session_id: | ||
| project.last_session_id = latest | ||
| project.last_activity_at = _utc_now() | ||
| db.session.commit() | ||
| return latest |
There was a problem hiding this comment.
A transient list error now invalidates an already-validated session.
_latest_project_session issues a fresh HTTP call inside this try block. If list_sessions raises requests.HTTPError, it is caught by the except requests.HTTPError handler below, which sets project.last_session_id = None and raises "Selected session could not be loaded" — even though get_session already succeeded and matched the project. A transient failure while probing for a newer session should not discard the valid candidate.
Consider isolating the reconciliation probe so it can't poison the candidate resolution:
🛡️ Guard the reconciliation probe
if _session_matches_project(project, session):
# Check if a newer session exists for this directory (e.g., created by TUI)
- latest = _latest_project_session(project, opencode_client)
+ try:
+ latest = _latest_project_session(project, opencode_client)
+ except requests.HTTPError:
+ latest = None
if latest and latest != candidate_session_id:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Check if a newer session exists for this directory (e.g., created by TUI) | |
| latest = _latest_project_session(project, opencode_client) | |
| if latest and latest != candidate_session_id: | |
| project.last_session_id = latest | |
| project.last_activity_at = _utc_now() | |
| db.session.commit() | |
| return latest | |
| # Check if a newer session exists for this directory (e.g., created by TUI) | |
| try: | |
| latest = _latest_project_session(project, opencode_client) | |
| except requests.HTTPError: | |
| latest = None | |
| if latest and latest != candidate_session_id: | |
| project.last_session_id = latest | |
| project.last_activity_at = _utc_now() | |
| db.session.commit() | |
| return latest |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/routes/helpers.py` around lines 751 - 757, The reconciliation
probe calling _latest_project_session can raise requests.HTTPError and currently
causes the outer except to clear project.last_session_id even when get_session
succeeded; isolate the probe by wrapping the _latest_project_session(...) call
in its own try/except that catches requests.HTTPError (and optionally logs it)
and falls back to not updating the project (i.e., treat latest as None or equal
to candidate_session_id), so that a transient list_sessions failure cannot
overwrite the validated candidate_session_id; update the logic around latest =
_latest_project_session(project, opencode_client) in the same block to use this
guarded call and avoid touching project.last_session_id or last_activity_at on
probe failures.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '_resolve_project_session\s*\(' -C2Repository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 3299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section in helpers.py
sed -n '700,820p' backend/app/routes/helpers.py | nl -ba | sed 's/^/helpers: /'
# Show call sites that pass requested_session_id (sessions.py)
sed -n '240,360p' backend/app/routes/sessions.py | nl -ba | sed 's/^/sessions: /'
# Show call sites that pass requested_session_id (messages.py)
sed -n '1,220p' backend/app/routes/messages.py | nl -ba | sed 's/^/messages: /'
sed -n '220,340p' backend/app/routes/messages.py | nl -ba | sed 's/^/messages: /'Repository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== helpers.py (around _resolve_project_session) ==="
sed -n '700,820p' backend/app/routes/helpers.py | cat -n
echo "=== sessions.py (around requested_session_id resolution) ==="
sed -n '220,360p' backend/app/routes/sessions.py | cat -n
echo "=== messages.py (around requested_session_id resolution) ==="
sed -n '1,220p' backend/app/routes/messages.py | cat -n
echo "=== messages.py (later occurrences) ==="
sed -n '220,360p' backend/app/routes/messages.py | cat -nRepository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 27953
Prevent overriding an explicitly requested session ID with “latest”
In backend/app/routes/helpers.py (_resolve_project_session), when the resolved candidate_session_id matches the project, the code always calls _latest_project_session(...) and, if it finds a different newer session, commits and returns latest—even when the caller supplied session_id. This means API endpoints that accept sessionId (e.g. send_project_message, compact/summarize/abort) can act on/return a different session than the one requested.
This also introduces an extra upstream call (list_sessions(limit=1, ...) via _latest_project_session) on every successful _resolve_project_session for message/approval/question paths where a candidate session exists.
# Check if a newer session exists for this directory (e.g., created by TUI)
latest = _latest_project_session(project, opencode_client)
if latest and latest != candidate_session_id:
project.last_session_id = latest
project.last_activity_at = _utc_now()
db.session.commit()
return latestConsider gating the “newer session” auto-advance to only run when session_id was not explicitly provided (i.e., when falling back to project.last_session_id), or by adding a parameter/flag to _resolve_project_session to control this behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/routes/helpers.py` around lines 751 - 757, The
_resolve_project_session function currently always calls _latest_project_session
and may override a caller-supplied session_id by updating
project.last_session_id and returning latest; change this so the "newer session"
auto-advance only happens when the caller did NOT explicitly provide session_id
(i.e., when resolving from project.last_session_id/fallback) or add a boolean
parameter (e.g., allow_auto_advance) to _resolve_project_session that defaults
to False for API paths that accept sessionId; ensure the check that assigns
project.last_session_id, updates last_activity_at, and commits only runs when
session_id was not provided (or allow_auto_advance is True), avoiding the extra
_latest_project_session call for explicit session requests.
| @api_bp.get("/opencode/sse-info") | ||
| @auth_required | ||
| def opencode_sse_info(): | ||
| try: | ||
| data = opencode_client.health() | ||
| base_url = settings.opencode_base_url | ||
| auth_header = opencode_client.auth_header | ||
| return jsonify({ | ||
| "url": base_url, | ||
| "auth": auth_header, | ||
| "healthy": True, | ||
| }) | ||
| except Exception as exc: | ||
| return jsonify({"healthy": False, "error": str(exc)}), 502 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'sse-info|\.auth\b' -g '*.ts' -g '*.tsx' -C3Repository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 64
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/app/auth.py =="
ls -la backend/app/auth.py || true
sed -n '1,220p' backend/app/auth.py 2>/dev/null || true
echo "== Find opencode_client definition and auth_header usage =="
rg -n "auth_header|opencode_base_url|opencode_client" backend/app -S || true
echo "== Find /opencode/sse-info usage anywhere (not just TS/TSX) =="
rg -n "/opencode/sse-info|sse-info" -S . || trueRepository: Rishabh-Bajpai/mobile-opencode-control
Length of output: 13728
Stop returning upstream Basic Auth credentials in /opencode/sse-info.
backend/app/routes/opencode.py exposes opencode_client.auth_header as JSON "auth" (constructed as Basic {base64(user:pass)} in backend/app/opencode.py), so any authenticated browser client receives the upstream username/password-equivalent credential. @auth_required only checks the Flask session (backend/app/auth.py) and doesn’t restrict what this endpoint returns.
Prefer proxying SSE through the backend (keep Authorization server-side) or use a short-lived scoped token instead of the static Basic header.
🧰 Tools
🪛 Ruff (0.15.14)
[warning] 81-81: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/routes/opencode.py` around lines 69 - 82, The endpoint
opencode_sse_info currently returns opencode_client.auth_header (the upstream
Basic {base64(user:pass)}) which leaks credentials; update the opencode_sse_info
handler to stop including auth in the JSON response (remove the "auth" key) and
instead either proxy the SSE through the backend (use
opencode_client.health()/settings.opencode_base_url only to validate/redirect
and stream events server-side) or implement and return a short-lived scoped
token from the server rather than opencode_client.auth_header; ensure any
client-facing payload contains only non-sensitive fields (e.g., "url" and
"healthy") and keep authentication details strictly server-side in
opencode_client or the proxy logic.
| if (partData && partData.text != null && partData.partID && (partData.messageID || activeSessionId)) { | ||
| const targetMsgId = partData.messageID || lastAssistantMessageIdBySessionRef.current[activeSessionId ?? ""]; | ||
| if (!targetMsgId) return; | ||
| setMessages((current) => { | ||
| const msgIndex = current.findIndex((m) => m.id === targetMsgId); | ||
| if (msgIndex < 0) return current; | ||
|
|
||
| if (msgIndex < 0) { | ||
| return [...current, { | ||
| id: targetMsgId, | ||
| role: "assistant", | ||
| createdAt: new Date().toISOString(), | ||
| text: partData.text || "", | ||
| parts: partData.text ? [{ type: "text", id: partData.partID, text: partData.text }] : [], | ||
| }]; |
There was a problem hiding this comment.
Advance lastAssistantMessageIdBySessionRef when SSE creates/upserts an assistant message.
Line 2392 falls back to lastAssistantMessageIdBySessionRef for deltas without messageID, but this ref is only refreshed by loadMessages(). Once a new assistant reply starts streaming, later chunks can still bind to the previous assistant message until a full reload happens.
Suggested fix
if (classification.hasPartDelta) {
const partData = extractMessagePartText(eventLines);
if (partData && partData.text != null && partData.partID && (partData.messageID || activeSessionId)) {
const targetMsgId = partData.messageID || lastAssistantMessageIdBySessionRef.current[activeSessionId ?? ""];
if (!targetMsgId) return;
+ const sessionKey = partData.sessionID ?? activeSessionId;
+ if (sessionKey) {
+ lastAssistantMessageIdBySessionRef.current = {
+ ...lastAssistantMessageIdBySessionRef.current,
+ [sessionKey]: targetMsgId,
+ };
+ }
setMessages((current) => {
const msgIndex = current.findIndex((m) => m.id === targetMsgId);
...
if (classification.hasMessageUpdate) {
const info = extractMessageFromEvent(eventLines);
if (info && typeof info.id === "string" && typeof info.role === "string") {
const infoId = info.id;
const infoRole = info.role as ChatMessage["role"];
+ if (infoRole === "assistant" && activeSessionId) {
+ lastAssistantMessageIdBySessionRef.current = {
+ ...lastAssistantMessageIdBySessionRef.current,
+ [activeSessionId]: infoId,
+ };
+ }
setMessages((current) => {Also applies to: 2483-2507
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/App.tsx` around lines 2391 - 2403, The SSE handler creates or
upserts assistant messages but doesn't update
lastAssistantMessageIdBySessionRef, so subsequent streaming chunks without
messageID bind to an older assistant message; update
lastAssistantMessageIdBySessionRef.current[activeSessionId ?? ""] whenever you
create or upsert an assistant message inside the setMessages block (the branch
that constructs a new assistant message and the branch that updates an existing
message), ensuring the ref is set to the new/updated message id immediately
after creating/upserting (so later SSE parts without messageID will resolve to
the just-created assistant message); also apply the same update in the other SSE
handling block around lines 2483-2507 where assistant messages are
created/updated.
…, and adjust reconnection backoff parameters
|
@copilot Please review the PR and all its comments, and then fix the issues. |
… narrowing in sendMessage handler
Reviewed all PR comments and made the following fixes:
All 10 backend tests pass and the frontend builds cleanly with no TypeScript errors. |
Summary by CodeRabbit
New Features
Performance
Improvements
known bugs: multiple session and chats are not working at the moment
The
per-session-runintentactive.mdrefactoring guide contained a logic error in thehandleAbortGenerationsection, incorrectly specifying thatrunIntentActiveshould be set totruewhen aborting.Changes:
setRunIntentActive(true)tosetRunIntentActive(false)in the "before" reference and updated the "after" state update accordinglyRationale:
When aborting a session,
runIntentActiveshould be cleared (set tofalse), not set totrue. Theabortingflag handles the abort operation state;runIntentActiveshould only betruewhen an intent/command is actively executing. This aligns with thefinallyblock cleanup which also clears the flag.