Skip to content

fix(cli): import cloud sessions before validation - #11223

Merged
catrielmuller merged 4 commits into
Kilo-Org:mainfrom
maphew:fix/cli-cloud-fork-session-import
Jul 6, 2026
Merged

fix(cli): import cloud sessions before validation#11223
catrielmuller merged 4 commits into
Kilo-Org:mainfrom
maphew:fix/cli-cloud-fork-session-import

Conversation

@maphew

@maphew maphew commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Import --cloud-fork sessions before validating the local session ID in TUI startup.
  • Apply the same ordering to daemon-backed TUI attach so validation uses the imported local session ID.
  • Add regression coverage and a patch changeset for the CLI fix.

Fixes #11222

Signed-off-by: gpt-5.5 on behalf of matt

return { data: { id: "ses_local" } }
},
},
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WARNING: expect inside mock will be silently swallowed

The expect(input.sessionId).toBe("ses_cloud") assertion sits inside the mocked import function. When importCloudSession is called, it invokes this mock — but the call site wraps the whole thing with .catch(() => undefined) (see thread.ts line 38). If this assertion throws (e.g. the wrong session ID is passed), the error is caught and id becomes undefined, causing an early return with fork.ok = false. The test then only fails on the later expect(seen).toEqual(["ses_local"]) with a confusing message rather than pointing directly at the bad session ID.

Consider moving the assertion out of the mock and instead capturing the received sessionId in a variable, then asserting after the await:

let importedId: string | undefined
mock.module("@kilocode/sdk/v2", () => ({
  createKiloClient: () => ({
    kilo: {
      cloud: {
        session: {
          import: async (input: { sessionId: string }) => {
            importedId = input.sessionId
            return { data: { id: "ses_local" } }
          },
        },
      },
    },
  }),
}))
// ... after `await mod.KiloTuiThreadDaemon.attach(...)`
expect(importedId).toBe("ses_cloud")

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/test/kilocode/cli/tui/thread.test.ts 34 expect inside mock is wrapped by .catch(() => undefined) — assertion failure is silently swallowed, test would fail with a misleading message on the later expect(seen) check
Other Observations (not in diff)

Mock-heavy test: The new test uses 5 mock.module calls. The AGENTS.md guide discourages heavy mocking. In this case the mocks appear necessary given the depth of integration (daemon client, SDK, TUI config), so this is a judgment call rather than a hard violation. Consider whether importCloudSession from @/kilocode/cloud-session (which is NOT mocked) could be stubbed at a higher level in a future refactor to reduce the mock surface.

process.exitCode = 1 side-effect in session() helper: packages/opencode/src/kilocode/cli/cmd/tui/thread.ts line 42 mutates process.exitCode on failure. The test doesn't verify this side-effect is absent on the success path; this is a pre-existing pattern and not introduced by this PR, but worth keeping in mind if test isolation becomes a concern later.

Files Reviewed (4 files)
  • .changeset/cloud-fork-session-import.md — changeset looks good, user-facing description is clear
  • packages/opencode/src/cli/cmd/tui/thread.ts — fix is correct; validateSession now runs after cloud import, using the local session ID
  • packages/opencode/src/kilocode/cli/cmd/tui/thread.ts — daemon path fix is correct; session() called before validateSession, fork.id passed through consistently
  • packages/opencode/test/kilocode/cli/tui/thread.test.ts — 1 issue (see above)

Fix these issues in Kilo Cloud


Reviewed by claude-4.6-sonnet-20260217 · 676,008 tokens

Review guidance: REVIEW.md from base branch main

@johnnyeric
johnnyeric requested a review from catrielmuller June 15, 2026 16:07
maphew and others added 2 commits June 15, 2026 13:10
…ssion-import

# Conflicts:
#	packages/opencode/src/cli/cmd/tui/thread.ts
#	packages/opencode/src/kilocode/cli/cmd/tui/thread.ts
#	packages/opencode/test/kilocode/cli/tui/thread.test.ts
@catrielmuller
catrielmuller merged commit ea0f5a0 into Kilo-Org:main Jul 6, 2026
22 checks passed
@maphew
maphew deleted the fix/cli-cloud-fork-session-import branch July 6, 2026 20:03
@maphew

maphew commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

thanks!

t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…ion-import

fix(cli): import cloud sessions before validation
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.

CLI cloud fork rejects valid cloud sessions as missing

2 participants