fix: preserve session start metadata#318
Conversation
Signed-off-by: Zavian <36817799+Zavianx@users.noreply.github.com>
|
@Zavianx is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR extends the session creation endpoint to accept and persist optional human-readable labels ( ChangesSession Label Initialization
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/triggers/api.ts (1)
546-558:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix Unicode truncation in
asSessionPreviewto prevent data corruption.The test reveals a real bug: truncating strings containing emojis or other non-BMP characters at certain boundaries creates invalid UTF-16 sequences. When the string contains an emoji (or similar 2-code-unit character) near the
maxLengthboundary,.slice(0, maxLength)splits the surrogate pair, leaving an orphaned surrogate that becomes a replacement character U+FFFD.For example,
asSessionPreview("a".repeat(199) + "👋", 200)produces a string ending with a replacement character instead of the emoji, corrupting the stored session metadata.Fix: Use a function that respects grapheme boundaries when truncating (e.g., by counting Unicode code points or using a proper grapheme library), or document the limitation. This affects
session.summaryandsession.firstPromptwhich are persisted and used throughout the application.🤖 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/triggers/api.ts` around lines 546 - 558, The asSessionPreview truncation is splitting UTF-16 surrogate pairs and corrupting emojis stored in session.summary and session.firstPrompt; update asSessionPreview to truncate by Unicode grapheme/code points (not by string index) — e.g., use Array.from(string) or Intl.Segmenter to count/slice user-perceived characters or integrate a grapheme library, then return the joined slice ensuring you preserve full code points; keep callers (where session.summary and session.firstPrompt are set) unchanged.
🧹 Nitpick comments (1)
test/api-session-start.test.ts (1)
33-55: ⚡ Quick winConsider adding test coverage for edge cases.
The current tests validate the core fallback and explicit-field behavior, but several edge cases are not covered:
- Empty or whitespace-only strings (should return sessions without summary/firstPrompt)
- Strings exceeding maxLength (200 for title/firstPrompt, 300 for summary)
- Multi-line strings with various whitespace patterns
- Unicode characters including surrogate pairs at truncation boundaries
These edge cases would increase confidence that
asSessionPreviewhandles all inputs correctly.📋 Example additional test cases
+ it("handles empty and whitespace-only inputs", async () => { + const { sdk, kv } = setupApi(); + + await sdk.trigger("api::session::start", { + body: { + sessionId: "ses_empty", + project: "/tmp/project", + cwd: "/tmp/project", + title: " ", + summary: "", + }, + }); + + const session = await kv.get<Session>(KV.sessions, "ses_empty"); + expect(session?.summary).toBeUndefined(); + expect(session?.firstPrompt).toBeUndefined(); + }); + + it("truncates long strings to maxLength", async () => { + const { sdk, kv } = setupApi(); + const longTitle = "a".repeat(250); + const longSummary = "b".repeat(350); + + await sdk.trigger("api::session::start", { + body: { + sessionId: "ses_long", + project: "/tmp/project", + cwd: "/tmp/project", + title: longTitle, + summary: longSummary, + }, + }); + + const session = await kv.get<Session>(KV.sessions, "ses_long"); + expect(session?.summary?.length).toBe(300); + expect(session?.firstPrompt?.length).toBe(200); + });🤖 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 `@test/api-session-start.test.ts` around lines 33 - 55, Add unit tests exercising asSessionPreview edge cases: (1) submit empty and whitespace-only title/summary/firstPrompt via sdk.trigger (using setupApi and KV.sessions) and assert stored session has no summary/firstPrompt; (2) submit strings exceeding the max lengths (title/firstPrompt 200, summary 300) and assert the persisted session fields are truncated to those limits; (3) supply multi-line strings with varied whitespace and assert trimming/normalization behavior matches asSessionPreview; (4) include Unicode surrogate-pair boundaries (e.g., emoji at truncation edge) and assert truncation does not split surrogate pairs. Use the same test harness patterns in the existing test (sdk.trigger, KV.sessions, Session) and add separate it() cases for each edge scenario.
🤖 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.
Outside diff comments:
In `@src/triggers/api.ts`:
- Around line 546-558: The asSessionPreview truncation is splitting UTF-16
surrogate pairs and corrupting emojis stored in session.summary and
session.firstPrompt; update asSessionPreview to truncate by Unicode
grapheme/code points (not by string index) — e.g., use Array.from(string) or
Intl.Segmenter to count/slice user-perceived characters or integrate a grapheme
library, then return the joined slice ensuring you preserve full code points;
keep callers (where session.summary and session.firstPrompt are set) unchanged.
---
Nitpick comments:
In `@test/api-session-start.test.ts`:
- Around line 33-55: Add unit tests exercising asSessionPreview edge cases: (1)
submit empty and whitespace-only title/summary/firstPrompt via sdk.trigger
(using setupApi and KV.sessions) and assert stored session has no
summary/firstPrompt; (2) submit strings exceeding the max lengths
(title/firstPrompt 200, summary 300) and assert the persisted session fields are
truncated to those limits; (3) supply multi-line strings with varied whitespace
and assert trimming/normalization behavior matches asSessionPreview; (4) include
Unicode surrogate-pair boundaries (e.g., emoji at truncation edge) and assert
truncation does not split surrogate pairs. Use the same test harness patterns in
the existing test (sdk.trigger, KV.sessions, Session) and add separate it()
cases for each edge scenario.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18710b31-39d6-48db-9f43-3c30791ff8b0
📒 Files selected for processing (3)
README.mdsrc/triggers/api.tstest/api-session-start.test.ts
1 participant
Summary
title,summary, andfirstPromptinPOST /agentmemory/session/starttitleas the fallback session label/first prompt so OpenCode-created sessions do not surface as raw IDs when prompt submission races session creationsummaryandfirstPromptvalues distinct when clients provide bothFixes #244
Related
title->summary/firstPromptpath. Happy to rebase if feat: add session agent metadata #306 lands first.Tests
npm run buildnpm testnpm test -- test/api-session-start.test.ts