mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 21:19:18 +00:00
v0.41.30.0 fix(brainstorm/lsd): --save writes the advertised .md file via canonical ingestion path (#1655)
* refactor: extract shared atomic writePageThrough helper Lift the v0.38 put_page disk write-through (operations.ts) into a shared src/core/write-through.ts helper and upgrade it to write atomically (unique temp file + rename) so a crash or a concurrent gbrain sync can never read a half-written .md. put_page now calls the helper; behavior is preserved (repo guards, source-awareness, provenance overrides) and its write is now atomic. brainstorm/lsd --save will call the same helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(brainstorm/lsd): --save writes the advertised file via canonical ingestion --save printed 'Saved to <slug>' unconditionally but only did a raw DB putPage: the promised wiki/ideas/<slug>.md file was never written, the page had no chunks (unsearchable, and churned by the next sync), and a failed DB write under PgBouncer still claimed success. Route save through importFromContent({noEmbed:true}) for a canonical row, then the shared writePageThrough helper renders the file from that row. persistSavedIdea + formatSaveOutcome report honestly which sinks landed and exit nonzero when nothing persisted. buildIdeaSlug gets a random nonce so same-day ideas don't clobber. New buildBrainstormFrontmatterObject feeds serializeMarkdown. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.41.30.0) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: document write-through.ts + brainstorm --save canonical path (v0.41.30.0) Add Key Files entry for the new shared atomic src/core/write-through.ts helper, note the brainstorm/lsd --save canonical-ingestion rewrite on the brainstorm entry, and note put_page now calls the shared atomic helper on the operations.ts entry. Regenerate llms-full.txt to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
041d89babe
commit
63977054af
@@ -2,6 +2,73 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.41.30.0] - 2026-05-30
|
||||
|
||||
**`gbrain lsd --save` (and `brainstorm --save`) now actually writes the
|
||||
committable `wiki/ideas/<slug>.md` file it always promised, makes the saved
|
||||
idea searchable, and tells you the truth when a save fails instead of always
|
||||
printing "Saved".**
|
||||
|
||||
The old behavior was a quiet lie. `--save` printed `Saved to <slug>` every
|
||||
time, but under the hood it only did a lightweight database write. The help
|
||||
text promised a `.md` file on disk for you to commit to your brain repo, and
|
||||
that file never appeared. Worse, when the database write itself failed (which
|
||||
happens under PgBouncer transaction-mode), it still said "Saved" — a real LSD
|
||||
run reported success while `gbrain get <slug>` returned `page_not_found`.
|
||||
Nothing landed anywhere, and the only way to recover the idea was to scrape it
|
||||
back out of the terminal.
|
||||
|
||||
Now the save runs through the same ingestion path the rest of gbrain uses. The
|
||||
page is written canonically (chunked and tagged, so `gbrain search` can find
|
||||
it), and when your `sync.repo_path` points at a real brain repo, the
|
||||
`wiki/ideas/<date>-<lsd|brainstorm>-<slug>.md` file is written to disk,
|
||||
rendered from the saved row so the two can't drift apart. The success message
|
||||
now names exactly which sinks landed. If nothing persisted, you get a loud
|
||||
`NOT persisted` error and a nonzero exit code, so a scripted `--save` can no
|
||||
longer be mistaken for success.
|
||||
|
||||
Same-day ideas no longer clobber each other either: the slug gets a short
|
||||
random suffix, so two quick `lsd` runs on similar questions keep both files.
|
||||
|
||||
How to use it:
|
||||
|
||||
```
|
||||
gbrain config set sync.repo_path /path/to/your/brain # if not already set
|
||||
gbrain lsd "why are agent tools converging" --save
|
||||
gbrain get wiki/ideas/2026-05-30-lsd-why-are-agent-tools-converging-<suffix>
|
||||
ls /path/to/your/brain/wiki/ideas/ # the .md file is really there
|
||||
```
|
||||
|
||||
No `sync.repo_path`? You still get the database page (queryable immediately),
|
||||
and the message tells you the file write was skipped. Nothing changes for
|
||||
`--json` callers (that path stays database-only, as before).
|
||||
|
||||
### For contributors
|
||||
|
||||
The disk write-through that `put_page` shipped in v0.38 was extracted into a
|
||||
shared `src/core/write-through.ts` helper that `put_page` and brainstorm save
|
||||
now both call, and it was upgraded to write atomically (unique temp file +
|
||||
rename) so a crash or a concurrently-running `gbrain sync` can never read a
|
||||
half-written `.md`. That hardens `put_page`'s own write-through as a side
|
||||
effect.
|
||||
|
||||
## To take advantage of v0.41.30.0
|
||||
|
||||
`gbrain upgrade` is all you need. There is no schema migration and no data
|
||||
backfill in this release. To confirm the fix:
|
||||
|
||||
1. **Set a repo path if you haven't:** `gbrain config set sync.repo_path /path/to/your/brain`
|
||||
2. **Save an idea and verify both sinks:**
|
||||
```bash
|
||||
gbrain lsd "test idea" --save
|
||||
gbrain get <printed-slug> # database page exists
|
||||
ls /path/to/your/brain/wiki/ideas/ # the .md file exists
|
||||
```
|
||||
3. **If a save ever reports `NOT persisted`,** that is the new honest failure
|
||||
path doing its job — check the stderr line above it for the underlying DB
|
||||
or file error, then please file an issue with `gbrain doctor` output:
|
||||
https://github.com/garrytan/gbrain/issues
|
||||
|
||||
## [0.41.29.0] - 2026-05-29
|
||||
|
||||
**Your meeting transcripts that look like `**Garry Tan:** ...` now actually
|
||||
|
||||
@@ -1,5 +1,13 @@
|
||||
# TODOS
|
||||
|
||||
## brainstorm/lsd --save source-awareness (v0.42+)
|
||||
|
||||
Filed from the `--save` dual-sink hardening wave (route through the canonical
|
||||
ingestion path: `importFromContent({noEmbed:true})` + the shared
|
||||
`writePageThrough` helper extracted from `put_page`).
|
||||
|
||||
- [ ] **v0.42+: make `gbrain brainstorm/lsd --save` source-aware.** Today the save path always writes to `source='default'` — `persistSavedIdea` (`src/commands/brainstorm.ts`) hardcodes `sourceId ?? 'default'`, and there is no `--save`-side `--source` flag. Both sinks stay consistent at default (no live bug), but on a multi-source brain a generated idea can't be filed to a non-default source. **What:** add a `--source <id>` option to brainstorm/lsd, resolve it via `resolveSourceWithTier`, and thread `sourceId` into `persistSavedIdea` → `importFromContent({sourceId})` + `writePageThrough({sourceId})`. **Why:** complete the multi-source story for generated ideas; the disk layout already handles it. **Context:** `writePageThrough` and `resolvePageFilePath` already take `sourceId` and emit `.sources/<id>/<slug>.md` for non-default sources, and `importFromContent` already accepts `sourceId` — so the only missing piece is the CLI flag + threading. `runBrainstorm` (orchestrator) already accepts `sourceId` for the close/far READ side. **Depends on:** nothing; purely additive. Priority: P3 (default-source is the common case).
|
||||
|
||||
## v0.41.29.0 orphan source-scoping follow-ups (v0.42+)
|
||||
|
||||
Filed from the v0.41.29.0 wave (bold-name-no-time pattern + orphan_ratio
|
||||
|
||||
+3
-2
File diff suppressed because one or more lines are too long
+1
-1
@@ -141,5 +141,5 @@
|
||||
"bun": ">=1.3.10"
|
||||
},
|
||||
"license": "MIT",
|
||||
"version": "0.41.29.0"
|
||||
"version": "0.41.30.0"
|
||||
}
|
||||
|
||||
+141
-29
@@ -16,13 +16,17 @@ import type { BrainEngine } from '../core/engine.ts';
|
||||
import {
|
||||
runBrainstorm,
|
||||
formatBrainstormMarkdown,
|
||||
buildBrainstormFrontmatter,
|
||||
buildBrainstormFrontmatterObject,
|
||||
BRAINSTORM_PROFILE,
|
||||
LSD_PROFILE,
|
||||
type BrainstormProfile,
|
||||
} from '../core/brainstorm/orchestrator.ts';
|
||||
import { loadConfig } from '../core/config.ts';
|
||||
import { StructuredAgentError } from '../core/errors.ts';
|
||||
import { serializeMarkdown } from '../core/markdown.ts';
|
||||
import { importFromContent } from '../core/import-file.ts';
|
||||
import { writePageThrough, type WriteThroughResult } from '../core/write-through.ts';
|
||||
import { randomBytes } from 'crypto';
|
||||
|
||||
export interface BrainstormCliArgs {
|
||||
question?: string;
|
||||
@@ -305,37 +309,144 @@ async function runBrainstormCli(
|
||||
const shouldSave = parsed.save ?? profile.default_save;
|
||||
if (shouldSave) {
|
||||
const slug = buildIdeaSlug(parsed.question, profile.label);
|
||||
const frontmatter = buildBrainstormFrontmatter(result, { slug });
|
||||
// Re-render content for save: include filtered ideas too so --retry-judge
|
||||
// (when implemented) has the full set to re-score.
|
||||
const title = `${profile.label === 'lsd' ? 'LSD' : 'Brainstorm'}: ${parsed.question.slice(0, 100)}`;
|
||||
// Build ONE frontmatter object and render via the canonical serializer so
|
||||
// the saved file round-trips through `gbrain sync` byte-for-byte. Include
|
||||
// filtered ideas (onlyPassed:false) so a future --retry-judge has the full
|
||||
// set to re-score.
|
||||
const fmObj = buildBrainstormFrontmatterObject(result);
|
||||
const body = formatBrainstormMarkdown(result, { onlyPassed: false, includeMeta: true });
|
||||
const content = frontmatter + body;
|
||||
try {
|
||||
await engine.putPage(slug, {
|
||||
title: `${profile.label === 'lsd' ? 'LSD' : 'Brainstorm'}: ${parsed.question.slice(0, 100)}`,
|
||||
type: 'note',
|
||||
compiled_truth: content,
|
||||
frontmatter: {
|
||||
mode: profile.frontmatter_mode,
|
||||
generated_at: new Date().toISOString(),
|
||||
question: parsed.question,
|
||||
judge_failed: result.judge_failed,
|
||||
unscored: result.judge_failed,
|
||||
close_slugs: result.close_set.map((c) => c.slug),
|
||||
far_slugs: result.far_set.map((f) => f.slug),
|
||||
},
|
||||
timeline: '',
|
||||
});
|
||||
console.log(`\n_Saved to \`${slug}\`._`);
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
console.error(`gbrain ${profile.label}: save failed: ${msg}`);
|
||||
}
|
||||
const content = serializeMarkdown(fmObj, body, '', { type: 'note', title, tags: [] });
|
||||
|
||||
const outcome = await persistSavedIdea(engine, { slug, content, provenanceVia: profile.label });
|
||||
const msg = formatSaveOutcome(outcome, { profileLabel: profile.label, slug });
|
||||
if (msg.stdout) console.log(msg.stdout);
|
||||
for (const line of msg.stderr) console.error(line);
|
||||
if (msg.exitCode) process.exitCode = msg.exitCode;
|
||||
}
|
||||
}
|
||||
|
||||
/** Slugify the question for the saved page path. Capped + collision-resistant via date prefix. */
|
||||
function buildIdeaSlug(question: string, label: 'brainstorm' | 'lsd'): string {
|
||||
/** Outcome of persisting a saved idea to both sinks. */
|
||||
export interface SaveOutcome {
|
||||
/** True when the canonical DB import (importFromContent) succeeded. */
|
||||
dbSaved: boolean;
|
||||
/** Set when the DB import threw. */
|
||||
dbError?: string;
|
||||
/** Disk write-through result (rendered from the saved row). */
|
||||
writeThrough: WriteThroughResult;
|
||||
}
|
||||
|
||||
export interface SaveMessage {
|
||||
/** Human-readable success line for stdout (omitted when nothing persisted). */
|
||||
stdout?: string;
|
||||
/** Error / warning lines for stderr. */
|
||||
stderr: string[];
|
||||
/** Nonzero ONLY when nothing was persisted (no DB row AND no file). */
|
||||
exitCode: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Persist a saved idea through the CANONICAL ingestion path: importFromContent
|
||||
* (chunks + tags + content_hash + source_path, but `noEmbed` so we don't pay
|
||||
* embedding cost at save time) writes the DB row, then the shared
|
||||
* `writePageThrough` helper renders that row to disk. Rendering from the row
|
||||
* means the two sinks cannot diverge, and the row matches what `gbrain sync`
|
||||
* would produce — so a later sync doesn't churn it. The file is only attempted
|
||||
* when the DB write landed (it's rendered from the row).
|
||||
*/
|
||||
export async function persistSavedIdea(
|
||||
engine: BrainEngine,
|
||||
args: { slug: string; content: string; sourceId?: string; provenanceVia: string },
|
||||
): Promise<SaveOutcome> {
|
||||
const sourceId = args.sourceId ?? 'default';
|
||||
let dbSaved = false;
|
||||
let dbError: string | undefined;
|
||||
try {
|
||||
await importFromContent(engine, args.slug, args.content, {
|
||||
noEmbed: true,
|
||||
sourceId,
|
||||
sourcePath: `${args.slug}.md`,
|
||||
});
|
||||
dbSaved = true;
|
||||
} catch (err) {
|
||||
dbError = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
const writeThrough: WriteThroughResult = dbSaved
|
||||
? await writePageThrough(engine, args.slug, {
|
||||
sourceId,
|
||||
frontmatterOverrides: { source_kind: args.provenanceVia },
|
||||
})
|
||||
: { written: false, skipped: 'page_not_found_after_write' };
|
||||
return { dbSaved, dbError, writeThrough };
|
||||
}
|
||||
|
||||
/**
|
||||
* Render an honest save message from the outcome. Every branch names the real
|
||||
* state; the only nonzero exit is the total-failure case (nothing persisted),
|
||||
* so scripts can't read a failed `--save` as success. A file-write failure when
|
||||
* the DB row landed stays exit 0 — the row is durable and `gbrain sync`
|
||||
* reconciles the disk file on the next run.
|
||||
*/
|
||||
export function formatSaveOutcome(
|
||||
outcome: SaveOutcome,
|
||||
ctx: { profileLabel: string; slug: string },
|
||||
): SaveMessage {
|
||||
const { dbSaved, dbError, writeThrough } = outcome;
|
||||
const stderr: string[] = [];
|
||||
if (dbError) stderr.push(`gbrain ${ctx.profileLabel}: DB save failed: ${dbError}`);
|
||||
if (writeThrough.error) {
|
||||
stderr.push(`gbrain ${ctx.profileLabel}: file write failed: ${writeThrough.error}`);
|
||||
}
|
||||
|
||||
if (dbSaved && writeThrough.written) {
|
||||
return {
|
||||
stdout: `\n_Saved to DB page \`${ctx.slug}\` and file \`${writeThrough.path}\`._`,
|
||||
stderr,
|
||||
exitCode: 0,
|
||||
};
|
||||
}
|
||||
if (dbSaved && writeThrough.skipped === 'no_repo_configured') {
|
||||
return {
|
||||
stdout: `\n_Saved to DB page \`${ctx.slug}\` (no \`sync.repo_path\` set — skipped file write)._`,
|
||||
stderr,
|
||||
exitCode: 0,
|
||||
};
|
||||
}
|
||||
if (dbSaved && writeThrough.skipped === 'repo_not_found') {
|
||||
return {
|
||||
stdout: `\n_Saved to DB page \`${ctx.slug}\` (\`sync.repo_path\` is not a directory — skipped file write)._`,
|
||||
stderr,
|
||||
exitCode: 0,
|
||||
};
|
||||
}
|
||||
if (dbSaved) {
|
||||
// File write attempted but errored (already on stderr). Row is durable.
|
||||
return {
|
||||
stdout: `\n_Saved to DB page \`${ctx.slug}\` (file NOT written — see error above; \`gbrain sync\` will reconcile)._`,
|
||||
stderr,
|
||||
exitCode: 0,
|
||||
};
|
||||
}
|
||||
// Nothing persisted — the silent-false-success bug class. Exit nonzero.
|
||||
stderr.push(
|
||||
`gbrain ${ctx.profileLabel}: save FAILED — neither DB page nor file was written. The idea is NOT persisted.`,
|
||||
);
|
||||
return { stderr, exitCode: 1 };
|
||||
}
|
||||
|
||||
/**
|
||||
* Slugify the question for the saved page path. Collision-resistant via a date
|
||||
* prefix AND a random nonce suffix — two same-day runs whose questions share
|
||||
* the first 60 slug chars (or both slugify to empty → `untitled`) would
|
||||
* otherwise produce the same slug, and both the DB upsert and the file write
|
||||
* would silently clobber the earlier idea. The nonce is injectable so tests are
|
||||
* deterministic; production uses crypto random.
|
||||
*/
|
||||
export function buildIdeaSlug(
|
||||
question: string,
|
||||
label: 'brainstorm' | 'lsd',
|
||||
nonce?: string,
|
||||
): string {
|
||||
const date = new Date().toISOString().slice(0, 10);
|
||||
const stem = question
|
||||
.toLowerCase()
|
||||
@@ -343,7 +454,8 @@ function buildIdeaSlug(question: string, label: 'brainstorm' | 'lsd'): string {
|
||||
.replace(/^-+|-+$/g, '')
|
||||
.slice(0, 60)
|
||||
.replace(/^-+|-+$/g, '');
|
||||
return `wiki/ideas/${date}-${label}-${stem || 'untitled'}`;
|
||||
const suffix = nonce ?? randomBytes(3).toString('hex');
|
||||
return `wiki/ideas/${date}-${label}-${stem || 'untitled'}-${suffix}`;
|
||||
}
|
||||
|
||||
/** CLI entry: `gbrain brainstorm`. */
|
||||
|
||||
@@ -1045,3 +1045,32 @@ cost_usd: ${result.cost.actual_usd.toFixed(4)}
|
||||
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Object form of {@link buildBrainstormFrontmatter} for callers that feed the
|
||||
* canonical `serializeMarkdown` serializer (which owns YAML escaping). The
|
||||
* string builder above is left untouched for back-compat — its output shape is
|
||||
* relied on by existing callers, so this is a SEPARATE helper, not a wrapper.
|
||||
* `title` is intentionally omitted: serializeMarkdown takes it via its `meta`
|
||||
* argument so it isn't duplicated in the frontmatter map.
|
||||
*/
|
||||
export function buildBrainstormFrontmatterObject(
|
||||
result: BrainstormResult,
|
||||
): Record<string, unknown> {
|
||||
const obj: Record<string, unknown> = {
|
||||
mode: result.profile_label,
|
||||
generated_at: new Date().toISOString(),
|
||||
date: new Date().toISOString().slice(0, 10),
|
||||
question: result.question.slice(0, 200),
|
||||
close_slugs: result.close_set.map((c) => c.slug),
|
||||
far_slugs: result.far_set.map((f) => f.slug),
|
||||
short_of_target: result.short_of_target,
|
||||
calibration_cold_start: result.active_bias_tags === null,
|
||||
cost_usd: Number(result.cost.actual_usd.toFixed(4)),
|
||||
};
|
||||
if (result.judge_failed) {
|
||||
obj.judge_failed = true;
|
||||
obj.unscored = true;
|
||||
}
|
||||
return obj;
|
||||
}
|
||||
|
||||
+15
-35
@@ -10,9 +10,7 @@ import { clampSearchLimit } from './engine.ts';
|
||||
import type { GBrainConfig } from './config.ts';
|
||||
import type { PageType } from './types.ts';
|
||||
import { importFromContent } from './import-file.ts';
|
||||
import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts';
|
||||
import { mkdirSync, writeFileSync, existsSync, statSync } from 'fs';
|
||||
import { dirname } from 'path';
|
||||
import { writePageThrough } from './write-through.ts';
|
||||
import { hybridSearch, hybridSearchCached } from './search/hybrid.ts';
|
||||
import { expandQuery } from './search/expansion.ts';
|
||||
import { dedupResults } from './search/dedup.ts';
|
||||
@@ -715,38 +713,20 @@ const put_page: Operation = {
|
||||
const isSandboxSubagent = ctx.viaSubagent === true
|
||||
&& !(Array.isArray(ctx.allowedSlugPrefixes) && ctx.allowedSlugPrefixes.length > 0);
|
||||
if (!ctx.dryRun && result.status !== 'error' && !isSandboxSubagent) {
|
||||
try {
|
||||
const repoPath = await ctx.engine.getConfig('sync.repo_path');
|
||||
if (!repoPath) {
|
||||
writeThrough = { written: false, skipped: 'no_repo_configured' };
|
||||
} else if (!existsSync(repoPath) || !statSync(repoPath).isDirectory()) {
|
||||
writeThrough = { written: false, skipped: 'repo_not_found' };
|
||||
} else {
|
||||
const sourceId = ctx.sourceId ?? 'default';
|
||||
const writtenPage = await ctx.engine.getPage(result.slug, { sourceId });
|
||||
if (writtenPage) {
|
||||
const tags = await ctx.engine.getTags(result.slug, { sourceId });
|
||||
const provenanceVia = ctx.remote === false ? 'put_page' : 'mcp:put_page';
|
||||
const md = serializePageToMarkdown(writtenPage, tags, {
|
||||
frontmatterOverrides: {
|
||||
ingested_via: provenanceVia,
|
||||
ingested_at: new Date().toISOString(),
|
||||
source_kind: provenanceVia,
|
||||
},
|
||||
});
|
||||
const filePath = resolvePageFilePath(repoPath as string, result.slug, sourceId);
|
||||
mkdirSync(dirname(filePath), { recursive: true });
|
||||
writeFileSync(filePath, md, 'utf8');
|
||||
writeThrough = { written: true, path: filePath };
|
||||
} else {
|
||||
writeThrough = { written: false, skipped: 'page_not_found_after_write' };
|
||||
}
|
||||
}
|
||||
} catch (e) {
|
||||
const msg = e instanceof Error ? e.message : String(e);
|
||||
ctx.logger.warn(`[put_page] write-through failed for ${result.slug}: ${msg}`);
|
||||
writeThrough = { written: false, error: msg };
|
||||
}
|
||||
const sourceId = ctx.sourceId ?? 'default';
|
||||
const provenanceVia = ctx.remote === false ? 'put_page' : 'mcp:put_page';
|
||||
// Shared canonical write-through (also used by `gbrain brainstorm/lsd
|
||||
// --save`). Renders the file from the saved DB row and writes it
|
||||
// atomically; never throws (failures land in skipped/error).
|
||||
writeThrough = await writePageThrough(ctx.engine, result.slug, {
|
||||
sourceId,
|
||||
frontmatterOverrides: {
|
||||
ingested_via: provenanceVia,
|
||||
ingested_at: new Date().toISOString(),
|
||||
source_kind: provenanceVia,
|
||||
},
|
||||
logger: ctx.logger,
|
||||
});
|
||||
} else if (isSandboxSubagent) {
|
||||
writeThrough = { written: false, skipped: 'subagent_sandbox' };
|
||||
} else if (ctx.dryRun) {
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* Shared disk write-through for the canonical ingestion path.
|
||||
*
|
||||
* After a page row lands in the DB (via importFromContent / putPage), this
|
||||
* renders the row to markdown via `serializePageToMarkdown` and writes it to
|
||||
* `sync.repo_path` so the brain repo has a committable `.md` artifact that
|
||||
* round-trips cleanly through `gbrain sync`. The file is rendered FROM the DB
|
||||
* row, so the two sinks cannot diverge.
|
||||
*
|
||||
* Extracted from the v0.38 `put_page` write-through (operations.ts) so the
|
||||
* `put_page` op AND `gbrain brainstorm/lsd --save` share one implementation
|
||||
* instead of hand-rolling parallel (and divergent) copies. The extraction also
|
||||
* upgraded the write to be ATOMIC — the original used a bare `writeFileSync`
|
||||
* into a live git tree that `gbrain sync` / autopilot actively walk, so a crash
|
||||
* mid-write left a partial `.md` that sync would fail to parse. We now write to
|
||||
* a unique temp sibling and `rename` into place (rename is atomic on the same
|
||||
* filesystem), matching the `.tmp + rename` convention used by
|
||||
* import-checkpoint.ts / op-checkpoint.ts.
|
||||
*
|
||||
* Trust gating (subagent sandbox, dry-run) stays at the CALLER — this helper
|
||||
* only does "row exists + repo is a real dir → render + atomic write".
|
||||
*/
|
||||
|
||||
import { existsSync, statSync, mkdirSync, writeFileSync, renameSync, unlinkSync } from 'fs';
|
||||
import { dirname } from 'path';
|
||||
import { randomBytes } from 'crypto';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts';
|
||||
|
||||
/** Minimal logger surface — structurally compatible with operations.ts `Logger`. */
|
||||
export interface WriteThroughLogger {
|
||||
warn(msg: string): void;
|
||||
}
|
||||
|
||||
export interface WriteThroughResult {
|
||||
written: boolean;
|
||||
path?: string;
|
||||
/**
|
||||
* Non-error reasons the file was not written:
|
||||
* - no_repo_configured: `sync.repo_path` is unset (DB-only by design).
|
||||
* - repo_not_found: `sync.repo_path` set but missing / not a directory.
|
||||
* - page_not_found_after_write: the DB row isn't readable back (the caller's
|
||||
* DB write failed or targeted a different source).
|
||||
*/
|
||||
skipped?: 'no_repo_configured' | 'repo_not_found' | 'page_not_found_after_write';
|
||||
/** Set when the render/write/rename itself threw (EACCES, ENOTDIR, disk full). */
|
||||
error?: string;
|
||||
}
|
||||
|
||||
export interface WritePageThroughOpts {
|
||||
sourceId?: string;
|
||||
/** Merged over the page's own frontmatter at render time (e.g. provenance). */
|
||||
frontmatterOverrides?: Record<string, unknown>;
|
||||
logger?: WriteThroughLogger;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the DB row for `slug` to markdown and atomically write it under
|
||||
* `sync.repo_path`. Never throws — failures are reported via the result's
|
||||
* `skipped` / `error` fields (the DB write is the durable sink; the file is
|
||||
* best-effort and reconciled by the next `gbrain sync`).
|
||||
*/
|
||||
export async function writePageThrough(
|
||||
engine: BrainEngine,
|
||||
slug: string,
|
||||
opts: WritePageThroughOpts = {},
|
||||
): Promise<WriteThroughResult> {
|
||||
const sourceId = opts.sourceId ?? 'default';
|
||||
try {
|
||||
const repoPath = await engine.getConfig('sync.repo_path');
|
||||
if (!repoPath) {
|
||||
return { written: false, skipped: 'no_repo_configured' };
|
||||
}
|
||||
if (!existsSync(repoPath) || !statSync(repoPath).isDirectory()) {
|
||||
return { written: false, skipped: 'repo_not_found' };
|
||||
}
|
||||
|
||||
const writtenPage = await engine.getPage(slug, { sourceId });
|
||||
if (!writtenPage) {
|
||||
return { written: false, skipped: 'page_not_found_after_write' };
|
||||
}
|
||||
|
||||
const tags = await engine.getTags(slug, { sourceId });
|
||||
const md = serializePageToMarkdown(writtenPage, tags, {
|
||||
frontmatterOverrides: opts.frontmatterOverrides,
|
||||
});
|
||||
|
||||
const filePath = resolvePageFilePath(repoPath, slug, sourceId);
|
||||
mkdirSync(dirname(filePath), { recursive: true });
|
||||
|
||||
// Atomic write: unique temp sibling + rename. Unique name (pid + random)
|
||||
// so two concurrent saves to the same target can't clobber each other's
|
||||
// temp file. Clean up the temp on any failure so we never leak a stray
|
||||
// `.tmp` next to the real file.
|
||||
const tmpPath = `${filePath}.tmp.${process.pid}.${randomBytes(4).toString('hex')}`;
|
||||
try {
|
||||
writeFileSync(tmpPath, md, 'utf8');
|
||||
renameSync(tmpPath, filePath);
|
||||
} catch (writeErr) {
|
||||
try {
|
||||
if (existsSync(tmpPath)) unlinkSync(tmpPath);
|
||||
} catch {
|
||||
// best-effort cleanup; surface the original write error below
|
||||
}
|
||||
throw writeErr;
|
||||
}
|
||||
|
||||
return { written: true, path: filePath };
|
||||
} catch (e) {
|
||||
const msg = e instanceof Error ? e.message : String(e);
|
||||
opts.logger?.warn(`[write-through] failed for ${slug}: ${msg}`);
|
||||
return { written: false, error: msg };
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,187 @@
|
||||
/**
|
||||
* brainstorm/lsd --save persistence tests.
|
||||
*
|
||||
* Covers persistSavedIdea (canonical importFromContent + shared write-through),
|
||||
* formatSaveOutcome (honest message + exit code), and buildIdeaSlug
|
||||
* (collision-resistant nonce). Includes the two regression cases for the
|
||||
* silent-false-success bug class: DB import throws → exit 1 "NOT persisted";
|
||||
* file write fails but DB row landed → exit 0 (durable, sync reconciles).
|
||||
*/
|
||||
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
import { PGLiteEngine } from '../../src/core/pglite-engine.ts';
|
||||
import { resetPgliteState } from '../helpers/reset-pglite.ts';
|
||||
import { resetGateway } from '../../src/core/ai/gateway.ts';
|
||||
import type { BrainEngine } from '../../src/core/engine.ts';
|
||||
import {
|
||||
persistSavedIdea,
|
||||
formatSaveOutcome,
|
||||
buildIdeaSlug,
|
||||
type SaveOutcome,
|
||||
} from '../../src/commands/brainstorm.ts';
|
||||
import { serializeMarkdown } from '../../src/core/markdown.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let tmpRoot: string;
|
||||
let brainDir: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
resetGateway();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetPgliteState(engine);
|
||||
resetGateway();
|
||||
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-bs-save-'));
|
||||
brainDir = path.join(tmpRoot, 'brain');
|
||||
fs.mkdirSync(brainDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tmpRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const sampleContent = () =>
|
||||
serializeMarkdown({ mode: 'lsd', question: 'why X' }, '# LSD: why X\n\nbody', '', {
|
||||
type: 'note',
|
||||
title: 'LSD: why X',
|
||||
tags: [],
|
||||
});
|
||||
|
||||
describe('persistSavedIdea', () => {
|
||||
test('both sinks: canonical DB import (chunks written) + file rendered from row', async () => {
|
||||
await engine.setConfig('sync.repo_path', brainDir);
|
||||
const slug = buildIdeaSlug('why X', 'lsd', 'nonce01');
|
||||
const o = await persistSavedIdea(engine, { slug, content: sampleContent(), provenanceVia: 'lsd' });
|
||||
|
||||
expect(o.dbSaved).toBe(true);
|
||||
expect(o.writeThrough.written).toBe(true);
|
||||
expect(fs.existsSync(o.writeThrough.path!)).toBe(true);
|
||||
|
||||
// Canonical proof: importFromContent wrote chunks. Raw engine.putPage (the
|
||||
// pre-fix path) would write ZERO chunks, so the page would be unsearchable
|
||||
// and the next sync would churn the row.
|
||||
const rows = await engine.executeRaw<{ n: number }>(
|
||||
'SELECT COUNT(*)::int AS n FROM content_chunks ch JOIN pages p ON p.id = ch.page_id WHERE p.slug = $1',
|
||||
[slug],
|
||||
);
|
||||
expect(Number(rows[0].n)).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('no repo configured → DB canonical, writeThrough skipped, exit 0', async () => {
|
||||
await engine.setConfig('sync.repo_path', '');
|
||||
const slug = buildIdeaSlug('why Y', 'lsd', 'nonce02');
|
||||
const o = await persistSavedIdea(engine, { slug, content: sampleContent(), provenanceVia: 'lsd' });
|
||||
|
||||
expect(o.dbSaved).toBe(true);
|
||||
expect(o.writeThrough.skipped).toBe('no_repo_configured');
|
||||
const m = formatSaveOutcome(o, { profileLabel: 'lsd', slug });
|
||||
expect(m.exitCode).toBe(0);
|
||||
expect(m.stdout).toContain('no `sync.repo_path`');
|
||||
});
|
||||
|
||||
test('[REGRESSION] DB import throws → nothing persisted → exit 1 NOT persisted', async () => {
|
||||
// A dead/garbage engine makes importFromContent throw immediately — the
|
||||
// shape of the original PgBouncer "DB write failed but said saved" bug.
|
||||
const deadEngine = {} as unknown as BrainEngine;
|
||||
const slug = buildIdeaSlug('why Z', 'lsd', 'nonce03');
|
||||
const o = await persistSavedIdea(deadEngine, { slug, content: sampleContent(), provenanceVia: 'lsd' });
|
||||
|
||||
expect(o.dbSaved).toBe(false);
|
||||
expect(typeof o.dbError).toBe('string');
|
||||
expect(o.writeThrough.written).toBe(false);
|
||||
|
||||
const m = formatSaveOutcome(o, { profileLabel: 'lsd', slug });
|
||||
expect(m.exitCode).toBe(1);
|
||||
expect(m.stdout).toBeUndefined();
|
||||
expect(m.stderr.some((l) => l.includes('NOT persisted'))).toBe(true);
|
||||
});
|
||||
|
||||
test('[REGRESSION] repo set but file write fails (ENOTDIR) → DB saved, exit 0, warns', async () => {
|
||||
await engine.setConfig('sync.repo_path', brainDir);
|
||||
fs.writeFileSync(path.join(brainDir, 'wiki'), 'blocker'); // blocks wiki/ideas/
|
||||
const slug = buildIdeaSlug('why W', 'lsd', 'nonce04'); // wiki/ideas/...-nonce04
|
||||
const o = await persistSavedIdea(engine, { slug, content: sampleContent(), provenanceVia: 'lsd' });
|
||||
|
||||
expect(o.dbSaved).toBe(true);
|
||||
expect(o.writeThrough.written).toBe(false);
|
||||
expect(typeof o.writeThrough.error).toBe('string');
|
||||
|
||||
const m = formatSaveOutcome(o, { profileLabel: 'lsd', slug });
|
||||
expect(m.exitCode).toBe(0); // row is durable; sync reconciles the file
|
||||
expect(m.stdout).toContain('file NOT written');
|
||||
expect(m.stderr.some((l) => l.includes('file write failed'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatSaveOutcome (pure)', () => {
|
||||
const ctx = { profileLabel: 'lsd', slug: 'wiki/ideas/x' };
|
||||
|
||||
test('both written → exit 0, names both sinks', () => {
|
||||
const o: SaveOutcome = { dbSaved: true, writeThrough: { written: true, path: '/r/x.md' } };
|
||||
const m = formatSaveOutcome(o, ctx);
|
||||
expect(m.exitCode).toBe(0);
|
||||
expect(m.stdout).toContain('and file');
|
||||
expect(m.stderr).toEqual([]);
|
||||
});
|
||||
|
||||
test('db only, repo_not_found → exit 0', () => {
|
||||
const o: SaveOutcome = { dbSaved: true, writeThrough: { written: false, skipped: 'repo_not_found' } };
|
||||
const m = formatSaveOutcome(o, ctx);
|
||||
expect(m.exitCode).toBe(0);
|
||||
expect(m.stdout).toContain('not a directory');
|
||||
});
|
||||
|
||||
test('db saved but file errored → exit 0, warns on stderr', () => {
|
||||
const o: SaveOutcome = { dbSaved: true, writeThrough: { written: false, error: 'EACCES' } };
|
||||
const m = formatSaveOutcome(o, ctx);
|
||||
expect(m.exitCode).toBe(0);
|
||||
expect(m.stdout).toContain('file NOT written');
|
||||
expect(m.stderr.some((l) => l.includes('EACCES'))).toBe(true);
|
||||
});
|
||||
|
||||
test('nothing persisted → exit 1, both error lines on stderr', () => {
|
||||
const o: SaveOutcome = {
|
||||
dbSaved: false,
|
||||
dbError: 'boom',
|
||||
writeThrough: { written: false, skipped: 'page_not_found_after_write' },
|
||||
};
|
||||
const m = formatSaveOutcome(o, ctx);
|
||||
expect(m.exitCode).toBe(1);
|
||||
expect(m.stdout).toBeUndefined();
|
||||
expect(m.stderr.some((l) => l.includes('DB save failed: boom'))).toBe(true);
|
||||
expect(m.stderr.some((l) => l.includes('NOT persisted'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildIdeaSlug', () => {
|
||||
test('distinct nonces → distinct slugs (no same-day clobber)', () => {
|
||||
const a = buildIdeaSlug('same question', 'lsd', 'aaa');
|
||||
const b = buildIdeaSlug('same question', 'lsd', 'bbb');
|
||||
expect(a).not.toBe(b);
|
||||
expect(a.startsWith('wiki/ideas/')).toBe(true);
|
||||
});
|
||||
|
||||
test('empty question → untitled stem, still unique by nonce', () => {
|
||||
const a = buildIdeaSlug('', 'lsd', 'aaa');
|
||||
const b = buildIdeaSlug('', 'lsd', 'bbb');
|
||||
expect(a).toContain('-untitled-');
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
|
||||
test('production nonce (no arg) is random → two calls differ', () => {
|
||||
const a = buildIdeaSlug('q', 'brainstorm');
|
||||
const b = buildIdeaSlug('q', 'brainstorm');
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,139 @@
|
||||
/**
|
||||
* Shared write-through helper tests (src/core/write-through.ts).
|
||||
*
|
||||
* Covers the skip/error branches and the atomic-write guarantee. The helper is
|
||||
* the canonical disk sink shared by `put_page` and `gbrain brainstorm/lsd
|
||||
* --save`, extracted from the v0.38 put_page write-through and upgraded to write
|
||||
* atomically (.tmp + rename).
|
||||
*/
|
||||
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
import { resetGateway } from '../src/core/ai/gateway.ts';
|
||||
import { writePageThrough } from '../src/core/write-through.ts';
|
||||
import { importFromContent } from '../src/core/import-file.ts';
|
||||
import { serializePageToMarkdown, resolvePageFilePath } from '../src/core/markdown.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let tmpRoot: string;
|
||||
let brainDir: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
resetGateway();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetPgliteState(engine);
|
||||
resetGateway();
|
||||
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-wt-helper-'));
|
||||
brainDir = path.join(tmpRoot, 'brain');
|
||||
fs.mkdirSync(brainDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(tmpRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function seedPage(slug: string): Promise<void> {
|
||||
await importFromContent(engine, slug, `---\ntitle: T\ntype: note\n---\n\n# Body ${slug}\n`, {
|
||||
noEmbed: true,
|
||||
sourceId: 'default',
|
||||
sourcePath: `${slug}.md`,
|
||||
});
|
||||
}
|
||||
|
||||
function walkFiles(dir: string): string[] {
|
||||
const out: string[] = [];
|
||||
const walk = (d: string) => {
|
||||
for (const e of fs.readdirSync(d, { withFileTypes: true })) {
|
||||
const p = path.join(d, e.name);
|
||||
if (e.isDirectory()) walk(p);
|
||||
else out.push(p);
|
||||
}
|
||||
};
|
||||
walk(dir);
|
||||
return out;
|
||||
}
|
||||
|
||||
describe('writePageThrough', () => {
|
||||
test('writes the file rendered from the saved row; no .tmp leftover', async () => {
|
||||
await engine.setConfig('sync.repo_path', brainDir);
|
||||
const slug = 'wiki/ideas/2026-01-01-lsd-foo-abc123';
|
||||
await seedPage(slug);
|
||||
|
||||
const res = await writePageThrough(engine, slug, {
|
||||
sourceId: 'default',
|
||||
frontmatterOverrides: { source_kind: 'lsd' },
|
||||
});
|
||||
|
||||
expect(res.written).toBe(true);
|
||||
const expectedPath = resolvePageFilePath(brainDir, slug, 'default');
|
||||
expect(res.path).toBe(expectedPath);
|
||||
expect(fs.existsSync(expectedPath)).toBe(true);
|
||||
|
||||
// Content is the canonical serialization of the saved row (the file is
|
||||
// rendered FROM the row, so the sinks can't diverge).
|
||||
const page = await engine.getPage(slug, { sourceId: 'default' });
|
||||
const tags = await engine.getTags(slug, { sourceId: 'default' });
|
||||
const expected = serializePageToMarkdown(page!, tags, {
|
||||
frontmatterOverrides: { source_kind: 'lsd' },
|
||||
});
|
||||
expect(fs.readFileSync(expectedPath, 'utf8')).toBe(expected);
|
||||
|
||||
// Atomic write left no temp sibling.
|
||||
const dir = path.dirname(expectedPath);
|
||||
expect(fs.readdirSync(dir).some((f) => f.includes('.tmp.'))).toBe(false);
|
||||
});
|
||||
|
||||
test('no sync.repo_path → skipped no_repo_configured', async () => {
|
||||
await engine.setConfig('sync.repo_path', '');
|
||||
const slug = 'wiki/ideas/x-1';
|
||||
await seedPage(slug);
|
||||
const res = await writePageThrough(engine, slug);
|
||||
expect(res).toEqual({ written: false, skipped: 'no_repo_configured' });
|
||||
});
|
||||
|
||||
test('sync.repo_path is a file, not a directory → skipped repo_not_found', async () => {
|
||||
const fileAsRepo = path.join(tmpRoot, 'not-a-dir');
|
||||
fs.writeFileSync(fileAsRepo, 'x');
|
||||
await engine.setConfig('sync.repo_path', fileAsRepo);
|
||||
const slug = 'wiki/ideas/x-2';
|
||||
await seedPage(slug);
|
||||
const res = await writePageThrough(engine, slug);
|
||||
expect(res).toEqual({ written: false, skipped: 'repo_not_found' });
|
||||
});
|
||||
|
||||
test('row missing → skipped page_not_found_after_write', async () => {
|
||||
await engine.setConfig('sync.repo_path', brainDir);
|
||||
const res = await writePageThrough(engine, 'wiki/ideas/does-not-exist');
|
||||
expect(res).toEqual({ written: false, skipped: 'page_not_found_after_write' });
|
||||
});
|
||||
|
||||
test('[REGRESSION] mkdir ENOTDIR (parent is a file) → error, no partial .md, no .tmp', async () => {
|
||||
await engine.setConfig('sync.repo_path', brainDir);
|
||||
// Block the `wiki/` directory by putting a FILE named "wiki" under the repo,
|
||||
// so `mkdir -p <repo>/wiki/ideas` throws ENOTDIR deterministically.
|
||||
fs.writeFileSync(path.join(brainDir, 'wiki'), 'blocker');
|
||||
const slug = 'wiki/ideas/blocked-1';
|
||||
await seedPage(slug);
|
||||
|
||||
const res = await writePageThrough(engine, slug, { sourceId: 'default' });
|
||||
|
||||
expect(res.written).toBe(false);
|
||||
expect(typeof res.error).toBe('string');
|
||||
const files = walkFiles(brainDir);
|
||||
expect(files.some((f) => f.endsWith('.md'))).toBe(false);
|
||||
expect(files.some((f) => f.includes('.tmp.'))).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user