fix(files): upload-raw persists small files + source-namespaced storage (#2297)

The small-file branch printed success and returned without copying the file into
the brain repo or inserting a files row — every small text/PDF raw was silently
lost. Both branches now copy into the repo and record a row via one shared writer.
storage_path (globally UNIQUE) is source-namespaced so the same page slug in two
sources can't ON CONFLICT-clobber each other; the physical copy stays repo-relative
under the source's own repo root. A new --source flag strictly disambiguates a slug
present in multiple sources (a scoped miss returns null, never falls back unscoped).

Adds test/silent-failure-wave.test.ts covering all four wave fixes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-24 15:19:14 -07:00
co-authored by Claude Opus 4.8
parent f399246db1
commit 33a842a7cc
2 changed files with 409 additions and 38 deletions
+197 -38
View File
@@ -86,7 +86,7 @@ export async function runFiles(engine: BrainEngine, args: string[]) {
console.error(`Usage: gbrain files <command> [args]`);
console.error(` list [slug] List files for a page (or all)`);
console.error(` upload <file> --page <slug> Upload file linked to page`);
console.error(` upload-raw <file> --page <slug> [--type <type>] Smart upload with .redirect.yaml pointer`);
console.error(` upload-raw <file> --page <slug> [--source <id>] [--type <type>] Smart upload with .redirect.yaml pointer`);
console.error(` signed-url <path> Generate signed URL for stored file`);
console.error(` sync <dir> Upload directory to storage`);
console.error(` verify Verify all uploads match local`);
@@ -170,23 +170,126 @@ async function uploadFile(engine: BrainEngine, args: string[]) {
console.log(`Uploaded: ${storagePath} (${humanSize(stat.size)})`);
}
/**
* Resolve the on-disk brain repo root for a source (#2297). Multi-source: a
* source's `local_path` is its checkout; the legacy/default source falls back
* to the `sync.repo_path` config anchor written by `gbrain sync`. Returns null
* when no repo is resolvable (pure-DB brain) — callers must fail loud rather
* than silently claim a git write succeeded.
*/
export async function resolveRepoRoot(engine: BrainEngine, sourceId: string | null): Promise<string | null> {
if (sourceId && sourceId !== 'default') {
const rows = await engine.executeRaw<{ local_path: string | null }>(
`SELECT local_path FROM sources WHERE id = $1`,
[sourceId],
);
if (rows[0]?.local_path) return rows[0].local_path;
}
return await engine.getConfig('sync.repo_path');
}
/**
* Look up a page by slug to link the file row to a real page + source (#2297).
* Slug uniqueness is (source_id, slug); with only `--page <slug>` we take the
* first live match. Returns null for a slug with no page (file row still records
* the slug + the default source).
*/
export async function lookupPageBySlug(
engine: BrainEngine,
pageSlug: string,
sourceHint?: string | null,
): Promise<{ id: number; source_id: string } | null> {
// Slug uniqueness is (source_id, slug), so a bare slug can match multiple
// sources (#2297). An explicit --source is honored STRICTLY: a scoped miss
// returns null (never falls back to an unscoped match, which could silently
// attach the file to the wrong source). Only a bare slug (no hint) takes the
// first live match deterministically (id ASC).
if (sourceHint) {
const scoped = await engine.executeRaw<{ id: number; source_id: string }>(
`SELECT id, source_id FROM pages WHERE slug = $1 AND source_id = $2 AND deleted_at IS NULL ORDER BY id LIMIT 1`,
[pageSlug, sourceHint],
);
return scoped[0] ?? null;
}
const rows = await engine.executeRaw<{ id: number; source_id: string }>(
`SELECT id, source_id FROM pages WHERE slug = $1 AND deleted_at IS NULL ORDER BY id LIMIT 1`,
[pageSlug],
);
return rows[0] ?? null;
}
/**
* Build the globally-unique `files.storage_path` key (#2297). The schema's
* UNIQUE is on storage_path alone, but slugs only disambiguate within a source,
* so a bare `<page>/<file>` collides across sources. Prefix non-default sources
* with the source id so two sources can hold the same slug+filename without one
* ON CONFLICT-clobbering the other. Default-source paths stay byte-identical
* (back-compat). NOTE: this is the DB key, not the on-disk path — the physical
* copy lives at repoRelPath under the source's own repo root.
*/
export function namespacedStoragePath(sourceId: string | null, repoRelPath: string): string {
return sourceId && sourceId !== 'default' ? `${sourceId}/${repoRelPath}` : repoRelPath;
}
/**
* Single canonical `files` row writer shared by BOTH the git (small-file) and
* cloud (large/media) branches of upload-raw (#2297, DRY). `storage_path` is
* UNIQUE in the schema, so it MUST be page-namespaced by the caller to avoid one
* page's attachment clobbering another's. metadata goes through executeRawJsonb
* (raw object, never JSON.stringify into a ::jsonb cast — the double-encode trap).
*/
export async function insertFileRow(
engine: BrainEngine,
row: {
sourceId: string | null;
pageId: number | null;
pageSlug: string | null;
filename: string;
storagePath: string;
mimeType: string | null;
size: number;
contentHash: string;
metadata: Record<string, unknown>;
},
): Promise<void> {
await executeRawJsonb(
engine,
`INSERT INTO files (source_id, page_id, page_slug, filename, storage_path, mime_type, size_bytes, content_hash, metadata)
VALUES (COALESCE($1, 'default'), $2, $3, $4, $5, $6, $7, $8, $9::jsonb)
ON CONFLICT (storage_path) DO UPDATE SET
content_hash = EXCLUDED.content_hash,
size_bytes = EXCLUDED.size_bytes,
mime_type = EXCLUDED.mime_type,
page_id = EXCLUDED.page_id,
page_slug = EXCLUDED.page_slug`,
[row.sourceId, row.pageId, row.pageSlug, row.filename, row.storagePath, row.mimeType, row.size, row.contentHash],
[row.metadata],
);
}
/**
* Smart upload with size routing and .redirect.yaml pointer creation.
*
* Size routing:
* < 100 MB text/PDF → stays in git (brain repo), no cloud upload
* < 100 MB text/PDF → copied into the brain repo under <page>/.raw/, recorded
* in the files table (storage = 'git')
* >= 100 MB OR media → upload to cloud storage, create .redirect.yaml pointer
* in the repo, recorded in the files table (storage = cloud)
*
* The .redirect.yaml pointer stays in the brain repo so git tracks what was stored.
* Both branches persist a files row AND write into the brain repo so git tracks
* what was stored. Neither silently returns success without persisting (#2297).
*/
async function uploadRaw(engine: BrainEngine, args: string[]) {
const filePath = args.find(a => !a.startsWith('--'));
const pageSlug = args.find((a, i) => args[i - 1] === '--page') || null;
const fileType = args.find((a, i) => args[i - 1] === '--type') || null;
// --source disambiguates a slug that exists in more than one source (#2297);
// without it, a bare slug resolves to the first matching source.
const sourceHint = args.find((a, i) => args[i - 1] === '--source') || null;
const noPointer = args.includes('--no-pointer');
if (!filePath || !existsSync(filePath)) {
console.error('Usage: gbrain files upload-raw <file> --page <slug> [--type <type>] [--no-pointer]');
console.error('Usage: gbrain files upload-raw <file> --page <slug> [--source <id>] [--type <type>] [--no-pointer]');
process.exit(1);
}
@@ -197,13 +300,56 @@ async function uploadRaw(engine: BrainEngine, args: string[]) {
const needsCloud = stat.size >= SIZE_THRESHOLD || isMedia;
if (!needsCloud) {
// Small text/PDF files stay in git
// Small text/PDF files are copied INTO the brain repo and recorded in the
// files table (#2297). Previously this branch printed success and returned
// without copying anything or inserting a row — every small "raw" was
// silently lost. Resolve the page + source so the row links correctly and
// the destination is page-namespaced (storage_path is UNIQUE).
const page = pageSlug ? await lookupPageBySlug(engine, pageSlug, sourceHint) : null;
const sourceId = page?.source_id ?? sourceHint ?? 'default';
const repoRoot = await resolveRepoRoot(engine, sourceId);
if (!repoRoot) {
console.error(JSON.stringify({
success: false,
reason: 'no_repo_path',
message: 'No brain repo on disk (sources.local_path / sync.repo_path unset). '
+ 'Run `gbrain sync` to anchor a repo, or configure cloud storage for raw uploads.',
}));
process.exit(1);
}
const content = readFileSync(filePath);
const hash = createHash('sha256').update(content).digest('hex');
// Physical copy lives at the repo-relative path UNDER the source's own repo
// root; the DB storage_path key is additionally source-namespaced so it stays
// globally unique across sources (#2297).
const repoRelPath = pageSlug
? `${pageSlug}/.raw/${filename}`
: `unsorted/.raw/${hash.slice(0, 8)}-${filename}`;
const storagePath = namespacedStoragePath(sourceId, repoRelPath);
const destAbs = join(repoRoot, repoRelPath);
mkdirSync(dirname(destAbs), { recursive: true });
writeFileSync(destAbs, content);
await insertFileRow(engine, {
sourceId,
pageId: page?.id ?? null,
pageSlug,
filename,
storagePath,
mimeType,
size: stat.size,
contentHash: 'sha256:' + hash,
metadata: { ...(fileType ? { type: fileType } : {}), upload_method: 'git' },
});
console.log(JSON.stringify({
success: true,
storage: 'git',
path: filePath,
path: storagePath,
abs_path: destAbs,
size: stat.size,
size_human: humanSize(stat.size),
hash: `sha256:${hash}`,
}));
return;
}
@@ -221,48 +367,61 @@ async function uploadRaw(engine: BrainEngine, args: string[]) {
const storage = await createStorage(config.storage as any);
const content = readFileSync(filePath);
const hash = createHash('sha256').update(content).digest('hex');
const storagePath = pageSlug ? `${pageSlug}/${filename}` : `unsorted/${hash.slice(0, 8)}-${filename}`;
const bucket = (config.storage as any).bucket || 'brain-files';
const page = pageSlug ? await lookupPageBySlug(engine, pageSlug, sourceHint) : null;
const sourceId = page?.source_id ?? sourceHint ?? 'default';
// Source-namespace the cloud storage key so the same slug+filename in two
// sources doesn't collide on the UNIQUE storage_path (#2297). Default source
// keeps its historical path.
const cloudRelPath = pageSlug ? `${pageSlug}/${filename}` : `unsorted/${hash.slice(0, 8)}-${filename}`;
const storagePath = namespacedStoragePath(sourceId, cloudRelPath);
const method = content.length >= SIZE_THRESHOLD ? 'TUS resumable' : 'standard';
console.error(`Uploading ${humanSize(stat.size)} via ${method}...`);
await storage.upload(storagePath, content, mimeType || undefined);
// Create .redirect.yaml pointer in the brain repo
// Create .redirect.yaml pointer IN THE BRAIN REPO, page-namespaced (#2297).
// Previously this was written next to the *input* file (filePath +
// '.redirect.yaml'), so the pointer never landed in the repo the comment
// claimed — git tracked nothing. Resolve the repo root and write it under
// the page's .raw/ sidecar so it travels with the page.
let pointerPath: string | null = null;
if (!noPointer && pageSlug) {
const { stringify } = await import('../core/yaml-lite.ts');
const pointer = stringify({
target: `supabase://${bucket}/${storagePath}`,
bucket,
storage_path: storagePath,
size: stat.size,
size_human: humanSize(stat.size),
hash: `sha256:${hash}`,
mime: mimeType || 'application/octet-stream',
uploaded: new Date().toISOString(),
...(fileType ? { type: fileType } : {}),
});
// Write pointer next to the original file
pointerPath = filePath + '.redirect.yaml';
writeFileSync(pointerPath, pointer);
console.error(`Pointer written: ${pointerPath}`);
const repoRoot = await resolveRepoRoot(engine, sourceId);
if (repoRoot) {
const { stringify } = await import('../core/yaml-lite.ts');
const pointer = stringify({
target: `supabase://${bucket}/${storagePath}`,
bucket,
storage_path: storagePath,
size: stat.size,
size_human: humanSize(stat.size),
hash: `sha256:${hash}`,
mime: mimeType || 'application/octet-stream',
uploaded: new Date().toISOString(),
...(fileType ? { type: fileType } : {}),
});
pointerPath = join(repoRoot, pageSlug, '.raw', `${filename}.redirect.yaml`);
mkdirSync(dirname(pointerPath), { recursive: true });
writeFileSync(pointerPath, pointer);
console.error(`Pointer written: ${pointerPath}`);
} else {
console.error('No brain repo on disk — skipping .redirect.yaml pointer (cloud upload + DB row still recorded).');
}
}
// Record in DB. files.metadata is JSONB — pass the object via
// executeRawJsonb with an explicit ::jsonb cast so post-v0.31 reads see
// an actual object, not a JSON-encoded string (D1 wave).
await executeRawJsonb(
engine,
`INSERT INTO files (page_slug, filename, storage_path, mime_type, size_bytes, content_hash, metadata)
VALUES ($1, $2, $3, $4, $5, $6, $7::jsonb)
ON CONFLICT (storage_path) DO UPDATE SET
content_hash = EXCLUDED.content_hash,
size_bytes = EXCLUDED.size_bytes,
mime_type = EXCLUDED.mime_type`,
[pageSlug, filename, storagePath, mimeType, stat.size, 'sha256:' + hash],
[{ type: fileType, upload_method: method }],
);
await insertFileRow(engine, {
sourceId,
pageId: page?.id ?? null,
pageSlug,
filename,
storagePath,
mimeType,
size: stat.size,
contentHash: 'sha256:' + hash,
metadata: { ...(fileType ? { type: fileType } : {}), upload_method: method },
});
// Output JSON for scripting
console.log(JSON.stringify({
+212
View File
@@ -0,0 +1,212 @@
/**
* Regression tests for the silent-failure cleanup wave (post-#2375).
*
* - #2251 updateSourceConfig array-branch guard (jsonb_each on a non-object)
* - #2330 getStats vs getHealth page-count parity (both exclude soft-deleted)
* - #2297 files upload-raw persistence helpers (page-namespaced, no clobber)
*
* Hermetic in-memory PGLite; Postgres parity covered by the e2e suite. The
* #2251 + #2330 fixes also land in postgres-engine.ts in lockstep.
*/
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { resetPgliteState } from './helpers/reset-pglite.ts';
import { resolveRepoRoot, lookupPageBySlug, insertFileRow, namespacedStoragePath } from '../src/commands/files.ts';
import { categorizeCheck } from '../src/core/doctor-categories.ts';
let engine: PGLiteEngine;
beforeAll(async () => {
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
});
afterAll(async () => {
await engine.disconnect();
});
beforeEach(async () => {
await resetPgliteState(engine);
});
async function seedSource(id: string, localPath: string | null): Promise<void> {
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config, created_at)
VALUES ($1, $2, $3, '{}'::jsonb, NOW())
ON CONFLICT (id) DO UPDATE SET local_path = EXCLUDED.local_path`,
[id, id, localPath],
);
}
async function seedPage(slug: string, opts: { deleted?: boolean; sourceId?: string } = {}): Promise<number> {
const rows = await engine.executeRaw<{ id: number }>(
`INSERT INTO pages (source_id, slug, type, title, deleted_at)
VALUES ($1, $2, 'note', $2, ${opts.deleted ? 'NOW()' : 'NULL'})
RETURNING id`,
[opts.sourceId ?? 'default', slug],
);
return rows[0].id;
}
describe('#2251 — updateSourceConfig array-branch guard', () => {
test('coerces a corrupted array config (with a non-object element) instead of throwing', async () => {
await seedSource('corrupt-arr', '/tmp/corrupt-arr');
// Force the historical bad shape: a JSONB ARRAY whose elements include a
// non-object scalar. Pre-fix, the array branch ran jsonb_each() on the
// string element and the whole UPDATE aborted with
// "cannot call jsonb_each on a non-object".
await engine.executeRaw(
`UPDATE sources SET config = '["stray-string", {"federated": true}]'::jsonb WHERE id = $1`,
['corrupt-arr'],
);
// Must NOT throw, and must return true (row matched).
const ok = await engine.updateSourceConfig('corrupt-arr', { tracked_branch: 'main' });
expect(ok).toBe(true);
const rows = await engine.executeRaw<{ typ: string; cfg: Record<string, unknown> }>(
`SELECT jsonb_typeof(config) AS typ, config AS cfg FROM sources WHERE id = $1`,
['corrupt-arr'],
);
expect(rows[0].typ).toBe('object');
// Object element flattened, scalar element skipped, patch merged.
expect(rows[0].cfg.federated).toBe(true);
expect(rows[0].cfg.tracked_branch).toBe('main');
});
test('still coerces a double-encoded string config', async () => {
await seedSource('corrupt-str', '/tmp/corrupt-str');
await engine.executeRaw(
`UPDATE sources SET config = to_jsonb('{"github_repo":"a/b"}'::text) WHERE id = $1`,
['corrupt-str'],
);
const ok = await engine.updateSourceConfig('corrupt-str', { federated: false });
expect(ok).toBe(true);
const rows = await engine.executeRaw<{ typ: string; cfg: Record<string, unknown> }>(
`SELECT jsonb_typeof(config) AS typ, config AS cfg FROM sources WHERE id = $1`,
['corrupt-str'],
);
expect(rows[0].typ).toBe('object');
expect(rows[0].cfg.github_repo).toBe('a/b');
expect(rows[0].cfg.federated).toBe(false);
});
});
describe('#2330 — skill_surface check is categorized as skill (not vacuous meta)', () => {
test('the focused-doctor skill_surface check lands in the skill category', () => {
// Without this wiring the explicit skill_surface check would fall through to
// meta, leaving category_scores.skill a vacuous 100 — the exact dishonesty
// #2330 reports. categorizeCheck must map it to skill.
expect(categorizeCheck('skill_surface')).toBe('skill');
});
});
describe('#2330 — getStats and getHealth agree on page_count (both exclude soft-deleted)', () => {
test('soft-deleted pages are excluded from BOTH surfaces', async () => {
await seedPage('live/one');
await seedPage('live/two');
await seedPage('gone/three', { deleted: true });
const stats = await engine.getStats();
const health = await engine.getHealth();
expect(stats.page_count).toBe(2);
expect(health.page_count).toBe(2);
expect(health.page_count).toBe(stats.page_count);
});
});
describe('#2297 — upload-raw persistence helpers', () => {
test('resolveRepoRoot prefers source.local_path, falls back to sync.repo_path', async () => {
await seedSource('teamco', '/repos/teamco');
expect(await resolveRepoRoot(engine, 'teamco')).toBe('/repos/teamco');
await engine.setConfig('sync.repo_path', '/repos/default');
expect(await resolveRepoRoot(engine, 'default')).toBe('/repos/default');
expect(await resolveRepoRoot(engine, null)).toBe('/repos/default');
});
test('lookupPageBySlug returns the live page id + source, honoring the source hint', async () => {
const id = await seedPage('people/alice-example');
const found = await lookupPageBySlug(engine, 'people/alice-example');
expect(found?.id).toBe(id);
expect(found?.source_id).toBe('default');
expect(await lookupPageBySlug(engine, 'people/nobody')).toBeNull();
// Same slug in a second source → the hint disambiguates (#2297).
await seedSource('teamco', '/repos/teamco');
const teamId = await seedPage('people/alice-example', { sourceId: 'teamco' });
const hinted = await lookupPageBySlug(engine, 'people/alice-example', 'teamco');
expect(hinted?.id).toBe(teamId);
expect(hinted?.source_id).toBe('teamco');
// Explicit --source is STRICT: a scoped miss returns null even though the
// slug exists in `default` — no silent cross-source fallback.
expect(await lookupPageBySlug(engine, 'people/alice-example', 'no-such-source')).toBeNull();
});
test('namespacedStoragePath keeps default-source paths but isolates other sources (#2297)', async () => {
// Default source: byte-identical to the historical path.
expect(namespacedStoragePath('default', 'people/alice/.raw/cv.pdf')).toBe('people/alice/.raw/cv.pdf');
expect(namespacedStoragePath(null, 'people/alice/.raw/cv.pdf')).toBe('people/alice/.raw/cv.pdf');
// Non-default source: prefixed so the same slug+filename can't collide on
// the globally-UNIQUE storage_path.
expect(namespacedStoragePath('teamco', 'people/alice/.raw/cv.pdf')).toBe('teamco/people/alice/.raw/cv.pdf');
// End to end: two sources, same slug+filename → two distinct rows.
await seedSource('src-a', '/repos/a');
await seedSource('src-b', '/repos/b');
const aId = await seedPage('shared/slug', { sourceId: 'src-a' });
const bId = await seedPage('shared/slug', { sourceId: 'src-b' });
await insertFileRow(engine, {
sourceId: 'src-a', pageId: aId, pageSlug: 'shared/slug', filename: 'f.pdf',
storagePath: namespacedStoragePath('src-a', 'shared/slug/.raw/f.pdf'),
mimeType: 'application/pdf', size: 1, contentHash: 'sha256:a', metadata: {},
});
await insertFileRow(engine, {
sourceId: 'src-b', pageId: bId, pageSlug: 'shared/slug', filename: 'f.pdf',
storagePath: namespacedStoragePath('src-b', 'shared/slug/.raw/f.pdf'),
mimeType: 'application/pdf', size: 2, contentHash: 'sha256:b', metadata: {},
});
const rows = await engine.executeRaw<{ n: number }>(`SELECT count(*)::int AS n FROM files`);
expect(rows[0].n).toBe(2);
});
test('insertFileRow persists a row; page-namespaced paths do not clobber across pages', async () => {
const aliceId = await seedPage('people/alice-example');
const bobId = await seedPage('people/bob-example');
// Same filename, different page → page-namespaced storage_path keeps both.
await insertFileRow(engine, {
sourceId: 'default', pageId: aliceId, pageSlug: 'people/alice-example',
filename: 'resume.pdf', storagePath: 'people/alice-example/.raw/resume.pdf',
mimeType: 'application/pdf', size: 10, contentHash: 'sha256:a', metadata: { upload_method: 'git' },
});
await insertFileRow(engine, {
sourceId: 'default', pageId: bobId, pageSlug: 'people/bob-example',
filename: 'resume.pdf', storagePath: 'people/bob-example/.raw/resume.pdf',
mimeType: 'application/pdf', size: 20, contentHash: 'sha256:b', metadata: { upload_method: 'git' },
});
const rows = await engine.executeRaw<{ n: number }>(`SELECT count(*)::int AS n FROM files`);
expect(rows[0].n).toBe(2);
// Re-uploading the SAME storage_path upserts (no duplicate), and metadata
// round-trips as a real JSONB object (not a double-encoded string).
await insertFileRow(engine, {
sourceId: 'default', pageId: aliceId, pageSlug: 'people/alice-example',
filename: 'resume.pdf', storagePath: 'people/alice-example/.raw/resume.pdf',
mimeType: 'application/pdf', size: 99, contentHash: 'sha256:a2', metadata: { upload_method: 'git', type: 'cv' },
});
const after = await engine.executeRaw<{ n: number; size_bytes: number; meta_typ: string }>(
`SELECT count(*)::int AS n,
max(size_bytes) AS size_bytes,
jsonb_typeof((SELECT metadata FROM files WHERE storage_path = 'people/alice-example/.raw/resume.pdf')) AS meta_typ
FROM files`,
);
expect(after[0].n).toBe(2); // upsert, not insert
expect(Number(after[0].size_bytes)).toBe(99); // updated row
expect(after[0].meta_typ).toBe('object'); // metadata is a real object
});
});