fix(notifications): match goal owner by canonical forms, not '%user%' substring

src/notifications/sources/open-goals.ts

@@ -2,9 +2,12 @@
  * Open-goals SessionStart summary.
  *
  * Reads the user's open goals from the dedicated hivemind_goals
- * table (owner + status filter on indexed columns, no LIKE scan)
- * and produces a short one-line summary the primary banner appends
- * to its body.
+ * table via the shared `listOpenGoals` reader (canonical owner-form
+ * matching + version=MAX dedup) and produces a short one-line summary
+ * the primary banner appends to its body. Sharing that reader keeps
+ * the banner and the SessionStart context block in agreement — both
+ * count one row per goal_id and never leak another user's goals via a
+ * substring collision.
  *
  * Returns null when:
  *   - creds are missing
@@ -19,7 +22,7 @@
 
 import type { Credentials } from "../../commands/auth-creds.js";
 import { DeeplakeApi } from "../../deeplake-api.js";
-import { sqlIdent, sqlStr } from "../../utils/sql.js";
+import { listOpenGoals } from "../../hooks/shared/context-renderer.js";
 import { log as _log } from "../../utils/debug.js";
 
 const log = (msg: string) => _log("notifications-open-goals", msg);
@@ -49,44 +52,23 @@ export async function fetchOpenGoals(
       creds.workspaceId ?? "default",
       goalsTableName,
     );
-    const safe = sqlIdent(goalsTableName);
-    // Latest version per goal_id, filtered to current owner +
-    // non-closed status. The (goal_id, version) and (owner, status)
-    // indexes both apply, so this stays a cheap point lookup even
-    // on a large hivemind_goals table.
-    //
-    // We tolerate userName in either short ("emanuele.fenocchi") or
-    // full-email form ("emanuele.fenocchi@activeloop.ai") by using
-    // a LIKE substring match on the owner column. Different agents
-    // historically populated this field with one or the other; the
-    // skill instructs the agent to use whatever userName the
-    // credentials carry, but staleness in older rows is real.
-    const sql =
-      // One row per goal_id (UPDATE-or-INSERT model), so a direct
-      // WHERE on owner+status is the cheap and correct path.
-      `SELECT goal_id, owner, status, content FROM "${safe}" ` +
-      `WHERE owner LIKE '%${sqlStr(creds.userName)}%' ` +
-      `  AND status IN ('opened', 'in_progress') ` +
-      `ORDER BY created_at DESC LIMIT 25`;
-    const rows = (await api.query(sql)) as Array<{
-      goal_id?: string;
-      owner?: string;
-      status?: string;
-      content?: string;
-    }>;
-    if (!Array.isArray(rows) || rows.length === 0) return null;
+    // Reuse the canonical goal reader: it matches the owner by exact
+    // full form, exact short form, and `short@%` alias (never a
+    // `'%user%'` substring scan, which collides — e.g. 'ali' matching
+    // 'malice@…') and keeps only the latest version per goal_id, so
+    // multiple stored versions of one goal count exactly once.
+    const rows = await listOpenGoals(
+      sql => api.query(sql) as Promise<Array<Record<string, unknown>>>,
+      goalsTableName,
+      creds.userName,
+      { limit: 25 },
+    );
+    if (rows.length === 0) return null;
 
     const goals: Array<{ label: string }> = [];
     for (const r of rows) {
-      const owner = String(r.owner ?? "");
-      const content = String(r.content ?? "");
-      if (!owner || !content) continue;
-      // Bi-directional substring match — same userName variant
-      // problem as the previous LIKE narrowing did at the SQL
-      // layer. Belt-and-braces guard for older rows.
-      const u = creds.userName;
-      if (owner !== u && !owner.includes(u) && !u.includes(owner)) continue;
-      goals.push({ label: firstLine(content) });
+      if (!r.content) continue;
+      goals.push({ label: firstLine(r.content) });
     }
     if (goals.length === 0) return null;
     return {

tests/claude-code/notifications-open-goals-source.test.ts

@@ -9,7 +9,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
  * `query` method) and capture the exact SQL the banner caused, so any
  * regression in:
  *   - SQL injection escaping of `creds.userName`
- *   - the owner-LIKE matching that tolerates short vs full-email forms
+ *   - the canonical owner-form matching that tolerates short vs
+ *     full-email forms without admitting substring collisions
  *   - the status filter (must not surface closed goals)
  *   - the limit (banner is bounded — never lets a flood of goals stretch)
  * fails fast.
@@ -72,26 +73,30 @@ describe("fetchOpenGoals — auth gating", () => {
 // ── SQL shape ───────────────────────────────────────────────────────────────
 
 describe("fetchOpenGoals — SQL shape", () => {
-  it("issues a single SELECT scoped to current owner with LIKE + non-closed status filter + limit 25", async () => {
+  it("issues a single SELECT scoped to current owner with canonical-form match + non-closed status filter + version dedup + limit 25", async () => {
     queryMock.mockResolvedValue([]);
     await fetchOpenGoals(BASE_CREDS as any, "hivemind_goals_test");
     expect(queryMock).toHaveBeenCalledTimes(1);
     const sql = queryMock.mock.calls[0][0] as string;
     expect(sql).toMatch(/^SELECT goal_id, owner, status, content FROM "hivemind_goals_test"/);
-    // owner LIKE is intentional — tolerates short vs full-email forms
-    expect(sql).toContain(`WHERE owner LIKE '%alice@activeloop.ai%'`);
+    // Canonical owner forms (exact full / exact short / `short@%` alias)
+    // instead of a `'%user%'` substring scan, which collides across users.
+    expect(sql).toContain(`(owner = 'alice@activeloop.ai' OR owner = 'alice' OR owner LIKE 'alice@%')`);
+    expect(sql).not.toContain(`LIKE '%alice`);
     expect(sql).toContain(`status IN ('opened', 'in_progress')`);
     // 'closed' status must never show up in the banner — that would
     // surface user's already-done work as if it were still open.
     expect(sql).not.toMatch(/closed/);
-    expect(sql).toContain("ORDER BY created_at DESC LIMIT 25");
+    // One row per goal_id — multiple stored versions count once.
+    expect(sql).toContain("version = (SELECT MAX(version)");
+    expect(sql).toContain("LIMIT 25");
   });
 
   it("escapes single quotes in userName (SQL injection guard)", async () => {
     queryMock.mockResolvedValue([]);
     await fetchOpenGoals({ ...BASE_CREDS, userName: "O'Brien" } as any, "hivemind_goals");
     const sql = queryMock.mock.calls[0][0] as string;
-    expect(sql).toContain(`'%O''Brien%'`);
+    expect(sql).toContain(`owner = 'O''Brien'`);
   });
 
   it("refuses to issue a query when goalsTableName is not a valid SQL identifier", async () => {
@@ -133,10 +138,10 @@ describe("fetchOpenGoals — result mapping", () => {
     expect(summary!.sample).toEqual(["Ship search", "Land memory", "Onboard team"]);
   });
 
-  it("filters out rows where owner doesn't substring-match userName (defense-in-depth past the LIKE)", async () => {
+  it("filters out rows whose owner isn't a canonical form of userName (defense-in-depth past the SQL gate)", async () => {
     queryMock.mockResolvedValue([
       { goal_id: "g1", owner: "alice@activeloop.ai", status: "opened", content: "mine" },
-      // A stray row that the LIKE allowed through but logically isn't this user
+      // A stray row that slipped past the SQL gate but logically isn't this user
       { goal_id: "g2", owner: "bob@activeloop.ai",   status: "opened", content: "his" },
     ]);
     const summary = await fetchOpenGoals(BASE_CREDS as any, "hivemind_goals");