fix(config): register sources.default as a known key + validate it at set time (#3941)

* fix(config): register sources.default as a known key + validate it at set time

`gbrain config set sources.default <id>` printed

    WARN: writing unknown key "sources.default" with --force.
    Nothing in gbrain reads this.

That last sentence is false. `source-resolver.ts` tier 5 reads
`getConfig('sources.default')` on every unqualified call, and
`gbrain sources default <id>` writes exactly this key. The key was
simply missing from KNOWN_CONFIG_KEYS.

The message matters more than the missing entry: pinning
`sources.default` is the documented way to stop tier 5.5
(sole-non-default-source auto-routing) from silently moving where
unqualified writes land. An operator who hits that and reaches for
this knob is told the knob does nothing.

- Register the exact key, not a `sources.` prefix. A prefix would
  bless arbitrary unread `sources.*` keys and weaken the unknown-key
  guard this list exists to provide.
- Validate at set time, mirroring `runDefault()`: reject a malformed
  id, and reject one that is not registered. Without this, registering
  the key would turn `config set` into a way around the check
  `sources default` already performs — and tier 5 calls
  assertSourceExists, so a typo would surface later as a throw on
  unrelated commands rather than at the point of the mistake.

Tests: membership, the absence of a blanket `sources.` prefix, and the
three set-time paths (malformed / unregistered / accepted).

* fix(config): keep flag literals out of the new error text + refresh flag registry

CI caught this: the #2185 freshness guard failed because the generated
flag registry no longer matched a fresh generator run.

Two separate causes, only one of them mine:

1. Mine. The "source is not registered" message named `--path` while
   suggesting `gbrain sources add`. The generator scans command sources
   for flag tokens, so a flag named in prose silently gets granted to
   `gbrain config` — exactly the prose-bleed class #3902 closed for
   safety flags. Reworded to carry no flag literals. (The first attempt
   at a warning comment reproduced the bug by quoting a placeholder
   flag in the comment itself; that is gone too.)

2. Pre-existing. Regenerating also adds `--federated-read`, which comes
   from `src/commands/auth.ts` and appears in no line this branch
   touches. The committed registry on master is stale with respect to
   its own sources, so the guard fails for any branch cut from master
   until it is refreshed. Committed here because the guard's contract is
   "committed registry == fresh generator run"; flagged in the PR body
   so it is not mistaken for part of this change.

`bun test test/cli-flag-validation.test.ts` — 24 pass / 0 fail.

* fix(config): propagate lookup failures, describe the real id grammar

Round-2 review findings:

- Dropped `.catch(() => null)` around `fetchSource`. It turned a
  connection failure / permission error / SQL regression into
  "source is not registered", sending the operator after a
  registration problem that doesn't exist while the real fault was
  swallowed. `fetchSource` already absorbs the one expected
  legacy-column case; everything else should surface as itself.
  Test added: `executeRaw` throws → the error propagates and nothing
  is written.
- The malformed-id message quoted `[a-z0-9-]{1,32}`, which is looser
  than `SOURCE_ID_RE` (`-wiki` reads as legal under the printed rule
  but is rejected). Now states the actual grammar.
- Tightened the accept-path assertion from `toContainEqual` to an
  exact `setCalls` equality.

Also corrects the previous commit's explanation of the flag-registry
drift: `--federated-read` is NOT pre-existing master drift. It enters
`config`'s flag set because this change adds
`await import('../core/source-id.ts')`, and that module's comment
mentions the flag — the generator follows imports, so prose-bleed
crosses module boundaries. Master is not stale; this branch pulled it
in. The regenerated entry is deterministic and harmless (`config` is
flag-validation-exempt), and stays in this PR because the guard's
contract is "committed registry == fresh generator run".

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Masa
2026-08-10 21:16:19 +07:00
committed by GitHub
co-authored by Claude Opus 5
parent 69472c24e7
commit e2ff128e35
4 changed files with 123 additions and 1 deletions
+36
View File
@@ -204,6 +204,42 @@ export async function runConfig(engine: BrainEngine, args: string[]) {
const coverageOverride =
args.includes('--coverage-override') || args.includes('--yes');
// Validate sources.default at set time. This key is read by
// source-resolver.ts tier 5 on EVERY unqualified call, and tier 5 calls
// assertSourceExists — so a syntactically valid but non-existent id set
// here would make every later unqualified command throw, far from the
// typo that caused it. `gbrain sources default <id>` already validates;
// config set is the lower-level door to the same key and must not be a
// way around that check.
if (key === 'sources.default') {
const { isValidSourceId } = await import('../core/source-id.ts');
if (!isValidSourceId(value)) {
console.error(
`[config] sources.default must be 1-32 lowercase alphanumerics with ` +
`optional interior hyphens (got '${value}').\n` +
`[config] gbrain sources default <id> # preferred — validates and reports`,
);
process.exit(1);
}
// No .catch() here: a connection failure or SQL regression must NOT be
// reported as "source is not registered". fetchSource already absorbs
// the one expected legacy-column case; anything else is a real error and
// should surface as itself.
const { fetchSource } = await import('../core/sources-load.ts');
const src = await fetchSource(engine, value);
if (!src) {
// NOTE: keep flag literals out of this message. The generated flag
// registry (#2185) scans command sources for flag tokens, so naming a
// flag in prose would silently grant it to `gbrain config`.
console.error(
`[config] source "${value}" is not registered; refusing to set sources.default.\n` +
`[config] gbrain sources list # see registered sources\n` +
`[config] gbrain sources add # register one first`,
);
process.exit(1);
}
}
// v0.42.42.0 (#2139): validate spend.posture at set time so a typo
// ('tokenMax', 'max') doesn't silently fall back to gated.
if (key === 'spend.posture') {
+1 -1
View File
@@ -30,7 +30,7 @@ export const CLI_FLAG_REGISTRY: Record<string, readonly string[]> = {
'code-callers': ['--aliases', '--all', '--all-sources', '--brain', '--chunker-debug', '--clone-dir', '--confirm-destructive', '--federated', '--force', '--help', '--include-null-signature', '--json', '--limit', '--no-extract', '--no-federated', '--no-json', '--path', '--pattern', '--pending', '--repo', '--reset', '--resolve', '--restore-only', '--source', '--stale', '--supersessions', '--thin', '--url', '--url-managed', '--yes'],
'code-def': ['--aliases', '--all', '--brain', '--chunker-debug', '--help', '--include-null-signature', '--json', '--lang', '--limit', '--no-extract', '--no-json', '--pattern', '--pending', '--pretty', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--thin', '--yes'],
'code-refs': ['--aliases', '--all', '--brain', '--chunker-debug', '--help', '--include-null-signature', '--json', '--lang', '--limit', '--no-extract', '--no-json', '--pattern', '--pending', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--thin', '--yes'],
'config': ['--aliases', '--all', '--brain', '--column', '--coverage-override', '--embedding-dimensions', '--embedding-model', '--fast', '--follow', '--force', '--from-pages', '--help', '--http', '--include-null-signature', '--json', '--markdown', '--model', '--multimodal', '--no-embedding', '--no-extract', '--pattern', '--pending', '--pglite', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--surface', '--thin', '--yes'],
'config': ['--aliases', '--all', '--brain', '--column', '--coverage-override', '--embedding-dimensions', '--embedding-model', '--fast', '--federated-read', '--follow', '--force', '--from-pages', '--help', '--http', '--include-null-signature', '--json', '--markdown', '--model', '--multimodal', '--no-embedding', '--no-extract', '--pattern', '--pending', '--pglite', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--surface', '--thin', '--yes'],
'connect': ['--agent', '--bearer-token-env-var', '--bind', '--brain', '--client-id', '--client-secret', '--force', '--grant-types', '--help', '--http', '--install', '--json', '--name', '--oauth', '--public-url', '--register', '--scopes', '--show-token', '--source', '--timeout-ms', '--token', '--token-endpoint-auth-method', '--url', '--version', '--yes'],
'conversation-parser': ['--aliases', '--all', '--brain', '--help', '--include-null-signature', '--json', '--no-extract', '--pattern', '--pending', '--reset', '--resolve', '--source', '--stale', '--supersessions', '--thin'],
'doctor': ['--ab', '--abi', '--aliases', '--all', '--allow-shell-jobs', '--auto', '--auto-fix', '--auto-update', '--background', '--batch', '--brain', '--brain-wide-max-cost-usd', '--break-lock', '--build-index', '--by-mention', '--by-type', '--check', '--column', '--compile', '--concurrency', '--confidence', '--content-audit', '--count', '--days', '--detach', '--dim', '--dir', '--drain', '--dry-run', '--embedding-dimensions', '--embedding-model', '--exclusive', '--explain', '--fast', '--fix', '--follow', '--force', '--force-break-lock', '--force-retry', '--force-schema', '--format', '--fresh', '--from-meetings', '--from-pages', '--full', '--grant-types', '--health-interval', '--help', '--history', '--http', '--include-flagged', '--include-frontmatter', '--include-null-signature', '--include-pseudo', '--index-audit', '--input', '--json', '--lang', '--limit', '--locks', '--markdown', '--max-age', '--max-cost', '--max-cost-usd', '--max-crashes', '--max-jobs', '--max-rss', '--max-usd', '--mcp-only', '--migrate-only', '--model', '--multimodal', '--name-only', '--name-status', '--near-symbol', '--nice', '--no', '--no-embed', '--no-embedding', '--no-extract', '--no-mutate', '--oauth-client-secret', '--older-than', '--once', '--overwrite', '--parallel', '--params', '--path', '--pattern', '--pending', '--pglite', '--phase', '--pid-file', '--porcelain', '--priority', '--progress-interval', '--progress-json', '--query', '--queue', '--quiet', '--rebuild-rollup', '--refresh', '--refresh-unqualified', '--regenerate', '--remediate', '--remediation-plan', '--repo', '--reset', '--resolve', '--restore-only', '--resume', '--review-lower', '--rollback', '--scope', '--scopes', '--since', '--skills-dir', '--skip-bare-tweet', '--skip-failed', '--skip-urls', '--skip-verify', '--slugs', '--source', '--source-id', '--stale', '--status', '--strategy', '--strict', '--supabase', '--supersessions', '--surface', '--symbol-kind', '--target', '--target-score', '--thin', '--timeout', '--to', '--top-k', '--type', '--undo-wave', '--unsafe-bypass-dream-guard', '--untracked-files', '--url', '--use-captured-snapshot', '--verbose', '--version', '--window', '--with-calibration', '--workers', '--yes'],
+6
View File
@@ -1110,6 +1110,12 @@ export const KNOWN_CONFIG_KEYS: readonly string[] = [
'embed.backfill_cooldown_min',
'embed.backfill_max_usd_per_source_24h',
'embed.backfill_max_usd',
// Brain-level default source. Read by source-resolver.ts tier 5
// (`engine.getConfig('sources.default')`) and written by
// `gbrain sources default <id>`. Listed here so `gbrain config set`
// stops claiming "Nothing in gbrain reads this" for a key the resolver
// reads on every unqualified call.
'sources.default',
];
/**
+80
View File
@@ -33,6 +33,23 @@ describe('KNOWN_CONFIG_KEYS', () => {
expect(KNOWN_CONFIG_KEYS).toContain('search.cache.enabled');
});
// Regression: `sources.default` is read by source-resolver.ts tier 5 on
// every unqualified call and written by `gbrain sources default <id>`, yet
// it was absent from this list — so `gbrain config set sources.default`
// warned "Nothing in gbrain reads this", which is false and misdirects an
// operator away from the one knob that pins brain-level source routing.
test('contains sources.default (read by the resolver, written by `sources default`)', () => {
expect(KNOWN_CONFIG_KEYS).toContain('sources.default');
});
// The fix registers the ONE key the resolver reads, not a `sources.` prefix:
// a prefix would bless arbitrary unread `sources.*` keys and weaken the
// unknown-key guard this list exists to provide.
test('does not bless arbitrary sources.* keys', () => {
expect(KNOWN_CONFIG_KEYS).not.toContain('sources.anything-else');
expect(KNOWN_CONFIG_KEY_PREFIXES).not.toContain('sources.');
});
test('contains the models-tier keys (v0.31.12)', () => {
expect(KNOWN_CONFIG_KEYS).toContain('models.default');
expect(KNOWN_CONFIG_KEYS).toContain('models.tier.subagent');
@@ -238,6 +255,69 @@ describe('#2753 — the doctor-proposed gateway-loop command is accepted by `con
return { logs, errs, exit };
}
// `sources.default` is the one config key whose value the resolver
// dereferences on every unqualified call (tier 5 → assertSourceExists).
// Registering it in KNOWN_CONFIG_KEYS without a set-time check would make
// `config set` a way around the validation `gbrain sources default <id>`
// already performs, and a typo would surface later as a throw on unrelated
// commands. These pin that `config set` refuses the same inputs.
function sourcesEngine(registered: string[]): { engine: BrainEngine; setCalls: Array<[string, string]> } {
const setCalls: Array<[string, string]> = [];
const engine = {
getConfig: async () => null,
setConfig: async (k: string, v: string) => { setCalls.push([k, v]); },
executeRaw: async (_sql: string, params?: unknown[]) => {
const id = String((params ?? [])[0] ?? '');
return registered.includes(id) ? [{ id, name: id }] : [];
},
} as unknown as BrainEngine;
return { engine, setCalls };
}
test('sources.default: refuses an id that is not a valid source id', async () => {
const { engine, setCalls } = sourcesEngine(['wiki']);
const { errs, exit } = await runConfigCapture(engine, ['set', 'sources.default', 'Not A Source']);
expect(exit).toBe(1);
expect(errs.join('\n')).toContain('lowercase alphanumerics');
expect(setCalls).toEqual([]);
});
test('sources.default: refuses an unregistered source instead of writing it', async () => {
const { engine, setCalls } = sourcesEngine(['wiki']);
const { errs, exit } = await runConfigCapture(engine, ['set', 'sources.default', 'ghost']);
expect(exit).toBe(1);
expect(errs.join('\n')).toContain('not registered');
expect(setCalls).toEqual([]);
});
test('sources.default: accepts a registered source without --force', async () => {
const { engine, setCalls } = sourcesEngine(['wiki']);
const { errs, exit } = await runConfigCapture(engine, ['set', 'sources.default', 'wiki']);
expect(exit).toBeNull();
// The false "Nothing in gbrain reads this" line is the bug this fixes.
expect(errs.join('\n')).not.toContain('Nothing in gbrain reads this');
expect(setCalls).toEqual([['sources.default', 'wiki']]);
});
// A DB failure must not be laundered into "source is not registered" — that
// would send an operator chasing a source-registration problem that doesn't
// exist while the real fault (connection, permissions, SQL regression) is
// swallowed.
test('sources.default: a lookup failure propagates instead of reading as unregistered', async () => {
const setCalls: Array<[string, string]> = [];
const engine = {
getConfig: async () => null,
setConfig: async (k: string, v: string) => { setCalls.push([k, v]); },
executeRaw: async () => { throw new Error('connection terminated unexpectedly'); },
} as unknown as BrainEngine;
// The real error must escape rather than be reshaped into a validation
// message, so this rejects instead of returning an exit code.
await expect(
runConfigCapture(engine, ['set', 'sources.default', 'wiki']),
).rejects.toThrow('connection terminated unexpectedly');
expect(setCalls).toEqual([]);
});
test('doctor-proposed command round-trips through `config set` without --force', async () => {
const check = await withEnv(
{ GBRAIN_HOME: home, GBRAIN_CHAT_MODEL: undefined, ANTHROPIC_API_KEY: undefined },