fix: parse remote-control session log entries without validation errors#216
Open
rymalia wants to merge 1 commit into
Open
fix: parse remote-control session log entries without validation errors#216rymalia wants to merge 1 commit into
rymalia wants to merge 1 commit into
Conversation
Claude Code v2.1.145+ emits new session log entries for its remote-control / bridge feature (continue a session on phone or web). Claude Code Viewer rejected these lines with schema validation errors: - last-prompt entries now use leafUuid instead of lastPrompt (d-kimuson#210) - mode entries, e.g. {"type":"mode","mode":"normal"} (d-kimuson#211) - bridge-session entries linking to a bridge session id (d-kimuson#212) - system entries with subtype "bridge_status" (d-kimuson#213) Add/extend Zod schemas for each and treat the two new metadata entry types (mode, bridge-session) like sibling metadata entries so they are filtered out of conversation rendering, sidechains, sync, and export.
📝 WalkthroughWalkthroughAdds schema and test coverage for ChangesConversation entry support and filtering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/lib/conversation-schema/entry/SystemEntrySchema.test.ts (1)
1-122: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftUse
testPlatformLayer-based Effect test setup in this test file.The test additions continue the direct Vitest pattern instead of the required reusable Effect test layers.
As per coding guidelines: "
**/*.test.{ts,tsx}: Use reusable Effect test layers fromsrc/testing/layers/for testing, withtestPlatformLayeras the foundation".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/conversation-schema/entry/SystemEntrySchema.test.ts` around lines 1 - 122, The SystemEntrySchema tests are still using the direct Vitest setup instead of the required reusable Effect test layers. Update the SystemEntrySchema.test.ts suite to use the shared testing layers from src/testing/layers with testPlatformLayer as the base, and wire the existing describe/test cases through that Effect-based setup while keeping the SystemEntrySchema assertions unchanged.Source: Coding guidelines
src/lib/conversation-schema/ConversationSchema.test.ts (1)
1-223: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftAdopt reusable Effect test layers (
testPlatformLayer) for this suite.The new cases are added in direct Vitest form, which diverges from the required test-layer pattern.
As per coding guidelines: "
**/*.test.{ts,tsx}: Use reusable Effect test layers fromsrc/testing/layers/for testing, withtestPlatformLayeras the foundation".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/conversation-schema/ConversationSchema.test.ts` around lines 1 - 223, The ConversationSchema test suite is using plain Vitest setup instead of the required reusable Effect test-layer pattern. Update the tests in ConversationSchema.test to use the shared layers from src/testing/layers/, with testPlatformLayer as the base foundation, and wire the suite through the existing Effect testing helpers rather than direct ad hoc parsing assertions. Keep the current test cases, but adapt the suite structure so it follows the project’s standard layer-based test setup.Source: Coding guidelines
src/server/core/session/services/ExportService.ts (1)
375-394: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove
ascast and use a type predicate in the filter.Line 394 uses
astype casting in a backend TS file; this violates the repository rule and can hide narrowing mistakes.Proposed fix
- const sidechainConversations = conversations.filter( - (conv) => + const sidechainConversations = conversations.filter( + ( + conv, + ): conv is Extract<Conversation, { type: "user" | "assistant" | "system" }> => conv.type !== "summary" && conv.type !== "file-history-snapshot" && conv.type !== "queue-operation" && conv.type !== "progress" && conv.type !== "custom-title" && conv.type !== "ai-title" && conv.type !== "agent-name" && conv.type !== "agent-setting" && conv.type !== "pr-link" && conv.type !== "last-prompt" && conv.type !== "permission-mode" && conv.type !== "mode" && conv.type !== "bridge-session" && conv.isSidechain === true, - ) as Array<Extract<Conversation, { type: "user" | "assistant" | "system" }>>; + );As per coding guidelines: "
**/*.{ts,tsx}: Never useastype casting in ANY context including test code (explain the problem to the user instead)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/core/session/services/ExportService.ts` around lines 375 - 394, The buildSidechainData helper is using an unsafe as cast after filtering conversations, which violates the TypeScript casting rule. Replace the current filter in buildSidechainData with a type predicate that narrows Conversation to the allowed user, assistant, and system variants while preserving the existing sidechain conditions, so the return type SidechainData is satisfied without any as assertion. Use the existing buildSidechainData symbol and the conversations.filter logic to locate the change.Source: Coding guidelines
src/lib/conversation-schema/entry/LastPromptEntrySchema.test.ts (1)
1-47: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftMigrate this test to reusable Effect test layers (
testPlatformLayer).This file uses plain Vitest directly instead of the required shared test layer foundation.
As per coding guidelines: "
**/*.test.{ts,tsx}: Use reusable Effect test layers fromsrc/testing/layers/for testing, withtestPlatformLayeras the foundation".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/conversation-schema/entry/LastPromptEntrySchema.test.ts` around lines 1 - 47, This test currently uses plain Vitest imports in LastPromptEntrySchema.test.ts instead of the shared Effect testing foundation. Update the test setup to use the reusable layers from src/testing/layers/, with testPlatformLayer as the base, and wire the suite through the Effect test utilities rather than direct describe/test usage. Keep the existing LastPromptEntrySchema assertions, but migrate the test structure so it follows the shared layer pattern used by other tests in the repo.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/conversation-schema/entry/BridgeSessionEntrySchema.ts`:
- Around line 5-10: Make BridgeSessionEntrySchema strict so unknown fields on
bridge-session entries are rejected instead of silently stripped. Update the
z.object definition in BridgeSessionEntrySchema to use z.strictObject while
keeping the existing type, sessionId, bridgeSessionId, and lastSequenceNum
fields unchanged.
In `@src/lib/conversation-schema/entry/LastPromptEntrySchema.ts`:
- Around line 7-10: The LastPromptEntrySchema currently allows a last-prompt
entry with no payload fields because both lastPrompt and leafUuid are optional.
Update the LastPromptEntrySchema definition so that at least one of those fields
is required, using the schema’s z.object shape and any appropriate
refinement/union logic, so valid entries always carry a prompt reference or leaf
reference.
---
Outside diff comments:
In `@src/lib/conversation-schema/ConversationSchema.test.ts`:
- Around line 1-223: The ConversationSchema test suite is using plain Vitest
setup instead of the required reusable Effect test-layer pattern. Update the
tests in ConversationSchema.test to use the shared layers from
src/testing/layers/, with testPlatformLayer as the base foundation, and wire the
suite through the existing Effect testing helpers rather than direct ad hoc
parsing assertions. Keep the current test cases, but adapt the suite structure
so it follows the project’s standard layer-based test setup.
In `@src/lib/conversation-schema/entry/LastPromptEntrySchema.test.ts`:
- Around line 1-47: This test currently uses plain Vitest imports in
LastPromptEntrySchema.test.ts instead of the shared Effect testing foundation.
Update the test setup to use the reusable layers from src/testing/layers/, with
testPlatformLayer as the base, and wire the suite through the Effect test
utilities rather than direct describe/test usage. Keep the existing
LastPromptEntrySchema assertions, but migrate the test structure so it follows
the shared layer pattern used by other tests in the repo.
In `@src/lib/conversation-schema/entry/SystemEntrySchema.test.ts`:
- Around line 1-122: The SystemEntrySchema tests are still using the direct
Vitest setup instead of the required reusable Effect test layers. Update the
SystemEntrySchema.test.ts suite to use the shared testing layers from
src/testing/layers with testPlatformLayer as the base, and wire the existing
describe/test cases through that Effect-based setup while keeping the
SystemEntrySchema assertions unchanged.
In `@src/server/core/session/services/ExportService.ts`:
- Around line 375-394: The buildSidechainData helper is using an unsafe as cast
after filtering conversations, which violates the TypeScript casting rule.
Replace the current filter in buildSidechainData with a type predicate that
narrows Conversation to the allowed user, assistant, and system variants while
preserving the existing sidechain conditions, so the return type SidechainData
is satisfied without any as assertion. Use the existing buildSidechainData
symbol and the conversations.filter logic to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 486a21d8-f942-446f-9c23-47fd60772baf
📒 Files selected for processing (15)
src/lib/conversation-schema/ConversationSchema.test.tssrc/lib/conversation-schema/entry/BridgeSessionEntrySchema.test.tssrc/lib/conversation-schema/entry/BridgeSessionEntrySchema.tssrc/lib/conversation-schema/entry/LastPromptEntrySchema.test.tssrc/lib/conversation-schema/entry/LastPromptEntrySchema.tssrc/lib/conversation-schema/entry/ModeEntrySchema.test.tssrc/lib/conversation-schema/entry/ModeEntrySchema.tssrc/lib/conversation-schema/entry/SystemEntrySchema.test.tssrc/lib/conversation-schema/entry/SystemEntrySchema.tssrc/lib/conversation-schema/index.tssrc/server/core/session/services/ExportService.tssrc/server/core/sync/services/SyncService.tssrc/web/app/projects/[projectId]/sessions/[sessionId]/components/conversationList/ConversationList.tsxsrc/web/app/projects/[projectId]/sessions/[sessionId]/components/conversationList/conversationRows.tssrc/web/app/projects/[projectId]/sessions/[sessionId]/hooks/useSidechain.ts
Comment on lines
+5
to
+10
| export const BridgeSessionEntrySchema = z.object({ | ||
| type: z.literal("bridge-session"), | ||
| sessionId: z.string(), | ||
| bridgeSessionId: z.string(), | ||
| lastSequenceNum: z.number().optional(), | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'src/lib/conversation-schema/**/*Schema.ts' 'src/lib/conversation-schema/**/*.ts' | sed -n '1,200p'
printf '\n== Outline: BridgeSessionEntrySchema.ts ==\n'
ast-grep outline src/lib/conversation-schema/entry/BridgeSessionEntrySchema.ts --view expanded || true
printf '\n== BridgeSessionEntrySchema.ts ==\n'
cat -n src/lib/conversation-schema/entry/BridgeSessionEntrySchema.ts
printf '\n== Search for strict schemas in conversation-schema ==\n'
rg -n "strictObject|\.strict\(" src/lib/conversation-schema -g '*.ts' -g '*.tsx' || true
printf '\n== Search for z.object in conversation-schema ==\n'
rg -n "z\.object\(" src/lib/conversation-schema -g '*.ts' -g '*.tsx' || trueRepository: d-kimuson/claude-code-viewer
Length of output: 8302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== BridgeSessionEntrySchema usages ==\n'
rg -n "BridgeSessionEntrySchema|BridgeSessionEntry" src -g '*.ts' -g '*.tsx'
printf '\n== BaseEntrySchema ==\n'
cat -n src/lib/conversation-schema/entry/BaseEntrySchema.ts
printf '\n== Related entry schemas with exported object shape ==\n'
for f in \
src/lib/conversation-schema/entry/AgentNameEntrySchema.ts \
src/lib/conversation-schema/entry/AgentSettingEntrySchema.ts \
src/lib/conversation-schema/entry/AiTitleEntrySchema.ts \
src/lib/conversation-schema/entry/AssistantEntrySchema.ts \
src/lib/conversation-schema/entry/AttachmentEntrySchema.ts \
src/lib/conversation-schema/entry/CustomTitleEntrySchema.ts \
src/lib/conversation-schema/entry/FileHIstorySnapshotEntrySchema.ts \
src/lib/conversation-schema/entry/LastPromptEntrySchema.ts \
src/lib/conversation-schema/entry/ModeEntrySchema.ts \
src/lib/conversation-schema/entry/PermissionModeEntrySchema.ts \
src/lib/conversation-schema/entry/PrLinkEntrySchema.ts \
src/lib/conversation-schema/entry/ProgressEntrySchema.ts \
src/lib/conversation-schema/entry/QueueOperationEntrySchema.ts \
src/lib/conversation-schema/entry/SummaryEntrySchema.ts \
src/lib/conversation-schema/entry/SystemEntrySchema.ts \
src/lib/conversation-schema/entry/UserEntrySchema.ts
do
echo "== $f =="
sed -n '1,40p' "$f"
doneRepository: d-kimuson/claude-code-viewer
Length of output: 11381
Make BridgeSessionEntrySchema strict.
z.object(...) strips unknown keys, so extra fields on "bridge-session" entries are accepted and discarded instead of rejected. Use z.strictObject(...) here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/conversation-schema/entry/BridgeSessionEntrySchema.ts` around lines 5
- 10, Make BridgeSessionEntrySchema strict so unknown fields on bridge-session
entries are rejected instead of silently stripped. Update the z.object
definition in BridgeSessionEntrySchema to use z.strictObject while keeping the
existing type, sessionId, bridgeSessionId, and lastSequenceNum fields unchanged.
Source: Coding guidelines
Comment on lines
7
to
+10
| export const LastPromptEntrySchema = z.object({ | ||
| type: z.literal("last-prompt"), | ||
| lastPrompt: z.string(), | ||
| lastPrompt: z.string().optional(), | ||
| leafUuid: z.string().optional(), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require at least one payload field for last-prompt entries.
Line 9 and Line 10 being optional allows { type: "last-prompt", sessionId } with no prompt reference, which weakens schema guarantees.
Proposed fix
export const LastPromptEntrySchema = z.object({
type: z.literal("last-prompt"),
lastPrompt: z.string().optional(),
leafUuid: z.string().optional(),
sessionId: z.string(),
-});
+}).refine((entry) => entry.lastPrompt !== undefined || entry.leafUuid !== undefined, {
+ error: "Either lastPrompt or leafUuid is required",
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const LastPromptEntrySchema = z.object({ | |
| type: z.literal("last-prompt"), | |
| lastPrompt: z.string(), | |
| lastPrompt: z.string().optional(), | |
| leafUuid: z.string().optional(), | |
| export const LastPromptEntrySchema = z.object({ | |
| type: z.literal("last-prompt"), | |
| lastPrompt: z.string().optional(), | |
| leafUuid: z.string().optional(), | |
| sessionId: z.string(), | |
| }).refine((entry) => entry.lastPrompt !== undefined || entry.leafUuid !== undefined, { | |
| error: "Either lastPrompt or leafUuid is required", | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/conversation-schema/entry/LastPromptEntrySchema.ts` around lines 7 -
10, The LastPromptEntrySchema currently allows a last-prompt entry with no
payload fields because both lastPrompt and leafUuid are optional. Update the
LastPromptEntrySchema definition so that at least one of those fields is
required, using the schema’s z.object shape and any appropriate refinement/union
logic, so valid entries always carry a prompt reference or leaf reference.
Contributor
Author
|
Thanks @coderabbitai. Declining all four recommendations and leaving the PR as-is:
|
|
Tip For best results, initiate chat on the files or code changes.
( ˘︹˘ )🐇 ✏️ Learnings added
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Claude Code v2.1.145+ emits new session-log entries for its remote-control / bridge feature (continue a session on phone or web). Claude Code Viewer rejected these lines with schema validation errors. This adds/extends the Zod schemas and threads the two new metadata entry types through every consumer.
last-promptwithleafUuidlastPromptmade optional, added optionalleafUuidmode({"type":"mode","mode":"normal"})ModeEntrySchemabridge-sessionBridgeSessionEntrySchemasystem/subtype: "bridge_status"modeandbridge-sessionare treated like sibling metadata entries (permission-mode,last-prompt) — filtered out of conversation rendering, sidechains, sync, and export. Thesatisfies neverguard inconversationRows.tsand the.cwd/.isSidechainaccess sites made several of these edits type-forced.Testing
.typeto prove the intended union member matched.Closes #210
Closes #211
Closes #212
Closes #213
Summary by CodeRabbit
New Features
Bug Fixes