Skip to content

fix(cli): accept JWT share tokens when importing sessions - #13183

Merged
eshurakov merged 3 commits into
mainfrom
eshurakov/fix-import-jwt-share-url
Aug 18, 2026
Merged

fix(cli): accept JWT share tokens when importing sessions#13183
eshurakov merged 3 commits into
mainfrom
eshurakov/fix-import-jwt-share-url

Conversation

@eshurakov

Copy link
Copy Markdown
Contributor

What

Cloud PR Kilo-Org/cloud#5300 replaced /s/{public_id} with purpose-bound JWT share tokens. kilo import https://app.kilo.ai/s/<token> then failed with Invalid URL format. Expected: https://app.kilo.ai/s/<id> because the parser only allowed [A-Za-z0-9_-].

Why

JWTs contain .. Import needs to accept the new share URL, and share() needs to read share_token from ingest instead of public_id.

Changes

  • Parse any non-empty /s/<token> path on app.kilo.ai
  • Build share URLs from share_token
  • Cover JWT import parsing and the updated share response

Verification

  • bun test ./test/cli/import.test.ts ./test/kilocode/session-share.test.ts from packages/opencode/
  • bun run typecheck from packages/opencode/

@kilo-code-bot

kilo-code-bot Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of commits since beba4a5: the branch was rebased onto current main, and the id.ts timestamp-unwrap change with its test is no longer part of this PR, so the two prior SUGGESTIONs no longer apply. The remaining delta (c8750fa) replaces the URL-constructor parser with the original anchored regex, now allowing dots in the token ([A-Za-z0-9_.-]+) so JWT share URLs import — this also resolves the earlier reviewer feedback about the bare catch. The parser change, share_token handling in share(), and the updated tests look correct; no stray public_id references remain, and the changeset is present and user-facing.

Files Reviewed (5 files)
  • .changeset/fix-import-jwt-share-url.md
  • packages/opencode/src/cli/cmd/import.ts
  • packages/opencode/src/kilo-sessions/kilo-sessions.ts
  • packages/opencode/test/cli/import.test.ts
  • packages/opencode/test/kilocode/session-share.test.ts
Previous Review Summaries (4 snapshots, latest commit beba4a5)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit beba4a5)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
SUGGESTION 2

Incremental review of commits since f23e9f2: the JWT share-token changes were previously approved; the new commits add a 48-bit timestamp unwrap in Identifier.timestamp for the truncation cleanup (only consumer: tool/truncate.ts), plus a regression test and changeset line. The unwrap logic is correct and timely — wall clock is currently within the 7-day retention window after a wrap boundary — both findings are optional suggestions.

Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/id/id.ts 79 Extract the Kilo-specific timestamp unwrap into a src/kilocode/id/id.ts mirror file per the fork isolation rule
packages/opencode/test/kilocode/id-timestamp.test.ts 8 Wrap-crossing test coverage is time-dependent; add a deterministic boundary case
Files Reviewed (3 files)
  • .changeset/fix-import-jwt-share-url.md
  • packages/opencode/src/id/id.ts - 1 issue
  • packages/opencode/test/kilocode/id-timestamp.test.ts - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit f23e9f2)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • packages/opencode/src/cli/cmd/import.ts

Previous review (commit 343be14)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • .changeset/fix-import-jwt-share-url.md
  • packages/opencode/src/cli/cmd/import.ts
  • packages/opencode/src/kilo-sessions/kilo-sessions.ts
  • packages/opencode/test/cli/import.test.ts
  • packages/opencode/test/kilocode/session-share.test.ts

Previous review (commit 43441e7)

Status: No Issues Found | Recommendation: Merge

Comment: lgtm — the parser change correctly handles JWT share tokens (allowing dots), share() now builds URLs from share_token, and both the import parsing and share response paths are covered by updated tests. Changes are confined to kilocode_change-marked/kilo-owned regions. A changeset is included.

Files Reviewed (5 files)
  • .changeset/fix-import-jwt-share-url.md
  • packages/opencode/src/cli/cmd/import.ts
  • packages/opencode/src/kilo-sessions/kilo-sessions.ts
  • packages/opencode/test/cli/import.test.ts
  • packages/opencode/test/kilocode/session-share.test.ts

Reviewed by kimi-k3 · Input: 57.5K · Output: 5K · Cached: 192.4K

Review guidance: REVIEW.md from base branch main

Comment thread packages/opencode/src/cli/cmd/import.ts Outdated
Comment thread packages/opencode/src/cli/cmd/import.ts Outdated
Comment thread packages/opencode/src/cli/cmd/import.ts Outdated
Comment thread packages/opencode/src/cli/cmd/import.ts Outdated

@intentionally-left-nil intentionally-left-nil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Comment thread packages/opencode/src/id/id.ts Outdated
Comment thread packages/opencode/test/kilocode/id-timestamp.test.ts Outdated
@eshurakov
eshurakov force-pushed the eshurakov/fix-import-jwt-share-url branch from beba4a5 to f23e9f2 Compare August 17, 2026 21:39
Cloud share URLs now use purpose-bound JWTs at /s/{token}. The import
parser rejected those tokens because it only allowed [A-Za-z0-9_-], and
share() still expected the old public_id field.
Allow dots in the token so JWT share URLs import, without the URL constructor and bare catch.
@eshurakov
eshurakov force-pushed the eshurakov/fix-import-jwt-share-url branch from f23e9f2 to c8750fa Compare August 18, 2026 09:04
@eshurakov
eshurakov merged commit 017410b into main Aug 18, 2026
46 of 49 checks passed
@eshurakov
eshurakov deleted the eshurakov/fix-import-jwt-share-url branch August 18, 2026 09:43
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.

3 participants