Compare commits

..
Author SHA1 Message Date
22022182f8 fix(cli): honor search --json output (#2042)
`gbrain search <q> --json` silently printed human text: the search/query
ops declare no json param, parseOpArgs dropped the undeclared trailing
flag, and formatResult only emitted human text or --explain output.

Treat --json as a CLI-local boolean formatter flag in parseOpArgs (never
added to the operation contract; MCP validateParams ignores the extra
key on the routed path) and have formatResult's search/query case emit
the raw result array as parseable JSON when set.

Takeover of #2531, rebased onto master (keeps bigintToStringReplacer at
the local-path formatResult call site).

Co-authored-by: javieraldape <javieraldape@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:20:25 -07:00
6 changed files with 82 additions and 63 deletions
+12 -3
View File
@@ -464,7 +464,7 @@ async function main() {
// routed path. Date → ISO string; bigint → string (postgres.js shape);
// Buffer → object. Microsecond-cost; eliminates a whole drift bug class.
const result = JSON.parse(JSON.stringify(rawResult, bigintToStringReplacer));
const output = formatResult(op.name, result);
const output = formatResult(op.name, result, params);
if (output) process.stdout.write(output);
} catch (e: unknown) {
// v0.42.20.0 (codex D4): on error, set exitCode + return so the `finally`
@@ -547,7 +547,7 @@ async function runThinClientRouted(
signal: sigintController.signal,
});
const result = unpackToolResult(raw);
const output = formatResult(op.name, result);
const output = formatResult(op.name, result, params);
if (output) process.stdout.write(output);
} catch (e: unknown) {
if (e instanceof RemoteMcpError) {
@@ -777,6 +777,10 @@ export function parseOpArgs(op: Operation, args: string[]): Record<string, unkno
const paramDef = op.params[key];
if (paramDef?.type === 'boolean') {
params[key] = true;
} else if (key === 'json') {
// Generic operation formatter flag. It is intentionally CLI-local:
// do not add it to the operation contract exposed over MCP/tools.
params[key] = true;
} else if (i + 1 < args.length) {
params[key] = args[++i];
if (paramDef?.type === 'number') params[key] = Number(params[key]);
@@ -838,7 +842,11 @@ async function makeContext(engine: BrainEngine, params: Record<string, unknown>)
}
// Exported for tests (same import-safety contract as cliAliases/printOpHelp).
export function formatResult(opName: string, result: unknown): string {
export function formatResult(
opName: string,
result: unknown,
params: Record<string, unknown> = {},
): string {
switch (opName) {
case 'volunteer_context': {
const r = result as any;
@@ -877,6 +885,7 @@ export function formatResult(opName: string, result: unknown): string {
case 'search':
case 'query': {
const results = result as any[];
if (params.json === true) return JSON.stringify(results, null, 2) + '\n';
if (results.length === 0) return 'No results.\n';
// v0.40.4 — --explain switches to per-stage attribution formatter.
// Reads CliOptions.explain via the module-level singleton.
+3 -11
View File
@@ -35,11 +35,10 @@
*
* The doctor renders both side by side.
*
* Drift contract: every check name that ships through doctor MUST appear in
* Drift contract: every check name that ships in doctor.ts MUST appear in
* exactly one set below. The drift-guard test in
* `test/doctor-categories.test.ts` enforces this by reading doctor check
* emitter sources via a tagged-string scan and asserting set membership
* exactly.
* `test/doctor-categories.test.ts` enforces this by reading doctor.ts source
* via a tagged-string scan and asserting set membership exactly.
*
* If you add a new doctor check, you MUST add its name to the appropriate
* set here. The categorize step in `src/commands/doctor.ts` falls through
@@ -68,15 +67,12 @@ export const BRAIN_CHECK_NAMES: ReadonlySet<string> = new Set([
'conversation_parser_probe_health',
'cross_modal_modality_backfill',
'cycle_freshness',
'dangling_aliases',
'effective_date_health',
'embed_staleness',
'embedding_column_registry',
'embedding_env_override',
'embedding_provider',
'embedding_width_consistency',
'embeddings',
'entity_link_coverage',
'eval_drift',
'extract_atoms_backlog',
'extract_health',
@@ -106,9 +102,7 @@ export const BRAIN_CHECK_NAMES: ReadonlySet<string> = new Set([
'stub_guard_24h',
'sync_failures',
'sync_freshness',
'takes_count',
'takes_weight_grid',
'timeline_coverage',
'unified_multimodal_coverage',
'voice_gate_health',
]);
@@ -176,14 +170,12 @@ export const META_CHECK_NAMES: ReadonlySet<string> = new Set([
'eval_capture',
'minions_migration',
'multi_source_drift',
'pack_upgrade_available',
'schema_pack_active',
'schema_pack_consistency',
'schema_pack_source_drift',
'schema_version',
'slug_fallback_audit',
'timeline_dedup_index',
'type_proliferation',
'upgrade_errors',
]);
+12 -1
View File
@@ -20,5 +20,16 @@ describe('parseOpArgs', () => {
source_id: 'gstack-code-repo-0e4763c9',
});
});
});
test('--json is a CLI-local formatter flag for shared operations', () => {
const params = parseOpArgs(operationsByName.search, [
'needle',
'--json',
]);
expect(params).toEqual({
query: 'needle',
json: true,
});
});
});
+28
View File
@@ -0,0 +1,28 @@
import { describe, expect, test } from 'bun:test';
import { formatResult } from '../src/cli.ts';
describe('formatResult - search/query --json', () => {
test('search --json renders the raw result array as parseable JSON', () => {
const out = formatResult('search', [
{
slug: 'docs/example',
score: 0.42,
chunk_text: 'Example result text',
},
], { json: true });
expect(JSON.parse(out)).toEqual([
{
slug: 'docs/example',
score: 0.42,
chunk_text: 'Example result text',
},
]);
});
test('query --json keeps empty results machine-readable', () => {
const out = formatResult('query', [], { json: true });
expect(JSON.parse(out)).toEqual([]);
});
});
+9
View File
@@ -49,6 +49,15 @@ describe('T5 — gbrain search dispatch', () => {
});
});
test('`search "<freetext>" --json` emits a parseable result array', () => {
withHome((home) => {
const { stdout, stderr, status } = run(['search', 'zzz-no-such-page-xyz', '--json'], home);
expect(status).toBe(0);
expect(stderr).not.toContain('Unknown subcommand');
expect(JSON.parse(stdout)).toEqual([]);
});
});
test('`search stats --json` routes to the dashboard', () => {
withHome((home) => {
const { stdout, status } = run(['search', 'stats', '--json'], home);
+18 -48
View File
@@ -1,10 +1,10 @@
/**
* Drift guard for src/core/doctor-categories.ts.
*
* Reads doctor check emitter source via a literal-string scan, enumerates every
* `name: '<...>'` Check name, and asserts each appears in exactly ONE category
* set. The union of the four sets must equal the discovered names exactly —
* no orphans, no extras.
* Reads src/commands/doctor.ts source via a literal-string scan, enumerates
* every `name: '<...>'` Check name, and asserts each appears in exactly ONE
* category set. The union of the four sets must equal the discovered names
* exactly — no orphans, no extras.
*
* This is the structural failure the v0.41.19.0 plan-eng-review caught:
* doctor.ts grows new checks regularly; without this guard, the
@@ -25,30 +25,26 @@ import {
} from '../src/core/doctor-categories.ts';
const DOCTOR_TS_PATH = join(import.meta.dir, '..', 'src', 'commands', 'doctor.ts');
const ONBOARD_CHECKS_TS_PATH = join(import.meta.dir, '..', 'src', 'core', 'onboard', 'checks.ts');
const CHECK_SOURCE_PATHS = [DOCTOR_TS_PATH, ONBOARD_CHECKS_TS_PATH];
function enumerateCheckNames(): Set<string> {
const source = readFileSync(DOCTOR_TS_PATH, 'utf-8');
const names = new Set<string>();
for (const path of CHECK_SOURCE_PATHS) {
const source = readFileSync(path, 'utf-8');
// 1) Inline object-literal form: `{ name: 'foo', ... }`.
for (const m of source.matchAll(/name:\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
// 2) Helper-function form: `const name = 'foo';` inside a check helper.
// Catches checks like `nightly_quality_probe_health` and
// `conversation_facts_backlog` that build the Check from a captured
// name constant.
for (const m of source.matchAll(/const\s+name\s*=\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
// 1) Inline object-literal form: `{ name: 'foo', ... }`.
for (const m of source.matchAll(/name:\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
// 2) Helper-function form: `const name = 'foo';` inside a check helper.
// Catches checks like `nightly_quality_probe_health` and
// `conversation_facts_backlog` that build the Check from a captured
// name constant.
for (const m of source.matchAll(/const\s+name\s*=\s*['"]([a-z][a-z0-9_]+)['"]/g)) {
names.add(m[1]);
}
return names;
}
describe('doctor-categories drift guard', () => {
test('every doctor-emitted check name belongs to exactly one category set', () => {
test('every check name in doctor.ts source belongs to exactly one category set', () => {
const discovered = enumerateCheckNames();
const allCategorized = new Set<string>([
...BRAIN_CHECK_NAMES,
@@ -63,7 +59,7 @@ describe('doctor-categories drift guard', () => {
}
if (missing.length > 0) {
throw new Error(
`These check names appear in doctor check emitters but are not categorized in ` +
`These check names appear in doctor.ts but are not categorized in ` +
`src/core/doctor-categories.ts: ${missing.sort().join(', ')}. ` +
`Add each to BRAIN/SKILL/OPS/META_CHECK_NAMES.`,
);
@@ -90,7 +86,7 @@ describe('doctor-categories drift guard', () => {
expect(dupes).toEqual([]);
});
test('every categorized name is currently used in doctor check emitters (no stale entries)', () => {
test('every categorized name is currently used in doctor.ts source (no stale entries)', () => {
const discovered = enumerateCheckNames();
const allCategorized = new Set<string>([
...BRAIN_CHECK_NAMES,
@@ -128,14 +124,6 @@ describe('categorizeCheck', () => {
expect(categorizeCheck('sync_freshness')).toBe('brain');
});
test('returns the right category for onboard data-quality check names', () => {
expect(categorizeCheck('embed_staleness')).toBe('brain');
expect(categorizeCheck('entity_link_coverage')).toBe('brain');
expect(categorizeCheck('timeline_coverage')).toBe('brain');
expect(categorizeCheck('takes_count')).toBe('brain');
expect(categorizeCheck('dangling_aliases')).toBe('brain');
});
test('returns the right category for a known skill name', () => {
expect(categorizeCheck('resolver_health')).toBe('skill');
expect(categorizeCheck('skill_conformance')).toBe('skill');
@@ -152,24 +140,6 @@ describe('categorizeCheck', () => {
expect(categorizeCheck('upgrade_errors')).toBe('meta');
});
test('returns the right category for onboard schema-pack check names without warning', () => {
const originalWrite = process.stderr.write.bind(process.stderr);
const captured: string[] = [];
(process.stderr as { write: typeof process.stderr.write }).write = ((
chunk: string | Uint8Array,
) => {
captured.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString());
return true;
}) as typeof process.stderr.write;
try {
expect(categorizeCheck('pack_upgrade_available')).toBe('meta');
expect(categorizeCheck('type_proliferation')).toBe('meta');
expect(captured.filter((c) => c.includes('[doctor-categories]'))).toEqual([]);
} finally {
(process.stderr as { write: typeof process.stderr.write }).write = originalWrite;
}
});
test('unknown check name falls through to meta with a stderr warn (once per process)', () => {
const originalWrite = process.stderr.write.bind(process.stderr);
const captured: string[] = [];