📋 Review Summary

This PR adds a --fork-session CLI flag that allows users to resume a session while creating a new forked session ID, preserving the original session unchanged. The implementation is clean and well-tested, with proper validation and error handling throughout.

🔍 General Feedback

• Good use of existing patterns (mirrors --continue/--resume architecture)
• Test coverage is comprehensive for the new functionality
• The validation logic correctly prevents invalid flag combinations
• Error messages are clear and actionable

🎯 Specific Feedback

🟡 High

• File: packages/cli/src/config/config.ts:1533-1536 - The error handling for --continue --fork-session when no session exists has a logic issue. The else if (argv.forkSession) only triggers when !sessionData in the argv.continue branch, but this means if --continue is used without an existing session AND --fork-session is set, it will error. However, this is actually correct behavior (can't fork nothing), but the error message "No saved session found to fork." could be clearer by mentioning that --continue requires an existing session. Consider: "Cannot use --fork-session with --continue: no saved session found to fork."

🟢 Medium

• File: packages/cli/src/config/config.ts:164 - The JSDoc comment says "Fork the resumed session into a new session before continuing" but this is slightly misleading since it works with both --resume AND --continue. Consider: "Create a new forked session from the resumed session. Must be used with --resume or --continue."

• File: packages/cli/src/config/config.ts:916-917 - The validation error message uses --fork-session but the actual flag in yargs is defined as 'fork-session' (kebab-case). While yargs handles both, being consistent in error messages helps users. The current message is correct, but consider adding the short form if one exists (none defined currently).

• File: packages/cli/src/config/config.test.ts:933 - The test assertion expect(config.getSessionId()).not.toBe(sourceSessionId) is good but could be stronger. Consider also asserting that it equals the expected forked ID: expect(config.getSessionId()).toBe('forked-session-id') to verify the fork actually changed the session ID as expected.

🔵 Low

• File: packages/cli/src/config/config.ts:809-814 - Consider adding an alias property for --fork-session if there's a natural short form (e.g., -f), though this should be checked against existing aliases to avoid conflicts.

• File: packages/cli/src/config/config.test.ts:438-443 - The test for "picker form" (--resume --fork-session) expects argv.resume to be '' (empty string). This is correct per the existing --resume behavior, but consider adding a comment explaining why empty string is expected (the picker form) to help future maintainers.

• File: packages/cli/src/config/config.ts:1552-1561 - The fork logic creates a new SessionService instance inside the if (argv.continue || argv.resume) block at line 1528, then uses it again here. This is fine, but consider extracting the fork logic into a helper function for clarity and testability:

  async function forkCurrentSession(
    sourceSessionId: string,
    sessionService: SessionService
  ): Promise<string> {
    const forkedSessionId = randomUUID();
    await sessionService.forkSession(sourceSessionId, forkedSessionId);
    const sessionData = await sessionService.loadSession(forkedSessionId);
    if (!sessionData) {
      writeStderrLine(`Failed to load forked session ${forkedSessionId}.`);
      process.exit(1);
    }
    return forkedSessionId;
  }

✅ Highlights

• File: packages/cli/src/config/config.ts:916-917 - Excellent validation preventing --fork-session from being used standalone without --resume or --continue

• File: packages/cli/src/config/config.test.ts:913-941 - Well-structured test that verifies the complete fork flow including mock setup, execution, and assertions on both the service call and resulting session ID

• File: packages/cli/src/config/config.test.ts:446-460 - Good coverage of the error case where --fork-session is used without required flags, including verification of the error message

• Clean integration with existing session resumption logic without disrupting the current flow

本地真实场景测试报告（tmux）

Head: bf80a88e (12:10Z) "guard sandbox session handoff flag"

Fork 链验证

3 个连续会话操作，构成完整 fork 链：

2d7fd674 (root, 初次 --prompt 创建)
   ↑
c5b911ef ← forked from 2d7fd674 (非交互式 --continue --fork-session)
   ↑
4f1faa62 ← forked from c5b911ef (tmux 交互式 --continue --fork-session)

每个 forked jsonl 的第一条记录都含 forkedFrom: { sessionId: <parent>, messageUuid: <…> }，parent 链完整可追溯。

tmux 交互 TUI 验证

启动：

cd /tmp/pr4159-tui
node dist/cli.js --continue --fork-session

1. ✅ 新建 forked session 4f1faa62，加载原 session 的历史（Remember: my favorite color is purple + What's my favorite color?）
2. ✅ 输入新问题 What was my favorite color again? → 模型回 紫色 (purple)。你之前告诉过我。 —— 证明 forked context 真的承袭了原 session 的对话历史，不是空白会话

两条 Suggestion 都修了

Suggestion 1 (09:22Z) — forkSession() 加 try-catch 友好错误

packages/cli/src/config/config.ts:1575-1586：

if (argv.forkSession && sessionId) {
  const sourceSessionId = sessionId;
  const forkedSessionId = randomUUID();
  try {
    await sessionService.forkSession(sourceSessionId, forkedSessionId);
  } catch (err) {
    writeStderrLine(
      `Failed to fork session ${sourceSessionId}: ${err instanceof Error ? err.message : String(err)}`,
    );
    process.exit(1);
  }
}

Suggestion 2 (11:09Z) — --sandbox-session-id gate on SANDBOX env

packages/cli/src/config/config.ts:1593-1597：

} else if (argv.sandboxSessionId) {
  if (!process.env['SANDBOX']) {
    writeStderrLine('--sandbox-session-id is for internal sandbox use only.');
    process.exit(1);
  }
  sessionId = argv.sandboxSessionId;
}

实测：

# 无 SANDBOX
$ node dist/cli.js --sandbox-session-id <uuid> --prompt "hi"
--sandbox-session-id is for internal sandbox use only.

# 有 SANDBOX=1
$ SANDBOX=1 node dist/cli.js --sandbox-session-id <uuid> --prompt "reply 'ok'"
ok

测试 & CI

LGTM ✅