mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-16 01:42:23 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6548e5cffc | ||
|
|
0ac3d4da9c |
@@ -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;
|
||||
|
||||
@@ -22,7 +22,6 @@
|
||||
*/
|
||||
|
||||
import { resolveRecipe } from './model-resolver.ts';
|
||||
import { listRecipes } from './recipes/index.ts';
|
||||
import { AIConfigError } from './errors.ts';
|
||||
|
||||
export interface ProviderCapabilities {
|
||||
@@ -78,10 +77,7 @@ export function getProviderCapabilities(modelString: string): ProviderCapabiliti
|
||||
if (!chat) {
|
||||
throw new AIConfigError(
|
||||
`Provider "${recipe.id}" does not offer a chat touchpoint.`,
|
||||
// Computed from the registry so the hint can't drift into listing
|
||||
// chat-less providers (the pre-fix list falsely included embedding-only
|
||||
// recipes, sending users in circles — #1157).
|
||||
`Known providers with chat: ${listRecipes().filter(r => r.touchpoints.chat).map(r => r.id).join(', ')}. Pick one for models.tier.subagent.`,
|
||||
`Known providers with chat: openai, anthropic, google, openrouter, litellm-proxy, deepseek, groq, together, azure-openai, dashscope, minimax, zhipu, ollama, llama-server. Pick one for models.tier.subagent.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,10 +1,9 @@
|
||||
import type { Recipe } from '../types.ts';
|
||||
|
||||
/**
|
||||
* Zhipu AI (智谱AI) BigModel Open Platform. OpenAI-compatible /embeddings and
|
||||
* /chat/completions endpoints at open.bigmodel.cn. Hosts embedding-2 (1024d),
|
||||
* embedding-3 (Matryoshka up to 2048d), and the GLM chat family (glm-5.1 etc.)
|
||||
* with native tool calling — usable for models.tier.subagent (#1157).
|
||||
* Zhipu AI (智谱AI) BigModel Open Platform. OpenAI-compatible /embeddings
|
||||
* endpoint at open.bigmodel.cn. Hosts embedding-2 (1024d) and embedding-3
|
||||
* (Matryoshka up to 2048d).
|
||||
*
|
||||
* embedding-3 at 2048 dims exceeds pgvector's HNSW cap of 2000 — those
|
||||
* brains fall back to exact vector scans (see
|
||||
@@ -26,20 +25,6 @@ export const zhipu: Recipe = {
|
||||
setup_url: 'https://open.bigmodel.cn/',
|
||||
},
|
||||
touchpoints: {
|
||||
chat: {
|
||||
// Informational list (openai-compat tier: assertTouchpoint doesn't
|
||||
// enforce it), so newer GLM ids pass without a recipe edit.
|
||||
models: ['glm-5.1', 'glm-4.6', 'glm-4.5'],
|
||||
supports_tools: true,
|
||||
// gbrain-side stable tool ids (v0.38 D11) decoupled the loop from
|
||||
// Anthropic response formats; GLM tool calling is stable through the
|
||||
// OpenAI-compat path, same as deepseek/groq.
|
||||
supports_subagent_loop: true,
|
||||
// Anthropic-style cache_control markers are not honored on the
|
||||
// OpenAI-compat path — the loop runs hot (degraded:no_caching warn).
|
||||
supports_prompt_cache: false,
|
||||
max_context_tokens: 128000,
|
||||
},
|
||||
embedding: {
|
||||
models: ['embedding-3', 'embedding-2'],
|
||||
default_dims: 1024,
|
||||
@@ -51,5 +36,5 @@ export const zhipu: Recipe = {
|
||||
},
|
||||
},
|
||||
setup_hint:
|
||||
'Get an API key at https://open.bigmodel.cn/, then `export ZHIPUAI_API_KEY=...`. Chat/subagent: use `zhipu:glm-5.1`.',
|
||||
'Get an API key at https://open.bigmodel.cn/, then `export ZHIPUAI_API_KEY=...`',
|
||||
};
|
||||
|
||||
@@ -69,45 +69,6 @@ describe('recipe: zhipu', () => {
|
||||
expect(sql.toLowerCase()).toContain('hnsw');
|
||||
});
|
||||
|
||||
test('chat touchpoint declares GLM models with tool + subagent-loop support (#1157)', () => {
|
||||
const r = getRecipe('zhipu')!;
|
||||
expect(r.touchpoints.chat).toBeDefined();
|
||||
expect(r.touchpoints.chat!.models).toContain('glm-5.1');
|
||||
expect(r.touchpoints.chat!.supports_tools).toBe(true);
|
||||
expect(r.touchpoints.chat!.supports_subagent_loop).toBe(true);
|
||||
expect(r.touchpoints.chat!.supports_prompt_cache).toBe(false);
|
||||
});
|
||||
|
||||
test('zhipu:glm-5.1 passes the subagent capability gate (degraded:no_caching, not refused)', async () => {
|
||||
// Pre-fix: getProviderCapabilities threw "does not offer a chat touchpoint"
|
||||
// and classifyCapabilities returned 'unknown' → subagent submit refused.
|
||||
const { getProviderCapabilities, classifyCapabilities } =
|
||||
await import('../../src/core/ai/capabilities.ts');
|
||||
const caps = getProviderCapabilities('zhipu:glm-5.1');
|
||||
expect(caps.supportsToolCalling).toBe(true);
|
||||
expect(classifyCapabilities('zhipu:glm-5.1')).toBe('degraded:no_caching');
|
||||
});
|
||||
|
||||
test('no-chat-touchpoint error hint lists only providers that actually have chat', async () => {
|
||||
// The hint is computed from the registry; every provider it names must
|
||||
// really carry a chat touchpoint (pre-fix it hardcoded zhipu/dashscope/
|
||||
// minimax, all embedding-only at the time).
|
||||
const { getProviderCapabilities } = await import('../../src/core/ai/capabilities.ts');
|
||||
const { listRecipes } = await import('../../src/core/ai/recipes/index.ts');
|
||||
let hint = '';
|
||||
try {
|
||||
getProviderCapabilities('voyage:voyage-3');
|
||||
throw new Error('expected AIConfigError for embedding-only provider');
|
||||
} catch (e) {
|
||||
hint = (e as { fix?: string }).fix ?? String(e);
|
||||
}
|
||||
const listed = hint.match(/chat: ([^.]+)\./)?.[1]?.split(', ') ?? [];
|
||||
expect(listed.length).toBeGreaterThan(0);
|
||||
const withChat = new Set(listRecipes().filter(r => r.touchpoints.chat).map(r => r.id));
|
||||
for (const id of listed) expect(withChat.has(id)).toBe(true);
|
||||
expect(listed).toContain('zhipu');
|
||||
});
|
||||
|
||||
test('dimsProviderOptions threads dimensions for embedding-3 (Matryoshka)', async () => {
|
||||
// Codex finding #1: Zhipu embedding-3 is Matryoshka 256-2048. Without
|
||||
// `dimensions` on the wire, user-selected non-default dims are
|
||||
|
||||
@@ -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');
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user