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 |
@@ -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;
|
||||
|
||||
@@ -908,10 +908,6 @@ export const KNOWN_CONFIG_KEYS: readonly string[] = [
|
||||
'facts.extraction_model',
|
||||
// #2113: output-token cap for the per-turn facts extractor (default 4000).
|
||||
'facts.extraction_max_tokens',
|
||||
// Owner opt-in: let the local stdio MCP pipe read this owner's private facts
|
||||
// (find_trajectory / recall). Default off; HTTP transport ignores it. See
|
||||
// src/core/facts/reader-trust.ts.
|
||||
'facts.trust_local_reads',
|
||||
// Dream cycle config
|
||||
'dream.synthesize.session_corpus_dir',
|
||||
'dream.synthesize.meeting_transcripts_dir',
|
||||
|
||||
@@ -591,8 +591,6 @@ export interface TrajectoryOpts {
|
||||
sourceIds?: string[];
|
||||
/** When true, filters to visibility='world' only. Set by MCP layer from ctx.remote. */
|
||||
remote?: boolean;
|
||||
/** Owner opt-in: read private facts despite `remote`. See facts/reader-trust.ts. */
|
||||
trustedFactReads?: boolean;
|
||||
/** Metric filter. When set, only facts with this canonical metric label participate. */
|
||||
metric?: string;
|
||||
/**
|
||||
|
||||
@@ -17,7 +17,6 @@
|
||||
import type { OperationContext } from './../operations.ts';
|
||||
import type { FactRow } from './../engine.ts';
|
||||
import { effectiveConfidence } from './decay.ts';
|
||||
import { readableFactVisibilities } from './reader-trust.ts';
|
||||
|
||||
const DEFAULT_TTL_MS = 30_000;
|
||||
const DEFAULT_TOP_K = 10;
|
||||
@@ -51,13 +50,7 @@ export async function getBrainHotMemoryMeta(
|
||||
const sessionId = (ctx as { source_session?: string }).source_session
|
||||
?? null;
|
||||
const allowListHash = hashAllowList(ctx.takesHoldersAllowList);
|
||||
// Visibility tier: untrusted remote → world-only; trusted local +
|
||||
// owner-trusted reads → all rows. Folded into the cache key (the header's
|
||||
// "cache entries don't bleed across tiers" invariant): trustedFactReads is
|
||||
// re-read from config per call, so a mid-session opt-out must not keep
|
||||
// serving a private-inclusive cached payload for the TTL window.
|
||||
const visibility = readableFactVisibilities(ctx);
|
||||
const cacheKey = `${sourceId}::${sessionId ?? '_'}::${allowListHash}::${visibility ? 'world' : 'all'}`;
|
||||
const cacheKey = `${sourceId}::${sessionId ?? '_'}::${allowListHash}`;
|
||||
|
||||
const ttl = Math.max(1000, opts.ttlMs ?? DEFAULT_TTL_MS);
|
||||
const topK = Math.max(1, Math.min(opts.topK ?? DEFAULT_TOP_K, 25));
|
||||
@@ -68,6 +61,10 @@ export async function getBrainHotMemoryMeta(
|
||||
return cached.payload;
|
||||
}
|
||||
|
||||
// Build a fresh payload. Visibility tier: remote → world-only;
|
||||
// local → all rows.
|
||||
const visibility = ctx.remote === false ? undefined : ['world'] as ('world' | 'private')[];
|
||||
|
||||
let rows: FactRow[] = [];
|
||||
if (sessionId) {
|
||||
rows = await ctx.engine.listFactsBySession(sourceId, sessionId, {
|
||||
|
||||
@@ -1,48 +0,0 @@
|
||||
/**
|
||||
* Fact-read visibility trust.
|
||||
*
|
||||
* Fact rows are tagged `private` | `world`. Remote/untrusted callers
|
||||
* (`remote === true`) see only `world` rows — the posture that keeps a
|
||||
* published or HTTP-served brain from leaking private claims to strangers.
|
||||
*
|
||||
* But the stdio MCP server is an unauthenticated LOCAL pipe: on a single-owner
|
||||
* machine the caller IS the owner, yet it still defaults `remote: true` for
|
||||
* safety, so the owner's own agent is denied the owner's own private facts
|
||||
* (e.g. `find_trajectory` returns empty over MCP even though the facts exist).
|
||||
*
|
||||
* `trustedFactReads` is a narrow, READ-ONLY trust elevation, deliberately
|
||||
* DECOUPLED from `remote` so every other remote protection — file_upload
|
||||
* confinement, source isolation, fence stripping, takes-holder scoping — stays
|
||||
* fully in force. The stdio MCP server sets it ONLY when the brain owner opts
|
||||
* in via the `facts.trust_local_reads` config (default off). The HTTP/published
|
||||
* transport never sets it, so a served brain stays world-only regardless.
|
||||
*/
|
||||
export interface FactReaderTrust {
|
||||
/**
|
||||
* Mirrors OperationContext.remote. FAIL-CLOSED: anything not strictly
|
||||
* `false` is treated as remote/untrusted (CLAUDE.md trust invariant).
|
||||
*/
|
||||
remote?: boolean;
|
||||
/** Owner opt-in: this remote caller may read private facts. */
|
||||
trustedFactReads?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the reader is restricted to `visibility = 'world'` rows.
|
||||
* Fail-closed: an unset/undefined `remote` is untrusted — only an explicit
|
||||
* `remote: false` (trusted local CLI) or an explicit owner opt-in
|
||||
* (`trustedFactReads: true`) reads private rows.
|
||||
*/
|
||||
export function factsWorldOnly(t: FactReaderTrust): boolean {
|
||||
return t.remote !== false && t.trustedFactReads !== true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Visibility filter for list-style fact reads: `['world']` when the reader is
|
||||
* world-only, `undefined` (no filter — all rows) when it is trusted.
|
||||
*/
|
||||
export function readableFactVisibilities(
|
||||
t: FactReaderTrust,
|
||||
): ('private' | 'world')[] | undefined {
|
||||
return factsWorldOnly(t) ? ['world'] : undefined;
|
||||
}
|
||||
+7
-14
@@ -18,7 +18,6 @@ import { captureEvalCandidate, isEvalCaptureEnabled, isEvalScrubEnabled } from '
|
||||
import type { HybridSearchMeta } from './types.ts';
|
||||
import { extractPageLinks, isAutoLinkEnabled, isAutoTimelineEnabled, isGlobalBasenameEnabled, parseTimelineEntries, makeResolver, type UnresolvedFrontmatterRef } from './link-extraction.ts';
|
||||
import { isFactsBackstopEligible } from './facts/eligibility.ts';
|
||||
import { readableFactVisibilities } from './facts/reader-trust.ts';
|
||||
import { stripTakesFence } from './takes-fence.ts';
|
||||
import { stripFactsFence } from './facts-fence.ts';
|
||||
import { getContentFlag } from './quarantine.ts';
|
||||
@@ -333,15 +332,6 @@ export interface OperationContext {
|
||||
* remote/untrusted (defense in depth in case the type is bypassed via cast).
|
||||
*/
|
||||
remote: boolean;
|
||||
/**
|
||||
* Owner opt-in (`facts.trust_local_reads`): allow this remote caller to read
|
||||
* `private` facts. A NARROW, read-only trust elevation decoupled from
|
||||
* `remote` — every other remote protection (file confinement, source
|
||||
* isolation, fence stripping, takes scoping) stays in force. Set ONLY by the
|
||||
* stdio MCP server when the config is on; the HTTP transport never sets it.
|
||||
* Consulted via `src/core/facts/reader-trust.ts`.
|
||||
*/
|
||||
trustedFactReads?: boolean;
|
||||
/**
|
||||
* Subagent runtime context (v0.16+). Set by the subagent tool dispatcher when
|
||||
* dispatching an op as a tool call from an LLM loop. Used to enforce per-op
|
||||
@@ -3639,7 +3629,6 @@ const find_trajectory: Operation = {
|
||||
entitySlug: p.entity_slug,
|
||||
...scope,
|
||||
remote: ctx.remote === true,
|
||||
trustedFactReads: ctx.trustedFactReads === true,
|
||||
metric,
|
||||
kind,
|
||||
since,
|
||||
@@ -4000,9 +3989,13 @@ const recall: Operation = {
|
||||
const includeExpired = p.include_expired === true;
|
||||
const grep = typeof p.grep === 'string' ? p.grep.toLowerCase() : null;
|
||||
|
||||
// Visibility filter: world-only for untrusted remote callers; all rows for
|
||||
// trusted local CLI and owner-trusted reads (facts.trust_local_reads).
|
||||
const visibility = readableFactVisibilities(ctx);
|
||||
// Visibility filter: remote callers see world-only unless their token
|
||||
// grants elevated visibility (future-proofing; v0.31 ships world-only
|
||||
// for remote, all for local CLI).
|
||||
const visibility =
|
||||
ctx.remote === false
|
||||
? undefined
|
||||
: ['world'] as ('private' | 'world')[];
|
||||
|
||||
let rows: Awaited<ReturnType<typeof ctx.engine.listFactsByEntity>> = [];
|
||||
|
||||
|
||||
@@ -18,7 +18,6 @@ import type {
|
||||
} from './engine.ts';
|
||||
import { MAX_SEARCH_LIMIT, clampSearchLimit } from './engine.ts';
|
||||
import { withRetry, BULK_RETRY_OPTS, resolveBulkRetryOpts, computeNextDelay, type BatchAuditSite } from './retry.ts';
|
||||
import { factsWorldOnly } from './facts/reader-trust.ts';
|
||||
import { logBatchRetry as auditLogBatchRetry, logBatchExhausted as auditLogBatchExhausted } from './audit/batch-retry-audit.ts';
|
||||
import { runMigrations } from './migrate.ts';
|
||||
import { PGLITE_SCHEMA_SQL, getPGLiteSchema } from './pglite-schema.ts';
|
||||
@@ -4319,10 +4318,7 @@ export class PGLiteEngine implements BrainEngine {
|
||||
const useArray = Array.isArray(opts.sourceIds) && opts.sourceIds.length > 0;
|
||||
const sourceIds = useArray ? opts.sourceIds! : null;
|
||||
const sourceId = opts.sourceId ?? 'default';
|
||||
// Direct-engine contract: unset `remote` here means a trusted in-process
|
||||
// caller (CLI/tests) — the fail-closed default lives in the op layer, which
|
||||
// always passes explicit booleans. Normalize before the fail-closed helper.
|
||||
const remoteFilter = factsWorldOnly({ remote: opts.remote === true, trustedFactReads: opts.trustedFactReads === true });
|
||||
const remoteFilter = opts.remote === true;
|
||||
|
||||
// Build SQL dynamically. PGLite uses $N positional params; we
|
||||
// assemble the WHERE clauses + params array in tandem to keep them
|
||||
|
||||
@@ -14,7 +14,6 @@ import type {
|
||||
SourceRow,
|
||||
} from './engine.ts';
|
||||
import { withRetry, BULK_RETRY_OPTS, resolveBulkRetryOpts, computeNextDelay, type BatchAuditSite } from './retry.ts';
|
||||
import { factsWorldOnly } from './facts/reader-trust.ts';
|
||||
import { logBatchRetry as auditLogBatchRetry, logBatchExhausted as auditLogBatchExhausted } from './audit/batch-retry-audit.ts';
|
||||
import type {
|
||||
DomainBankSampleOpts, CorpusSampleOpts, DomainBankRow,
|
||||
@@ -4533,10 +4532,7 @@ export class PostgresEngine implements BrainEngine {
|
||||
const useArray = Array.isArray(opts.sourceIds) && opts.sourceIds.length > 0;
|
||||
const sourceIds = useArray ? opts.sourceIds! : null;
|
||||
const sourceId = opts.sourceId ?? 'default';
|
||||
// Direct-engine contract: unset `remote` here means a trusted in-process
|
||||
// caller (CLI/tests) — the fail-closed default lives in the op layer, which
|
||||
// always passes explicit booleans. Normalize before the fail-closed helper.
|
||||
const remoteFilter = factsWorldOnly({ remote: opts.remote === true, trustedFactReads: opts.trustedFactReads === true });
|
||||
const remoteFilter = opts.remote === true;
|
||||
|
||||
// Source-scope predicate: array path (federated) wins over scalar.
|
||||
// Engine.ts contract: returns chronological points; regressions +
|
||||
|
||||
@@ -30,13 +30,6 @@ export interface ToolResult {
|
||||
export interface DispatchOpts {
|
||||
/** Defaults to true (remote/untrusted). Local CLI callers (`gbrain call`) pass false. */
|
||||
remote?: boolean;
|
||||
/**
|
||||
* Owner opt-in (`facts.trust_local_reads`): let this remote caller read
|
||||
* `private` facts. Set ONLY by the stdio MCP server; the HTTP transport
|
||||
* leaves it unset so a served brain stays world-only. See
|
||||
* `src/core/facts/reader-trust.ts`.
|
||||
*/
|
||||
trustedFactReads?: boolean;
|
||||
/** Override the default stderr logger (e.g. CLI uses console.* directly). */
|
||||
logger?: OperationContext['logger'];
|
||||
/**
|
||||
@@ -210,7 +203,6 @@ export function buildOperationContext(
|
||||
logger: opts.logger || stderrLogger,
|
||||
dryRun: !!params.dry_run,
|
||||
remote: opts.remote ?? true,
|
||||
trustedFactReads: opts.trustedFactReads === true,
|
||||
takesHoldersAllowList: opts.takesHoldersAllowList,
|
||||
// v0.34 D4: sourceId is REQUIRED at the type level. Auto-fill 'default'
|
||||
// for single-source brains and any caller who didn't resolve a sourceId.
|
||||
|
||||
@@ -35,16 +35,6 @@ export async function startMcpServer(engine: BrainEngine) {
|
||||
// shape and cast through `any` (the SDK accepts it via the ServerResult union).
|
||||
server.setRequestHandler(CallToolRequestSchema, async (request: any): Promise<any> => {
|
||||
const { name, arguments: params } = request.params;
|
||||
// Owner opt-in: the stdio pipe is local + unauthenticated, so on a
|
||||
// single-owner machine its caller is the owner. When facts.trust_local_reads
|
||||
// is on, let fact reads (find_trajectory / recall) see this owner's own
|
||||
// private facts. Narrow + read-only — every other remote protection stays
|
||||
// on (remote stays true). HTTP transport never sets this. Best-effort: a
|
||||
// config read blip falls back to the safe world-only default.
|
||||
let trustedFactReads = false;
|
||||
try {
|
||||
trustedFactReads = (await engine.getConfig('facts.trust_local_reads')) === 'true';
|
||||
} catch { /* keep world-only default */ }
|
||||
// v0.28: stdio MCP has no per-token auth (local pipe). Default the
|
||||
// takes-holder allow-list to ['world'] so agent-facing callers don't
|
||||
// see private hunches via takes_list / takes_search / query. Operators
|
||||
@@ -52,7 +42,6 @@ export async function startMcpServer(engine: BrainEngine) {
|
||||
// `gbrain call <op>` (sets remote=false in src/cli.ts).
|
||||
return dispatchToolCall(engine, name, params, {
|
||||
remote: true,
|
||||
trustedFactReads,
|
||||
takesHoldersAllowList: ['world'],
|
||||
// v0.31: source defaults to 'default' for stdio (no per-token scope).
|
||||
// Operators who want a different source on stdio MCP should set
|
||||
|
||||
@@ -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');
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -165,22 +165,6 @@ describe('findTrajectory — visibility filter (D-CDX-1 / R6)', () => {
|
||||
const all = await engine.findTrajectory({ entitySlug: 'traj-vis-default' });
|
||||
expect(all.length).toBe(2);
|
||||
});
|
||||
|
||||
test('remote=true + trustedFactReads bypasses world-only (owner-trusted reads)', async () => {
|
||||
await insertTyped({ entity_slug: 'traj-vis-trusted', metric: 'mrr', value: 50000, visibility: 'private', valid_from: new Date('2026-01-15') });
|
||||
await insertTyped({ entity_slug: 'traj-vis-trusted', metric: 'mrr', value: 99999, visibility: 'world', valid_from: new Date('2026-04-12') });
|
||||
|
||||
// Untrusted remote: world only.
|
||||
const untrusted = await engine.findTrajectory({ entitySlug: 'traj-vis-trusted', remote: true });
|
||||
expect(untrusted.length).toBe(1);
|
||||
expect(untrusted[0].value).toBe(99999);
|
||||
|
||||
// Owner-trusted remote: sees the private point too. remote stays true — only
|
||||
// fact-read visibility is elevated.
|
||||
const trusted = await engine.findTrajectory({ entitySlug: 'traj-vis-trusted', remote: true, trustedFactReads: true });
|
||||
expect(trusted.length).toBe(2);
|
||||
expect(trusted.map(p => p.value).sort((a, b) => (a! - b!))).toEqual([50000, 99999]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('findTrajectory — metric + since + until filters', () => {
|
||||
|
||||
@@ -1,43 +0,0 @@
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import {
|
||||
factsWorldOnly,
|
||||
readableFactVisibilities,
|
||||
} from '../src/core/facts/reader-trust.ts';
|
||||
|
||||
describe('factsWorldOnly', () => {
|
||||
test('FAIL-CLOSED: unset remote is untrusted (world-only)', () => {
|
||||
expect(factsWorldOnly({})).toBe(true);
|
||||
expect(factsWorldOnly({ remote: undefined })).toBe(true);
|
||||
expect(factsWorldOnly({ trustedFactReads: false })).toBe(true);
|
||||
});
|
||||
|
||||
test('explicit remote=false (trusted local CLI) sees all', () => {
|
||||
expect(factsWorldOnly({ remote: false })).toBe(false);
|
||||
expect(factsWorldOnly({ remote: false, trustedFactReads: false })).toBe(false);
|
||||
});
|
||||
|
||||
test('untrusted remote callers are world-only', () => {
|
||||
expect(factsWorldOnly({ remote: true })).toBe(true);
|
||||
expect(factsWorldOnly({ remote: true, trustedFactReads: false })).toBe(true);
|
||||
});
|
||||
|
||||
test('owner-trusted remote reads bypass the world-only filter', () => {
|
||||
expect(factsWorldOnly({ remote: true, trustedFactReads: true })).toBe(false);
|
||||
});
|
||||
|
||||
test('trustedFactReads is a no-op for an already-trusted local caller', () => {
|
||||
expect(factsWorldOnly({ remote: false, trustedFactReads: true })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('readableFactVisibilities', () => {
|
||||
test("world-only readers get the ['world'] filter (incl. unset remote)", () => {
|
||||
expect(readableFactVisibilities({ remote: true })).toEqual(['world']);
|
||||
expect(readableFactVisibilities({})).toEqual(['world']);
|
||||
});
|
||||
|
||||
test('trusted readers get undefined (no filter — all rows)', () => {
|
||||
expect(readableFactVisibilities({ remote: false })).toBeUndefined();
|
||||
expect(readableFactVisibilities({ remote: true, trustedFactReads: true })).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user