fix(advisor): preserve long consult audit records

.github/workflows/advisor-hooks.yml

@@ -0,0 +1,18 @@
+name: Test Advisor hooks
+
+on:
+  pull_request:
+    paths:
+      - "advisor/hooks/**"
+      - ".github/workflows/advisor-hooks.yml"
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 20
+      - name: Test consult persistence
+        run: node --test advisor/hooks/record-consult.test.mjs

advisor/hooks/record-consult.sh

@@ -24,18 +24,22 @@ if [[ "$SUBAGENT_TYPE" != "advisor-subagent" || "$STATUS" != "completed" ]]; the
 fi
 
 NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ)
-advisor_state_update '.consults = ((.consults // 0) + 1) | .last_consult_at = $now' --arg now "$NOW"
-rm -f "$PENDING_FILE"
-
 DESCRIPTION=$(jq -r '.description // "Advisor consult"' <<< "$HOOK_INPUT")
-SUMMARY=$(jq -r '.summary // empty' <<< "$HOOK_INPUT" | head -c 6000)
+# Consume jq fully before bounding the text. With pipefail, piping a long
+# summary to head can terminate jq with SIGPIPE and lose the consult record.
+SUMMARY=$(jq -r '.summary // empty' <<< "$HOOK_INPUT")
+SUMMARY=$(head -c 6000 <<< "$SUMMARY")
 {
   printf '## %s — %s\n\n' "$NOW" "$DESCRIPTION"
   if [[ -n "$SUMMARY" ]]; then
     printf '%s\n\n' "$SUMMARY"
   else
     printf '_No summary captured._\n\n'
   fi
-} >> "$LOG_FILE"
+} >> "$LOG_FILE" || exit 1
+
+# Mark the consult complete only after its advice was persisted successfully.
+advisor_state_update '.consults = ((.consults // 0) + 1) | .last_consult_at = $now' --arg now "$NOW"
+rm -f "$PENDING_FILE"
 
 exit 0

advisor/hooks/record-consult.test.mjs

@@ -0,0 +1,64 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, mkdirSync, readFileSync, writeFileSync, existsSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join, dirname } from "node:path";
+import { fileURLToPath } from "node:url";
+import { spawnSync } from "node:child_process";
+
+const hook = join(dirname(fileURLToPath(import.meta.url)), "record-consult.sh");
+function consult(t, input, { brokenLog = false } = {}) {
+  const root = mkdtempSync(join(tmpdir(), "advisor-consult-"));
+  t.after(() => rmSync(root, { recursive: true, force: true }));
+  const dir = join(root, ".cursor", "advisor");
+  mkdirSync(dir, { recursive: true });
+  writeFileSync(join(dir, "state.json"), JSON.stringify({ enabled: true, conversation_id: "current", consults: 0 }));
+  writeFileSync(join(dir, "pending"), "");
+  if (brokenLog) mkdirSync(join(dir, "log.md"));
+  const result = spawnSync("bash", [hook], {
+    input: JSON.stringify({ conversation_id: "current", subagent_type: "advisor-subagent", status: "completed", ...input }),
+    encoding: "utf8", env: { ...process.env, CURSOR_PROJECT_DIR: root }, timeout: 5000,
+  });
+  return { ...result, dir, state: JSON.parse(readFileSync(join(dir, "state.json"))), pending: existsSync(join(dir, "pending")) };
+}
+
+for (const [name, summary] of [
+  ["short", "Advice remains intact"],
+  ["long", "A".repeat(500_000)],
+  ["unicode", "é".repeat(3000) + "tail"],
+]) {
+  test(`persists ${name} consult before marking it complete`, (t) => {
+    const result = consult(t, { summary, description: "Review" });
+    assert.equal(result.status, 0, result.stderr);
+    assert.equal(result.state.consults, 1);
+    assert.equal(result.pending, false);
+    const log = readFileSync(join(result.dir, "log.md"));
+    const expected = Buffer.from(summary).subarray(0, 6000);
+    assert.equal(log.includes(expected), true);
+    assert.equal(log.subarray(log.indexOf(Buffer.from("Review\n\n")) + 8).length, expected.length + 2);
+  });
+}
+
+test("records absent summaries without dropping the consult", (t) => {
+  const result = consult(t, {});
+  assert.equal(result.status, 0, result.stderr);
+  assert.match(readFileSync(join(result.dir, "log.md"), "utf8"), /No summary captured/);
+  assert.equal(result.state.consults, 1);
+});
+
+test("failed log writes leave the consult pending and uncounted", (t) => {
+  const result = consult(t, { summary: "advice" }, { brokenLog: true });
+  assert.notEqual(result.status, 0);
+  assert.equal(result.state.consults, 0);
+  assert.equal(result.pending, true);
+});
+
+for (const input of [{ status: "error" }, { status: "aborted" }, { conversation_id: "other" }, { subagent_type: "worker" }]) {
+  test(`ignores unrelated or unfinished consult ${JSON.stringify(input)}`, (t) => {
+    const result = consult(t, { summary: "advice", ...input });
+    assert.equal(result.status, 0, result.stderr);
+    assert.equal(result.state.consults, 0);
+    assert.equal(result.pending, true);
+    assert.equal(existsSync(join(result.dir, "log.md")), false);
+  });
+}