Flag tasks with secret findings in Hub sync

CLAUDE.md

@@ -171,11 +171,12 @@ you'd cause a bug by not knowing:
 - **What crosses the sync wire, and what doesn't.** `sync` uploads the reconciled rows *and the
   interpretations*: sessions (with the model's title/summary), usage, `resolved_tasks` (the full
   `TaskFact` — outcome, frustration, signals), interactions (with `task_seq`), invocations, and user
-  **labels**. The things that never leave the machine: the retained prompt/response **text**
-  (`resolved_interaction_text`, toggleable via `retainText`), **secret-scan findings**
-  (`resolved_secret_findings` — redacted locators only, and even those stay local; see
-  `docs/internals/secret-scanning.md`), and **BYO API keys** (`secrets.ts`) — so the interpretations
-  *derived from* the text upload, but the raw text does not.
+  **labels**, plus one derived boolean per task (`flagged` — whether a secret finding landed in it;
+  it ignores dismissal on purpose). The things that never leave the machine: the retained prompt/response **text**
+  (`resolved_interaction_text`, toggleable via `retainText`), **secret-scan finding rows**
+  (`resolved_secret_findings` — no category, hint, digest, or dismissal value is ever uploaded, only
+  that `flagged` bit; see `docs/internals/secret-scanning.md`), and **BYO API keys** (`secrets.ts`) —
+  so the interpretations *derived from* the text upload, but the raw text does not.
 - **Canonical tool/MCP parsing lives in `tool-categories.ts`** (`categorizeTool`, `parseMcpTool` — the
   `mcp__server__tool` split). Route through it so categorization and MCP naming stay consistent.
 - **All LLM access goes through `src/llm/`.** `registry.ts` is the single source of truth (adding a
@@ -206,10 +207,12 @@ imports (the Hub backend had inlined its own copies and dropped it too).
 `Dashboard` still backs the web app's per-view response types (imported type-only by `web/src/types.ts`
 from `src/types.ts`); the wire contract itself is being reworked separately.
 
-Three things stay off the wire entirely: the retained prompt/response text
-(`resolved_interaction_text`), secret-scan findings (`resolved_secret_findings`, #327 — the findings
-hold only redacted locators, and even those stay local), and BYO API keys (`secrets.ts`). The task
-*interpretations* built from that text (outcome, frustration, chapter span) do upload — the raw text
-doesn't. Separately, `store/store-contract.ts` (the parse→store
+Two things stay off the wire entirely: the retained prompt/response text
+(`resolved_interaction_text`) and BYO API keys (`secrets.ts`). Secret-scan findings
+(`resolved_secret_findings`, #327) are nearly a third: no finding row is ever uploaded — not the
+category, hint, digest, or dismissal value — but each uploaded task carries a derived `flagged`
+boolean saying whether a finding landed in it (dismissal doesn't clear it: the banner is one user's,
+the flag is the org's). The task *interpretations* built from the
+retained text (outcome, frustration, chapter span) do upload — the raw text doesn't. Separately, `store/store-contract.ts` (the parse→store
 fact contract, including `PARSED_FRAGMENT_CONTRACT_VERSION`) is its own contract, distinct from the
 `Dashboard`/`SessionRow` types above.

docs/internals/database-schema.md

@@ -357,8 +357,11 @@ hash of the session's whole finding set) is denormalized onto every row so a dis
 digest stored on `resolved_sessions.secret_scan_dismissed` — can be compared in SQL, and lapses when a
 re-scan produces different findings. Two writers: materialize (from the scan it just ran) and the
 version-stamped rescan drain's `writeSessionSecretFindings` (#335), which catches up sessions the
-current scanner hasn't stamped. **Local-only, never synced** (the push path never reads it).
-PK `(session_id, seq)`, FK → `resolved_sessions`. See [secret-scanning.md](./secret-scanning.md).
+current scanner hasn't stamped. **No row ever crosses the wire**: `push.ts` reads the table only to
+derive one boolean per uploaded task (`flagged`), never a category, hint, digest, or dismissal value.
+That boolean deliberately ignores dismissal; see [secret-scanning.md](./secret-scanning.md) for why.
+PK `(session_id, seq)`, FK → `resolved_sessions`.
+See [secret-scanning.md](./secret-scanning.md).
 
 ## Tier 3 — freshness & ownership
 

docs/internals/secret-scanning.md

@@ -90,11 +90,30 @@ forever".
 
 ## What syncs
 
-**Nothing, in v1.** `push.ts` never reads `resolved_secret_findings` or `secret_scan_dismissed`, so
-findings can't cross the wire. The issue floated syncing a bare count as an org-level signal; that
-is an explicit future decision, not a default. If it ever happens, the shape should be a count per
-session at most — never a category-and-hint row, since even a redacted locator tells an org admin
-which of a user's sessions contained a credential.
+Finding details remain local. `push.ts` derives one boolean on each uploaded task by joining a finding
+to its interaction's `task_seq`; the Hub stores that boolean with the task so it can flag affected
+work. No category, hint, digest, dismissal state, or raw text crosses the wire. A finding that is
+not linked to a task produces no task flag.
+
+Three properties that boolean has to have, none of which come for free:
+
+- **It ignores dismissal**, alone among readers of the findings table (`readSecretFindingCounts`,
+  `readSecretFindingsRollup`, `readSessionIdsWithSecretFindings` all filter on
+  `findings_digest IS NOT secret_scan_dismissed`). The divergence is deliberate, so don't reconcile it:
+  dismissal means "I've seen these findings" and silences one user's banner, while this flag is an
+  org-level record that a piece of work touched a credential. If each user's banner state decided what
+  their org sees, the signal would depend on who clicked Dismiss, which is not something an org can
+  reason about. `FLAGGED_TASK_EXISTS_SQL` in `push.ts` carries this note too.
+- **It re-syncs.** Findings are written after materialize (inline, or by the drain), touching none of
+  the fields `computeSessionDigest` normally watches, so the digest also folds in *which* tasks the
+  session's findings flag. Without that, the sessions the drain just flagged are exactly the ones
+  `sync` skips. It's per-task rather than a bare "has findings" bit so that re-interpretation moving a
+  finding between tasks still re-syncs. Because the flag ignores dismissal, the digest does too: a
+  dismissal changes nothing on the wire and so triggers no upload.
+- **It degrades.** `sync` opens the store read-only and never migrates it, so a store last written
+  before schema v24 has no findings table. `push.ts` probes for the table and the dismissal column and
+  reports no flags when they're absent, rather than failing the upload with a raw SQLite error and
+  losing the Hub's version answer.
 
 ## Surfacing
 

src/push.ts

@@ -80,6 +80,8 @@ export interface HubUploadTask {
   source: string;
   ts: number | null;
   task_json: string;
+  /** True when a local secret-scan finding belongs to an interaction in this task. */
+  flagged: boolean;
 }
 
 export interface HubUploadInteraction {
@@ -298,11 +300,18 @@ export function readHubUploadPayload(
       )
       .all();
 
+    const flaggedExpr = hasSecretScanSurface(db)
+      ? `CASE WHEN ${FLAGGED_TASK_EXISTS_SQL} THEN 1 ELSE 0 END`
+      : "0";
     const allTasks = db
-      .query<HubUploadTask, []>(
-        "SELECT session_id, seq, source, ts, task_json FROM resolved_tasks",
+      .query<Omit<HubUploadTask, "flagged"> & { flagged: number }, []>(
+        `SELECT t.session_id, t.seq, t.source, t.ts, t.task_json,
+                ${flaggedExpr} AS flagged
+         FROM resolved_tasks t
+         ORDER BY t.session_id, t.seq`,
       )
-      .all();
+      .all()
+      .map((task) => ({ ...task, flagged: task.flagged !== 0 }));
 
     const allInteractions = db
       .query<HubUploadInteraction, []>(
@@ -358,6 +367,39 @@ export function readSessionIds(dbPath: string, filters: HubUploadFilters = {}):
 /** Read all applied labels (active definitions only), denormalized with the label name/origin and
  *  the owning session's source, for the Hub upload. Shared by the payload read and the cursor scan
  *  so both see the same label state. */
+/** Whether this store has the secret-scan surface (the findings table + the dismissal column, both
+ *  schema v24). `sync` opens `argus.db` READ-ONLY and never migrates it, so a store last written by
+ *  an older build simply doesn't have them. Probe rather than assume: without this, preparing the
+ *  flag query throws a raw `no such table` SQLiteError, which surfaces as `Upload failed (0):
+ *  SQLiteError: …` and repeats every `--watch` interval. Degrading to "no flags" instead lets the
+ *  upload proceed to the Hub's version check, whose 422 carries the "re-index" guidance. */
+function hasSecretScanSurface(db: Database): boolean {
+  return !!db
+    .query<{ name: string }, []>(
+      "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'resolved_secret_findings'",
+    )
+    .get();
+}
+
+// A task is flagged when a secret finding belongs to one of its interactions. Findings carry
+// `interaction_seq`; interactions carry `task_seq`, so the link is a two-hop join with no task pointer
+// to keep in sync.
+//
+// This DELIBERATELY ignores `secret_scan_dismissed`, unlike every local reader of this table
+// (readSecretFindingCounts, readSecretFindingsRollup, readSessionIdsWithSecretFindings). The
+// divergence is the point, so don't "fix" it into consistency: dismissal means "I've seen these
+// findings" and silences one user's banner, while this flag is an org-level record of which work
+// touched a credential. Letting each user's banner state decide what their org sees would make the
+// signal depend on who clicked Dismiss, which is not something an org can reason about. Nothing about
+// the finding itself crosses the wire either way.
+const FLAGGED_TASK_EXISTS_SQL = `EXISTS (
+  SELECT 1
+  FROM resolved_secret_findings f
+  JOIN resolved_interactions i
+    ON i.session_id = f.session_id AND i.seq = f.interaction_seq
+  WHERE f.session_id = t.session_id AND i.task_seq = t.seq
+)`;
+
 function readLabelRows(db: Database): HubUploadLabel[] {
   return db
     .query<HubUploadLabel, []>(
@@ -386,9 +428,39 @@ function labelFingerprint(
   );
 }
 
+/** Which of a session's tasks a secret finding flags, keyed by session id — the same link the payload
+ *  uploads (same shared predicate, so the two can't disagree), read as rows and folded in JS like the
+ *  label fingerprint. This feeds the content digest: the scan drain (#335) writes findings AFTER
+ *  materialize and touches none of the other digest inputs, so without it a session whose only change
+ *  is "we just found a credential in it" keeps its old digest and `sync` skips it forever. Empty when
+ *  the store predates the findings table. */
+function readFlaggedTaskSeqs(db: Database): Map<string, number[]> {
+  const bySession = new Map<string, number[]>();
+  if (!hasSecretScanSurface(db)) return bySession;
+  const rows = db
+    .query<{ session_id: string; task_seq: number }, []>(
+      `SELECT DISTINCT t.session_id AS session_id, t.seq AS task_seq
+       FROM resolved_tasks t
+       WHERE ${FLAGGED_TASK_EXISTS_SQL}`,
+    )
+    .all();
+  for (const row of rows) {
+    const list = bySession.get(row.session_id);
+    if (list) list.push(row.task_seq);
+    else bySession.set(row.session_id, [row.task_seq]);
+  }
+  return bySession;
+}
+
+/** Stable fingerprint of which tasks a session's findings flag. Per-task rather than a bare "has
+ *  findings" bit, so re-interpretation moving a finding from one task to another still re-syncs. */
+function flaggedTaskFingerprint(taskSeqs: number[] | undefined): string {
+  return taskSeqs?.length ? [...taskSeqs].sort((a, b) => a - b).join(",") : "";
+}
+
 /** Compute a stable digest from session fields that can change without advancing last_ts:
  *  archive state, message count, task count, first_prompt, the model-generated title/summary
- *  (#234), and the applied-label fingerprint. */
+ *  (#234), the applied-label fingerprint, and which tasks a secret finding flags (#335). */
 function computeSessionDigest(
   sessionId: string,
   archived: number,
@@ -398,11 +470,12 @@ function computeSessionDigest(
   title: string | null,
   summary: string | null,
   labelFp: string,
+  flaggedFp: string,
 ): string {
   return createHash("sha256")
     .update(
       `${sessionId}|${archived}|${messageCount}|${taskCount}|${firstPrompt ?? ""}` +
-        `|${title ?? ""}|${summary ?? ""}|${labelFp}`,
+        `|${title ?? ""}|${summary ?? ""}|${labelFp}|${flaggedFp}`,
     )
     .digest("hex");
 }
@@ -482,6 +555,10 @@ export function readChangedHubSessionIds(
       else labelsBySession.set(label.session_id, [label]);
     }
 
+    // Same reason as labels: findings are written after materialize, independently of session
+    // activity, so they have to feed the digest or a newly-flagged session never re-uploads (#335).
+    const flaggedBySession = readFlaggedTaskSeqs(db);
+
     const allSessionData = new Map<string, HubSessionCursorRow>();
     const changed: HubSessionCursorRow[] = [];
     for (const row of rows) {
@@ -494,6 +571,7 @@ export function readChangedHubSessionIds(
         row.title,
         row.summary,
         labelFingerprint(labelsBySession.get(row.session_id) ?? []),
+        flaggedTaskFingerprint(flaggedBySession.get(row.session_id)),
       );
       const cursorRow: HubSessionCursorRow = {
         sessionId: row.session_id,
@@ -655,6 +733,13 @@ export async function pushHubJson(
           session.session_id, session.archived, session.message_count, 0, session.first_prompt,
           session.title, session.summary,
           labelFingerprint(payload.rows.labels.filter((l) => l.session_id === session.session_id)),
+          // From the payload we're about to upload, so this fallback digest agrees with the one
+          // readChangedHubSessionIds computes from the store for the same finding state (#335).
+          flaggedTaskFingerprint(
+            payload.rows.tasks
+              .filter((t) => t.session_id === session.session_id && t.flagged)
+              .map((t) => t.seq),
+          ),
         ),
         parserVersion: cursor?.parserVersion ?? PARSED_FRAGMENT_CONTRACT_VERSION,
       };

src/store/store-contract.ts

@@ -934,7 +934,9 @@ export interface ReadModelStore {
   readMcpServerTools(query?: ResolvedQuery): Promise<Array<{ server: string; tool: string; count: number }>>;
   /** Cross-session + per-project friction and the high-token-growth count. */
   readHealthRollups(query?: ResolvedQuery): Promise<HealthRollups>;
-  // ---- Secret-scan findings (#327; local-only, never synced) ----
+  // ---- Secret-scan findings (#327). No finding row is ever synced; push.ts reads the table only to
+  // derive one boolean per uploaded task (never a category, hint, or digest). That boolean ignores
+  // dismissal by design, unlike the readers below; see FLAGGED_TASK_EXISTS_SQL in push.ts. ----
   /** A session's secret-scan findings (redacted locators only) plus whether the user dismissed
    *  exactly this finding set. Backs the session-detail warning banner. */
   readSessionSecretFindings(sessionId: string): Promise<SessionSecretFindings>;

src/store/store.ts

@@ -634,7 +634,8 @@ const CREATE_SCHEMA_SQL = `
   ${RESOLVED_SESSIONS_FTS_DDL}
 
   -- Secret-scan findings (#327): likely exposed credentials spotted in session text. Redacted
-  -- locators only, never secret values. Local-only; never synced.
+  -- locators only, never secret values. No row is ever synced: push.ts reads this table only to
+  -- derive one boolean per uploaded task, never a category, hint, or digest.
   ${RESOLVED_SECRET_FINDINGS_DDL}
 
   -- Per-source freshness attestation: lets a consumer know whether the store is current.