fix(votes): persist upvote counts reliably across merge and hook dispatch

src/__tests__/hook-dispatch-cli.test.ts

@@ -0,0 +1,59 @@
+import { describe, it, expect } from 'vitest';
+
+import { parseStdin } from '../hook-dispatch-cli.js';
+
+describe('parseStdin', () => {
+  it('degrades malformed JSON to an empty object instead of null', () => {
+    // RED BASELINE: before the fix this returns null (short-circuiting all
+    // dispatch). After the fix it degrades to {} so non-stdin-dependent
+    // background handlers still run, and records diagnostics to debug.log.
+    const result = parseStdin('{broken', 'stop');
+    expect(result).not.toBeNull();
+    expect(result).toBeTypeOf('object');
+    expect(result.hook_event_name).toBe('Stop');
+  });
+
+  it('returns an empty object (plus event name) for blank STDIN', () => {
+    const result = parseStdin('', 'stop');
+    expect(result).toEqual({ hook_event_name: 'Stop' });
+  });
+
+  it('parses well-formed JSON and keeps its fields (regression)', () => {
+    const result = parseStdin('{"transcript_path":"/x"}', 'stop');
+    expect(result.transcript_path).toBe('/x');
+    expect(result.hook_event_name).toBe('Stop');
+  });
+
+  it('maps lower-case event aliases to their canonical hook names', () => {
+    const result = parseStdin('', 'session-start');
+    expect(result.hook_event_name).toBe('SessionStart');
+  });
+
+  it('degrades JSON `null` to {} instead of throwing', () => {
+    // RED BASELINE: before the fix, JSON.parse('null') returns null, and the
+    // subsequent `stdin.hook_event_name` access throws TypeError in ESM strict
+    // mode, short-circuiting all dispatch (the very failure mode the original
+    // malformed-JSON fix was meant to prevent).
+    const result = parseStdin('null', 'stop');
+    expect(Array.isArray(result)).toBe(false);
+    expect(result.hook_event_name).toBe('Stop');
+  });
+
+  it('degrades JSON number to {} instead of throwing', () => {
+    // RED BASELINE: before the fix, JSON.parse('123') returns 123, and
+    // assigning a property on a number primitive throws TypeError in ESM
+    // strict mode.
+    const result = parseStdin('123', 'stop');
+    expect(Array.isArray(result)).toBe(false);
+    expect(result.hook_event_name).toBe('Stop');
+  });
+
+  it('degrades JSON array to a plain object (arrays are not records)', () => {
+    // RED BASELINE: before the fix, JSON.parse('[1,2]') returns an array,
+    // which is typeof 'object' but not a plain record — downstream handlers
+    // indexing string keys would misbehave.
+    const result = parseStdin('[1,2]', 'stop');
+    expect(Array.isArray(result)).toBe(false);
+    expect(result).toEqual({ hook_event_name: 'Stop' });
+  });
+});

src/__tests__/votes.test.ts

@@ -190,6 +190,90 @@ describe('mergeDeltas', () => {
     expect(merged.votes['doc-a'].upvoted_count).toBe(0);
     expect(merged.votes['doc-a'].recalled_count).toBe(1);
   });
+
+  it('does not keep last_upvoted_at when upvoted_count is 0', () => {
+    const local: UserVotesV2 = {
+      version: 2,
+      votes: {
+        'doc-bug': {
+          recalled_count: 1,
+          upvoted_count: 0,
+          last_recalled_at: '2026-09-15T00:00:00Z',
+          last_upvoted_at: '2026-09-15T00:00:00Z',
+        },
+      },
+      deltas: { 'doc-bug': { recalled_delta: 1, upvoted_delta: 0 } },
+    };
+    const remote: UserVotesV2 = {
+      version: 2,
+      votes: {
+        'doc-bug': { recalled_count: 0, upvoted_count: 0, last_recalled_at: '' },
+      },
+      deltas: {},
+    };
+
+    const merged = mergeDeltas(local, remote);
+    expect(merged.votes['doc-bug'].upvoted_count).toBe(0);
+    expect(merged.votes['doc-bug'].last_upvoted_at).toBeUndefined();
+  });
+
+  it('does not keep last_recalled_at when recalled_count is 0', () => {
+    const local: UserVotesV2 = {
+      version: 2,
+      votes: {
+        'doc-bug': {
+          recalled_count: 0,
+          upvoted_count: 0,
+          last_recalled_at: '2026-09-15T00:00:00Z',
+        },
+      },
+      deltas: { 'doc-bug': { recalled_delta: -1, upvoted_delta: 0 } },
+    };
+    const remote: UserVotesV2 = {
+      version: 2,
+      votes: {
+        'doc-bug': { recalled_count: 1, upvoted_count: 0, last_recalled_at: '2026-09-05T00:00:00Z' },
+      },
+      deltas: {},
+    };
+
+    const merged = mergeDeltas(local, remote);
+    expect(merged.votes['doc-bug'].recalled_count).toBe(0);
+    expect(merged.votes['doc-bug'].last_recalled_at).toBe('');
+  });
+
+  it('preserves timestamps when counts stay positive (regression)', () => {
+    const local: UserVotesV2 = {
+      version: 2,
+      votes: {
+        'doc-a': {
+          recalled_count: 5,
+          upvoted_count: 2,
+          last_recalled_at: '2026-06-10T00:00:00Z',
+          last_upvoted_at: '2026-06-10T00:00:00Z',
+        },
+      },
+      deltas: { 'doc-a': { recalled_delta: 3, upvoted_delta: 1 } },
+    };
+    const remote: UserVotesV2 = {
+      version: 2,
+      votes: {
+        'doc-a': {
+          recalled_count: 2,
+          upvoted_count: 1,
+          last_recalled_at: '2026-06-05T00:00:00Z',
+          last_upvoted_at: '2026-06-04T00:00:00Z',
+        },
+      },
+      deltas: {},
+    };
+
+    const merged = mergeDeltas(local, remote);
+    expect(merged.votes['doc-a'].recalled_count).toBe(5);
+    expect(merged.votes['doc-a'].upvoted_count).toBe(2);
+    expect(merged.votes['doc-a'].last_recalled_at).toBe('2026-06-10T00:00:00Z');
+    expect(merged.votes['doc-a'].last_upvoted_at).toBe('2026-06-10T00:00:00Z');
+  });
 });
 
 describe('syncVotesToTeam', () => {
@@ -281,4 +365,24 @@ describe('recallFeedback', () => {
     // Should not throw
     await expect(recallFeedback({ negative: 'nonexistent' })).resolves.not.toThrow();
   });
+
+  it('negative deletes last_upvoted_at when upvoted_count reaches 0', async () => {
+    const votesDir = path.join(tmpDir, '.teamai', 'votes');
+    fs.mkdirSync(votesDir, { recursive: true });
+    const votePath = path.join(votesDir, 'testuser.yaml');
+
+    // Set up: recalled_count=1, upvoted_count=1 with last_upvoted_at populated
+    await incrementRecalled(votePath, ['doc-zero']);
+    await incrementUpvoted(votePath, ['doc-zero']);
+
+    const before = await loadUserVotes(votePath);
+    expect(before.votes['doc-zero'].upvoted_count).toBe(1);
+    expect(before.votes['doc-zero'].last_upvoted_at).toBeTruthy();
+
+    await recallFeedback({ negative: 'doc-zero' });
+
+    const after = YAML.parse(fs.readFileSync(votePath, 'utf-8')) as UserVotesV2;
+    expect(after.votes['doc-zero'].upvoted_count).toBe(0);
+    expect(after.votes['doc-zero'].last_upvoted_at).toBeUndefined();
+  });
 });

src/hook-dispatch-cli.ts

@@ -108,17 +108,37 @@ function spawnBackground(
 }
 
 /** Parse STDIN JSON and normalize the event name for downstream handlers. */
-function parseStdin(raw: string, event: string): Record<string, unknown> | null {
+export function parseStdin(raw: string, event: string): Record<string, unknown> {
   let stdin: Record<string, unknown> = {};
   if (raw.trim()) {
     try {
       stdin = JSON.parse(raw);
     } catch {
-      log.debug(`hook-dispatch: failed to parse STDIN JSON for event=${event}`);
-      return null;
+      // Degrade to {} instead of short-circuiting: handlers that depend on
+      // stdin fields (votes-sync, contribute-check) self-skip when
+      // transcript_path is absent, while background handlers that don't read
+      // stdin (version-check, etc.) still get to run. Include a bounded
+      // preview so concurrent STDIN corruption is diagnosable in debug.log.
+      const preview = raw.length > 160
+        ? `${raw.slice(0, 80)}...${raw.slice(-80)}`
+        : raw;
+      log.debug(
+        `hook-dispatch: failed to parse STDIN JSON for event=${event}` +
+          ` (len=${raw.length}, body=${JSON.stringify(preview)})`,
+      );
     }
   }
 
+  // JSON.parse succeeds for non-object values (`null`, numbers, booleans,
+  // strings, arrays) that are not valid hook payloads. Without this guard,
+  // the next `stdin.hook_event_name` access/assign throws TypeError in ESM
+  // strict mode, which the outer try/catch swallows and short-circuits the
+  // whole dispatch — the exact failure the malformed-JSON path above was
+  // meant to prevent. Degrade any non-plain-object to {}.
+  if (!stdin || typeof stdin !== 'object' || Array.isArray(stdin)) {
+    stdin = {};
+  }
+
   // WorkBuddy/CodeBuddy may pass hook_event_name: "" — normalize to the
   // CLI-derived event name so downstream handlers (parseHookEvent, etc.)
   // can correctly determine the event type.
@@ -168,7 +188,6 @@ export async function hookDispatchCli(
   try {
     const raw = await readStdin();
     const stdin = parseStdin(raw, event);
-    if (stdin === null) return;
 
     // Provider-config gate: HTTP-only teams must not receive git-provider-only
     // hook prompts (contribute / mr-hint / votes). Prefer the project-scope

src/votes.ts

@@ -116,6 +116,12 @@ export async function incrementUpvoted(votePath: string, docIds: string[]): Prom
 /**
  * Merge local deltas into a remote votes snapshot.
  * Returns merged result with empty deltas.
+ *
+ * Scope note: this is a delta-merge, not a full-snapshot consistency sweep.
+ * The zeroed-counter timestamp cleanup below only runs for docs that appear
+ * in `local.deltas` — pure-remote docs with no local delta are copied as-is
+ * and are NOT re-validated. Callers needing a full consistency pass must run
+ * it separately; do not assume mergeDeltas sanitizes the entire result.
  */
 export function mergeDeltas(local: UserVotesV2, remote: UserVotesV2): UserVotesV2 {
   const votes: Record<string, VoteEntryV2> = {};
@@ -144,6 +150,14 @@ export function mergeDeltas(local: UserVotesV2, remote: UserVotesV2): UserVotesV
         votes[docId].last_upvoted_at = localEntry.last_upvoted_at;
       }
     }
+
+    // Consistency constraint: a zeroed counter must not retain a timestamp.
+    if (votes[docId].recalled_count === 0) {
+      votes[docId].last_recalled_at = '';
+    }
+    if (votes[docId].upvoted_count === 0) {
+      delete votes[docId].last_upvoted_at;
+    }
   }
 
   return { version: 2, votes, deltas: {} };
@@ -204,11 +218,17 @@ export async function recallFeedback(opts: { positive?: string; negative?: strin
       return;
     }
     const existingDelta = data.deltas[opts.negative] ?? { recalled_delta: 0, upvoted_delta: 0 };
+    const decrementedCount = entry.upvoted_count - 1;
+    const updatedEntry: VoteEntryV2 = { ...entry, upvoted_count: decrementedCount };
+    // Consistency: a zeroed counter must not retain a timestamp.
+    if (decrementedCount === 0) {
+      delete updatedEntry.last_upvoted_at;
+    }
     const updated: UserVotesV2 = {
       ...data,
       votes: {
         ...data.votes,
-        [opts.negative]: { ...entry, upvoted_count: entry.upvoted_count - 1 },
+        [opts.negative]: updatedEntry,
       },
       deltas: {
         ...data.deltas,