mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
feat(sync): --missing-path skip — classify absent-local_path sources in --all instead of failing (#3426)
sources.local_path is machine-specific state in a brain-wide table. Any brain whose sources were registered from more than one machine — or a sanctioned setup mid-migration (topologies.md Topology 2, or the system-of-record git flow before every repo is cloned) — has sources whose checkout is not present on the machine running sync --all. Each surfaced as a hard failure and forced rc=1 every run; on one observed fleet that was 12 phantom failures per hour, training operators to ignore the exit code. --missing-path skip classifies them honestly: ⊘ in the human aggregate, status skipped_missing_path + local_path in the --json envelope, new skipped_count, excluded from error_count and the rc=1 gate. Using the flag outside --all warns instead of silently no-oping. Default stays fail: on a single-machine brain a missing local_path usually means an unmounted volume or deleted checkout, and silently skipping would hide data loss. Skip is explicit opt-in. Pure helpers (parseMissingPathMode, partitionMissingPathSources) exported and unit-tested in the sync-all-parallel style — no DB, no fs. Docs: sync --help, docs/TESTING.md inventory, KEY_FILES.md sync entry. CHANGELOG/VERSION deliberately untouched per the release process. Co-authored-by: Ziggy <lazyclaw137@gmail.com> Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Lazydayz137 <Lazydayz137@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Ziggy
Garry Tan
Lazydayz137
Claude Fable 5
parent
16782aee7f
commit
10079efe40
@@ -192,6 +192,7 @@ Unit tests and what they cover:
|
||||
- `test/sync-pull-failed-anchor.serial.test.ts` — #3068 regression: a failed internal `git pull` (local-path origin vs `protocol.file.allow=never`) with zero imports returns `partial`/`pull_failed` (not `up_to_date`), freezes `last_commit` + `last_sync_at`, recovers after a manual pull; fall-through import of local commits preserved. Serial: pins `GBRAIN_HOME` to a temp dir for the whole file.
|
||||
- `test/sync-concurrency.test.ts` — `autoConcurrency()` thresholds + PGLite-forces-serial + explicit-override clamping; `shouldRunParallel()` explicit-bypasses-floor contract; `parseWorkers()` validation rejecting `'0'`/`'-3'`/`'foo'`/`'1.5'`/trailing chars.
|
||||
- `test/sync-parallel.test.ts` — PGLite-routed coverage of the bookmark gate under concurrency, head-drift gate, vanished-file failure capture, PGLite-stays-serial, and the `gbrain-sync` writer-lock contract.
|
||||
- `test/sync-all-missing-path.test.ts` — `sync --all --missing-path <fail|skip>` pure helpers: `parseMissingPathMode` (default fail, explicit values, loud rejection of bad/dangling values, never swallows a following flag) and `partitionMissingPathSources` (classification driven only by the injected pathExists predicate — no fs; null `local_path` passes through runnable; order preserved).
|
||||
- `test/sync-failures.test.ts` — `classifyErrorCode` regex coverage for all 12 codes against literal production message strings from `markdown.ts` and `import-file.ts`; `summarizeFailuresByCode` sort + pre-classified-honor; `recordSyncFailures` code-field persistence; `acknowledgeSyncFailures` `AcknowledgeResult` shape + backfill on legacy entries.
|
||||
- `test/doctor.test.ts` — doctor command; assertions that `jsonb_integrity` scans the four JSONB write sites and `markdown_body_completeness` is present.
|
||||
- `test/utils.test.ts` — shared SQL utilities + `tryParseEmbedding` null-return and single-warn semantics.
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -2372,6 +2372,8 @@ IMPORT/EXPORT
|
||||
import <dir> [--no-embed] Import markdown directory
|
||||
sync [--repo <path>] [flags] Git-to-brain incremental sync
|
||||
sync --watch [--interval N] Continuous sync (loops until stopped)
|
||||
sync --all --missing-path skip Classify sources whose local_path is absent
|
||||
on this machine as skipped, not failed
|
||||
See also: autopilot --install (continuous daemon).
|
||||
export [--dir ./out/] Export to markdown
|
||||
export --restore-only [--repo <p>] Restore missing supabase-only files
|
||||
|
||||
+130
-13
@@ -4169,12 +4169,22 @@ Options:
|
||||
connections per wave ≈ parallel × workers × 2
|
||||
(per-file pool) + parent pool. Pass --parallel 1
|
||||
to force serial.
|
||||
--missing-path M (with --all) What to do when a source's local_path
|
||||
does not exist on this machine: 'fail' (default —
|
||||
loud, current behavior) or 'skip' (classify as
|
||||
skipped_missing_path: ⊘ in the aggregate, excluded
|
||||
from error_count and the rc=1 gate). Use skip on
|
||||
brains whose sources were registered from more
|
||||
than one machine.
|
||||
--json Emit a structured JSON envelope on stdout
|
||||
({schema_version: 1, sources, parallel,
|
||||
ok_count, error_count}). Human banners route to
|
||||
stderr so '--json | jq' parses cleanly.
|
||||
Exit codes: 0 = all sources ok, 1 = any error,
|
||||
2 = cost-prompt-not-confirmed.
|
||||
ok_count, error_count, skipped_count}). Sources
|
||||
skipped by --missing-path skip appear with
|
||||
status 'skipped_missing_path' and their
|
||||
local_path. Human banners route to stderr so
|
||||
'--json | jq' parses cleanly.
|
||||
Exit codes: 0 = all sources ok or skipped,
|
||||
1 = any error, 2 = cost-prompt-not-confirmed.
|
||||
--yes Accept any interactive prompts (CI / non-TTY).
|
||||
|
||||
See also:
|
||||
@@ -4198,6 +4208,19 @@ See also:
|
||||
const noSchemaPack = args.includes('--no-schema-pack'); // v0.41.37.0 #1569
|
||||
const includeGitignored = args.includes('--include-gitignored');
|
||||
const syncAll = args.includes('--all');
|
||||
let missingPathMode: MissingPathMode = 'fail';
|
||||
try {
|
||||
missingPathMode = parseMissingPathMode(args);
|
||||
} catch (e) {
|
||||
console.error(e instanceof Error ? e.message : String(e));
|
||||
process.exit(2);
|
||||
}
|
||||
if (missingPathMode !== 'fail' && !syncAll) {
|
||||
// Single-source sync on a missing path should stay loud — an explicit
|
||||
// `--source X` naming an absent checkout is an operator error, not a
|
||||
// multi-machine artifact. Warn instead of silently ignoring the flag.
|
||||
console.error('[gbrain] WARN: --missing-path only applies to `sync --all`; ignored here.');
|
||||
}
|
||||
const jsonOut = args.includes('--json');
|
||||
const yesFlag = args.includes('--yes');
|
||||
// v0.41.6.0 D3: lock-recovery flags. --break-lock (safe) verifies the
|
||||
@@ -4484,14 +4507,40 @@ See also:
|
||||
writeHuman(`Skipping ${disabledCount} disabled source(s).`);
|
||||
}
|
||||
|
||||
if (activeSources.length === 0) {
|
||||
// --missing-path skip: classify sources whose checkout is not on this
|
||||
// machine instead of failing them (see parseMissingPathMode's rationale).
|
||||
// Under the default 'fail' this is a no-op and behavior is unchanged.
|
||||
let skippedMissingPath: typeof activeSources = [];
|
||||
let runnableSources = activeSources;
|
||||
if (missingPathMode === 'skip') {
|
||||
const parts = partitionMissingPathSources(activeSources, existsSync);
|
||||
runnableSources = parts.runnable;
|
||||
skippedMissingPath = parts.missing;
|
||||
for (const src of skippedMissingPath) {
|
||||
writeHuman(` ⊘ ${src.name}: skipped — local_path not present on this host (${src.local_path})`);
|
||||
}
|
||||
if (skippedMissingPath.length > 0) {
|
||||
writeHuman(`Skipped ${skippedMissingPath.length} source(s) whose local_path is not present on this host (--missing-path skip).`);
|
||||
}
|
||||
}
|
||||
|
||||
if (runnableSources.length === 0) {
|
||||
if (jsonOut) {
|
||||
console.log(JSON.stringify({
|
||||
schema_version: 1,
|
||||
sources: [],
|
||||
sources: skippedMissingPath
|
||||
.slice()
|
||||
.sort((a, b) => a.id.localeCompare(b.id))
|
||||
.map((s) => ({
|
||||
source_id: s.id,
|
||||
name: s.name,
|
||||
status: 'skipped_missing_path',
|
||||
local_path: s.local_path,
|
||||
})),
|
||||
parallel: 0,
|
||||
ok_count: 0,
|
||||
error_count: 0,
|
||||
skipped_count: skippedMissingPath.length,
|
||||
}));
|
||||
}
|
||||
return;
|
||||
@@ -4501,11 +4550,20 @@ See also:
|
||||
type PerSourceResult = {
|
||||
sourceId: string;
|
||||
sourceName: string;
|
||||
status: 'ok' | 'error';
|
||||
status: 'ok' | 'error' | 'skipped_missing_path';
|
||||
result?: SyncResult;
|
||||
error?: string;
|
||||
localPath?: string;
|
||||
};
|
||||
const perSourceResults: PerSourceResult[] = [];
|
||||
for (const src of skippedMissingPath) {
|
||||
perSourceResults.push({
|
||||
sourceId: src.id,
|
||||
sourceName: src.name,
|
||||
status: 'skipped_missing_path',
|
||||
localPath: src.local_path ?? undefined,
|
||||
});
|
||||
}
|
||||
|
||||
// #1633 (Part B): one shared SIGINT controller for the whole --all fan-out.
|
||||
// process-cleanup.ts doesn't own SIGINT, so without this Ctrl-C hard-cuts the
|
||||
@@ -4615,7 +4673,7 @@ See also:
|
||||
};
|
||||
|
||||
const parallelEligible =
|
||||
v2Enabled && !serialFlag && engine.kind !== 'pglite' && activeSources.length > 1;
|
||||
v2Enabled && !serialFlag && engine.kind !== 'pglite' && runnableSources.length > 1;
|
||||
|
||||
// v0.42.42.0 (#2139, D13C): the v0.40.6.0 (D15) refusal of --skip-failed /
|
||||
// --retry-failed under parallel sync is LIFTED. It existed because the
|
||||
@@ -4629,7 +4687,7 @@ See also:
|
||||
// know how the run was actually dispatched. 1 in the serial fallback,
|
||||
// capped at min(sourceCount, --max-sources, 8) in the parallel path.
|
||||
const effectiveParallel = parallelEligible
|
||||
? Math.min(activeSources.length, maxSources ?? 8)
|
||||
? Math.min(runnableSources.length, maxSources ?? 8)
|
||||
: 1;
|
||||
|
||||
process.on('SIGINT', onAllSigint);
|
||||
@@ -4653,8 +4711,8 @@ See also:
|
||||
);
|
||||
}
|
||||
|
||||
writeHuman(`\nParallel sync: ${activeSources.length} sources, ${cap} concurrent workers.\n`);
|
||||
const results = await pMapAllSettled(activeSources, cap, async (src) => {
|
||||
writeHuman(`\nParallel sync: ${runnableSources.length} sources, ${cap} concurrent workers.\n`);
|
||||
const results = await pMapAllSettled(runnableSources, cap, async (src) => {
|
||||
const r = await runOne(src);
|
||||
return { name: src.name, result: r };
|
||||
});
|
||||
@@ -4662,7 +4720,7 @@ See also:
|
||||
writeHuman('\n--- sync --all aggregate ---');
|
||||
for (let i = 0; i < results.length; i++) {
|
||||
const r = results[i];
|
||||
const src = activeSources[i];
|
||||
const src = runnableSources[i];
|
||||
if (r.status === 'fulfilled') {
|
||||
writeHuman(` ✓ ${src.name}: ${r.value.result.status} (added=${r.value.result.added}, modified=${r.value.result.modified}, deleted=${r.value.result.deleted})`);
|
||||
perSourceResults.push({
|
||||
@@ -4683,7 +4741,7 @@ See also:
|
||||
}
|
||||
}
|
||||
} else {
|
||||
for (const src of activeSources) {
|
||||
for (const src of runnableSources) {
|
||||
writeHuman(`\n--- Syncing source: ${src.name} ---`);
|
||||
try {
|
||||
const result = await runOne(src);
|
||||
@@ -4723,6 +4781,7 @@ See also:
|
||||
source_id: r.sourceId,
|
||||
name: r.sourceName,
|
||||
status: r.status,
|
||||
...(r.localPath ? { local_path: r.localPath } : {}),
|
||||
...(r.result ? {
|
||||
sync_status: r.result.status,
|
||||
// #3068: surface the partial reason (e.g. pull_failed) so JSON
|
||||
@@ -4742,6 +4801,7 @@ See also:
|
||||
parallel: effectiveParallel,
|
||||
ok_count: okCount,
|
||||
error_count: errCount,
|
||||
skipped_count: perSourceResults.filter((r) => r.status === 'skipped_missing_path').length,
|
||||
}));
|
||||
}
|
||||
|
||||
@@ -4938,6 +4998,63 @@ See also:
|
||||
}
|
||||
}
|
||||
|
||||
/** Mode for `sync --all --missing-path`: what to do when a source's
|
||||
* local_path does not exist on this machine. */
|
||||
export type MissingPathMode = 'fail' | 'skip';
|
||||
|
||||
/**
|
||||
* Parse `--missing-path <fail|skip>` (default: fail).
|
||||
*
|
||||
* Why the flag exists: `sources.local_path` is machine-specific state in a
|
||||
* brain-wide table. Any brain whose sources were registered from more than
|
||||
* one machine — or a sanctioned setup mid-migration (topologies.md Topology 2,
|
||||
* or the system-of-record git flow before every repo is cloned here) — has
|
||||
* sources whose checkout simply is not present on the machine running
|
||||
* `sync --all`. Each used to surface as a hard failure ("Not a git
|
||||
* repository: <path>") and force rc=1 on every run; on one observed fleet
|
||||
* that was 12 phantom failures per hour, which trains operators to ignore
|
||||
* the exit code.
|
||||
*
|
||||
* The DEFAULT stays `fail`: on a single-machine brain a missing local_path
|
||||
* usually means an unmounted volume or a deleted checkout, and silently
|
||||
* skipping it would hide real data loss. Skip is an explicit opt-in.
|
||||
*
|
||||
* Throws on a bad/absent value with a paste-ready hint (caller converts to
|
||||
* stderr + exit 2, same as other flag-misuse exits).
|
||||
*/
|
||||
export function parseMissingPathMode(args: string[]): MissingPathMode {
|
||||
const idx = args.indexOf('--missing-path');
|
||||
if (idx === -1) return 'fail';
|
||||
const val = args[idx + 1];
|
||||
if (val === 'fail' || val === 'skip') return val;
|
||||
throw new Error(
|
||||
`--missing-path expects 'fail' or 'skip', got: ${val ?? '(nothing)'}. ` +
|
||||
`Use \`--missing-path skip\` to classify sources whose local_path is not ` +
|
||||
`present on this machine as skipped instead of failed, or \`--missing-path ` +
|
||||
`fail\` (the default) to keep them loud.`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Partition `--all` sources by whether their local_path exists on THIS
|
||||
* machine. Classification is driven only by the injected predicate so tests
|
||||
* never touch the filesystem. A null local_path passes through as runnable —
|
||||
* pure-DB sources are already excluded from `--all` by the
|
||||
* `local_path IS NOT NULL` SELECT; this is defensive, not load-bearing.
|
||||
*/
|
||||
export function partitionMissingPathSources<T extends { local_path: string | null }>(
|
||||
sources: T[],
|
||||
pathExists: (p: string) => boolean,
|
||||
): { runnable: T[]; missing: T[] } {
|
||||
const runnable: T[] = [];
|
||||
const missing: T[] = [];
|
||||
for (const s of sources) {
|
||||
if (s.local_path != null && !pathExists(s.local_path)) missing.push(s);
|
||||
else runnable.push(s);
|
||||
}
|
||||
return { runnable, missing };
|
||||
}
|
||||
|
||||
/**
|
||||
* v0.40.3.0 — resolve effective per-source concurrency for `sync --all`.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* Tests for `sync --all --missing-path <fail|skip>`.
|
||||
*
|
||||
* Why this exists:
|
||||
* `sources.local_path` is machine-specific state in a brain-wide table.
|
||||
* Any brain whose sources were registered from more than one machine —
|
||||
* or a sanctioned setup mid-migration (docs/architecture/topologies.md
|
||||
* Topology 2, or the system-of-record git flow before every repo is
|
||||
* cloned) — has sources whose checkout is simply not present on the
|
||||
* machine running `sync --all`. Today each of those is reported as a
|
||||
* hard per-source failure ("Not a git repository: <path>") and the run
|
||||
* exits rc=1, every run. On one observed fleet that was 12 phantom
|
||||
* failures per hour with zero actionable signal, which trains operators
|
||||
* to ignore the exit code — the worst possible property for a cron.
|
||||
*
|
||||
* `--missing-path skip` classifies those sources honestly instead:
|
||||
* status `skipped_missing_path` in the --json envelope, a ⊘ line in the
|
||||
* human aggregate, excluded from error_count and from the rc=1 gate.
|
||||
*
|
||||
* The DEFAULT stays `fail`: on a single-machine brain a missing
|
||||
* local_path usually means an unmounted volume or a deleted checkout,
|
||||
* and silently skipping it would hide real data loss. Skip is opt-in.
|
||||
*
|
||||
* These tests pin the two pure helpers (same style as
|
||||
* sync-all-parallel.test.ts — no DB, no fs):
|
||||
* 1. parseMissingPathMode(): flag parsing, default, loud rejection of
|
||||
* bad values (paste-ready hint names the flag and both values).
|
||||
* 2. partitionMissingPathSources(): classification is driven ONLY by
|
||||
* the injected pathExists predicate; null local_path passes through
|
||||
* as runnable (pure-DB sources are already excluded from --all by
|
||||
* the local_path IS NOT NULL SELECT — defensive, not load-bearing).
|
||||
*/
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import {
|
||||
parseMissingPathMode,
|
||||
partitionMissingPathSources,
|
||||
} from '../src/commands/sync.ts';
|
||||
|
||||
// ── parseMissingPathMode ────────────────────────────────────────────
|
||||
|
||||
describe('parseMissingPathMode', () => {
|
||||
test('defaults to fail when the flag is absent', () => {
|
||||
expect(parseMissingPathMode([])).toBe('fail');
|
||||
expect(parseMissingPathMode(['--all', '--no-embed'])).toBe('fail');
|
||||
});
|
||||
|
||||
test('parses skip and fail explicitly', () => {
|
||||
expect(parseMissingPathMode(['--all', '--missing-path', 'skip'])).toBe('skip');
|
||||
expect(parseMissingPathMode(['--all', '--missing-path', 'fail'])).toBe('fail');
|
||||
});
|
||||
|
||||
test('rejects an unknown value with a paste-ready hint', () => {
|
||||
expect(() => parseMissingPathMode(['--missing-path', 'ignore']))
|
||||
.toThrow(/--missing-path.*(fail|skip)/);
|
||||
});
|
||||
|
||||
test('rejects a dangling flag (no value)', () => {
|
||||
expect(() => parseMissingPathMode(['--all', '--missing-path']))
|
||||
.toThrow(/--missing-path/);
|
||||
});
|
||||
|
||||
test('does not swallow a following flag as its value', () => {
|
||||
// `--missing-path --json` is a mistake, not "mode --json".
|
||||
expect(() => parseMissingPathMode(['--missing-path', '--json']))
|
||||
.toThrow(/--missing-path/);
|
||||
});
|
||||
});
|
||||
|
||||
// ── partitionMissingPathSources ─────────────────────────────────────
|
||||
|
||||
type Src = { id: string; local_path: string | null };
|
||||
const src = (id: string, local_path: string | null): Src => ({ id, local_path });
|
||||
|
||||
describe('partitionMissingPathSources', () => {
|
||||
test('splits sources by the injected pathExists predicate', () => {
|
||||
const sources = [src('here', '/present'), src('elsewhere', '/absent')];
|
||||
const { runnable, missing } = partitionMissingPathSources(
|
||||
sources, (p) => p === '/present');
|
||||
expect(runnable.map((s) => s.id)).toEqual(['here']);
|
||||
expect(missing.map((s) => s.id)).toEqual(['elsewhere']);
|
||||
});
|
||||
|
||||
test('null local_path stays runnable (pure-DB sources are not "missing")', () => {
|
||||
const { runnable, missing } = partitionMissingPathSources(
|
||||
[src('db-only', null)], () => false);
|
||||
expect(runnable.map((s) => s.id)).toEqual(['db-only']);
|
||||
expect(missing).toEqual([]);
|
||||
});
|
||||
|
||||
test('all-missing and empty inputs are well-formed', () => {
|
||||
const allMissing = partitionMissingPathSources(
|
||||
[src('a', '/x'), src('b', '/y')], () => false);
|
||||
expect(allMissing.runnable).toEqual([]);
|
||||
expect(allMissing.missing.map((s) => s.id)).toEqual(['a', 'b']);
|
||||
|
||||
const empty = partitionMissingPathSources([], () => true);
|
||||
expect(empty.runnable).toEqual([]);
|
||||
expect(empty.missing).toEqual([]);
|
||||
});
|
||||
|
||||
test('never consults the predicate for null paths and preserves order', () => {
|
||||
const asked: string[] = [];
|
||||
const sources = [src('a', '/1'), src('b', null), src('c', '/2')];
|
||||
const { runnable } = partitionMissingPathSources(sources, (p) => {
|
||||
asked.push(p);
|
||||
return true;
|
||||
});
|
||||
expect(asked).toEqual(['/1', '/2']);
|
||||
expect(runnable.map((s) => s.id)).toEqual(['a', 'b', 'c']);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user