From f23f964a5c604f46961c98f0db166344c30ed2af Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 21 Jul 2026 14:58:16 -0700 Subject: [PATCH] fix(takes): persist think --take rows + review-first take proposals (#2556, #2411) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Takeover of two community PRs, rebased onto master and repaired: PR #2618 (think --take, fixes #2556): runThink declared `take` but never consumed it — `--take` silently wrote nothing ("Takes: 0"). Adds persistThinkTake (anchor-page take append, refuses empty/no-LLM synthesis, source-scope forwarded to getPage), explicit CLI error when no row is written, REMOTE_PERSISTED_BLOCKED warning at the trust boundary. Rebased onto master's thinkSourceScopeOpts (#2739) split. PR #2418 (takes propose CLI + ops, fixes #2411): implements the documented `gbrain takes propose list/accept/reject` promotion path (D17) plus takes_propose_* MCP ops. Repairs from review: - Source scoping now routes through sourceScopeOpts(ctx) (federated array > scalar > nothing, fail-closed) instead of the fail-open `ctx.remote ? ctx.sourceId : undefined`; listTakeProposals/accept/reject accept sourceIds[]. - takes_propose_accept is localOnly (writes source markdown under sync.repo_path — same class as sync; D17 operator-only promotion) and both write ops carry mutating: true. - Rebased onto master's source-resolver-threaded takes dispatcher. Also corrects the propose-takes.ts D17 doc comment to the shipped syntax (`takes propose accept `). Co-authored-by: javieraldape Co-authored-by: Mr-B-1 Co-Authored-By: Claude Fable 5 --- src/commands/takes.ts | 87 ++++++ src/commands/think.ts | 22 +- src/core/cycle/propose-takes.ts | 2 +- src/core/operations.ts | 85 +++++- src/core/take-proposals.ts | 321 ++++++++++++++++++++ src/core/think/index.ts | 56 ++++ test/take-proposals.test.ts | 386 ++++++++++++++++++++++++ test/takes-mcp-allowlist.serial.test.ts | 3 +- test/think-pipeline.serial.test.ts | 64 +++- 9 files changed, 1020 insertions(+), 6 deletions(-) create mode 100644 src/core/take-proposals.ts create mode 100644 test/take-proposals.test.ts diff --git a/src/commands/takes.ts b/src/commands/takes.ts index 4cad6b7a7..90e155aea 100644 --- a/src/commands/takes.ts +++ b/src/commands/takes.ts @@ -29,6 +29,11 @@ import { } from '../core/takes-fence.ts'; import { withPageLock } from '../core/page-lock.ts'; import { resolveSourceId } from '../core/source-resolver.ts'; +import { + acceptTakeProposal, + listTakeProposals, + rejectTakeProposal, +} from '../core/take-proposals.ts'; // --- Helpers --- @@ -189,6 +194,81 @@ async function cmdSearch(engine: BrainEngine, args: string[]): Promise { } } +async function cmdPropose(engine: BrainEngine, args: string[]): Promise { + const sub = args[0]; + const rest = args.slice(1); + const json = flagPresent(rest, '--json') || flagPresent(args, '--json'); + if (!sub || sub === 'list') { + const status = flagValue(rest, '--status') as 'pending' | 'accepted' | 'rejected' | 'superseded' | undefined; + const limit = parseInt(flagValue(rest, '--limit') ?? '30', 10); + const offset = parseInt(flagValue(rest, '--offset') ?? '0', 10); + const rows = await listTakeProposals(engine, { status: status ?? 'pending', limit, offset }); + if (json) { + console.log(JSON.stringify(rows, null, 2)); + return; + } + if (rows.length === 0) { + console.log(`No ${status ?? 'pending'} take proposals.`); + return; + } + for (const row of rows) { + const date = row.effective_date ? row.effective_date.slice(0, 10) : ''; + const dateSource = row.effective_date_source ? ` ${row.effective_date_source}` : ''; + const brier = row.predicted_brier === null || row.predicted_brier === undefined + ? '' + : ` • brier=${row.predicted_brier.toFixed(3)}`; + console.log( + `#${row.id} [${row.kind} • ${row.holder} • w=${Number(row.weight).toFixed(2)}${date ? ` • ${date}${dateSource}` : ''}${brier}]\n` + + ` ${row.page_slug}\n` + + ` ${row.claim_text}\n`, + ); + } + return; + } + + if (sub === 'accept') { + const id = parseInt(rest[0] ?? flagValue(rest, '--id') ?? '', 10); + if (!Number.isFinite(id)) { + console.error('Usage: gbrain takes propose accept [--by ] [--dir ] [--json]'); + process.exit(1); + } + const result = await acceptTakeProposal(engine, id, { + actedBy: flagValue(rest, '--by') ?? 'gbrain-cli', + brainDir: flagValue(rest, '--dir'), + }); + if (json) { + console.log(JSON.stringify(result, null, 2)); + return; + } + const since = result.since_date ? ` since=${result.since_date}` : ' since=(unset: no real claim-date)'; + const suffix = result.idempotent ? ' (already accepted)' : ''; + console.log(`Accepted proposal #${result.proposal_id} → ${result.page_slug}#${result.row_num}${since}${suffix}.`); + return; + } + + if (sub === 'reject') { + const id = parseInt(rest[0] ?? flagValue(rest, '--id') ?? '', 10); + if (!Number.isFinite(id)) { + console.error('Usage: gbrain takes propose reject [--reason "..."] [--by ] [--json]'); + process.exit(1); + } + const result = await rejectTakeProposal(engine, id, { + actedBy: flagValue(rest, '--by') ?? 'gbrain-cli', + reason: flagValue(rest, '--reason'), + }); + if (json) { + console.log(JSON.stringify(result, null, 2)); + return; + } + const suffix = result.idempotent ? ' (already rejected)' : ''; + console.log(`Rejected proposal #${result.proposal_id}${suffix}.`); + return; + } + + console.error(`Unknown takes propose subcommand: ${sub}`); + process.exit(1); +} + async function cmdAdd(engine: BrainEngine, args: string[], sourceId?: string): Promise { const slug = args[0]; if (!slug) { @@ -556,6 +636,12 @@ Subcommands: List takes for a page takes search "" [--limit N] [--json] Keyword search across all takes + takes propose list [--status pending] [--limit N] [--offset N] [--json] + Review pending take proposals + takes propose accept [--by ] [--json] + Promote a reviewed proposal into the page's takes fence + takes propose reject [--reason "..."] [--by ] [--json] + Reject a proposal so it is not re-proposed takes add --claim "..." --kind --who [--weight 0.5] [--source "..."] [--since YYYY-MM] Append a take (markdown + DB) @@ -584,6 +670,7 @@ Common flags: switch (sub) { case 'search': return cmdSearch(engine, rest); + case 'propose': return cmdPropose(engine, rest); case 'add': return cmdAdd(engine, rest, await resolveTakesSourceId(engine)); case 'update': return cmdUpdate(engine, rest, await resolveTakesSourceId(engine)); case 'supersede': return cmdSupersede(engine, rest, await resolveTakesSourceId(engine)); diff --git a/src/commands/think.ts b/src/commands/think.ts index e3c10882f..09162f512 100644 --- a/src/commands/think.ts +++ b/src/commands/think.ts @@ -6,7 +6,7 @@ * degrades to gather-only output with a warning if missing. */ import type { BrainEngine } from '../core/engine.ts'; -import { runThink, persistSynthesis } from '../core/think/index.ts'; +import { runThink, persistSynthesis, persistThinkTake } from '../core/think/index.ts'; import { loadConfig, isThinClient } from '../core/config.ts'; import { callRemoteTool, unpackToolResult } from '../core/mcp-client.ts'; @@ -92,6 +92,8 @@ prints what would have been the input (exit 0). let result: any; let savedSlug: string | undefined; let evidenceInserted = 0; + let takeRow: number | null = null; + let takeInserted = 0; const cfg = loadConfig(); if (isThinClient(cfg)) { if (save || take) { @@ -138,6 +140,19 @@ prints what would have been the input (exit 0). process.exit(1); } } + if (take) { + const persistedTake = await persistThinkTake(engine, result, { anchor }); + takeRow = persistedTake.rowNum; + takeInserted = persistedTake.inserted; + for (const w of persistedTake.warnings) result.warnings.push(w); + if (!persistedTake.rowNum) { + console.error( + 'think: --take requested but no take row was written (missing anchor, ' + + 'missing anchor page, or empty synthesis).', + ); + process.exit(1); + } + } } catch (e) { // #1698: an unresolvable explicit --model throws here. Clean non-zero exit // with the actionable message, not a stack trace. @@ -151,6 +166,8 @@ prints what would have been the input (exit 0). ...result, saved_slug: savedSlug ?? null, evidence_inserted: evidenceInserted, + take_row: takeRow, + take_inserted: takeInserted, }, null, 2)); return; } @@ -169,6 +186,9 @@ prints what would have been the input (exit 0). if (savedSlug) { console.log(`Saved: ${savedSlug} (${evidenceInserted} evidence rows)`); } + if (takeRow !== null) { + console.log(`Take: ${anchor}#${takeRow} (${takeInserted} row)`); + } if (result.warnings.length > 0) { console.error(`Warnings: ${result.warnings.join(', ')}`); } diff --git a/src/core/cycle/propose-takes.ts b/src/core/cycle/propose-takes.ts index 63ada141e..4eb28c6af 100644 --- a/src/core/cycle/propose-takes.ts +++ b/src/core/cycle/propose-takes.ts @@ -20,7 +20,7 @@ * Auto-resolve posture: * propose_takes only WRITES proposals to the queue. Nothing here mutates * the canonical takes table. Operator opt-in via `gbrain takes propose - * --accept N` is the only path from queue to canonical fence (D17). + * accept ` is the only path from queue to canonical fence (D17). * * Prompt tuning status (v0.36.1.0 ship state): * The default extractor prompt was tuned against the synthetic corpus at diff --git a/src/core/operations.ts b/src/core/operations.ts index ab2d4cbf1..5808b76db 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -26,6 +26,11 @@ import { isSearchMode } from './search/mode.ts'; import { stampEvidence } from './search/evidence.ts'; import type { SearchResult } from './types.ts'; import { CJK_SLUG_CHARS } from './cjk.ts'; +import { + acceptTakeProposal, + listTakeProposals, + rejectTakeProposal, +} from './take-proposals.ts'; import * as db from './db.ts'; import { VERSION } from '../version.ts'; import { @@ -1782,6 +1787,65 @@ const takes_search: Operation = { cliHints: { name: 'takes-search', positional: ['query'] }, }; +const takes_propose_list: Operation = { + name: 'takes_propose_list', + description: 'List pending reviewed take proposals before promotion into canonical takes.', + scope: 'read', + params: { + limit: { type: 'number', description: 'Max rows (default 50, cap 500)' }, + offset: { type: 'number', description: 'Skip first N rows' }, + status: { type: 'string', description: 'pending | accepted | rejected | superseded (default pending)' }, + }, + handler: async (ctx, p) => { + return listTakeProposals(ctx.engine, { + limit: p.limit as number | undefined, + offset: p.offset as number | undefined, + status: p.status as never, + // Source-isolation invariant: always route through sourceScopeOpts + // (federated array > scalar > nothing) — never gate on ctx.remote. + ...sourceScopeOpts(ctx), + }); + }, +}; + +const takes_propose_accept: Operation = { + name: 'takes_propose_accept', + description: 'Accept one reviewed take proposal, append it to the source markdown takes fence, mirror it to DB, and stamp the proposal accepted.', + scope: 'write', + mutating: true, + // localOnly: accept writes the source markdown file under sync.repo_path — + // same class as `sync` (filesystem mutation), and D17 says operator opt-in + // via the local CLI is the only queue→canonical-fence path. + localOnly: true, + params: { + proposal_id: { type: 'number', required: true }, + }, + handler: async (ctx, p) => { + return acceptTakeProposal(ctx.engine, p.proposal_id as number, { + actedBy: ctx.auth?.clientName ?? 'mcp', + ...sourceScopeOpts(ctx), + }); + }, +}; + +const takes_propose_reject: Operation = { + name: 'takes_propose_reject', + description: 'Reject one take proposal so review-first queues do not re-offer it.', + scope: 'write', + mutating: true, + params: { + proposal_id: { type: 'number', required: true }, + reason: { type: 'string', description: 'Operator note. Current schema records acted_by/acted_at, not a reason column.' }, + }, + handler: async (ctx, p) => { + return rejectTakeProposal(ctx.engine, p.proposal_id as number, { + actedBy: ctx.auth?.clientName ?? 'mcp', + reason: p.reason as string | undefined, + ...sourceScopeOpts(ctx), + }); + }, +}; + /** * v0.30.0 (Slice A1): aggregate calibration scorecard. Pure SQL aggregation. * @@ -1866,7 +1930,7 @@ const think: Operation = { // forwards to findTrajectory. CLI callers don't go through this op // and get default scope + remote=false from runThink's CLI path. const thinkScope = thinkSourceScopeOpts(ctx); - const { runThink, persistSynthesis } = await import('./think/index.ts'); + const { runThink, persistSynthesis, persistThinkTake } = await import('./think/index.ts'); const result = await runThink(ctx.engine, { question: String(p.question), anchor: p.anchor ? String(p.anchor) : undefined, @@ -1885,16 +1949,31 @@ const think: Operation = { ...thinkScope, remote: ctx.remote === true, }); + if (remote && (Boolean(p.save) || Boolean(p.take))) { + result.warnings.push('REMOTE_PERSISTED_BLOCKED'); + } // Persist if --save was passed locally let savedSlug: string | undefined; let evidenceInserted = 0; + let takeRow: number | null = null; + let takeInserted = 0; if (safeSave) { const persisted = await persistSynthesis(ctx.engine, result); savedSlug = persisted.slug; evidenceInserted = persisted.evidenceInserted; for (const w of persisted.warnings) result.warnings.push(w); } + if (safeTake) { + const persistedTake = await persistThinkTake(ctx.engine, result, { + anchor: p.anchor ? String(p.anchor) : undefined, + ...(thinkScope.sourceId !== undefined ? { sourceId: thinkScope.sourceId } : {}), + ...(thinkScope.allowedSources !== undefined ? { sourceIds: thinkScope.allowedSources } : {}), + }); + takeRow = persistedTake.rowNum; + takeInserted = persistedTake.inserted; + for (const w of persistedTake.warnings) result.warnings.push(w); + } return { ...result, @@ -1902,6 +1981,8 @@ const think: Operation = { // falsy) to null so callers never see an empty-string "slug". saved_slug: savedSlug || null, evidence_inserted: evidenceInserted, + take_row: takeRow, + take_inserted: takeInserted, remote_persisted_blocked: remote && (Boolean(p.save) || Boolean(p.take)), }; }, @@ -5402,7 +5483,7 @@ export const operations: Operation[] = [ // v0.36.1.0 (T7) — Hindsight calibration wave: read profile via MCP get_calibration_profile, // v0.28: Takes + think - takes_list, takes_search, think, + takes_list, takes_search, takes_propose_list, takes_propose_accept, takes_propose_reject, think, // v0.30: calibration aggregates over takes takes_scorecard, takes_calibration, // v0.28: whoami + scoped sources management diff --git a/src/core/take-proposals.ts b/src/core/take-proposals.ts new file mode 100644 index 000000000..491959c25 --- /dev/null +++ b/src/core/take-proposals.ts @@ -0,0 +1,321 @@ +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import type { BrainEngine, TakeKind } from './engine.ts'; +import { parseTakesFence, upsertTakeRow } from './takes-fence.ts'; +import { withPageLock } from './page-lock.ts'; + +export interface TakeProposalRow { + id: number; + source_id: string; + page_slug: string; + status: 'pending' | 'accepted' | 'rejected' | 'superseded'; + claim_text: string; + kind: TakeKind; + holder: string; + weight: number; + domain?: string | null; + dedup_against_fence_rows?: unknown; + model_id: string; + proposed_at: string; + acted_at?: string | null; + acted_by?: string | null; + promoted_row_num?: number | null; + predicted_brier?: number | null; + predicted_brier_bucket_n?: number | null; + effective_date?: string | null; + effective_date_source?: string | null; +} + +export interface TakeProposalAcceptResult { + ok: true; + proposal_id: number; + page_slug: string; + row_num: number; + status: 'accepted'; + idempotent: boolean; + since_date?: string; +} + +export interface TakeProposalRejectResult { + ok: true; + proposal_id: number; + status: 'rejected'; + idempotent: boolean; + reason?: string; +} + +function isoOrNull(value: unknown): string | null { + if (value === null || value === undefined) return null; + if (value instanceof Date) return value.toISOString(); + return String(value); +} + +function dateOnlyOrUndefined(value: unknown, source: unknown): string | undefined { + if (value === null || value === undefined) return undefined; + if (source === 'fallback') return undefined; + if (value instanceof Date) return value.toISOString().slice(0, 10); + const raw = String(value); + return /^\d{4}-\d{2}-\d{2}/.test(raw) ? raw.slice(0, 10) : undefined; +} + +function numberOrNull(value: unknown): number | null { + if (value === null || value === undefined) return null; + const n = Number(value); + return Number.isFinite(n) ? n : null; +} + +function mapProposalRow(row: Record): TakeProposalRow { + return { + id: Number(row.id), + source_id: String(row.source_id), + page_slug: String(row.page_slug), + status: row.status as TakeProposalRow['status'], + claim_text: String(row.claim_text), + kind: String(row.kind) as TakeKind, + holder: String(row.holder), + weight: Number(row.weight), + domain: row.domain === undefined ? null : row.domain as string | null, + dedup_against_fence_rows: row.dedup_against_fence_rows, + model_id: String(row.model_id), + proposed_at: isoOrNull(row.proposed_at) ?? '', + acted_at: isoOrNull(row.acted_at), + acted_by: row.acted_by === undefined ? null : row.acted_by as string | null, + promoted_row_num: numberOrNull(row.promoted_row_num), + predicted_brier: numberOrNull(row.predicted_brier), + predicted_brier_bucket_n: numberOrNull(row.predicted_brier_bucket_n), + effective_date: isoOrNull(row.effective_date), + effective_date_source: row.effective_date_source === undefined ? null : row.effective_date_source as string | null, + }; +} + +async function resolveBrainDir(engine: BrainEngine, explicitDir?: string): Promise { + if (explicitDir) return explicitDir; + const configured = await engine.getConfig('sync.repo_path'); + if (configured) return configured; + throw new Error('No brain directory configured. Pass brainDir or set sync.repo_path.'); +} + +function pageFilePath(brainDir: string, slug: string): string { + return join(brainDir, `${slug}.md`); +} + +export async function listTakeProposals( + engine: BrainEngine, + opts: { + limit?: number; + offset?: number; + status?: TakeProposalRow['status']; + /** Scalar source scope. Ignored when `sourceIds` is present (federated array wins). */ + sourceId?: string; + /** Federated source scope (ctx.auth.allowedSources) — takes precedence over sourceId. */ + sourceIds?: string[]; + } = {}, +): Promise { + const limit = Math.max(1, Math.min(500, Math.floor(opts.limit ?? 50))); + const offset = Math.max(0, Math.floor(opts.offset ?? 0)); + // Source-isolation invariant: federated array > scalar > no filter. + const scopeIds = opts.sourceIds && opts.sourceIds.length > 0 + ? opts.sourceIds + : opts.sourceId + ? [opts.sourceId] + : null; + const rows = await engine.executeRaw( + `SELECT + tp.id, tp.source_id, tp.page_slug, tp.status, tp.claim_text, + tp.kind, tp.holder, tp.weight, tp.domain, tp.dedup_against_fence_rows, + tp.model_id, tp.proposed_at, tp.acted_at, tp.acted_by, + tp.promoted_row_num, tp.predicted_brier, tp.predicted_brier_bucket_n, + p.effective_date, p.effective_date_source + FROM take_proposals tp + LEFT JOIN pages p ON p.slug = tp.page_slug AND p.source_id = tp.source_id + WHERE ($1::text IS NULL OR tp.status = $1) + AND ($2::text[] IS NULL OR tp.source_id = ANY($2::text[])) + ORDER BY + CASE WHEN tp.predicted_brier IS NULL THEN 1 ELSE 0 END, + tp.predicted_brier ASC NULLS LAST, + tp.proposed_at DESC + LIMIT $3 OFFSET $4`, + [opts.status ?? 'pending', scopeIds, limit, offset], + ); + return rows.map((r) => mapProposalRow(r as Record)); +} + +/** True when `sourceId` falls outside the caller's scope (array > scalar > unscoped). */ +function outsideScope(sourceId: string, opts: { sourceId?: string; sourceIds?: string[] }): boolean { + if (opts.sourceIds && opts.sourceIds.length > 0) return !opts.sourceIds.includes(sourceId); + if (opts.sourceId) return sourceId !== opts.sourceId; + return false; +} + +export async function acceptTakeProposal( + engine: BrainEngine, + proposalId: number, + opts: { brainDir?: string; actedBy?: string; sourceId?: string; sourceIds?: string[] } = {}, +): Promise { + const proposalLookup = await engine.executeRaw<{ page_slug: string; source_id: string; status: string; promoted_row_num: number | null }>( + `SELECT page_slug, source_id, status, promoted_row_num + FROM take_proposals WHERE id = $1 LIMIT 1`, + [proposalId], + ); + const existing = proposalLookup[0]; + if (!existing) throw new Error(`take proposal not found: ${proposalId}`); + if (outsideScope(existing.source_id, opts)) { + throw new Error(`take proposal ${proposalId} is outside source scope`); + } + + const actedBy = opts.actedBy ?? 'gbrain-cli'; + const brainDir = await resolveBrainDir(engine, opts.brainDir); + + return withPageLock(existing.page_slug, async () => { + const path = pageFilePath(brainDir, existing.page_slug); + if (!existsSync(path)) { + throw new Error(`source markdown page not found: ${path}`); + } + + let originalBody: string | null = null; + let wroteBody = false; + + try { + return await engine.transaction(async (tx) => { + const rows = await tx.executeRaw>( + `SELECT + tp.id, tp.source_id, tp.page_slug, tp.status, tp.claim_text, + tp.kind, tp.holder, tp.weight, tp.model_id, tp.promoted_row_num, + p.id AS page_id, p.effective_date, p.effective_date_source + FROM take_proposals tp + JOIN pages p ON p.slug = tp.page_slug AND p.source_id = tp.source_id + WHERE tp.id = $1 + FOR UPDATE OF tp`, + [proposalId], + ); + const row = rows[0]; + if (!row) throw new Error(`take proposal not found: ${proposalId}`); + if (outsideScope(String(row.source_id), opts)) { + throw new Error(`take proposal ${proposalId} is outside source scope`); + } + + const promotedRowNum = numberOrNull(row.promoted_row_num); + if (row.status === 'accepted' && promotedRowNum !== null) { + return { + ok: true, + proposal_id: proposalId, + page_slug: String(row.page_slug), + row_num: promotedRowNum, + status: 'accepted', + idempotent: true, + since_date: dateOnlyOrUndefined(row.effective_date, row.effective_date_source), + } satisfies TakeProposalAcceptResult; + } + if (row.status !== 'pending') { + throw new Error(`take proposal ${proposalId} is ${row.status}; only pending proposals can be accepted`); + } + + const pageId = Number(row.page_id); + await tx.executeRaw('SELECT pg_advisory_xact_lock($1::bigint)', [pageId]); + + const dupes = await tx.executeRaw<{ row_num: number }>( + `SELECT row_num + FROM takes + WHERE page_id = $1 AND active = true + AND lower(trim(claim)) = lower(trim($2)) + LIMIT 1`, + [pageId, row.claim_text], + ); + if (dupes.length > 0) { + throw new Error(`take proposal ${proposalId} duplicates existing take row #${dupes[0].row_num}`); + } + + originalBody = readFileSync(path, 'utf-8'); + const sinceDate = dateOnlyOrUndefined(row.effective_date, row.effective_date_source); + const { body: nextBody, rowNum } = upsertTakeRow(originalBody, { + claim: String(row.claim_text), + kind: String(row.kind), + holder: String(row.holder), + weight: Number(row.weight), + source: `proposal:${proposalId}`, + sinceDate, + active: true, + }); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, nextBody, 'utf-8'); + wroteBody = true; + + await tx.addTakesBatch([{ + page_id: pageId, + row_num: rowNum, + claim: String(row.claim_text), + kind: String(row.kind), + holder: String(row.holder), + weight: Number(row.weight), + since_date: sinceDate, + source: `proposal:${proposalId}`, + active: true, + superseded_by: null, + }]); + const stamped = await tx.executeRaw<{ promoted_row_num: number }>( + `UPDATE take_proposals + SET status = 'accepted', + acted_at = now(), + acted_by = $2, + promoted_row_num = $3 + WHERE id = $1 AND status = 'pending' + RETURNING promoted_row_num`, + [proposalId, actedBy, rowNum], + ); + if (stamped.length === 0) { + throw new Error(`take proposal ${proposalId} was not stamped accepted`); + } + return { + ok: true, + proposal_id: proposalId, + page_slug: String(row.page_slug), + row_num: rowNum, + status: 'accepted', + idempotent: false, + since_date: sinceDate, + } satisfies TakeProposalAcceptResult; + }); + } catch (err) { + if (wroteBody && originalBody !== null) { + writeFileSync(path, originalBody, 'utf-8'); + } + throw err; + } + }); +} + +export async function rejectTakeProposal( + engine: BrainEngine, + proposalId: number, + opts: { actedBy?: string; reason?: string; sourceId?: string; sourceIds?: string[] } = {}, +): Promise { + const actedBy = opts.actedBy ?? 'gbrain-cli'; + return engine.transaction(async (tx) => { + const rows = await tx.executeRaw>( + `SELECT id, source_id, status, promoted_row_num + FROM take_proposals WHERE id = $1 + FOR UPDATE`, + [proposalId], + ); + const row = rows[0]; + if (!row) throw new Error(`take proposal not found: ${proposalId}`); + if (outsideScope(String(row.source_id), opts)) { + throw new Error(`take proposal ${proposalId} is outside source scope`); + } + if (row.status === 'rejected') { + return { ok: true, proposal_id: proposalId, status: 'rejected', idempotent: true, reason: opts.reason }; + } + if (row.status === 'accepted' || numberOrNull(row.promoted_row_num) !== null) { + throw new Error(`take proposal ${proposalId} is already accepted and cannot be rejected`); + } + await tx.executeRaw( + `UPDATE take_proposals + SET status = 'rejected', + acted_at = now(), + acted_by = $2 + WHERE id = $1`, + [proposalId, actedBy], + ); + return { ok: true, proposal_id: proposalId, status: 'rejected', idempotent: false, reason: opts.reason }; + }); +} diff --git a/src/core/think/index.ts b/src/core/think/index.ts index e11d7475f..f01bda718 100644 --- a/src/core/think/index.ts +++ b/src/core/think/index.ts @@ -150,6 +150,18 @@ export interface ThinkResult { }; } +export interface PersistThinkTakeOpts { + anchor?: string; + sourceId?: string; + sourceIds?: string[]; +} + +export interface PersistThinkTakeResult { + rowNum: number | null; + inserted: number; + warnings: string[]; +} + const DEFAULT_MAX_OUTPUT_TOKENS = 4000; // Thinking-by-default Claude 5 models (`anthropic:claude-*-5`) spend a large @@ -605,6 +617,50 @@ export async function persistSynthesis( return { slug, evidenceInserted: persisted.inserted, warnings: persisted.warnings }; } +/** + * Persist `gbrain think --take` as the next append-only take row on the anchor page. + * The synthesis answer is the claim; holder=brain because this is gbrain's own analysis. + */ +export async function persistThinkTake( + engine: BrainEngine, + result: ThinkResult, + opts: PersistThinkTakeOpts, +): Promise { + const anchor = opts.anchor?.trim(); + if (!anchor) { + return { rowNum: null, inserted: 0, warnings: ['TAKE_REQUIRES_ANCHOR'] }; + } + if (result.synthesisOk === false || result.answer.trim().length === 0) { + return { rowNum: null, inserted: 0, warnings: ['TAKE_EMPTY_NOT_PERSISTED'] }; + } + + const pageOpts: { sourceId?: string; sourceIds?: string[] } = {}; + if (opts.sourceIds !== undefined) pageOpts.sourceIds = opts.sourceIds; + else if (opts.sourceId !== undefined) pageOpts.sourceId = opts.sourceId; + const page = await engine.getPage(anchor, pageOpts); + if (!page) { + return { rowNum: null, inserted: 0, warnings: [`TAKE_ANCHOR_NOT_FOUND: ${anchor}`] }; + } + + const rows = await engine.executeRaw<{ next: number | string }>( + `SELECT (COALESCE(MAX(row_num), 0) + 1)::int AS next FROM takes WHERE page_id = $1`, + [page.id], + ); + const rowNum = Number(rows[0]?.next ?? 1); + const inserted = await engine.addTakesBatch([{ + page_id: page.id, + row_num: rowNum, + claim: result.answer.trim(), + kind: 'take', + holder: 'brain', + weight: 0.5, + source: 'gbrain think', + active: true, + }]); + + return { rowNum, inserted, warnings: [] }; +} + // ───────────────────────────────────────────────────────────────── // Gateway adapter for #952 (think over MCP returns "no LLM available"). // ───────────────────────────────────────────────────────────────── diff --git a/test/take-proposals.test.ts b/test/take-proposals.test.ts new file mode 100644 index 000000000..36405716a --- /dev/null +++ b/test/take-proposals.test.ts @@ -0,0 +1,386 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { operations } from '../src/core/operations.ts'; +import { buildToolDefs } from '../src/mcp/tool-defs.ts'; +import { + acceptTakeProposal, + listTakeProposals, + rejectTakeProposal, + type TakeProposalRow, +} from '../src/core/take-proposals.ts'; +import type { BrainEngine } from '../src/core/engine.ts'; + +type ProposalStatus = TakeProposalRow['status']; + +interface ProposalRecord { + id: number; + source_id: string; + page_slug: string; + status: ProposalStatus; + claim_text: string; + kind: string; + holder: string; + weight: number; + domain?: string | null; + dedup_against_fence_rows?: unknown; + model_id: string; + proposed_at: Date; + acted_at?: Date | null; + acted_by?: string | null; + promoted_row_num?: number | null; + predicted_brier?: number | null; + predicted_brier_bucket_n?: number | null; +} + +interface PageRecord { + id: number; + source_id: string; + slug: string; + effective_date?: Date | null; + effective_date_source?: string | null; +} + +const tmpRoots: string[] = []; + +afterEach(() => { + for (const root of tmpRoots.splice(0)) { + rmSync(root, { recursive: true, force: true }); + } +}); + +function makeBrainDir(): string { + const root = mkdtempSync(join(tmpdir(), 'gbrain-take-proposals-')); + tmpRoots.push(root); + return root; +} + +function writePage(brainDir: string, slug: string, body = '# Page\n'): string { + const path = join(brainDir, `${slug}.md`); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, body, 'utf-8'); + return path; +} + +class FakeProposalEngine { + readonly kind = 'postgres' as const; + readonly proposals = new Map(); + readonly pages = new Map(); + readonly config = new Map(); + readonly addedBatches: unknown[][] = []; + readonly queries: Array<{ sql: string; params: unknown[] }> = []; + failAddTakes = false; + activeTakes: Array<{ page_id: number; row_num: number; claim: string; active: boolean }> = []; + + constructor(brainDir: string) { + this.config.set('sync.repo_path', brainDir); + } + + addPage(page: PageRecord): void { + this.pages.set(`${page.source_id}:${page.slug}`, page); + } + + addProposal(record: Partial & Pick): void { + this.proposals.set(record.id, { + source_id: 'default', + status: 'pending', + kind: 'take', + holder: 'people/garry-tan', + weight: 0.7, + model_id: 'test-model', + proposed_at: new Date('2026-06-25T00:00:00Z'), + acted_at: null, + acted_by: null, + promoted_row_num: null, + predicted_brier: null, + predicted_brier_bucket_n: null, + ...record, + }); + } + + async getConfig(key: string): Promise { + return this.config.get(key) ?? null; + } + + async transaction(fn: (engine: BrainEngine) => Promise): Promise { + return fn(this as unknown as BrainEngine); + } + + async addTakesBatch(rows: unknown[]): Promise { + if (this.failAddTakes) throw new Error('addTakesBatch failed'); + this.addedBatches.push(rows); + for (const row of rows as Array<{ page_id: number; row_num: number; claim: string; active: boolean }>) { + this.activeTakes.push(row); + } + return rows.length; + } + + async executeRaw>(sql: string, params: unknown[] = []): Promise { + this.queries.push({ sql, params }); + const compact = sql.replace(/\s+/g, ' ').trim(); + + if (compact.includes('FROM take_proposals tp LEFT JOIN pages p')) { + const [status, scopeIds, limitRaw, offsetRaw] = params as [string | null, string[] | null, number, number]; + const limit = Number(limitRaw ?? 50); + const offset = Number(offsetRaw ?? 0); + const rows = [...this.proposals.values()] + .filter((p) => status === null || p.status === status) + .filter((p) => scopeIds === null || scopeIds.includes(p.source_id)) + .sort((a, b) => b.proposed_at.getTime() - a.proposed_at.getTime()) + .slice(offset, offset + limit) + .map((p) => ({ + ...p, + ...this.pageFields(p.source_id, p.page_slug), + })); + return rows as T[]; + } + + if (compact.startsWith('SELECT page_slug, source_id, status, promoted_row_num FROM take_proposals WHERE id')) { + const p = this.proposals.get(Number(params[0])); + return (p ? [{ + page_slug: p.page_slug, + source_id: p.source_id, + status: p.status, + promoted_row_num: p.promoted_row_num ?? null, + }] : []) as T[]; + } + + if (compact.includes('FROM take_proposals tp JOIN pages p')) { + const p = this.proposals.get(Number(params[0])); + if (!p) return [] as T[]; + const page = this.pages.get(`${p.source_id}:${p.page_slug}`); + if (!page) return [] as T[]; + return [{ + ...p, + page_id: page.id, + effective_date: page.effective_date ?? null, + effective_date_source: page.effective_date_source ?? null, + }] as T[]; + } + + if (compact.startsWith('SELECT pg_advisory_xact_lock')) { + return [] as T[]; + } + + if (compact.includes('FROM takes WHERE page_id')) { + const [pageId, claim] = params; + const normalizedClaim = String(claim).trim().toLowerCase(); + const duplicate = this.activeTakes.find((t) => + t.page_id === Number(pageId) && + t.active === true && + t.claim.trim().toLowerCase() === normalizedClaim + ); + return (duplicate ? [{ row_num: duplicate.row_num }] : []) as T[]; + } + + if (compact.startsWith("UPDATE take_proposals SET status = 'accepted'")) { + const [id, actedBy, rowNum] = params; + const p = this.proposals.get(Number(id)); + if (!p || p.status !== 'pending') return [] as T[]; + p.status = 'accepted'; + p.acted_at = new Date('2026-06-25T01:00:00Z'); + p.acted_by = String(actedBy); + p.promoted_row_num = Number(rowNum); + return [{ promoted_row_num: p.promoted_row_num }] as T[]; + } + + if (compact.startsWith('SELECT id, source_id, status, promoted_row_num FROM take_proposals WHERE id')) { + const p = this.proposals.get(Number(params[0])); + return (p ? [{ + id: p.id, + source_id: p.source_id, + status: p.status, + promoted_row_num: p.promoted_row_num ?? null, + }] : []) as T[]; + } + + if (compact.startsWith("UPDATE take_proposals SET status = 'rejected'")) { + const [id, actedBy] = params; + const p = this.proposals.get(Number(id)); + if (p) { + p.status = 'rejected'; + p.acted_at = new Date('2026-06-25T01:00:00Z'); + p.acted_by = String(actedBy); + } + return [] as T[]; + } + + throw new Error(`unhandled SQL in fake engine: ${compact}`); + } + + private pageFields(sourceId: string, slug: string) { + const page = this.pages.get(`${sourceId}:${slug}`); + return { + effective_date: page?.effective_date ?? null, + effective_date_source: page?.effective_date_source ?? null, + }; + } +} + +describe('take proposal review helpers', () => { + test('lists pending proposals with page effective-date metadata', async () => { + const brainDir = makeBrainDir(); + const engine = new FakeProposalEngine(brainDir); + engine.addPage({ id: 10, source_id: 'default', slug: 'topics/a', effective_date: new Date('2024-03-02T00:00:00Z'), effective_date_source: 'frontmatter' }); + engine.addProposal({ id: 1, page_slug: 'topics/a', claim_text: 'A will happen' }); + + const rows = await listTakeProposals(engine as unknown as BrainEngine, { status: 'pending', sourceId: 'default' }); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ + id: 1, + status: 'pending', + page_slug: 'topics/a', + effective_date_source: 'frontmatter', + }); + expect(rows[0].effective_date).toContain('2024-03-02'); + }); + + test('accept writes markdown, mirrors DB, stamps proposal, and uses real effective date', async () => { + const brainDir = makeBrainDir(); + const pagePath = writePage(brainDir, 'topics/a', '# A\n\nBody\n'); + const engine = new FakeProposalEngine(brainDir); + engine.addPage({ id: 10, source_id: 'default', slug: 'topics/a', effective_date: new Date('2024-03-02T00:00:00Z'), effective_date_source: 'frontmatter' }); + engine.addProposal({ id: 1, page_slug: 'topics/a', claim_text: 'A will happen' }); + + const result = await acceptTakeProposal(engine as unknown as BrainEngine, 1, { actedBy: 'test' }); + + expect(result).toMatchObject({ ok: true, proposal_id: 1, page_slug: 'topics/a', status: 'accepted', idempotent: false, since_date: '2024-03-02' }); + expect(result.row_num).toBe(1); + expect(engine.addedBatches).toHaveLength(1); + expect((engine.addedBatches[0][0] as Record)).toMatchObject({ + page_id: 10, + row_num: 1, + claim: 'A will happen', + since_date: '2024-03-02', + source: 'proposal:1', + active: true, + }); + expect(engine.proposals.get(1)).toMatchObject({ status: 'accepted', acted_by: 'test', promoted_row_num: 1 }); + const body = readFileSync(pagePath, 'utf-8'); + expect(body).toContain('A will happen'); + expect(body).toContain('2024-03-02'); + expect(body).toContain('proposal:1'); + }); + + test('accept omits since_date when only fallback effective date is available', async () => { + const brainDir = makeBrainDir(); + writePage(brainDir, 'topics/fallback', '# Fallback\n'); + const engine = new FakeProposalEngine(brainDir); + engine.addPage({ id: 11, source_id: 'default', slug: 'topics/fallback', effective_date: new Date('2024-03-02T00:00:00Z'), effective_date_source: 'fallback' }); + engine.addProposal({ id: 2, page_slug: 'topics/fallback', claim_text: 'Fallback date should not be trusted' }); + + const result = await acceptTakeProposal(engine as unknown as BrainEngine, 2, { actedBy: 'test' }); + + expect(result.since_date).toBeUndefined(); + expect((engine.addedBatches[0][0] as Record).since_date).toBeUndefined(); + }); + + test('accept rolls back markdown if DB mirror fails', async () => { + const brainDir = makeBrainDir(); + const original = '# Rollback\n'; + const pagePath = writePage(brainDir, 'topics/rollback', original); + const engine = new FakeProposalEngine(brainDir); + engine.failAddTakes = true; + engine.addPage({ id: 12, source_id: 'default', slug: 'topics/rollback', effective_date: new Date('2024-03-02T00:00:00Z'), effective_date_source: 'frontmatter' }); + engine.addProposal({ id: 3, page_slug: 'topics/rollback', claim_text: 'Should not persist' }); + + await expect(acceptTakeProposal(engine as unknown as BrainEngine, 3, { actedBy: 'test' })).rejects.toThrow('addTakesBatch failed'); + expect(readFileSync(pagePath, 'utf-8')).toBe(original); + expect(engine.proposals.get(3)).toMatchObject({ status: 'pending', promoted_row_num: null }); + }); + + test('accept is idempotent after a proposal has already been promoted', async () => { + const brainDir = makeBrainDir(); + writePage(brainDir, 'topics/done', '# Done\n'); + const engine = new FakeProposalEngine(brainDir); + engine.addPage({ id: 13, source_id: 'default', slug: 'topics/done', effective_date: new Date('2024-03-02T00:00:00Z'), effective_date_source: 'frontmatter' }); + engine.addProposal({ id: 4, page_slug: 'topics/done', claim_text: 'Already accepted', status: 'accepted', promoted_row_num: 9 }); + + const result = await acceptTakeProposal(engine as unknown as BrainEngine, 4, { actedBy: 'test' }); + expect(result).toMatchObject({ ok: true, proposal_id: 4, row_num: 9, status: 'accepted', idempotent: true, since_date: '2024-03-02' }); + expect(engine.addedBatches).toHaveLength(0); + }); + + test('reject stamps pending proposals and is idempotent for rejected proposals', async () => { + const brainDir = makeBrainDir(); + const engine = new FakeProposalEngine(brainDir); + engine.addProposal({ id: 5, page_slug: 'topics/reject', claim_text: 'Reject me' }); + + const first = await rejectTakeProposal(engine as unknown as BrainEngine, 5, { actedBy: 'reviewer', reason: 'not supported' }); + expect(first).toEqual({ ok: true, proposal_id: 5, status: 'rejected', idempotent: false, reason: 'not supported' }); + expect(engine.proposals.get(5)).toMatchObject({ status: 'rejected', acted_by: 'reviewer' }); + + const second = await rejectTakeProposal(engine as unknown as BrainEngine, 5, { actedBy: 'reviewer', reason: 'not supported' }); + expect(second).toEqual({ ok: true, proposal_id: 5, status: 'rejected', idempotent: true, reason: 'not supported' }); + }); + + test('reject refuses accepted proposals', async () => { + const brainDir = makeBrainDir(); + const engine = new FakeProposalEngine(brainDir); + engine.addProposal({ id: 6, page_slug: 'topics/accepted', claim_text: 'Accepted', status: 'accepted', promoted_row_num: 1 }); + + await expect(rejectTakeProposal(engine as unknown as BrainEngine, 6, { actedBy: 'reviewer' })).rejects.toThrow('already accepted'); + }); + + test('source isolation: federated sourceIds scope filters list and blocks accept/reject outside scope', async () => { + const brainDir = makeBrainDir(); + writePage(brainDir, 'topics/other', '# Other\n'); + const engine = new FakeProposalEngine(brainDir); + engine.addPage({ id: 20, source_id: 'other', slug: 'topics/other', effective_date: null, effective_date_source: null }); + engine.addProposal({ id: 7, page_slug: 'topics/other', claim_text: 'Foreign claim', source_id: 'other' }); + engine.addProposal({ id: 8, page_slug: 'topics/mine', claim_text: 'My claim', source_id: 'mine' }); + + // Federated array scope (ctx.auth.allowedSources) filters the list. + const scoped = await listTakeProposals(engine as unknown as BrainEngine, { sourceIds: ['mine'] }); + expect(scoped.map((r) => r.id)).toEqual([8]); + // Array takes precedence over scalar. + const arrayWins = await listTakeProposals(engine as unknown as BrainEngine, { sourceIds: ['mine'], sourceId: 'other' }); + expect(arrayWins.map((r) => r.id)).toEqual([8]); + + // Accept and reject refuse proposals outside the federated scope. + await expect(acceptTakeProposal(engine as unknown as BrainEngine, 7, { sourceIds: ['mine'] })).rejects.toThrow('outside source scope'); + await expect(rejectTakeProposal(engine as unknown as BrainEngine, 7, { sourceIds: ['mine'] })).rejects.toThrow('outside source scope'); + // Scalar scope still enforced too. + await expect(rejectTakeProposal(engine as unknown as BrainEngine, 7, { sourceId: 'mine' })).rejects.toThrow('outside source scope'); + expect(engine.proposals.get(7)).toMatchObject({ status: 'pending' }); + }); +}); + +describe('take proposal MCP operation schema', () => { + test('exposes list/accept/reject through the shared operation registry with correct scopes', () => { + const byName = Object.fromEntries(operations.map((op) => [op.name, op])); + expect(byName.takes_propose_list?.scope).toBe('read'); + expect(byName.takes_propose_accept?.scope).toBe('write'); + expect(byName.takes_propose_reject?.scope).toBe('write'); + expect(byName.takes_propose_accept.params.proposal_id.required).toBe(true); + expect(byName.takes_propose_reject.params.proposal_id.required).toBe(true); + + // accept writes the source markdown under sync.repo_path — local CLI only (D17). + expect(byName.takes_propose_accept?.localOnly).toBe(true); + + const defs = Object.fromEntries(buildToolDefs(operations).map((def) => [def.name, def])); + expect(defs.takes_propose_accept.inputSchema.required).toEqual(['proposal_id']); + expect(defs.takes_propose_reject.inputSchema.required).toEqual(['proposal_id']); + expect(Object.keys(defs.takes_propose_list.inputSchema.properties)).toEqual(['limit', 'offset', 'status']); + }); + + test('takes_propose_list handler scopes via sourceScopeOpts — fail-closed when ctx.remote is undefined', async () => { + const engine = new FakeProposalEngine(makeBrainDir()); + engine.addProposal({ id: 1, page_slug: 'topics/a', claim_text: 'mine', source_id: 'mine' }); + engine.addProposal({ id: 2, page_slug: 'topics/b', claim_text: 'other', source_id: 'other' }); + const op = operations.find((o) => o.name === 'takes_propose_list')!; + + // ctx.remote undefined (the invariant's fail-closed case) with a scalar sourceId: + // scoping must still apply — the old `ctx.remote ? ctx.sourceId : undefined` dropped it. + const scalar = await op.handler({ engine, sourceId: 'mine', remote: undefined } as never, {}); + expect((scalar as TakeProposalRow[]).map((r) => r.id)).toEqual([1]); + + // Federated array (ctx.auth.allowedSources) wins over scalar. + const federated = await op.handler( + { engine, sourceId: 'mine', remote: true, auth: { allowedSources: ['other'] } } as never, + {}, + ); + expect((federated as TakeProposalRow[]).map((r) => r.id)).toEqual([2]); + }); +}); diff --git a/test/takes-mcp-allowlist.serial.test.ts b/test/takes-mcp-allowlist.serial.test.ts index 3720bf4a8..4217bd51b 100644 --- a/test/takes-mcp-allowlist.serial.test.ts +++ b/test/takes-mcp-allowlist.serial.test.ts @@ -218,9 +218,10 @@ describe('think op — read-only on remote callers (Lane D landed)', () => { saved_slug: string | null; warnings: string[]; }; - // Codex P1 #7: remote save/take is silently disabled. + // Codex P1 #7: remote save/take is blocked explicitly at the trust boundary. expect(env.remote_persisted_blocked).toBe(true); expect(env.saved_slug).toBeNull(); + expect(env.warnings).toContain('REMOTE_PERSISTED_BLOCKED'); // Without API key, gather succeeds but synthesis is skipped. expect(env.warnings).toContain('NO_ANTHROPIC_API_KEY'); }); diff --git a/test/think-pipeline.serial.test.ts b/test/think-pipeline.serial.test.ts index 8c85e6d0b..230e58199 100644 --- a/test/think-pipeline.serial.test.ts +++ b/test/think-pipeline.serial.test.ts @@ -1,7 +1,7 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; import { PGLiteEngine } from '../src/core/pglite-engine.ts'; import { operationsByName } from '../src/core/operations.ts'; -import { runThink, persistSynthesis, type ThinkLLMClient } from '../src/core/think/index.ts'; +import { runThink, persistSynthesis, persistThinkTake, type ThinkLLMClient } from '../src/core/think/index.ts'; import { sanitizeTakeForPrompt, renderTakesBlock } from '../src/core/think/sanitize.ts'; import { resolveCitations, parseInlineCitations, normalizeStructuredCitations } from '../src/core/think/cite-render.ts'; import { runGather } from '../src/core/think/gather.ts'; @@ -369,6 +369,68 @@ describe('runThink + persistSynthesis — #1698 never persist empty', () => { const saved = await persistSynthesis(engine, legacy); expect(saved.slug).toContain('synthesis/legacy-backcompat'); }); + + test('persistThinkTake appends the synthesis answer as the next anchor take (#2556)', async () => { + const target = await engine.putPage('notes/think-take-target-example', { + title: 'Think take target', + type: 'note', + compiled_truth: 'A safe placeholder page for think take persistence.', + }); + const result: any = { + question: 'what should this page remember?', + answer: 'This page should remember the synthesized placeholder insight.', + citations: [], + gaps: [], + pagesGathered: 0, + takesGathered: 0, + graphHits: 0, + modelUsed: 'stub', + rounds: 1, + warnings: [], + synthesisOk: true, + diagnostics: { pagesFromHybrid: 0, takesFromKeyword: 0, takesFromVector: 0, graphHits: 0 }, + }; + + const persisted = await persistThinkTake(engine, result, { anchor: target.slug }); + + expect(persisted).toEqual({ rowNum: 1, inserted: 1, warnings: [] }); + const takes = await engine.listTakes({ page_id: target.id }); + expect(takes).toHaveLength(1); + expect(takes[0]).toMatchObject({ + row_num: 1, + claim: 'This page should remember the synthesized placeholder insight.', + kind: 'take', + holder: 'brain', + source: 'gbrain think', + }); + }); + + test('persistThinkTake refuses empty/no-LLM synthesis instead of writing a blank take (#2556)', async () => { + const target = await engine.putPage('notes/think-take-empty-example', { + title: 'Think take empty target', + type: 'note', + compiled_truth: 'A safe placeholder page for empty think take persistence.', + }); + const result: any = { + question: 'empty synthesis', + answer: '(no LLM available)', + citations: [], + gaps: [], + pagesGathered: 0, + takesGathered: 0, + graphHits: 0, + modelUsed: 'stub', + rounds: 0, + warnings: [], + synthesisOk: false, + diagnostics: { pagesFromHybrid: 0, takesFromKeyword: 0, takesFromVector: 0, graphHits: 0 }, + }; + + const persisted = await persistThinkTake(engine, result, { anchor: target.slug }); + + expect(persisted).toEqual({ rowNum: null, inserted: 0, warnings: ['TAKE_EMPTY_NOT_PERSISTED'] }); + expect(await engine.listTakes({ page_id: target.id })).toHaveLength(0); + }); }); describe('think MCP op — #1698 C3 + #10', () => {