Skip to content

First-run + resume session briefs, kept to the user channel#223

Merged
khustup2 merged 7 commits into
mainfrom
feat/session-briefs-and-channel-safety
May 31, 2026
Merged

First-run + resume session briefs, kept to the user channel#223
khustup2 merged 7 commits into
mainfrom
feat/session-briefs-and-channel-safety

Conversation

@khustup2
Copy link
Copy Markdown
Contributor

@khustup2 khustup2 commented May 30, 2026

Summary

Adds two SessionStart briefs and makes a hard rule that every user-facing notification stays out of the model's prompt context. The channel-safety half is prompted by a user flagging our credit notice as a prompt-injection attempt — it was.

Briefs

  • cold-start-brief — first-run insight mined from local history. CTA + cadence are sign-in-aware:
    • anonymous → a sign-in nudge that re-surfaces at most once/24h until they convert (the install→sign-in conversion slot);
    • signed-in → a one-time onboarding brief, after which resume-brief takes over.
  • resume-brief — "where you left off" for signed-in users, from the latest project summary. Query bounded at 1.5s so it can't stall the hook; pointer skips # Session/metadata/**Label** boilerplate and returns the first prose sentence (verified against real summaries).

Channel safety (the important part)

A user's agent flagged repeated HIVEMIND ALERT system-reminders as prompt-injection. They were right — it was our own capture.ts writing "HIVEMIND ALERT … Tell the user clearly: … top up at <url>" into the model's additionalContext on every failed capture.

  • Notifications banner is now userVisibleOnly across the board — welcome / savings / briefs render to systemMessage, never additionalContext. Mined and summary-derived prose can no longer reach the model.
  • capture.ts no longer emits the mid-session credit notice into the model channel (no safe user channel exists for PostToolUse, so we don't smuggle it via the model). The credit-exhausted condition reaches the user via the SessionStart balance banner.
  • balance-exhausted / low-balance notifications → userVisibleOnly.
  • session-start no-auth note is neutral/factual (dropped the imperative "ask the user to run /hivemind:login").
  • localMinedRule unregistered: the cold-start signup brief owns the single anonymous conversion slot — no double login nudge.

Tests

  • New channel-split regression: a resume brief carrying an injection-shaped payload reaches systemMessage but never additionalContext.
  • New prose-extraction unit tests (boilerplate skip, mid-token-dot safety, empty-on-no-prose).
  • Updated welcome/savings assertions to the user channel.
  • Full tests/claude-code + tests/shared suites green (2833 tests); tsc --noEmit clean.

Known follow-ups (minor, same class, not in this PR)

  • updateNotice and localMinedNote are still appended to session-start's additionalContext — milder (no "tell the user / pay"), but should move to the user-only path.
  • Add a CI guard that fails if any additionalContext write contains billing / hivemind login / hivemind:update-style user-directed strings.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Session briefs now appear on welcome, showing insights from your last project or suggesting next steps on first run
    • Personalized greetings for first-time and returning users
    • Simplified messaging when not logged into Deeplake
  • Bug Fixes

    • Removed disruptive mid-session balance notifications
    • Billing notices now properly handled as user-visible content only
  • Tests

    • Added test coverage for session brief extraction and welcome banner logic

Review Change Stack

…ser channel

Adds two SessionStart briefs and ensures every user-facing notification
stays out of the model's prompt context.

Briefs
- cold-start-brief: first-run insight mined from local history. CTA and
  cadence are sign-in-aware — anonymous users get a sign-in nudge that
  re-surfaces at most once/24h until they convert; signed-in users get a
  one-time onboarding brief, after which resume-brief takes over.
- resume-brief: "where you left off" for signed-in users, from the latest
  project summary. Query bounded at 1.5s so it can't stall the hook;
  pointer skips boilerplate and returns the first prose sentence.

Channel safety (a user flagged our credit notice as a prompt-injection)
- The notifications banner is userVisibleOnly across the board:
  welcome / savings / briefs render to systemMessage, never
  additionalContext. Mined and summary-derived prose can no longer enter
  the model's context.
- capture.ts no longer writes a "HIVEMIND ALERT ... tell the user ... top
  up at <url>" notice into additionalContext on capture failure. The
  credit-exhausted condition reaches the user via the SessionStart banner.
- balance-exhausted / low-balance notifications are userVisibleOnly.
- session-start no-auth note is neutral/factual (dropped the imperative
  "ask the user to run /hivemind:login").
- localMinedRule unregistered: the cold-start signup brief owns the single
  anonymous conversion slot, so no double login nudge.

Tests: channel-split regression (resume prose hits systemMessage, never
additionalContext), prose extraction, updated welcome/savings channel
assertions.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 891cab19-46ae-4bd8-b657-1b5780955cc0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR refactors the session notification pipeline by removing mid-session credit signaling and local-mining-derived rules, introducing cold-start and resume brief generators that provide contextual session-startup messaging, and ensuring all user-facing content (briefs and banners) routes through the user-visible channel only.

Changes

Session Notification Pipeline Refactoring and Context Briefs

Layer / File(s) Summary
Mid-session error handling cleanup
src/hooks/capture.ts
Removes the mid-session "credits exhausted" inline notification path; the main().catch handler now exits cleanly with process.exit(0) instead of emitting stdout banners on DeepLake 402 errors.
Balance/billing notification user-visibility marking
src/deeplake-api.ts
Marks balance-exhausted and low-balance-warning notifications with userVisibleOnly: true so they render in user-facing channels.
Session notification rule and field cleanup
src/hooks/session-notifications.ts
Removes localMinedRule registration and local computation of localSkillsCount/latestInsightEntry; simplifies drainSessionStart to only pass credential and session metadata.
Session-start login context update
src/hooks/session-start.ts
Shortens the "not logged in" injected context to state that memory search is unavailable this session, removing explicit login instructions.
Cold-start brief notification source
src/notifications/sources/cold-start-brief.ts
Implements a new brief generator that mines local Claude .jsonl session files for first-run and re-nudge users, derives signals from session patterns (recall-seeking openers, abandoned threads, project concentration), and renders a 3–4 sentence contextual brief with rate-limiting and persistent state tracking.
Resume brief notification source
src/notifications/sources/resume-brief.ts
Implements resume brief source for signed-in users; queries Deeplake for the latest session summary matching the current project, extracts the first meaningful prose sentence via pattern-based parsing, and returns a display-ready brief with relative-age suffix.
Primary banner brief integration and channel routing
src/notifications/sources/primary-banner.ts
Integrates cold-start and resume briefs into primary-banner selection: anonymous users with a cold-start brief receive a signup-brief notification; signed-in users get a brief prefix prepended to welcome or savings-recap, with all notifications marked userVisibleOnly: true and briefs excluded from model additionalContext.
Primary banner test mocking
tests/claude-code/notifications-primary-banner.test.ts
Adds resumeMock to control resume-brief resolution without live Deeplake queries, defaulting to null.
Integration test updates for channel routing
tests/claude-code/notifications.test.ts
Updates notification flow tests to verify channel splitting: welcome and recap banners appear in payload.systemMessage with userVisibleOnly: true and additionalContext undefined; adds new test validating resume-brief text reaches systemMessage only.
Resume brief prose-extraction unit tests
tests/claude-code/resume-brief.test.ts
Adds Vitest suite for firstProseSentence, verifying it correctly skips non-prose regions (headers, bullets, labels) and extracts sentences while handling internal punctuation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • activeloopai/hivemind#216: Both PRs touch src/deeplake-api.ts's low-balance/balance-exhausted notification flow—Hivemind: surface low-balance warning banner via X-Activeloop-Balance-Cents #216 adds/enqueues the low-balance-warning from X-Activeloop-Balance-Cents, and this PR updates those same queued notification payloads with userVisibleOnly: true.

  • activeloopai/hivemind#96: This PR modifies src/hooks/session-notifications.ts to align its drainSessionStart/rule wiring with the centralized notification framework introduced in #96, and updates src/notifications/* banner sources that feed into the same pipeline.

  • activeloopai/hivemind#128: Both PRs modify the session-start notification pipeline—especially src/hooks/session-notifications.ts and how drainSessionStart is invoked with sessionId—to control which notifications are generated and emitted.

Suggested reviewers

  • efenocchi
  • kaghni

Poem

🐰 Briefs bloom bright on session starts,
Cold-mined signals, resume arts,
No more mid-streams, just clean exits flow,
User-visible paths all aglow!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description comprehensively covers the summary, implementation details, testing approach, and known follow-ups, but the Version Bump section required by the template is missing. Add the Version Bump section to specify the appropriate version change (likely minor, as this adds new features for session briefs).
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the two main changes: adding session briefs (first-run and resume) and ensuring they stay in the user-visible channel, not the model context.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/session-briefs-and-channel-safety

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot requested review from efenocchi and kaghni May 30, 2026 18:30
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/notifications/sources/cold-start-brief.ts (2)

121-126: ⚡ Quick win

Add defensive check for state directory existence.

writeFileSync at line 124 will throw if the parent directory (~/.claude) doesn't exist. While the code reads from ~/.claude/projects earlier (making it likely to exist), adding an explicit mkdirSync guard would prevent edge-case failures.

🛡️ Proposed defensive mkdir
+import { existsSync, readdirSync, statSync, writeFileSync, readFileSync, openSync, readSync, closeSync, mkdirSync } from "node:fs";
+import { join, dirname } from "node:path";

...

 function writeState(sessionsScanned: number): void {
   try {
+    const stateFile = STATE_FILE();
+    const dir = dirname(stateFile);
+    if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
     writeFileSync(
-      STATE_FILE(),
+      stateFile,
       JSON.stringify({ lastBriefTs: new Date().toISOString(), fireReason: "first_run", sessionsScanned }),
     );
   } catch (e: unknown) {
     log(`writeState failed: ${(e as Error).message}`);
   }
 }
🤖 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/notifications/sources/cold-start-brief.ts` around lines 121 - 126, The
writeState function currently calls writeFileSync(STATE_FILE(), ...) which will
throw if the parent directory (~/.claude) does not exist; before writing, ensure
the directory exists by resolving the directory via path.dirname(STATE_FILE())
and calling mkdirSync(dir, { recursive: true }) (or an equivalent async check)
to create the parent directory, then proceed with writeFileSync; update the
writeState function to perform this defensive mkdir check using path and fs
utilities.

30-30: ⚖️ Poor tradeoff

Consider asynchronous file operations to avoid blocking the event loop.

The module uses synchronous fs operations extensively (readFileSync, readdirSync, statSync, etc.), which can block Node's event loop during I/O. While the HARD_TIMEOUT_MS budget and chunked-read strategy mitigate unbounded work, switching to async operations with Promise.race for timeout would maintain responsiveness.

🤖 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/notifications/sources/cold-start-brief.ts` at line 30, The file currently
imports and uses blocking fs functions (readFileSync, readdirSync, statSync,
writeFileSync, openSync, readSync, closeSync); replace those with their async
equivalents from fs/promises or the asynchronous stream APIs (readdir, stat,
readFile, writeFile, open + filehandle.read/close or createReadStream) inside
the ColdStartBrief code paths, and wrap each long-running I/O operation or the
whole processing routine with a Promise.race against the existing
HARD_TIMEOUT_MS to enforce the timeout; update functions that perform chunked
reads to use async reads or streams (using FileHandle.read or a readable stream)
and ensure proper try/finally close semantics and error propagation so behavior
matches current logic but without blocking the event loop.
tests/claude-code/resume-brief.test.ts (1)

20-48: ⚡ Quick win

Consider adding test cases for additional sentence-ending punctuation.

The current tests thoroughly cover period-ending sentences and edge cases, but the regex pattern /^.*?[.!?](\s|$)/ also supports ! and ?. Consider adding test cases for:

  • Sentence ending with ! → ensures exclamation marks are recognized
  • Sentence ending with ? → ensures question marks are recognized
  • Multiple sentences in prose → verifies only the first is returned
  • Empty string input → confirms graceful empty-string return
📝 Suggested additional test cases
it("handles exclamation mark sentence endings", () => {
  const s = "## What Happened\nGreat progress today! More work tomorrow.";
  expect(firstProseSentence(s)).toBe("Great progress today!");
});

it("handles question mark sentence endings", () => {
  const s = "## What Happened\nWhat's the plan? We'll figure it out.";
  expect(firstProseSentence(s)).toBe("What's the plan?");
});

it("returns only the first sentence when multiple exist", () => {
  const s = "## What Happened\nFirst sentence here. Second sentence. Third.";
  expect(firstProseSentence(s)).toBe("First sentence here.");
});

it("returns empty string for empty input", () => {
  expect(firstProseSentence("")).toBe("");
});
🤖 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 `@tests/claude-code/resume-brief.test.ts` around lines 20 - 48, Tests only
assert period-ending sentences but the regex in firstProseSentence supports !
and ? and edge cases; add unit tests covering exclamation and question endings,
multiple sentences to ensure only the first is returned, and an empty-string
input to ensure it returns "" so behavior matches the regex. Add tests
referencing firstProseSentence such as: a prose line ending with "!" returning
that sentence, a line ending with "?" returning that sentence, a multi-sentence
prose returning only the first sentence, and calling firstProseSentence("")
expecting "". Ensure these new cases follow the existing test structure and
edge-case patterns used in the file.
🤖 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/notifications/sources/cold-start-brief.ts`:
- Around line 121-130: The writeState function always hardcodes fireReason as
"first_run", which mislabels subsequent anonymous re-nudges; change writeState
to accept a fireReason parameter (e.g., 'first_run' | 're_nudge' | string),
write that value to the JSON instead of the constant, and update every call site
(the cold-start notifier caller that currently invokes writeState(...) around
the 450s) to pass the appropriate reason (pass 'first_run' on initial run and
're_nudge' for 24-hour re-notifications); keep using STATE_FILE() and the same
JSON shape and preserve the existing try/catch logging in writeState.
- Line 254: The code that computes projDirName uses a hardcoded "/" separator
which breaks on Windows; update the extraction in cold-start-brief.ts to use
Node's path utilities (e.g., path.sep or better path.dirname/path.basename)
instead of splitting on "/", so replace the split("/") logic that assigns
projDirName with a path-aware approach (use path.basename(path.dirname(path)) or
split using path.sep) to reliably get the project directory name across
platforms.

In `@src/notifications/sources/resume-brief.ts`:
- Around line 128-144: The SQL identifier for the Deeplake table is used without
validation in pickResumeBrief (cfg?.tableName ?? "memory") and can lead to SQL
injection when interpolated into DeeplakeApi.query; validate the table name
using the repository's sqlIdent() (or a strict identifier regex) before building
the query, and if validation fails, log an error (include context: table value
and source) and return null instead of proceeding to call DeeplakeApi.query;
update pickResumeBrief to reference the validated table identifier and avoid
using sqlStr() for identifiers.

---

Nitpick comments:
In `@src/notifications/sources/cold-start-brief.ts`:
- Around line 121-126: The writeState function currently calls
writeFileSync(STATE_FILE(), ...) which will throw if the parent directory
(~/.claude) does not exist; before writing, ensure the directory exists by
resolving the directory via path.dirname(STATE_FILE()) and calling
mkdirSync(dir, { recursive: true }) (or an equivalent async check) to create the
parent directory, then proceed with writeFileSync; update the writeState
function to perform this defensive mkdir check using path and fs utilities.
- Line 30: The file currently imports and uses blocking fs functions
(readFileSync, readdirSync, statSync, writeFileSync, openSync, readSync,
closeSync); replace those with their async equivalents from fs/promises or the
asynchronous stream APIs (readdir, stat, readFile, writeFile, open +
filehandle.read/close or createReadStream) inside the ColdStartBrief code paths,
and wrap each long-running I/O operation or the whole processing routine with a
Promise.race against the existing HARD_TIMEOUT_MS to enforce the timeout; update
functions that perform chunked reads to use async reads or streams (using
FileHandle.read or a readable stream) and ensure proper try/finally close
semantics and error propagation so behavior matches current logic but without
blocking the event loop.

In `@tests/claude-code/resume-brief.test.ts`:
- Around line 20-48: Tests only assert period-ending sentences but the regex in
firstProseSentence supports ! and ? and edge cases; add unit tests covering
exclamation and question endings, multiple sentences to ensure only the first is
returned, and an empty-string input to ensure it returns "" so behavior matches
the regex. Add tests referencing firstProseSentence such as: a prose line ending
with "!" returning that sentence, a line ending with "?" returning that
sentence, a multi-sentence prose returning only the first sentence, and calling
firstProseSentence("") expecting "". Ensure these new cases follow the existing
test structure and edge-case patterns used in the file.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 60c8182d-1bd6-404e-8b57-3964ad909025

📥 Commits

Reviewing files that changed from the base of the PR and between 8c938bc and c294af9.

📒 Files selected for processing (10)
  • src/deeplake-api.ts
  • src/hooks/capture.ts
  • src/hooks/session-notifications.ts
  • src/hooks/session-start.ts
  • src/notifications/sources/cold-start-brief.ts
  • src/notifications/sources/primary-banner.ts
  • src/notifications/sources/resume-brief.ts
  • tests/claude-code/notifications-primary-banner.test.ts
  • tests/claude-code/notifications.test.ts
  • tests/claude-code/resume-brief.test.ts

Comment on lines +121 to +130
function writeState(sessionsScanned: number): void {
try {
writeFileSync(
STATE_FILE(),
JSON.stringify({ lastBriefTs: new Date().toISOString(), fireReason: "first_run", sessionsScanned }),
);
} catch (e: unknown) {
log(`writeState failed: ${(e as Error).message}`);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

fireReason is always "first_run" but anonymous users can fire multiple times.

The writeState function hardcodes fireReason: "first_run", but anonymous users can receive re-nudges every 24 hours (lines 440-441). This makes the persisted state misleading for debugging or telemetry.

📊 Proposed fix to distinguish first-run from re-nudge
-function writeState(sessionsScanned: number): void {
+function writeState(sessionsScanned: number, isFirstRun: boolean): void {
   try {
     writeFileSync(
       STATE_FILE(),
-      JSON.stringify({ lastBriefTs: new Date().toISOString(), fireReason: "first_run", sessionsScanned }),
+      JSON.stringify({ 
+        lastBriefTs: new Date().toISOString(), 
+        fireReason: isFirstRun ? "first_run" : "renudge", 
+        sessionsScanned 
+      }),
     );
   } catch (e: unknown) {
     log(`writeState failed: ${(e as Error).message}`);
   }
 }

Then update the call site at line 454:

-    writeState(sessions.length);
+    writeState(sessions.length, !hadState);
🤖 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/notifications/sources/cold-start-brief.ts` around lines 121 - 130, The
writeState function always hardcodes fireReason as "first_run", which mislabels
subsequent anonymous re-nudges; change writeState to accept a fireReason
parameter (e.g., 'first_run' | 're_nudge' | string), write that value to the
JSON instead of the constant, and update every call site (the cold-start
notifier caller that currently invokes writeState(...) around the 450s) to pass
the appropriate reason (pass 'first_run' on initial run and 're_nudge' for
24-hour re-notifications); keep using STATE_FILE() and the same JSON shape and
preserve the existing try/catch logging in writeState.

const projectCwd = first.cwd ?? last.cwd;
if (last.ts < cutoff) return null;

const projDirName = path.split("/").slice(-2, -1)[0] ?? "unknown";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Path separator assumption breaks on Windows.

Line 254 hardcodes "/" as the path separator, which fails on Windows where paths use "\". The project directory name extraction will return incorrect results.

🔧 Proposed fix using path.sep
+import { join, sep } from "node:path";

...

-  const projDirName = path.split("/").slice(-2, -1)[0] ?? "unknown";
+  const projDirName = path.split(sep).slice(-2, -1)[0] ?? "unknown";
🤖 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/notifications/sources/cold-start-brief.ts` at line 254, The code that
computes projDirName uses a hardcoded "/" separator which breaks on Windows;
update the extraction in cold-start-brief.ts to use Node's path utilities (e.g.,
path.sep or better path.dirname/path.basename) instead of splitting on "/", so
replace the split("/") logic that assigns projDirName with a path-aware approach
(use path.basename(path.dirname(path)) or split using path.sep) to reliably get
the project directory name across platforms.

Comment on lines +128 to +144
const table = cfg?.tableName ?? "memory";
const api = new DeeplakeApi(
creds.token,
creds.apiUrl ?? "https://api.deeplake.ai",
creds.orgId,
creds.workspaceId ?? "default",
table,
);

// Most recent summary for THIS project by THIS user. summary != ''
// skips placeholder rows; ORDER BY last_update_date DESC takes the
// latest. LIMIT 1 — we only need the one resume pointer.
const rows = await withTimeout(
api.query(
`SELECT summary, project, last_update_date FROM "${table}" ` +
`WHERE project = '${sqlStr(project)}' AND author = '${sqlStr(creds.userName)}' ` +
`AND summary <> '' ORDER BY last_update_date DESC LIMIT 1`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check DeeplakeApi.query signature and usage patterns

# Search for DeeplakeApi class definition and query method signature
ast-grep --pattern $'class DeeplakeApi {
  $$$
  query($$$) {
    $$$
  }
  $$$
}'

# Search for all query() call sites to see if any use parameterization
rg -n -C3 'api\.query\(' --type ts

Repository: activeloopai/hivemind

Length of output: 44147


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Locate DeeplakeApi implementation
rg -n --hidden --no-ignore -S "class DeeplakeApi" src tests || true
rg -n --hidden --no-ignore -S "knownTablesOrNull\\(" src tests || true
rg -n --hidden --no-ignore -S "ensureSessionsTable\\(" src tests || true

# If class not found, locate the file exporting DeeplakeApi
rg -n --hidden --no-ignore -S "export (class|default function|const) DeeplakeApi" src tests || true

# 2) Print the relevant part of resume-brief.ts (table construction + query)
sed -n '1,220p' src/notifications/sources/resume-brief.ts

# 3) Find loadConfig() / HIVEMIND_TABLE / tableName validation (if any)
rg -n --hidden --no-ignore -S "HIVEMIND_TABLE|tableName|loadConfig\\(" src || true
fd -a "loadConfig" src || true
fd -a "*config*" src/notifications/sources || true

# 4) If sqlStr() is in this module or imported, locate it
rg -n --hidden --no-ignore -S "function sqlStr|const sqlStr|sqlStr\\(" src/notifications/sources/resume-brief.ts src || true

Repository: activeloopai/hivemind

Length of output: 36790


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect DeeplakeApi.query and any identifier escaping/validation helpers
sed -n '240,520p' src/deeplake-api.ts
echo "----"
sed -n '520,720p' src/deeplake-api.ts

# Inspect sql identifier helper usage (sqlIdent) if present
rg -n "function sqlIdent|sqlIdent\\(" src/deeplake-api.ts src/utils -S || true
echo "----"
sed -n '1,120p' src/utils/sql.ts

# Inspect config.ts to see how tableName/HIVEMIND_TABLE is sourced/validated
sed -n '1,120p' src/config.ts
sed -n '120,240p' src/config.ts

# Inspect resume-brief.ts around table/query
sed -n '110,180p' src/notifications/sources/resume-brief.ts

# Inspect tests for invalid SQL identifier behavior (for ensureSessionsTable)
sed -n '280,420p' tests/claude-code/sessions-table.test.ts

Repository: activeloopai/hivemind

Length of output: 34596


SQL injection risk: unvalidated table identifier in resume-brief.ts

In src/notifications/sources/resume-brief.ts (lines ~127-144), pickResumeBrief uses cfg?.tableName ?? "memory" (sourced from process.env.HIVEMIND_TABLE in loadConfig) directly in FROM "${table}" without any identifier validation. sqlStr() only escapes string literals, and DeeplakeApi.query accepts a raw SQL string (no parameterized queries), so a malicious HIVEMIND_TABLE containing " can break out of the quoted identifier and modify the query.

Fix: validate table with the repo’s existing sqlIdent() (or equivalent strict regex) before interpolating; if invalid, log and return null.

🤖 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/notifications/sources/resume-brief.ts` around lines 128 - 144, The SQL
identifier for the Deeplake table is used without validation in pickResumeBrief
(cfg?.tableName ?? "memory") and can lead to SQL injection when interpolated
into DeeplakeApi.query; validate the table name using the repository's
sqlIdent() (or a strict identifier regex) before building the query, and if
validation fails, log an error (include context: table value and source) and
return null instead of proceeding to call DeeplakeApi.query; update
pickResumeBrief to reference the validated table identifier and avoid using
sqlStr() for identifiers.

- tests: update the bundle-artifact (built session-notifications.js) cases
  to the new user-only channel — welcome renders to systemMessage and never
  to additionalContext (the source-level cases were already updated; these
  run the compiled bundle, which CI rebuilds).
- resume-brief: validate the table identifier with sqlIdent() before
  interpolating into FROM "<table>". sqlStr escapes literals, not
  identifiers, so a HIVEMIND_TABLE containing a double-quote could break out
  — bail to a plain welcome on an invalid name.
- cold-start-brief: split jsonl/cwd paths on both / and \ so project-label
  derivation is correct on Windows.
- cold-start-brief: writeState records fireReason "first_run" vs "renudge"
  instead of always "first_run", so the anonymous 24h re-nudge isn't
  mislabelled in state.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

Coverage Report

Scope: files changed in this PR. Enforced threshold: 90% per metric (per file via vitest.config.ts).

Status Category Percentage Covered / Total
🔴 Lines 78.70% (🎯 90%) 632 / 803
🔴 Statements 75.92% (🎯 90%) 725 / 955
🔴 Functions 79.84% (🎯 90%) 103 / 129
🔴 Branches 68.06% (🎯 90%) 422 / 620
File Coverage — 10 files changed
File Stmts Branches Functions Lines
src/deeplake-api.ts 🟢 98.9% 🔴 89.0% 🟢 100.0% 🟢 99.6%
src/hooks/capture.ts 🟢 97.4% 🔴 81.8% 🟢 100.0% 🟢 100.0%
src/hooks/session-notifications.ts 🟢 92.3% 🟢 100.0% 🔴 75.0% 🟢 100.0%
src/hooks/session-start.ts 🟢 100.0% 🟢 96.2% 🟢 100.0% 🟢 100.0%
src/notifications/index.ts 🟢 100.0% 🟢 95.0% 🟢 100.0% 🟢 100.0%
src/notifications/sources/backend.ts 🟢 97.4% 🟢 97.0% 🔴 80.0% 🟢 100.0%
src/notifications/sources/cold-start-brief.ts 🔴 22.0% 🔴 9.7% 🔴 40.9% 🔴 24.1%
src/notifications/sources/org-stats.ts 🔴 87.3% 🔴 76.9% 🟢 90.9% 🟢 91.7%
src/notifications/sources/primary-banner.ts 🔴 88.9% 🔴 88.2% 🟢 92.3% 🔴 88.8%
src/notifications/sources/resume-brief.ts 🔴 19.7% 🔴 20.4% 🔴 10.0% 🔴 21.1%

Generated for commit ddc52db.

@kaghni
Copy link
Copy Markdown
Collaborator

kaghni commented May 30, 2026

Reviewed end-to-end. The client-channel fix is solid:

  • All client enqueues (balance-exhausted, low-balance-warning, welcome, savings, briefs) carry userVisibleOnly: true — verified in source and in the built bundle.
  • emitClaudeCode correctly splits: modelSafe = notifications.filter(n => !n.userVisibleOnly), so flagged notifications only reach systemMessage.
  • The mid-session emitBalanceExhaustedInline "HIVEMIND ALERT" path in capture.ts is gone — that was the loudest offender.
  • 85/85 tests pass; CI fully green incl. the new Windows leg.

One gap I'd flag before declaring the injection vector closed: src/notifications/sources/backend.ts:toClient doesn't set userVisibleOnly on backend-pushed rows. So org-targeted notifications coming from GET /me/notifications — including the deeplake-api MaybeWarn row whose body reads "Low Deeplake balance: … Top up to avoid service interruption" — still flow to additionalContext (model-visible). Same prompt-injection shape, different source.

Two ways to close it:

  1. Backend (deeplake-api): set Targets: []string{"email"} on the MaybeWarn notification so hivemind never fetches it as a banner row.
  2. This PR: mark backend rows userVisibleOnly: true in toClient for billing-class ids (backend:low_balance_warning_…).

(Disregard my earlier mention of CodeRabbit findings — I checked HEAD and the path-separator + fireReason ones are already fixed in f575ae3f, the threads just weren't replied to with the SHA.)

khustup2 added 2 commits May 30, 2026 22:13
…atim

renderBrief now emits generic copy ('I found context from your recent
sessions...') instead of the mined signal description + verbatim snippet.
The signal is kept only as a fire/skip gate. Removes the surveillance-y
'your last session ended with "<quote>"' first-contact message.
…rver-push injection gap)

Kamo's review caught that toClient() didn't set userVisibleOnly, so
server-pushed rows from GET /me/notifications — including the deeplake-api
low-balance 'top up to avoid service interruption' push — still reached the
model's additionalContext. Same prompt-injection shape as the client-side
notices we already fixed, different source.

Mark all backend rows userVisibleOnly: the body is server-controlled free
text shown as a banner; no server push should enter the agent prompt.
Defense-in-depth regardless of any deeplake-api Targets change.

Tests: backend-source cases now assert systemMessage + additionalContext
undefined; added a billing-class regression (low-balance push reaches the
user, never the model).
@kaghni
Copy link
Copy Markdown
Collaborator

kaghni commented May 30, 2026

Re-reviewed against 0e84c706 — the backend-push gap is closed. toClient now sets userVisibleOnly: true on every server-pushed notification, with a comment that explicitly calls out the deeplake-api low-balance "top up to avoid service interruption" push as the motivating case. 85/85 tests still green after the change. From my side this clears the residual injection vector I flagged earlier.

khustup2 added 3 commits May 30, 2026 22:35
Reorder drainSessionStart so the welcome/savings banner leads, then
low-balance / backend / rule notifications follow. Previously the queued
low-balance notice rendered above 'Welcome back', which read backwards.
…nto banner

Low-balance warnings were produced on the SDK query path and enqueued, so
they only surfaced at the NEXT SessionStart — a session behind — and never
co-occurred with the first-run brief (the queue was empty on a fresh
session). They could also double with anything live.

Now: org-stats reads X-Activeloop-Balance-Cents from its own SessionStart
call (OrgStats.balanceCents), and pickPrimaryBanner merges a low-balance
line straight into the (userVisibleOnly) welcome/savings banner — shown the
moment it's detected, in the message the user is already reading.

- Remove signalLowBalanceFromHeader + the query-path enqueue + flag (the
  lagging path). Hard exhaustion (402) still flows through the
  balance-exhausted queue notice.
- Scope the live line to the soft warning (0 < bal < $2); exhausted stays
  on its 402 path to avoid a double.
- Drop deeplake-api-low-balance.test.ts (tested the removed path); add
  coverage for the header parse and the live banner merge.
Completes bce3f95 (which only carried the test deletion): the actual
org-stats balance-header read, the pickPrimaryBanner appendBalance merge,
and removal of the lagging SDK signalLowBalanceFromHeader enqueue, plus the
new header-parse and banner-merge tests.
@khustup2 khustup2 merged commit 9498772 into main May 31, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants