mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-16 09:52:22 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6548e5cffc | ||
|
|
0ac3d4da9c |
+2
-11
@@ -808,20 +808,12 @@ async function makeContext(engine: BrainEngine, params: Record<string, unknown>)
|
||||
// 'default'. Wrapped in try/catch so a doctor / single-source brain that
|
||||
// never set up sources still returns 'default' silently.
|
||||
let sourceId: string | undefined;
|
||||
// #2561: when the source resolved via a NON-explicit tier (path-match /
|
||||
// brain default / sole-non-default / seed default), unqualified search-shaped
|
||||
// reads span every `config.federated = true` source. Computed here (the
|
||||
// trusted local boundary) and consumed by federatedSearchScope in
|
||||
// operations.ts, which additionally gates on ctx.remote === false.
|
||||
let localFederated: string[] | undefined;
|
||||
try {
|
||||
const { resolveSourceWithTier, localFederatedSourceIds } = await import('./core/source-resolver.ts');
|
||||
const { resolveSourceId } = await import('./core/source-resolver.ts');
|
||||
// params.source is set when a CLI flag was parsed for the op (rare; most
|
||||
// CLI ops don't take --source). Falls through to env/dotfile/path-match.
|
||||
const explicit = (params.source as string | undefined) ?? null;
|
||||
const resolved = await resolveSourceWithTier(engine, explicit);
|
||||
sourceId = resolved.source_id;
|
||||
localFederated = await localFederatedSourceIds(engine, resolved.source_id, resolved.tier);
|
||||
sourceId = await resolveSourceId(engine, explicit);
|
||||
} catch {
|
||||
// Source resolution failed (e.g. sources table doesn't exist on a fresh
|
||||
// pre-init brain). Leave sourceId unset; engine read methods fall through
|
||||
@@ -842,7 +834,6 @@ async function makeContext(engine: BrainEngine, params: Record<string, unknown>)
|
||||
// table). Matches dispatch.ts's auto-fill so the contract holds across
|
||||
// every transport.
|
||||
sourceId: sourceId ?? 'default',
|
||||
...(localFederated ? { localFederatedSourceIds: localFederated } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -151,6 +151,100 @@ export function shouldSpawnAutopilotWorker(args: string[]): boolean {
|
||||
return !args.includes('--no-worker');
|
||||
}
|
||||
|
||||
/**
|
||||
* #1525 — positional subcommand translation.
|
||||
*
|
||||
* Pre-fix, `gbrain autopilot status` silently fell through to "start daemon"
|
||||
* because `runAutopilot()` only branched on flag forms (`--status`, etc.).
|
||||
* `status` was treated as a stray positional and ignored.
|
||||
*
|
||||
* This translator maps known positional subcommands to their flag form so
|
||||
* `autopilot status` is equivalent to `autopilot --status`, then rejects
|
||||
* any unrecognized positional with a fail-loud error before any side
|
||||
* effect (lockfile, daemon spawn, sync dispatch) runs.
|
||||
*
|
||||
* Scope decisions:
|
||||
* - Known aliases: `status` → `--status`, `install` → `--install`,
|
||||
* `uninstall` → `--uninstall`, `start` → (drop; default daemon launch).
|
||||
* - `stop` is intentionally NOT aliased here. Stopping a running daemon
|
||||
* is a new behavior (read PID from lock, SIGTERM, drain) that deserves
|
||||
* its own design and PR. Users typing `gbrain autopilot stop` today get
|
||||
* the unknown-positional error with the canonical alternatives.
|
||||
* - At most one positional allowed; multiple positionals fail loud.
|
||||
*/
|
||||
// Every flag that consumes the NEXT argv token. Missing one here makes the
|
||||
// translator misread the flag's value as a positional subcommand and exit 2
|
||||
// (e.g. `--install --target linux-cron`). Keep in sync with parseArg call sites.
|
||||
const AUTOPILOT_VALUE_FLAGS = new Set(['--repo', '--interval', '--target']);
|
||||
const AUTOPILOT_POSITIONAL_ALIASES: Record<string, string | null> = {
|
||||
status: '--status',
|
||||
install: '--install',
|
||||
uninstall: '--uninstall',
|
||||
start: null, // drop the positional; default behavior is daemon launch
|
||||
};
|
||||
|
||||
export type PositionalTranslation =
|
||||
| { ok: true; args: string[] }
|
||||
| {
|
||||
ok: false;
|
||||
reason: 'unknown_subcommand' | 'multiple_subcommands';
|
||||
message: string;
|
||||
};
|
||||
|
||||
export function translatePositionalSubcommands(args: string[]): PositionalTranslation {
|
||||
const out: string[] = [];
|
||||
let positionalSeen = false;
|
||||
let i = 0;
|
||||
while (i < args.length) {
|
||||
const a = args[i];
|
||||
if (AUTOPILOT_VALUE_FLAGS.has(a)) {
|
||||
// Pass through the flag and its value untouched. If the value is
|
||||
// missing at end-of-argv, fall through so the existing parseArg
|
||||
// path can report the broken usage.
|
||||
out.push(a);
|
||||
if (i + 1 < args.length) {
|
||||
out.push(args[i + 1]);
|
||||
i += 2;
|
||||
} else {
|
||||
i += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (a.startsWith('-')) {
|
||||
out.push(a);
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
// Positional subcommand.
|
||||
if (positionalSeen) {
|
||||
const known = Object.keys(AUTOPILOT_POSITIONAL_ALIASES).join(', ');
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'multiple_subcommands',
|
||||
message: `Multiple subcommands given. Use only one of: ${known}.`,
|
||||
};
|
||||
}
|
||||
positionalSeen = true;
|
||||
if (a in AUTOPILOT_POSITIONAL_ALIASES) {
|
||||
const alias = AUTOPILOT_POSITIONAL_ALIASES[a];
|
||||
if (alias) out.push(alias);
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
const known = Object.keys(AUTOPILOT_POSITIONAL_ALIASES).join(', ');
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'unknown_subcommand',
|
||||
message:
|
||||
`Unknown subcommand: \`${a}\`.\n` +
|
||||
`Allowed subcommands: ${known}.\n` +
|
||||
`Or use the flag form: --status, --install, --uninstall.\n` +
|
||||
`Run \`gbrain autopilot --help\` for full usage.`,
|
||||
};
|
||||
}
|
||||
return { ok: true, args: out };
|
||||
}
|
||||
|
||||
export function isPidAlive(pid: number): boolean {
|
||||
if (!Number.isFinite(pid) || pid <= 0) return false;
|
||||
try {
|
||||
@@ -363,6 +457,11 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
' gbrain autopilot --install [--repo <path>]\n' +
|
||||
' gbrain autopilot --uninstall\n' +
|
||||
' gbrain autopilot --status [--json]\n\n' +
|
||||
'Subcommand aliases:\n' +
|
||||
' gbrain autopilot status → --status\n' +
|
||||
' gbrain autopilot install → --install\n' +
|
||||
' gbrain autopilot uninstall → --uninstall\n' +
|
||||
' gbrain autopilot start → (default daemon launch)\n\n' +
|
||||
'Self-maintaining brain daemon. Runs the full maintenance cycle\n' +
|
||||
'(lint + backlinks + sync + extract + embed + orphans) on an interval.\n\n' +
|
||||
'For a one-shot cron-triggered cycle, see `gbrain dream`.',
|
||||
@@ -370,6 +469,16 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
return;
|
||||
}
|
||||
|
||||
// #1525: translate positional subcommands to their flag form BEFORE any
|
||||
// side effect (lockfile, daemon spawn, sync dispatch). Unknown positionals
|
||||
// fail loud here rather than silently starting the daemon.
|
||||
const translated = translatePositionalSubcommands(args);
|
||||
if (!translated.ok) {
|
||||
console.error(translated.message);
|
||||
process.exit(2);
|
||||
}
|
||||
args = translated.args;
|
||||
|
||||
if (args.includes('--install')) {
|
||||
await installDaemon(engine, args);
|
||||
return;
|
||||
|
||||
+2
-61
@@ -424,23 +424,6 @@ export interface OperationContext {
|
||||
* satisfied even on single-source brains.
|
||||
*/
|
||||
sourceId: string;
|
||||
/**
|
||||
* #2561 — federated read scope for UNQUALIFIED local CLI reads.
|
||||
*
|
||||
* Set ONLY by the local CLI's context builder (src/cli.ts makeContext), and
|
||||
* only when the source resolved via a non-explicit tier (local_path /
|
||||
* brain_default / sole_non_default / seed_default — NOT --source, NOT
|
||||
* GBRAIN_SOURCE, NOT a .gbrain-source dotfile). Contains the resolved
|
||||
* source first, then every other `config.federated = true` source, so an
|
||||
* unqualified `gbrain search "X"` spans federated sources as
|
||||
* docs/guides/multi-source-brains.md promises.
|
||||
*
|
||||
* Consumed exclusively by `federatedSearchScope` and ONLY when
|
||||
* `ctx.remote === false` — a remote caller's scope stays governed by
|
||||
* `ctx.auth.allowedSources` / scalar `ctx.sourceId` (source-isolation
|
||||
* invariant, fail-closed).
|
||||
*/
|
||||
localFederatedSourceIds?: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -556,45 +539,6 @@ export function resolveRequestedScope(
|
||||
return sourceScopeOpts(ctx);
|
||||
}
|
||||
|
||||
/**
|
||||
* #2561 — source scope for the search-shaped read ops (`search`, `query`).
|
||||
*
|
||||
* Delegates to `resolveRequestedScope` (the single trust+grant resolver), then
|
||||
* widens an UNQUALIFIED trusted-local scalar scope to the CLI-computed
|
||||
* federated set (`ctx.localFederatedSourceIds`, resolved source first). This is
|
||||
* what makes `sources add --federated` mean something for local search: a
|
||||
* federated source participates in unqualified `gbrain search "X"` results.
|
||||
*
|
||||
* The expansion NEVER applies when:
|
||||
* - the caller is not strictly trusted-local (`ctx.remote !== false`) —
|
||||
* remote scope stays grant-governed (fail-closed source isolation);
|
||||
* - a per-call `source_id` was passed (explicit wins, including `__all__`);
|
||||
* - the resolver already produced a federated array (OAuth grant);
|
||||
* - the CLI resolved the source from an explicit signal (--source / env /
|
||||
* dotfile) — makeContext leaves `localFederatedSourceIds` unset then.
|
||||
*
|
||||
* Deliberately NOT inside `sourceScopeOpts`: code-intel ops collapse a
|
||||
* multi-element scope to an error (`resolveCodeIntelScope`), and non-search
|
||||
* reads (get_page, get_links, …) keep their long-standing scalar behavior.
|
||||
*/
|
||||
export function federatedSearchScope(
|
||||
ctx: OperationContext,
|
||||
sourceIdParam?: string,
|
||||
): { sourceId?: string; sourceIds?: string[] } {
|
||||
const scope = resolveRequestedScope(ctx, sourceIdParam);
|
||||
if (
|
||||
ctx.remote === false &&
|
||||
sourceIdParam === undefined &&
|
||||
scope.sourceId !== undefined &&
|
||||
scope.sourceIds === undefined &&
|
||||
ctx.localFederatedSourceIds !== undefined &&
|
||||
ctx.localFederatedSourceIds.length > 1
|
||||
) {
|
||||
return { sourceIds: ctx.localFederatedSourceIds };
|
||||
}
|
||||
return scope;
|
||||
}
|
||||
|
||||
/**
|
||||
* Code-intel adapter for `resolveRequestedScope`. Graph traversal
|
||||
* (code_callers/code_callees/code_blast/code_flow) is single-source by design —
|
||||
@@ -1504,8 +1448,7 @@ const search: Operation = {
|
||||
const queryText = p.query as string;
|
||||
const limit = (p.limit as number) || 20;
|
||||
const offset = (p.offset as number) || 0;
|
||||
// #2561: unqualified trusted-local search spans federated sources.
|
||||
const scope = federatedSearchScope(ctx);
|
||||
const scope = sourceScopeOpts(ctx);
|
||||
|
||||
// T4/D5 — per-call mode honored ONLY for trusted/local callers so a remote
|
||||
// OAuth client can't escalate to the costly tokenmax bundle. Local + unknown
|
||||
@@ -1667,9 +1610,7 @@ const query: Operation = {
|
||||
// is spread into BOTH the image-similarity searchVector path and the text
|
||||
// hybridSearch path below, so both honor the same grant.
|
||||
const sourceIdParam = typeof p.source_id === 'string' ? p.source_id : undefined;
|
||||
// #2561: unqualified trusted-local query spans federated sources (per-call
|
||||
// source_id / remote grants still resolve through resolveRequestedScope).
|
||||
const querySourceScope = federatedSearchScope(ctx, sourceIdParam);
|
||||
const querySourceScope = resolveRequestedScope(ctx, sourceIdParam);
|
||||
|
||||
// v0.27.1: image-similarity branch. Bypasses hybridSearch (which is
|
||||
// text-only); embeds the image via embedMultimodal and runs a direct
|
||||
|
||||
@@ -353,45 +353,6 @@ export async function resolveSourceWithTier(
|
||||
return { source_id: 'default', tier: 'seed_default' };
|
||||
}
|
||||
|
||||
/**
|
||||
* #2561 — compute the federated read scope for an UNQUALIFIED local CLI call.
|
||||
*
|
||||
* `sources add --federated` promises that a `config.federated = true` source
|
||||
* "participates in unqualified `gbrain search` results"
|
||||
* (docs/guides/multi-source-brains.md). This helper turns that promise into a
|
||||
* scope: given the resolved source and WHICH tier resolved it, return
|
||||
* `[resolvedSource, ...other federated source ids]` — or `undefined` when the
|
||||
* expansion must not apply:
|
||||
*
|
||||
* - 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.
|
||||
*
|
||||
* Archived sources are excluded (same rationale as pickSoleNonDefaultSource);
|
||||
* the archived column is v34+, so fall back to the un-archived query on older
|
||||
* brains. Callers put the result on `OperationContext.localFederatedSourceIds`
|
||||
* — consumed only by `federatedSearchScope` and only when `remote === false`.
|
||||
*/
|
||||
export async function localFederatedSourceIds(
|
||||
engine: BrainEngine,
|
||||
sourceId: string,
|
||||
tier: SourceTier,
|
||||
): Promise<string[] | undefined> {
|
||||
if (tier === 'flag' || tier === 'env' || tier === 'dotfile') return undefined;
|
||||
let rows: Array<{ id: string }>;
|
||||
try {
|
||||
rows = await engine.executeRaw<{ id: string }>(
|
||||
`SELECT id FROM sources WHERE config->>'federated' = 'true' AND archived = false ORDER BY id`,
|
||||
);
|
||||
} catch {
|
||||
rows = await engine.executeRaw<{ id: string }>(
|
||||
`SELECT id FROM sources WHERE config->>'federated' = 'true' ORDER BY id`,
|
||||
);
|
||||
}
|
||||
const ids = [sourceId, ...rows.map((r) => r.id).filter((id) => id !== sourceId)];
|
||||
return ids.length > 1 ? ids : undefined;
|
||||
}
|
||||
|
||||
/** Exposed for tests. */
|
||||
export const __testing = {
|
||||
readDotfileWalk,
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
/**
|
||||
* Tests for translatePositionalSubcommands() — the v0.41.x #1525 fix that
|
||||
* prevents `gbrain autopilot status` from silently starting the daemon.
|
||||
*
|
||||
* IRON RULE regression guard: the exact ticket repro (`gbrain autopilot
|
||||
* status`) MUST translate to `--status`, not fall through to the default
|
||||
* daemon launch. Verified by the "ticket-exact repro" case below.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { translatePositionalSubcommands } from '../src/commands/autopilot.ts';
|
||||
|
||||
describe('translatePositionalSubcommands — known aliases', () => {
|
||||
test('IRON RULE — `autopilot status` translates to `--status` (ticket #1525 repro)', () => {
|
||||
const r = translatePositionalSubcommands(['status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--status']);
|
||||
});
|
||||
|
||||
test('`install` translates to `--install`', () => {
|
||||
const r = translatePositionalSubcommands(['install']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--install']);
|
||||
});
|
||||
|
||||
test('`uninstall` translates to `--uninstall`', () => {
|
||||
const r = translatePositionalSubcommands(['uninstall']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--uninstall']);
|
||||
});
|
||||
|
||||
test('`start` drops the positional (default daemon launch)', () => {
|
||||
const r = translatePositionalSubcommands(['start']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual([]);
|
||||
});
|
||||
|
||||
test('`start --json` drops only the positional, keeps the flag', () => {
|
||||
const r = translatePositionalSubcommands(['start', '--json']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--json']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('translatePositionalSubcommands — flag/positional interleaving', () => {
|
||||
test('`status --json` preserves the trailing flag', () => {
|
||||
const r = translatePositionalSubcommands(['status', '--json']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--status', '--json']);
|
||||
});
|
||||
|
||||
test('`--json status` preserves the leading flag', () => {
|
||||
const r = translatePositionalSubcommands(['--json', 'status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--json', '--status']);
|
||||
});
|
||||
|
||||
test('`--repo /foo status` does not mis-classify the path as positional', () => {
|
||||
const r = translatePositionalSubcommands(['--repo', '/foo', 'status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--repo', '/foo', '--status']);
|
||||
});
|
||||
|
||||
test('`--interval 300 install` does not mis-classify the number as positional', () => {
|
||||
const r = translatePositionalSubcommands(['--interval', '300', 'install']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--interval', '300', '--install']);
|
||||
});
|
||||
|
||||
test('`--install --target linux-cron` does not mis-classify the target as positional', () => {
|
||||
// --target is installDaemon's value flag; its value must never be read
|
||||
// as a positional subcommand (regression guard for the review fix).
|
||||
const r = translatePositionalSubcommands(['--install', '--target', 'linux-cron']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--install', '--target', 'linux-cron']);
|
||||
});
|
||||
|
||||
test('`install --target macos` keeps the alias translation and the target value', () => {
|
||||
const r = translatePositionalSubcommands(['install', '--target', 'macos']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--install', '--target', 'macos']);
|
||||
});
|
||||
|
||||
test('value-flag at end of argv with missing value passes through (so parseArg can report it)', () => {
|
||||
const r = translatePositionalSubcommands(['--repo']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--repo']);
|
||||
});
|
||||
|
||||
test('value-flag whose value looks like an alias is NOT translated', () => {
|
||||
// `--repo status` means "use repo path 'status'", not "show status".
|
||||
// Translator must not destructure the value of --repo.
|
||||
const r = translatePositionalSubcommands(['--repo', 'status']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--repo', 'status']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('translatePositionalSubcommands — pass-through cases', () => {
|
||||
test('empty args returns empty args', () => {
|
||||
const r = translatePositionalSubcommands([]);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual([]);
|
||||
});
|
||||
|
||||
test('flag-only invocation passes through unchanged', () => {
|
||||
const r = translatePositionalSubcommands(['--status', '--json']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['--status', '--json']);
|
||||
});
|
||||
|
||||
test('short flag `-h` passes through unchanged', () => {
|
||||
const r = translatePositionalSubcommands(['-h']);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(['-h']);
|
||||
});
|
||||
|
||||
test('all known bare flags pass through unchanged', () => {
|
||||
const flags = ['--help', '--install', '--uninstall', '--status', '--json', '--inline', '--no-worker'];
|
||||
const r = translatePositionalSubcommands(flags);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.args).toEqual(flags);
|
||||
});
|
||||
});
|
||||
|
||||
describe('translatePositionalSubcommands — rejection of unknown positionals', () => {
|
||||
test('unknown positional `foo` fails with reason=unknown_subcommand + structured message', () => {
|
||||
const r = translatePositionalSubcommands(['foo']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('Unknown subcommand: `foo`');
|
||||
expect(r.message).toContain('status');
|
||||
expect(r.message).toContain('install');
|
||||
expect(r.message).toContain('uninstall');
|
||||
expect(r.message).toContain('--help');
|
||||
}
|
||||
});
|
||||
|
||||
test('unknown positional `stop` fails with reason=unknown_subcommand (NOT silently aliased)', () => {
|
||||
// Stop is mentioned in the ticket but deliberately NOT aliased in this
|
||||
// PR — stopping a running daemon is a new behavior, not just an alias.
|
||||
// Until that feature lands separately, `stop` must fail loud rather
|
||||
// than starting the daemon (the bug we're fixing).
|
||||
const r = translatePositionalSubcommands(['stop']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('Unknown subcommand: `stop`');
|
||||
}
|
||||
});
|
||||
|
||||
test('unknown positional `status-detail` (close-but-not-matching) fails', () => {
|
||||
const r = translatePositionalSubcommands(['status-detail']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('Unknown subcommand: `status-detail`');
|
||||
}
|
||||
});
|
||||
|
||||
test('multiple positionals fail with reason=multiple_subcommands (`start install`)', () => {
|
||||
const r = translatePositionalSubcommands(['start', 'install']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('multiple_subcommands');
|
||||
expect(r.message).toContain('Multiple subcommands');
|
||||
}
|
||||
});
|
||||
|
||||
test('multiple positionals fail even when both are known aliases (`status install`)', () => {
|
||||
const r = translatePositionalSubcommands(['status', 'install']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('multiple_subcommands');
|
||||
expect(r.message).toContain('Multiple subcommands');
|
||||
}
|
||||
});
|
||||
|
||||
test('known-then-unknown rejects with multiple_subcommands (first-positional-wins)', () => {
|
||||
// First positional is known, second is not. Rejection comes from the
|
||||
// multiple-positional rule, which fires before the unknown check; the
|
||||
// intent is "only one subcommand allowed."
|
||||
const r = translatePositionalSubcommands(['status', 'garbage']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) expect(r.reason).toBe('multiple_subcommands');
|
||||
});
|
||||
|
||||
test('unknown-then-known rejects on the unknown (unknown fires before second-positional check)', () => {
|
||||
const r = translatePositionalSubcommands(['garbage', 'status']);
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(r.reason).toBe('unknown_subcommand');
|
||||
expect(r.message).toContain('garbage');
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,154 +0,0 @@
|
||||
/**
|
||||
* #2561 — sources.config.federated participates in UNQUALIFIED local CLI
|
||||
* search/query.
|
||||
*
|
||||
* Pre-fix: the local CLI always emitted a scalar `{sourceId}` scope (required
|
||||
* field, auto-filled 'default'), so a source registered with
|
||||
* `gbrain sources add --federated` was invisible to an unqualified
|
||||
* `gbrain search "X"` — contradicting docs/guides/multi-source-brains.md
|
||||
* ("Source participates in unqualified `gbrain search` results").
|
||||
*
|
||||
* Fix: the CLI context builder computes `ctx.localFederatedSourceIds`
|
||||
* (resolved source + every other federated source) whenever the source
|
||||
* resolved via a NON-explicit tier; `federatedSearchScope` widens the scalar
|
||||
* scope to that set for the `search` / `query` ops — trusted-local only
|
||||
* (`ctx.remote === false`), never for remote callers, never when a per-call
|
||||
* `source_id` or an explicit --source/env/dotfile was given.
|
||||
*/
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { localFederatedSourceIds } from '../src/core/source-resolver.ts';
|
||||
import {
|
||||
federatedSearchScope,
|
||||
operations,
|
||||
type OperationContext,
|
||||
} from '../src/core/operations.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
const search = operations.find((o) => o.name === 'search')!;
|
||||
|
||||
function ctxOf(overrides: Partial<OperationContext> = {}): OperationContext {
|
||||
return {
|
||||
engine: engine as any,
|
||||
config: {} as any,
|
||||
logger: console as any,
|
||||
dryRun: false,
|
||||
remote: false,
|
||||
sourceId: 'default',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
// Seeded 'default' source is federated=true. Add:
|
||||
// wiki — federated (must join unqualified search)
|
||||
// private — NOT federated (must stay invisible unless explicitly named)
|
||||
// oldnews — federated but archived (must stay excluded)
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config) VALUES ('wiki', 'wiki', '/tmp/wiki', '{"federated": true}'::jsonb)`,
|
||||
);
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config) VALUES ('private', 'private', '/tmp/private', '{}'::jsonb)`,
|
||||
);
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config, archived) VALUES ('oldnews', 'oldnews', '/tmp/oldnews', '{"federated": true}'::jsonb, true)`,
|
||||
);
|
||||
const pages: Array<[slug: string, sourceId: string, where: string]> = [
|
||||
['notes/home', 'default', 'default'],
|
||||
['wiki/topic', 'wiki', 'wiki'],
|
||||
['private/topic', 'private', 'private'],
|
||||
['old/topic', 'oldnews', 'oldnews'],
|
||||
];
|
||||
for (const [slug, sourceId, where] of pages) {
|
||||
await engine.putPage(slug, {
|
||||
type: 'note', title: `Topic in ${where}`, compiled_truth: `the zebra telescope in ${where}`, frontmatter: {},
|
||||
}, { sourceId });
|
||||
await engine.upsertChunks(slug, [
|
||||
{ chunk_index: 0, chunk_text: `the zebra telescope in ${where}`, chunk_source: 'compiled_truth' },
|
||||
], { sourceId });
|
||||
}
|
||||
// Keyword-only search path: no embedding provider needed in tests.
|
||||
await engine.setConfig('search.mcp_keyword_only', 'true');
|
||||
}, 60_000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (engine) await engine.disconnect();
|
||||
}, 60_000);
|
||||
|
||||
describe('localFederatedSourceIds — CLI-side scope computation', () => {
|
||||
test('non-explicit tier: resolved source first, then other federated, archived excluded', async () => {
|
||||
expect(await localFederatedSourceIds(engine, 'default', 'seed_default')).toEqual(['default', 'wiki']);
|
||||
});
|
||||
|
||||
test('non-federated resolved source still joins its own scope', async () => {
|
||||
expect(await localFederatedSourceIds(engine, 'private', 'brain_default')).toEqual(['private', 'default', 'wiki']);
|
||||
});
|
||||
|
||||
test('explicit tiers (--source / env / dotfile) never expand', async () => {
|
||||
expect(await localFederatedSourceIds(engine, 'default', 'flag')).toBeUndefined();
|
||||
expect(await localFederatedSourceIds(engine, 'default', 'env')).toBeUndefined();
|
||||
expect(await localFederatedSourceIds(engine, 'default', 'dotfile')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('single federated source (the resolved one) keeps the scalar fast path', async () => {
|
||||
const solo = { executeRaw: async () => [{ id: 'default' }] } as any;
|
||||
expect(await localFederatedSourceIds(solo, 'default', 'seed_default')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('federatedSearchScope — trust + explicitness matrix', () => {
|
||||
test('trusted local + unqualified widens to the federated set', () => {
|
||||
const ctx = ctxOf({ localFederatedSourceIds: ['default', 'wiki'] });
|
||||
expect(federatedSearchScope(ctx)).toEqual({ sourceIds: ['default', 'wiki'] });
|
||||
});
|
||||
|
||||
test('remote caller NEVER widens (fail-closed), even if the field is set', () => {
|
||||
const ctx = ctxOf({ remote: true, localFederatedSourceIds: ['default', 'wiki'] });
|
||||
expect(federatedSearchScope(ctx)).toEqual({ sourceId: 'default' });
|
||||
});
|
||||
|
||||
test('per-call source_id wins over the federated set', () => {
|
||||
const ctx = ctxOf({ localFederatedSourceIds: ['default', 'wiki'] });
|
||||
expect(federatedSearchScope(ctx, 'wiki')).toEqual({ sourceId: 'wiki' });
|
||||
});
|
||||
|
||||
test('per-call __all__ keeps the whole-brain semantics for trusted local', () => {
|
||||
const ctx = ctxOf({ localFederatedSourceIds: ['default', 'wiki'] });
|
||||
expect(federatedSearchScope(ctx, '__all__')).toEqual({});
|
||||
});
|
||||
|
||||
test('a federated OAuth grant wins over the local set', () => {
|
||||
const ctx = ctxOf({
|
||||
localFederatedSourceIds: ['default', 'wiki'],
|
||||
auth: { allowedSources: ['a', 'b'] } as OperationContext['auth'],
|
||||
});
|
||||
expect(federatedSearchScope(ctx)).toEqual({ sourceIds: ['a', 'b'] });
|
||||
});
|
||||
|
||||
test('no local federated set → unchanged scalar scope', () => {
|
||||
expect(federatedSearchScope(ctxOf())).toEqual({ sourceId: 'default' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('search op — unqualified local search spans federated sources', () => {
|
||||
test('federated source results appear; non-federated + archived stay invisible', async () => {
|
||||
const ctx = ctxOf({
|
||||
localFederatedSourceIds: await localFederatedSourceIds(engine, 'default', 'seed_default'),
|
||||
});
|
||||
const results = (await search.handler(ctx, { query: 'zebra telescope' })) as Array<{ slug: string }>;
|
||||
const slugs = results.map((r) => r.slug);
|
||||
expect(slugs).toContain('notes/home');
|
||||
expect(slugs).toContain('wiki/topic'); // pre-#2561 this was missing
|
||||
expect(slugs).not.toContain('private/topic');
|
||||
expect(slugs).not.toContain('old/topic');
|
||||
});
|
||||
|
||||
test('explicit source resolution (no federated set on ctx) stays single-source', async () => {
|
||||
const results = (await search.handler(ctxOf(), { query: 'zebra telescope' })) as Array<{ slug: string }>;
|
||||
const slugs = results.map((r) => r.slug);
|
||||
expect(slugs).toEqual(['notes/home']);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user