From d4f4be068c1aca4b06974630bf871ce00f375349 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Thu, 19 Feb 2026 13:18:20 -0800 Subject: [PATCH] fix(minio): support mc_path and harden sync against transient objects --- README.md | 4 +- docs/plans/state.json | 35 +++++++++- src/backup/run.test.ts | 12 ++++ src/backup/run.ts | 31 +++++++-- src/config/schema.test.ts | 3 + src/config/schema.ts | 1 + src/tools/builtin/minio-ingest.test.ts | 15 +++- src/tools/builtin/minio-ingest.ts | 10 +-- src/tools/builtin/minio-share.test.ts | 14 +++- src/tools/builtin/minio-share.ts | 9 +-- src/tools/builtin/minio-sync.test.ts | 96 ++++++++++++++++++++++++++ src/tools/builtin/minio-sync.ts | 29 ++++++-- 12 files changed, 238 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index c833efe..08115e0 100644 --- a/README.md +++ b/README.md @@ -818,6 +818,8 @@ backup: bucket: flynn-backups prefix: flynn secure: true + # Optional absolute path to MinIO client binary if not on PATH + mc_path: /usr/local/bin/mc ``` ## MinIO Share Tool @@ -1631,7 +1633,7 @@ src/ ## System Prompt -Flynn assembles its system prompt from layered template files (`SOUL.md`, `AGENTS.md`, `IDENTITY.md`, `USER.md`, `TOOLS.md`) searched in configurable directories. The first match per file wins. +Flynn assembles its system prompt from layered template files (`SOUL.md`, `AGENTS.md`, `IDENTITY.md`, `USER.md`, `TOOLS.md`, `BOOTSTRAP.md`, `HEARTBEAT.md`) searched in configurable directories. The first match per file wins. A **Runtime Context** section is automatically appended to every system prompt with the current date and time, so the model always knows when "now" is without needing a tool call. diff --git a/docs/plans/state.json b/docs/plans/state.json index 63f63db..279fbc8 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -5860,10 +5860,43 @@ "docs/plans/state.json" ], "test_status": "docs-only change" + }, + "minio-mc-path-and-enoent-hardening": { + "status": "completed", + "date": "2026-02-19", + "updated": "2026-02-19", + "summary": "Added backup-level MinIO CLI path override (`backup.minio.mc_path`) and wired it through MinIO backup upload + `minio.share`/`minio.ingest`/`minio.sync`. Added consistent ENOENT guidance so missing `mc` now returns actionable setup errors instead of raw spawn failures.", + "files_modified": [ + "src/config/schema.ts", + "src/config/schema.test.ts", + "src/backup/run.ts", + "src/backup/run.test.ts", + "src/tools/builtin/minio-share.ts", + "src/tools/builtin/minio-share.test.ts", + "src/tools/builtin/minio-ingest.ts", + "src/tools/builtin/minio-ingest.test.ts", + "src/tools/builtin/minio-sync.ts", + "src/tools/builtin/minio-sync.test.ts", + "README.md", + "docs/plans/state.json" + ], + "test_status": "pnpm test:run src/config/schema.test.ts src/tools/builtin/minio-sync.test.ts src/tools/builtin/minio-ingest.test.ts src/tools/builtin/minio-share.test.ts src/backup/run.test.ts + pnpm typecheck passing" + }, + "minio-sync-keep-marker-and-race-hardening": { + "status": "completed", + "date": "2026-02-19", + "updated": "2026-02-19", + "summary": "Hardened `minio.sync` against noisy object listings and race conditions by skipping `.keep` marker objects and treating missing-object read errors (objects deleted after listing) as per-object skips instead of failing the entire sync task.", + "files_modified": [ + "src/tools/builtin/minio-sync.ts", + "src/tools/builtin/minio-sync.test.ts", + "docs/plans/state.json" + ], + "test_status": "pnpm test:run src/tools/builtin/minio-sync.test.ts + pnpm typecheck passing" } }, "overall_progress": { - "total_test_count": 1933, + "total_test_count": 1941, "all_tests_passing": true, "p0_completion": "3/3 (100%)", "p1_completion": "4/4 (100%)", diff --git a/src/backup/run.test.ts b/src/backup/run.test.ts index 8c42fda..b77ea14 100644 --- a/src/backup/run.test.ts +++ b/src/backup/run.test.ts @@ -18,4 +18,16 @@ describe('backup internals', () => { expect(backupInternals.buildObjectKey('/flynn/daily/', 'a.tar.gz')).toBe('flynn/daily/a.tar.gz'); expect(backupInternals.buildObjectKey('', 'a.tar.gz')).toBe('a.tar.gz'); }); + + it('resolves custom mc path with fallback', () => { + expect(backupInternals.resolveMcPath('/usr/local/bin/mc')).toBe('/usr/local/bin/mc'); + expect(backupInternals.resolveMcPath('')).toBe('mc'); + expect(backupInternals.resolveMcPath(undefined)).toBe('mc'); + }); + + it('formats missing mc binary errors with setup hint', () => { + const error = new Error('spawn mc ENOENT') as Error & { code?: string }; + error.code = 'ENOENT'; + expect(backupInternals.formatMinioCliError(error, '/custom/mc')).toContain('MinIO client binary not found: /custom/mc'); + }); }); diff --git a/src/backup/run.ts b/src/backup/run.ts index b754bc4..4968f49 100644 --- a/src/backup/run.ts +++ b/src/backup/run.ts @@ -61,6 +61,21 @@ function buildObjectKey(prefix: string, fileName: string): string { return trimmed.length > 0 ? `${trimmed}/${fileName}` : fileName; } +function resolveMcPath(pathValue: string | undefined): string { + const trimmed = pathValue?.trim(); + return trimmed && trimmed.length > 0 ? trimmed : 'mc'; +} + +function formatMinioCliError(error: unknown, mcPath: string): string { + if (error && typeof error === 'object' && 'code' in error && (error as { code?: unknown }).code === 'ENOENT') { + return `MinIO client binary not found: ${mcPath}. Install MinIO Client (mc) or set \`backup.minio.mc_path\` to the full binary path.`; + } + if (error instanceof Error) { + return error.message; + } + return String(error); +} + function collectExistingEntries(opts: { dataDir: string; backupConfig: BackupConfig; @@ -137,12 +152,18 @@ export async function runBackupSnapshot(opts: BackupRunOptions): Promise { expect(result.backup.minio.enabled).toBe(false); expect(result.backup.minio.prefix).toBe('flynn'); expect(result.backup.minio.secure).toBe(true); + expect(result.backup.minio.mc_path).toBeUndefined(); }); it('accepts custom backup settings', () => { @@ -307,6 +308,7 @@ describe('configSchema — backup', () => { bucket: 'flynn-backups', prefix: 'daily', secure: false, + mc_path: '/usr/local/bin/mc', }, }, }); @@ -325,6 +327,7 @@ describe('configSchema — backup', () => { expect(result.backup.minio.bucket).toBe('flynn-backups'); expect(result.backup.minio.prefix).toBe('daily'); expect(result.backup.minio.secure).toBe(false); + expect(result.backup.minio.mc_path).toBe('/usr/local/bin/mc'); }); }); diff --git a/src/config/schema.ts b/src/config/schema.ts index 5dace4d..3c36383 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -940,6 +940,7 @@ const backupSchema = z.object({ bucket: z.string().optional(), prefix: z.string().default('flynn'), secure: z.boolean().default(true), + mc_path: z.string().optional(), }).default({}), }).default({}); diff --git a/src/tools/builtin/minio-ingest.test.ts b/src/tools/builtin/minio-ingest.test.ts index b65729d..d3cc358 100644 --- a/src/tools/builtin/minio-ingest.test.ts +++ b/src/tools/builtin/minio-ingest.test.ts @@ -49,7 +49,18 @@ describe('createMinioIngestTool', () => { stderr: '', })); - const tool = createMinioIngestTool(makeBackupConfig(), store, { + const tool = createMinioIngestTool(makeBackupConfig({ + minio: { + enabled: true, + endpoint: 'localhost:9000', + access_key: 'minio-admin', + secret_key: 'minio-secret', + bucket: 'flynn-knowledge', + prefix: 'flynn', + secure: false, + mc_path: '/opt/bin/mc', + }, + }), store, { execRunner, now: () => new Date('2026-02-16T15:00:00.000Z'), }); @@ -68,7 +79,7 @@ describe('createMinioIngestTool', () => { 'append', ); expect(execRunner).toHaveBeenCalledWith( - 'mc', + '/opt/bin/mc', ['cat', 'flynningest/flynn-knowledge/knowledge/runbook.md'], expect.objectContaining({ env: expect.any(Object) }), ); diff --git a/src/tools/builtin/minio-ingest.ts b/src/tools/builtin/minio-ingest.ts index fefc4c6..e6a83b6 100644 --- a/src/tools/builtin/minio-ingest.ts +++ b/src/tools/builtin/minio-ingest.ts @@ -58,20 +58,21 @@ function isExtractableBinaryObject(objectKey: string): boolean { async function readObjectText( runner: ExecRunner, + mcPath: string, remotePath: string, objectKey: string, env: NodeJS.ProcessEnv, ): Promise { const ext = extname(objectKey).toLowerCase(); if (!isExtractableBinaryObject(objectKey)) { - const { stdout } = await runner('mc', ['cat', remotePath], { env, maxBuffer: 20 * 1024 * 1024 }); + const { stdout } = await runner(mcPath, ['cat', remotePath], { env, maxBuffer: 20 * 1024 * 1024 }); return toText(stdout); } const tempDir = mkdtempSync(join(tmpdir(), 'flynn-minio-ingest-')); const localPath = join(tempDir, 'object.bin'); try { - await runner('mc', ['cp', remotePath, localPath], { env, maxBuffer: 20 * 1024 * 1024 }); + await runner(mcPath, ['cp', remotePath, localPath], { env, maxBuffer: 20 * 1024 * 1024 }); if (ext === '.pdf') { const { stdout } = await runner('pdftotext', ['-q', localPath, '-'], { maxBuffer: 20 * 1024 * 1024 }); return toText(stdout); @@ -187,13 +188,14 @@ export function createMinioIngestTool(config: BackupConfig, store: MemoryStore, secure: minio.secure, }); const env = { ...process.env, [`MC_HOST_${alias}`]: host }; + const mcPath = backupInternals.resolveMcPath(minio.mc_path); const runner = deps?.execRunner ?? (async (file: string, cmdArgs: string[], options?: { env?: NodeJS.ProcessEnv; maxBuffer?: number }) => { return execFileAsync(file, cmdArgs, options); }); const remotePath = `${alias}/${bucket}/${objectKey}`; try { - const text = await readObjectText(runner, remotePath, objectKey, env); + const text = await readObjectText(runner, mcPath, remotePath, objectKey, env); if (!force && !isExtractableBinaryObject(objectKey) && !isLikelyText(text)) { return { @@ -227,7 +229,7 @@ export function createMinioIngestTool(config: BackupConfig, store: MemoryStore, return { success: false, output: '', - error: error instanceof Error ? error.message : String(error), + error: backupInternals.formatMinioCliError(error, mcPath), }; } }, diff --git a/src/tools/builtin/minio-share.test.ts b/src/tools/builtin/minio-share.test.ts index 0893244..d2bcd8f 100644 --- a/src/tools/builtin/minio-share.test.ts +++ b/src/tools/builtin/minio-share.test.ts @@ -63,7 +63,18 @@ describe('createMinioShareTool', () => { return { stdout: '', stderr: '' }; }); - const tool = createMinioShareTool(makeBackupConfig(), { + const tool = createMinioShareTool(makeBackupConfig({ + minio: { + enabled: true, + endpoint: 'localhost:9000', + access_key: 'minio-admin', + secret_key: 'minio-secret', + bucket: 'flynn-shared', + prefix: 'flynn', + secure: false, + mc_path: '/opt/bin/mc', + }, + }), { execRunner, now: () => new Date('2026-02-16T10:00:00.000Z'), }); @@ -72,6 +83,7 @@ describe('createMinioShareTool', () => { expect(result.success).toBe(true); expect(result.output).toContain('https://minio.local/share/abc'); expect(execRunner).toHaveBeenCalledTimes(3); + expect(execRunner.mock.calls[0]?.[0]).toBe('/opt/bin/mc'); expect(execRunner.mock.calls[1]?.[1]).toContain('cp'); expect(execRunner.mock.calls[2]?.[1]).toContain('share'); }); diff --git a/src/tools/builtin/minio-share.ts b/src/tools/builtin/minio-share.ts index 875b814..f130a0d 100644 --- a/src/tools/builtin/minio-share.ts +++ b/src/tools/builtin/minio-share.ts @@ -116,14 +116,15 @@ export function createMinioShareTool(config: BackupConfig, deps?: MinioShareDeps secure: minio.secure, }); const env = { ...process.env, [`MC_HOST_${alias}`]: host }; + const mcPath = backupInternals.resolveMcPath(minio.mc_path); const runner = deps?.execRunner ?? (async (file: string, cmdArgs: string[], options?: { env?: NodeJS.ProcessEnv }) => { return execFileAsync(file, cmdArgs, options); }); try { - await runner('mc', ['mb', '--ignore-existing', `${alias}/${minio.bucket}`], { env }); - await runner('mc', ['cp', localPath, remotePath], { env }); - const { stdout } = await runner('mc', ['share', 'download', '--json', '--expire', expires, remotePath], { env }); + await runner(mcPath, ['mb', '--ignore-existing', `${alias}/${minio.bucket}`], { env }); + await runner(mcPath, ['cp', localPath, remotePath], { env }); + const { stdout } = await runner(mcPath, ['share', 'download', '--json', '--expire', expires, remotePath], { env }); const shareUrl = parseShareUrl(typeof stdout === 'string' ? stdout : stdout.toString('utf-8')); if (!shareUrl) { return { success: false, output: '', error: 'Failed to parse MinIO share URL from mc output' }; @@ -137,7 +138,7 @@ export function createMinioShareTool(config: BackupConfig, deps?: MinioShareDeps return { success: false, output: '', - error: error instanceof Error ? error.message : String(error), + error: backupInternals.formatMinioCliError(error, mcPath), }; } }, diff --git a/src/tools/builtin/minio-sync.test.ts b/src/tools/builtin/minio-sync.test.ts index 3a26992..9418d9e 100644 --- a/src/tools/builtin/minio-sync.test.ts +++ b/src/tools/builtin/minio-sync.test.ts @@ -41,6 +41,17 @@ describe('minio sync internals', () => { ]); }); + it('ignores keep-marker objects from recursive listings', () => { + const stdout = [ + '{"status":"success","type":"file","key":".keep"}', + '{"status":"success","type":"file","key":"reports/.keep"}', + '{"status":"success","type":"file","key":"reports/daily.md"}', + ].join('\n'); + expect(minioSyncInternals.parseListedObjectKeys(stdout)).toEqual([ + 'reports/daily.md', + ]); + }); + it('normalizes object paths into namespace-safe segments', () => { expect(minioSyncInternals.normalizeNamespaceSegment('knowledge/team runbook.v2.md')).toBe('knowledge/team_runbook_v2'); }); @@ -163,4 +174,89 @@ describe('createMinioSyncTool', () => { expect(result.success).toBe(false); expect(result.error).toContain('backup.minio.enabled=true'); }); + + it('uses configured backup.minio.mc_path and returns setup hint when missing', async () => { + const write = vi.fn(); + const store = { write } as unknown as MemoryStore; + const execRunner = vi.fn(async () => { + const error = new Error('spawn /opt/minio/mc ENOENT') as Error & { code?: string }; + error.code = 'ENOENT'; + throw error; + }); + + const tool = createMinioSyncTool(makeBackupConfig({ + minio: { + enabled: true, + endpoint: 'localhost:9000', + access_key: 'minio-admin', + secret_key: 'minio-secret', + bucket: 'flynn-knowledge', + prefix: 'flynn', + secure: false, + mc_path: '/opt/minio/mc', + }, + }), store, { execRunner }); + + const result = await tool.execute({ prefix: 'knowledge/' }); + expect(result.success).toBe(false); + expect(result.error).toContain('MinIO client binary not found: /opt/minio/mc'); + expect(result.error).toContain('backup.minio.mc_path'); + expect(execRunner).toHaveBeenCalledWith( + '/opt/minio/mc', + ['ls', '--json', '--recursive', 'flynnsync/flynn-knowledge/knowledge/'], + expect.objectContaining({ env: expect.any(Object) }), + ); + }); + + it('skips objects that disappear between ls and read', async () => { + const write = vi.fn(); + const store = { write } as unknown as MemoryStore; + const execRunner = vi.fn(async (_file: string, args: string[]) => { + if (args[0] === 'ls') { + return { + stdout: [ + '{"status":"success","type":"file","key":"reports/.keep"}', + '{"status":"success","type":"file","key":"reports/summary.md"}', + ].join('\n'), + stderr: '', + }; + } + if (args[0] === 'cat' && args[1]?.endsWith('reports/summary.md')) { + return { stdout: 'OK summary', stderr: '' }; + } + throw new Error(`Unexpected call: ${args.join(' ')}`); + }); + + const tool = createMinioSyncTool(makeBackupConfig(), store, { execRunner }); + const result = await tool.execute({ prefix: 'reports/' }); + expect(result.success).toBe(true); + expect(result.output).toContain('Scanned: 1 object(s)'); + expect(result.output).toContain('Imported: 1'); + expect(result.output).toContain('Skipped: 0'); + expect(write).toHaveBeenCalledTimes(1); + }); + + it('treats missing-object read errors as skip instead of task failure', async () => { + const write = vi.fn(); + const store = { write } as unknown as MemoryStore; + const execRunner = vi.fn(async (_file: string, args: string[]) => { + if (args[0] === 'ls') { + return { + stdout: '{"status":"success","type":"file","key":"reports/missing.md"}', + stderr: '', + }; + } + if (args[0] === 'cat') { + throw new Error('mcli: Unable to read from flynnsync/flynn/reports/missing.md. Object does not exist.'); + } + return { stdout: '', stderr: '' }; + }); + + const tool = createMinioSyncTool(makeBackupConfig(), store, { execRunner }); + const result = await tool.execute({ prefix: 'reports/' }); + expect(result.success).toBe(true); + expect(result.output).toContain('Imported: 0'); + expect(result.output).toContain('Skipped: 1'); + expect(write).not.toHaveBeenCalled(); + }); }); diff --git a/src/tools/builtin/minio-sync.ts b/src/tools/builtin/minio-sync.ts index 51c3a33..3d166cd 100644 --- a/src/tools/builtin/minio-sync.ts +++ b/src/tools/builtin/minio-sync.ts @@ -44,6 +44,7 @@ function parseListedObjectKeys(stdout: string): string[] { if (!key) {continue;} if (type && type !== 'file') {continue;} if (key.endsWith('/')) {continue;} + if (key === '.keep' || key.endsWith('/.keep')) {continue;} keys.push(key); } catch { continue; @@ -52,6 +53,15 @@ function parseListedObjectKeys(stdout: string): string[] { return keys; } +function isBenignMissingObjectError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + const lowered = message.toLowerCase(); + return lowered.includes('object does not exist') + || lowered.includes('unable to read from') + || lowered.includes('no such key') + || lowered.includes('not found'); +} + function normalizeNamespaceSegment(value: string): string { return value .replace(/\.[^.]+$/, '') @@ -61,6 +71,7 @@ function normalizeNamespaceSegment(value: string): string { } export const minioSyncInternals = { + isBenignMissingObjectError, parseListedObjectKeys, normalizeNamespaceSegment, }; @@ -137,13 +148,14 @@ export function createMinioSyncTool(config: BackupConfig, store: MemoryStore, de secure: minio.secure, }); const env = { ...process.env, [`MC_HOST_${alias}`]: host }; + const mcPath = backupInternals.resolveMcPath(minio.mc_path); const runner = deps?.execRunner ?? (async (file: string, cmdArgs: string[], options?: { env?: NodeJS.ProcessEnv; maxBuffer?: number }) => { return execFileAsync(file, cmdArgs, options); }); try { const basePath = `${alias}/${bucket}/${prefix}`; - const { stdout: listed } = await runner('mc', ['ls', '--json', '--recursive', basePath], { + const { stdout: listed } = await runner(mcPath, ['ls', '--json', '--recursive', basePath], { env, maxBuffer: 20 * 1024 * 1024, }); @@ -167,8 +179,17 @@ export function createMinioSyncTool(config: BackupConfig, store: MemoryStore, de continue; } - const remotePath = `${alias}/${bucket}/${key}`; - const text = await minioIngestInternals.readObjectText(runner, remotePath, key, env); + let text = ''; + try { + const remotePath = `${alias}/${bucket}/${key}`; + text = await minioIngestInternals.readObjectText(runner, mcPath, remotePath, key, env); + } catch (error) { + if (isBenignMissingObjectError(error)) { + skipped++; + continue; + } + throw error; + } if (!force && !minioIngestInternals.isExtractableBinaryObject(key) && !minioIngestInternals.isLikelyText(text)) { skipped++; @@ -207,7 +228,7 @@ export function createMinioSyncTool(config: BackupConfig, store: MemoryStore, de return { success: false, output: '', - error: error instanceof Error ? error.message : String(error), + error: backupInternals.formatMinioCliError(error, mcPath), }; } },