Skip to content

fix(web): stabilize /web handoff and preserve session migration#1073

Open
Fengzdadi wants to merge 2 commits intoMoonshotAI:mainfrom
Fengzdadi:fix/web-switch-session-handoff
Open

fix(web): stabilize /web handoff and preserve session migration#1073
Fengzdadi wants to merge 2 commits intoMoonshotAI:mainfrom
Fengzdadi:fix/web-switch-session-handoff

Conversation

@Fengzdadi
Copy link

@Fengzdadi Fengzdadi commented Feb 9, 2026

Related Issue

Related discussions:

Description

This PR fixes /web session handoff reliability when switching from CLI shell mode to Web UI.

What changed:

  1. Preserve session on /web switch, including empty sessions, so handoff does not lose the current session context.
  2. Carry the current CLI session_id into Web startup and append it to browser URL as ?session=... (coexisting with token query if present).
  3. Before launching Web server in the /web switch path, restore process stderr to avoid same-process stderr redirection interaction causing server hang/timeouts.

Manual verification performed locally:

  • Start CLI via uv run python -m kimi_cli.cli
  • Run /web
  • Verify /healthz responds quickly
  • Verify Web URL includes session=<current_session_id>
  • Verify Web connects to the intended session instead of hanging on “Connecting to session...”

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked the related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.
    Tested locally; I intentionally did not include additional test files in this PR to keep scope minimal.
  • I have run make gen-changelog to update the changelog.
  • I have run make gen-docs to update the user documentation.

Open with Devin

Copilot AI review requested due to automatic review settings February 9, 2026 20:03
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves reliability when switching from CLI shell mode to the Web UI (/web) by preserving the active session across the handoff, propagating the session id into the Web UI URL, and avoiding stderr redirection side-effects during Web startup.

Changes:

  • Add initial_session_id support to Web server startup and include it in the browser URL query string.
  • Preserve empty sessions during /web switching so session context isn’t dropped.
  • Add a stderr “restore” path to undo process-level stderr redirection before starting the Web server.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/kimi_cli/web/app.py Adds initial_session_id param and builds browser URL query via urlencode.
src/kimi_cli/utils/logging.py Introduces stderr redirector uninstall + a restore_stderr() helper.
src/kimi_cli/cli/__init__.py Preserves empty sessions on /web switch and restores stderr before launching Web server with session id.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +427 to +432
query: dict[str, str] = {}
if session_token:
query["token"] = session_token
if initial_session_id:
query["session"] = initial_session_id
browser_url = f"{url}/?{urlencode(query)}" if query else url
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_url() now builds browser_url with session when initial_session_id is set, but downstream the banner currently only prints the “URL with token” when session_token is present. This means in local/no-token mode the displayed URLs can omit the session=... parameter even though the browser-open URL includes it, making copy/paste handoff inconsistent. Consider treating any non-empty query (token or session) as the “URL with params” and updating the banner logic/variable naming accordingly.

Copilot uses AI. Check for mistakes.
Comment on lines 48 to 55
def uninstall(self) -> None:
with self._lock:
if not self._installed:
return
if self._original_fd is not None:
with contextlib.suppress(OSError):
os.dup2(self._original_fd, 2)
self._installed = False
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uninstall() marks the redirector as not installed even if _original_fd is None (e.g., if os.dup(2) failed during install() due to an OSError being suppressed). In that case fd=2 may remain pointing at the pipe, but future logic will believe stderr is restored. Consider making install() fail (or skip redirecting) if the original fd can’t be duplicated, or make uninstall() restore from sys.__stderr__.fileno() / otherwise ensure fd=2 is not left redirected before setting _installed = False.

Copilot uses AI. Check for mistakes.
Comment on lines 575 to +579
if e.session_id is not None:
session = await Session.find(work_dir, e.session_id)
if session is not None:
await _post_run(session, True)
return True
await _post_run(session, True, preserve_empty_session=True)
return True, e.session_id
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /web switch path now has new behavior that’s easy to regress (preserving empty sessions and passing initial_session_id into run_web_server). The repo already has pytest coverage for CLI/session behavior; adding a focused test around the SwitchToWeb handling (metadata update + empty-session preservation and/or asserted URL query composition) would help keep the handoff stable.

Copilot uses AI. Check for mistakes.
@Fengzdadi
Copy link
Author

make_url already included session, but the banner previously selected display output based only on session_token, which could hide ?session=... in local/no-token mode.

I updated banner rendering to always print the full URL returned by make_url(...) (including query params when present), so copy/paste from terminal remains consistent with the browser-open URL.

I also validated this manually in /web flow:

  • banner shows ?session=<id>
  • Web opens and connects to the intended session
  • /healthz and session APIs respond normally
image

Displays normally in a large window.

image

However, it might be rendered like this in a small window; is that acceptable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments