feat: add session agent metadata#306
Conversation
Signed-off-by: flamerged <34665379+flamerged@users.noreply.github.com>
|
@flamerged is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughSession-start flow extended to optionally capture agent identity, model, and custom metadata. Client hooks send these fields; server APIs/events normalize, validate, and redact metadata before storing. Attribution helpers, search/context enrichment, viewer UI, README, and tests were updated to surface the new session attribution data. ChangesSession metadata capture
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
Signed-off-by: flamerged <34665379+flamerged@users.noreply.github.com>
![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
🤖 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/functions/session-metadata.ts`:
- Around line 17-20: The helper asRecord currently treats null like undefined
which suppresses the "must be an object when provided" validation for
metadata/agent; change asRecord so it returns undefined only for undefined and
returns null for an explicit null input (i.e., separate the checks: if value ===
undefined return undefined; if value === null return null) so callers
(validation around metadata and agent) can detect explicit null and emit the
intended errors.
In `@src/triggers/events.ts`:
- Around line 12-19: The code stores data.metadata and data.agent directly which
skips the normalization and secret-redaction logic used by api::session::start;
update the event handler in events.ts to run the same normalization/redaction
routine used by api::session::start on data.metadata and data.agent before
persisting them (i.e., import and call the session start normalization/redaction
helper or extract that logic into a shared helper and use it here) so that
metadata and agent values are normalized and secrets are redacted consistently
when sessions are created via events.
🪄 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: 44c5e84a-d3d3-4132-be4a-4f000ba940e5
📒 Files selected for processing (9)
README.mdplugin/scripts/session-start.mjssrc/functions/session-metadata.tssrc/hooks/session-start.tssrc/triggers/api.tssrc/triggers/events.tssrc/types.tstest/context-injection.test.tstest/session-metadata.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/viewer/index.html`:
- Around line 1091-1097: The sessionAgentLabel function only reads
agent.agentType and agent.sessionSource; update identity extraction logic (in
sessionAgentLabel and the related sessionAgent function) to also check for
alternative keys role and agent_type for agent type and source and source for
sessionSource (e.g., prefer agent.agentType || agent.role || agent.agent_type
and agent.sessionSource || agent.source) so role/source and agent_type/source
appear in the parts array and render as badges/panel fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Signed-off-by: flamerged <34665379+flamerged@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/functions/search.ts (1)
128-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid failing the entire search when session lookup rejects
loadSession()currently letskv.get()rejections bubble, andPromise.allon session enrichment can fail the whole request. This should degrade tonullsession attribution instead.Suggested patch
const loadSession = async (sessionId: string): Promise<Session | null> => { if (sessionCache.has(sessionId)) return sessionCache.get(sessionId)! - const s = await kv.get<Session>(KV.sessions, sessionId) + const s = await kv.get<Session>(KV.sessions, sessionId).catch(() => null) sessionCache.set(sessionId, s ?? null) return s ?? null }Also applies to: 164-166
🤖 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/functions/search.ts` around lines 128 - 133, The session lookup currently lets kv.get(...) rejections bubble through loadSession and can cause the surrounding Promise.all (session enrichment) to reject; update loadSession to catch any errors from kv.get and treat failures as a missing session by logging/ignoring the error, setting sessionCache.set(sessionId, null), and returning null so enrichment degrades gracefully; apply the same defensive try/catch pattern wherever kv.get is used for session reads (the other instance around lines 164-166) so those calls also return null on error instead of throwing.test/search.test.ts (1)
11-51: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdopt the required
iii-sdkmock pattern instead of custom local SDK/KV mocks.This test currently uses bespoke
mockSdk()/mockKV()helpers, which diverges from the repository’s required test harness pattern. Please switch tovi.mock("iii-sdk")and provide the expected mockedsdk.triggerandkv.get/set/listthere, matching the established test style.As per coding guidelines,
test/**/*.test.tsmust “usevi.mock("iii-sdk")with mocksdk.trigger,kv.get/set/list” and “follow existing patterns fromtest/crystallize.test.ts.”🤖 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/search.test.ts` around lines 11 - 51, Replace the local mock helpers mockKV() and mockSdk() with the repository-standard vi.mock("iii-sdk") pattern: remove mockKV and mockSdk, add vi.mock("iii-sdk") at the top of the test, and in its factory return an object with sdk.trigger implemented (to dispatch to your registered handlers) and kv.get, kv.set, kv.list, kv.delete implementations that mirror the behavior currently in mockKV; follow the same mock shapes and call signatures used in test/crystallize.test.ts so existing tests using sdk.trigger and kv.get/set/list continue to work.
🤖 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/functions/session-attribution.ts`:
- Around line 46-47: sessionAttributionLabel currently omits role when client is
absent; change the logic that builds the parts array so that when client is
present you push `${client}/${role}` (if role exists) and when client is missing
but role exists you push just `role`; update the block around parts.push in
sessionAttributionLabel to conditionally push client, client+role, or role so
role-only sessions get preserved.
---
Outside diff comments:
In `@src/functions/search.ts`:
- Around line 128-133: The session lookup currently lets kv.get(...) rejections
bubble through loadSession and can cause the surrounding Promise.all (session
enrichment) to reject; update loadSession to catch any errors from kv.get and
treat failures as a missing session by logging/ignoring the error, setting
sessionCache.set(sessionId, null), and returning null so enrichment degrades
gracefully; apply the same defensive try/catch pattern wherever kv.get is used
for session reads (the other instance around lines 164-166) so those calls also
return null on error instead of throwing.
In `@test/search.test.ts`:
- Around line 11-51: Replace the local mock helpers mockKV() and mockSdk() with
the repository-standard vi.mock("iii-sdk") pattern: remove mockKV and mockSdk,
add vi.mock("iii-sdk") at the top of the test, and in its factory return an
object with sdk.trigger implemented (to dispatch to your registered handlers)
and kv.get, kv.set, kv.list, kv.delete implementations that mirror the behavior
currently in mockKV; follow the same mock shapes and call signatures used in
test/crystallize.test.ts so existing tests using sdk.trigger and kv.get/set/list
continue to work.
🪄 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: 9ab024ec-5a6c-466f-a5fd-f8b3fc0930c0
📒 Files selected for processing (13)
README.mdsrc/functions/context.tssrc/functions/search.tssrc/functions/session-attribution.tssrc/functions/session-metadata.tssrc/functions/smart-search.tssrc/triggers/events.tssrc/types.tssrc/viewer/index.htmltest/context-slots.test.tstest/search.test.tstest/session-metadata.test.tstest/smart-search.test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- test/session-metadata.test.ts
- src/functions/session-metadata.ts
- src/triggers/events.ts
1 participant
Summary
Session.agentandSession.metadatafields for client/model/role attributionagent,metadata, convenience agent fields, and commonrole/agent_type/sourcealiases onPOST /agentmemory/session/startmodel,agent_type, andsourcein the SessionStart hook payloadmem::search,mem::smart-search, andmem::contextso recalled context names the originating client/model/roleNotes
This is a focused first slice of session metadata support. It stores and returns metadata on sessions, makes identity visible in the UI, and carries attribution into agent-facing recall/context output. Update/filter APIs can land as a follow-up without changing the stored shape.
Refs #259
CodeRabbit Follow-up
sessionAttributionLabel()mem::searchtreat failed session lookups as missing attribution instead of failing the whole searchtest/search.test.tsto the repo-standardvi.mock("iii-sdk")test harness patternTests
npm run buildnode --check <(awk '/<script nonce/{flag=1;next}/<\\/script>/{flag=0}flag' src/viewer/index.html) && git diff --checknpm test -- --run test/session-metadata.test.ts test/search.test.ts test/smart-search.test.ts test/context-slots.test.ts test/context-injection.test.tsSANDBOX_HOME=$(mktemp -d); env -i HOME="$SANDBOX_HOME" USERPROFILE="$SANDBOX_HOME" PATH="$PATH" AGENTMEMORY_URL="http://127.0.0.1:9" AGENTMEMORY_PROBE_TIMEOUT_MS="10" npm test; rc=$?; rm -rf "$SANDBOX_HOME"; exit $rcSummary by CodeRabbit
New Features
Documentation
Tests