mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
fix(sources): narrow #2928 fix to the read path — keep tier 5.5 write routing
CI (test/sync-sole-non-default-routing.test.ts) caught that excluding explicitly-isolated sources from pickSoleNonDefaultSource redirected unqualified WRITES (sync/import) to 'default' — silent data misplacement, worse than the read bug. --no-federated governs read mixing, not write routing. Reverted tier 5.5 to master behavior. The fix now lives in localFederatedSourceIds only: an explicitly isolated resolved source (config.federated === false) is never widened into the federated set, so unqualified reads anchored on it stay scalar — its pages are not mixed with 'default' and vice versa. That helper feeds federatedSearchScope exclusively (reads); writes never touch it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
cfab0c31a3
commit
ba1aedd100
+26
-18
@@ -200,32 +200,27 @@ export function resolveSourceIdEngineFree(
|
||||
* doesn't auto-resolve. Shared by `resolveSourceId` and `resolveSourceWithTier`
|
||||
* so the heuristic can't drift between the two entry points.
|
||||
*
|
||||
* #2928: also refuses to auto-pick a source EXPLICITLY marked isolated
|
||||
* (`gbrain sources unfederate <id>` / `--no-federated` → config.federated =
|
||||
* false). Auto-routing there made an unqualified `query`/`search`/`think`
|
||||
* surface the isolated source's pages — the exact thing unfederate promises
|
||||
* to prevent ("only searched when explicitly named"). UNSET federated stays
|
||||
* eligible: a fresh `sources add` writes no federated key, and the #1434
|
||||
* sole-source convenience must keep working for it. Narrowing-only by
|
||||
* design — an isolated sole source falls through to 'default'; the tier
|
||||
* never widens to some OTHER source it wouldn't have picked before.
|
||||
* NOTE (#2928): this tier deliberately does NOT consult config.federated —
|
||||
* `--no-federated` governs READ mixing, not write routing, and unqualified
|
||||
* `sync`/`import` on a single-vault brain must keep landing in the vault
|
||||
* (#1434, pinned by test/sync-sole-non-default-routing.test.ts). The
|
||||
* unfederate read fix lives in `localFederatedSourceIds` below.
|
||||
*/
|
||||
async function pickSoleNonDefaultSource(engine: BrainEngine): Promise<string | null> {
|
||||
// archived column was added in v34 (v0.26.5). Older brains may not have
|
||||
// it — fall back to the un-archived query in that case via try/catch.
|
||||
let rows: Array<{ id: string; config: unknown }>;
|
||||
let rows: Array<{ id: string }>;
|
||||
try {
|
||||
rows = await engine.executeRaw<{ id: string; config: unknown }>(
|
||||
`SELECT id, config FROM sources WHERE local_path IS NOT NULL AND id != 'default' AND archived = false`,
|
||||
rows = await engine.executeRaw<{ id: string }>(
|
||||
`SELECT id FROM sources WHERE local_path IS NOT NULL AND id != 'default' AND archived = false`,
|
||||
);
|
||||
} catch {
|
||||
rows = await engine.executeRaw<{ id: string; config: unknown }>(
|
||||
`SELECT id, config FROM sources WHERE local_path IS NOT NULL AND id != 'default'`,
|
||||
rows = await engine.executeRaw<{ id: string }>(
|
||||
`SELECT id FROM sources WHERE local_path IS NOT NULL AND id != 'default'`,
|
||||
);
|
||||
}
|
||||
if (rows.length !== 1) return null;
|
||||
if (parseSourceConfig(rows[0].config).federated === false) return null;
|
||||
return rows[0].id;
|
||||
if (rows.length === 1) return rows[0].id;
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -404,7 +399,10 @@ export async function resolveSourceWithTier(
|
||||
*
|
||||
* - explicit tiers (`flag` / `env` / `dotfile`): the user named a source;
|
||||
* scalar scope stands (that IS the qualified case);
|
||||
* - no other federated source exists: keep the scalar fast path unchanged.
|
||||
* - no other federated source exists: keep the scalar fast path unchanged;
|
||||
* - #2928: the resolved source is explicitly isolated (config.federated =
|
||||
* false): it must not be mixed into a cross-source read in EITHER
|
||||
* direction, so the scalar scope stands.
|
||||
*
|
||||
* Archived sources are excluded (same rationale as pickSoleNonDefaultSource);
|
||||
* the archived column is v34+, so fall back to the un-archived query on older
|
||||
@@ -427,6 +425,16 @@ export async function localFederatedSourceIds(
|
||||
`SELECT id, config FROM sources ORDER BY id`,
|
||||
);
|
||||
}
|
||||
// #2928: an EXPLICITLY isolated anchor (`sources unfederate` /
|
||||
// `--no-federated` → config.federated = false) opted out of cross-source
|
||||
// read mixing — never widen it into the federated set (which would drag
|
||||
// other sources' pages into its unqualified reads and vice versa). Scalar
|
||||
// scope stands. UNSET federated keeps the pre-#2928 widening behavior;
|
||||
// write routing (tier 5.5 above) is deliberately untouched.
|
||||
const resolvedRow = rows.find((row) => row.id === sourceId);
|
||||
if (resolvedRow && parseSourceConfig(resolvedRow.config).federated === false) {
|
||||
return undefined;
|
||||
}
|
||||
const ids = [
|
||||
sourceId,
|
||||
...rows
|
||||
|
||||
@@ -2,21 +2,24 @@
|
||||
* #2928 — `gbrain sources unfederate <id>` (config.federated = false) must
|
||||
* keep the isolated source out of UNQUALIFIED reads.
|
||||
*
|
||||
* Root cause is NOT the functions the issue points at (`sourceScopeOpts` /
|
||||
* the dead `federatedOnly` loader flag): the read-widening path
|
||||
* (`localFederatedSourceIds` → `federatedSearchScope`) already excludes
|
||||
* non-federated sources. The intent is lost one step earlier, in source
|
||||
* RESOLUTION: `pickSoleNonDefaultSource` (tier 5.5, #1434) auto-picks the
|
||||
* single non-default source as the anchor of an unqualified call without
|
||||
* consulting config.federated — so an explicitly isolated source becomes the
|
||||
* scalar read scope and its pages surface in unqualified query/search/think.
|
||||
* The leak: tier 5.5 (#1434) anchors an unqualified call on the sole
|
||||
* non-default source — correct for writes and for the anchor itself — but
|
||||
* `localFederatedSourceIds` then widened that anchor into the federated set
|
||||
* (`[isolated-src, default]`), mixing the isolated source's pages with
|
||||
* federated sources' pages in unqualified query/search/think, both
|
||||
* directions. The fix is READ-ONLY: an explicitly isolated anchor
|
||||
* (config.federated === false) is never widened; scalar scope stands.
|
||||
*
|
||||
* Deliberately unchanged (pinned below):
|
||||
* - tier 5.5 write routing — `--no-federated` governs read mixing, not
|
||||
* where unqualified sync/import land (#1434,
|
||||
* test/sync-sole-non-default-routing.test.ts);
|
||||
* - the unscoped-local invariant that sank #3470/#3497: an empty scope
|
||||
* must stay UNSCOPED ({}), never collapse to 'default'.
|
||||
*
|
||||
* All imports here exist on master, so this file runs against an unmodified
|
||||
* master checkout and fails BEHAVIORALLY there (resolution returns the
|
||||
* isolated source).
|
||||
*
|
||||
* Also pins the unscoped-local invariant that sank #3470/#3497: an empty
|
||||
* scope must stay UNSCOPED ({}), never collapse to 'default'.
|
||||
* master checkout and fails BEHAVIORALLY there (the widened scope contains
|
||||
* both sources).
|
||||
*/
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { mkdtempSync } from 'node:fs';
|
||||
@@ -80,29 +83,36 @@ function ctxOf(overrides: Partial<OperationContext> = {}): OperationContext {
|
||||
} as OperationContext;
|
||||
}
|
||||
|
||||
describe('#2928 — unfederated source and the sole_non_default tier', () => {
|
||||
test('explicitly isolated sole source no longer anchors unqualified resolution', async () => {
|
||||
describe('#2928 — isolated anchor is never widened into a cross-source read', () => {
|
||||
test('explicitly isolated resolved source gets NO federated widening set', async () => {
|
||||
await addSource('isolated-src', { federated: false });
|
||||
const resolved = await resolveClean(null, outsideCwd);
|
||||
// Master auto-picks the isolated source (tier sole_non_default), which
|
||||
// makes an unqualified query/search return its pages — the exact thing
|
||||
// `sources unfederate` promises to prevent.
|
||||
expect(resolved.source_id).toBe('default');
|
||||
expect(resolved.tier).toBe('seed_default');
|
||||
// Master returns ['isolated-src', 'default'] here (the seeded default is
|
||||
// federated), which mixes the isolated source's pages with default's in
|
||||
// every unqualified query/search/think — the #2928 report.
|
||||
const localFed = await localFederatedSourceIds(engine, 'isolated-src', 'sole_non_default');
|
||||
expect(localFed).toBeUndefined();
|
||||
});
|
||||
|
||||
test('full unqualified read chain excludes the isolated source from the scope', async () => {
|
||||
test('full unqualified read chain stays scalar: no default pages mixed in', async () => {
|
||||
await addSource('isolated-src', { federated: false });
|
||||
const resolved = await resolveClean(null, outsideCwd);
|
||||
// Write/anchor routing is UNCHANGED: the sole vault still resolves (#1434).
|
||||
expect(resolved.source_id).toBe('isolated-src');
|
||||
expect(resolved.tier).toBe('sole_non_default');
|
||||
const localFed = await localFederatedSourceIds(engine, resolved.source_id, resolved.tier);
|
||||
const ctx = ctxOf({
|
||||
sourceId: resolved.source_id,
|
||||
...(localFed ? { localFederatedSourceIds: localFed } : {}),
|
||||
});
|
||||
const scope = federatedSearchScope(ctx);
|
||||
expect(scope.sourceId).not.toBe('isolated-src');
|
||||
expect(scope.sourceIds ?? []).not.toContain('isolated-src');
|
||||
expect(scope).toEqual({ sourceId: 'default' });
|
||||
// Scalar scope: the isolated source is not mixed with 'default' (and
|
||||
// vice versa). Master produces { sourceIds: ['isolated-src','default'] }.
|
||||
expect(federatedSearchScope(ctx)).toEqual({ sourceId: 'isolated-src' });
|
||||
});
|
||||
|
||||
test('isolated brain_default anchor is not widened either (same seam)', async () => {
|
||||
await addSource('isolated-src', { federated: false });
|
||||
const localFed = await localFederatedSourceIds(engine, 'isolated-src', 'brain_default');
|
||||
expect(localFed).toBeUndefined();
|
||||
});
|
||||
|
||||
test('UNSET federated keeps the #1434 sole-source convenience (no over-narrowing)', async () => {
|
||||
@@ -110,6 +120,10 @@ describe('#2928 — unfederated source and the sole_non_default tier', () => {
|
||||
const resolved = await resolveClean(null, outsideCwd);
|
||||
expect(resolved.source_id).toBe('vault');
|
||||
expect(resolved.tier).toBe('sole_non_default');
|
||||
// Only an EXPLICIT federated:false suppresses widening; unset keeps the
|
||||
// pre-#2928 behavior (the seeded 'default' is federated).
|
||||
const localFed = await localFederatedSourceIds(engine, 'vault', 'sole_non_default');
|
||||
expect(localFed).toEqual(['vault', 'default']);
|
||||
});
|
||||
|
||||
test('federated: true sole source still auto-resolves', async () => {
|
||||
|
||||
Reference in New Issue
Block a user