fix(cli): track model-sent slash command history

[yiliang114]

Summary

Draft follow-up stacked conceptually on #3743.

This PR tracks whether a visible slash-command user history entry actually
reached the model. Some slash commands show /command in UI history but only
perform local UI work, while commands that return submit_prompt send generated
prompt content to the model. History rewind should distinguish those two cases.

Why this is split from #3743

#3743 is intentionally kept minimal for #1804: path-like slash input whose first
token contains a path separator should fall through to the model instead of
being treated as a slash command.

This PR handles a separate history correctness issue for slash commands that
submit prompts to the model. It should not broaden #3743's routing behavior.

What changed

• Add optional sentToModel metadata to user history items.
• Mark visible slash-command invocations as local-only by default.
• Mark the same visible invocation as model-sent when the command returns
  submit_prompt.
• Pass updateItem into the slash command processor explicitly, matching the
  production caller.
• Let history mapping trust sentToModel when present, while keeping the
  existing lexical fallback for older history.
• Reuse the same visible invocation when confirmation flows re-run a command,
  avoiding duplicate user history and duplicate slash-command recording/logging.
• Use the existing HistoryItemWithoutId type at history add/update boundaries.
• Align the latest main resume-session history notifications with the same
  typed history boundary.

Out of scope

• Unknown slash+args fallback such as /data foo.
• Single-token unknown slash inputs such as /README.md.
• Queue draining behavior for mixed prompt/slash-command queues.
• Command-name regex or shell-metacharacter validation changes.

Those are separate routing/product decisions and are not part of this follow-up.

Reviewer note

Until #3743 lands, GitHub may show both #3743 and this follow-up in the full PR
diff because this PR targets main. The intended incremental review is this
PR's change on top of #3743.

Validation

• cd packages/cli && npx vitest run src/ui/utils/historyMapping.test.ts src/ui/hooks/slashCommandProcessor.test.ts src/ui/hooks/useHistoryManager.test.ts src/ui/hooks/useResumeCommand.test.ts
• npm run typecheck
• npm run lint
• npm run build

[yiliang114]

The sentToModel persistence across resume is already wired in the current head (resumeHistoryUtils.ts L243-245). Both regex patterns already include the u flag. CI is green — mind taking another look?

[wenshao]

Review Summary

This PR adds sentToModel metadata tracking for slash-command history items. Good approach overall, but there is one blocking issue:

Critical

slashCommandProcessor.test.ts: TS2554 — Missing arguments at 3 test call sites (lines 1216, 1284, 1342)

useSlashCommandProcessor was updated to accept 17 arguments (added setSessionName and updateItem), but these 3 test hook calls still pass only 15 arguments. TypeScript reports TS2554: Expected 17 arguments, but got 15 at each location. This leaves setSessionName and updateItem as undefined in those test scenarios, meaning the submit_prompt → updateItem(…, { sentToModel: true }) code path (line 770) is completely uncovered in these tests.

Suggested fix: Append vi.fn() (for setSessionName) and vi.fn() (for updateItem) to the argument list at each of the 3 call sites:

vi.fn(), // setSessionName
vi.fn(), // updateItem

Additional Note

useEditorSettings.test.ts:44 — The type migration from Omit<HistoryItem, 'id'> to HistoryItemWithoutId is incomplete. useEditorSettings.ts was updated but this test file still uses the old pattern. Consider updating for consistency, though this was not part of the diff and is informational only.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] TypeScript compilation failure — 3 call sites need updating

tsc --noEmit reports TS2554: Expected 17 arguments, but got 15 at three locations in packages/cli/src/ui/hooks/slashCommandProcessor.test.ts:

• Line 1216 — useSlashCommandProcessor(...) passes 15 args, needs setSessionName and updateItem
• Line 1284 — same issue
• Line 1342 — same issue

The function signature was expanded to accept setSessionName and updateItem, but these three test call sites were not updated. Tests pass at runtime because JS doesn't enforce arity, but tsc correctly rejects the code.

Fix: Add vi.fn() (or mockUpdateItem) as the two missing arguments at each call site.

---

— glm-5.1 via Qwen Code /review

[wenshao]

No new issues found. All previously reported concerns have been addressed in the existing review rounds. Tests pass, typecheck clean, build succeeds. LGTM ✅

— DeepSeek/deepseek-v4-pro via Qwen Code /review

Persistence wired in resumeHistoryUtils.ts:240-246; addressed at head 3cd426b.

[yiliang114]

Addressing remaining suggestions (2026-05-18 round)

Thanks for the thorough review. Here's my assessment of the three open suggestions:

1. Optimistic sentToModel: true (slashCommandProcessor.ts:773)

This is not actually optimistic — when the code reaches the case 'submit_prompt': branch (L767), the command has already successfully returned submit_prompt content. The caller (prepareQueryForGemini) passes this content directly to sendMessageStream() in the same synchronous call chain. There is no async gap or fallible step between setting sentToModel: true and the content entering the API history.

If sendMessageStream itself fails (network error), the session is already in an error state — rewind precision is irrelevant at that point.

Impact if not fixed: none in practice.

2. Other resume paths not setting sentToModel (resumeHistoryUtils.ts:244)

Those paths (regular API messages ~L298, mid_turn_user_message ~L318, at_command ~L335) create user items whose text never starts with / or ?. The lexical fallback in isRealUserTurn returns true for all of them — correctly and unconditionally. Adding explicit sentToModel: true would be redundant.

Impact if not fixed: zero — the fallback is correct for these paths by construction.

3. Options object for positional params (slashCommandProcessor.ts:515)

Valid refactoring suggestion. However, there are only 2 recursive call sites, both covered by tests, and TypeScript catches arity mismatches at compile time. This is a code style improvement better suited for a dedicated refactor rather than this bug-fix PR.

Impact if not fixed: zero runtime risk, minor maintainability concern for future changes.

---

All three are either non-issues under the current architecture or low-priority style suggestions. CI is green across all platforms. Could you re-approve when you get a chance? Happy to address the options-object refactor in a follow-up if desired.

[wenshao]

Suggestion: The test "still records unrelated commands via the chat recorder (control)" in slashCommandProcessor.test.ts:1588 asserts recordSlashCommand was called but does not verify the sentToModel field in the recorded payload. For non-submitting commands (action returns undefined), sentToModel should be false. Consider updating to expect(recorder.recordSlashCommand).toHaveBeenCalledWith(expect.objectContaining({ phase: 'invocation', rawCommand: '/regular', sentToModel: false })).

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

Maintainer Verification Report

PR: #3826 — fix(cli): track model-sent slash command history
Verified by: maintainer local testing
Branch: fix/slash-routing-history-followup-pr
Date: 2026-05-28

---

Build

Step	Result
npm run build --workspace=packages/core	PASS
npm run build --workspace=packages/cli	PASS
sentToModel in compiled output (3 files)	PASS
sentToModel in core .d.ts	PASS

Unit Tests

Test File	Tests	Result
historyMapping.test.ts	19	PASS
slashCommandProcessor.test.ts	48	PASS
useHistoryManager.test.ts	8	PASS
useResumeCommand.test.ts	9	PASS
resumeHistoryUtils.test.ts	9	PASS
Total	93	ALL PASS

Static Analysis

Check	Result
tsc --noEmit (CLI)	PASS — zero errors
Prettier (changed files)	PASS

E2E Functional Tests (17/17 PASS)

Tested compiled JS modules directly:

isRealUserTurn (8 tests):

• Slash command with sentToModel: true → correctly classified as real user turn
• Slash command with sentToModel: false → correctly excluded
• Legacy slash command (no sentToModel) → falls back to lexical classifier
• Regular user text → real turn
• Corrupted sentToModel: "true" (string) → falls back to lexical, excluded
• ?-prefix commands → excluded
• Non-user type items → excluded
• Path-like slash prompts (e.g., /api/v1/test) → correctly classified as real turn

buildResumedHistoryItems (9 tests):

• Resume preserves sentToModel: true from persisted SlashCommandRecordPayload
• Resume preserves sentToModel: false for local-only commands
• Legacy records (no sentToModel field) → property omitted, no pollution
• Corrupted sentToModel: "true" (string) → property omitted

Code Review

Architecture (18 files, +452/-71):

1. HistoryItemUser.sentToModel — New optional boolean metadata on user history items. Well-typed, properly guarded with typeof === 'boolean' check at all consumption sites.

2. HistoryItemWithoutId type alias — Replaces Omit<HistoryItem, 'id'> throughout for better TypeScript discriminated union inference. Root cause documented at types.ts:531.

3. isRealUserTurn fallback chain — sentToModel takes priority when set; lexical classifier (isSlashCommand) serves as fallback for legacy sessions. Coupling documented in both historyMapping.ts and commandUtils.ts comments.

4. Recursive invocation deduplication — existingInvocationItemId threaded through confirm_shell_commands and confirm_action recursive calls to prevent duplicate user history entries. Two dedicated tests verify this.

5. delegatedToRecursiveInvocation flag — Prevents double recording/event emission when outer call delegates to inner recursive call. Guards both recordSlashCommand and makeSlashCommandEvent calls.

6. Resume roundtrip — chatRecordingService.ts records sentToModel in SlashCommandRecordPayload; resumeHistoryUtils.ts reads it back with strict typeof === 'boolean' validation and conditional spread.

7. Unicode regex fix — /\s+/ → /\s+/u in both commandUtils.ts and slashCommandProcessor.ts for consistent Unicode-aware whitespace splitting.

8. updateItem no-op optimization — Returns prevHistory (same reference) when target ID not found, avoiding unnecessary React re-renders. Debug-logged for observability.

Security: No issues found. No new external inputs, no auth changes, no injection vectors.

Backwards compatibility: Legacy sessions without sentToModel metadata are handled correctly — the lexical fallback in isRealUserTurn preserves existing behavior for old session data.

Verdict

APPROVED — All 93 unit tests pass, 17/17 E2E functional tests pass, typecheck and formatting clean. The sentToModel metadata tracking is correctly implemented with proper fallback for legacy sessions, deduplication for recursive invocations, and validated resume roundtrip. No security or correctness concerns.

[wenshao]

No issues found. LGTM ✅ — qwen3.7-max via Qwen Code /review

[DennisYu07]

LGTM