Compare commits

..
Author SHA1 Message Date
Garry TanandClaude Fable 5 6548e5cffc fix(autopilot): add --target to value-flag set so install targets survive positional translation
Review finding on #3103: --target is installDaemon's value flag
(macos | linux-systemd | ephemeral-container | linux-cron). The
translator only knew --repo/--interval, so `gbrain autopilot
--install --target linux-cron` misread the target value as an
unknown positional subcommand and exited 2 before installDaemon ran.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-22 12:05:46 -07:00
0ac3d4da9c fix(autopilot): translate positional subcommands so autopilot status doesn't start the daemon
Takeover/rebase of #1529 onto current master. `gbrain autopilot status`
(and install/uninstall/start) previously fell through the flag-only
branches in runAutopilot and silently started the daemon (lockfile +
worker spawn + sync dispatch). A pure translatePositionalSubcommands()
now maps known positionals to their flag form before any side effect,
is value-flag aware (--repo/--interval), and fails loud (exit 2) on
unknown positionals. 22 tests including the exact #1525 repro.

Fixes #1525

Co-authored-by: Oszkar <Oszkar@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:28:16 -07:00
20 changed files with 337 additions and 165 deletions
-1
View File
@@ -457,7 +457,6 @@ unresolvable+true|false, pre-v80 NULL/NULL rows survive).
- `src/core/think/prompt.ts` extension — anti-bias rewrite. `withCalibration` option on `buildThinkSystemPrompt` adds anti-bias rules. `buildCalibrationBlock()` emits the `<calibration>` XML. `buildThinkUserMessage` has TWO shapes: default (question first), and with-calibration (retrieval → calibration → question) when opt-in. Wired into `runThink` via `opts.withCalibration` + `opts.calibrationHolder`.
- `src/commands/calibration.ts` — CLI: `gbrain calibration` (read + print), `--regenerate`, `--undo-wave <ver>`, `ab-report`. MCP op `get_calibration_profile` (scope: read) backs the same data path. Source-scoped via `sourceScopeOpts(ctx)`.
- `src/commands/serve-http.ts` extension — three admin routes: `/admin/api/calibration/profile`, `/admin/api/calibration/charts/:type` (image/svg+xml; type in {brier-trend, domain-bars, pattern-statements, abandoned-threads}), `/admin/api/calibration/pattern/:id` (drill-down).
- `src/core/owner-holder.ts` — single source of truth for "the brain owner" holder string. `DEFAULT_OWNER_HOLDER = 'self'` (matches the consolidate facts→takes writer + `docs/takes-vs-facts.md`); `resolveOwnerHolder({override, configValue})` returns override > `emotional_weight.user_holder` config > `'self'`. Consumed by the calibration_profile cycle phase, `gbrain calibration` CLI, the `get_calibration_profile` op, `think`'s calibration block, `emotional-weight`'s `DEFAULT_USER_HOLDER`, and doctor's `calibration_freshness`. Pure; unit-tested in `test/owner-holder.test.ts`. Does NOT unify owner-identity fragmentation (`self`/`brain`/`people-<owner>`) — tracked separately.
- `src/commands/takes.ts` extension — `gbrain takes revisit <slug>` opens $EDITOR on the source page with a `<!-- gbrain:revisit -->` cursor marker.
- `src/commands/doctor.ts` extension — 4 checks: `abandoned_threads`, `calibration_freshness`, `grade_confidence_drift` (mitigation surface; math ships later), `voice_gate_health`.
- `admin/src/pages/Calibration.tsx` — Calibration tab. Single-column layout. `<TrustedSVG>` wrapper handles `dangerouslySetInnerHTML` for the server-rendered SVG.
-15
View File
@@ -91,18 +91,3 @@ First full takes extraction run on a ~100K-page brain:
4. **Self-reported ≠ verified.** "Reports 7 figures" → holder=person, weight=0.75, NOT world/1.0
5. **No false precision.** Use 0.05 increments (0.35, 0.55, 0.75), not 0.74 or 0.82
6. **"So what" test.** Skip Twitter handles, follower counts, obvious metadata
## Owner-holder canonicalization
"The brain owner" is, by convention, the holder string **`self`** — the value the
dream `consolidate` phase stamps when it promotes the owner's hot facts into cold
takes. Calibration, `think`, and the `doctor` calibration check resolve the owner
holder through `resolveOwnerHolder` (`src/core/owner-holder.ts`): explicit override
> `emotional_weight.user_holder` config > `self`.
Known limitation (tracked in garrytan/gbrain#2465): the owner can also
appear under `brain` (a take the owner asserts, via `propose_takes`) and
`people/<owner>` (extraction that names the owner). The resolver selects the
*default* canonical owner string for reads; it does not merge those other
strings. Per-take attribution for other people (e.g. `people/george`) is
unaffected and correct.
+109
View File
@@ -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;
+3 -10
View File
@@ -23,7 +23,6 @@ import { runPhaseCalibrationProfile } from '../core/cycle/calibration-profile.ts
import { sourceScopeOpts, type OperationContext } from '../core/operations.ts';
import type { GBrainConfig } from '../core/config.ts';
import { GBrainError } from '../core/types.ts';
import { resolveOwnerHolder } from '../core/owner-holder.ts';
export interface CalibrationProfileRow {
/** BIGSERIAL → string (postgres.js int8 wire shape; never Number() — int8
@@ -168,10 +167,7 @@ export async function runCalibration(
config: GBrainConfig,
): Promise<void> {
const { opts } = parseArgs(args);
const holder = resolveOwnerHolder({
override: opts.holder,
configValue: await engine.getConfig('emotional_weight.user_holder'),
});
const holder = opts.holder ?? 'garry';
// Resolve --source / GBRAIN_SOURCE / .gbrain-source so the (now reachable, #2035)
// calibration command targets the right source in a multi-source brain instead
// of always reading `default`. No signal → 'default' (prior behavior).
@@ -257,15 +253,12 @@ export async function getCalibrationProfileOp(
ctx: OperationContext,
params: { holder?: string },
): Promise<CalibrationProfileRow | null> {
const holder = resolveOwnerHolder({
override: params.holder,
configValue: await ctx.engine.getConfig('emotional_weight.user_holder'),
});
const holder = params.holder ?? 'garry';
if (typeof holder !== 'string' || holder.length === 0) {
throw new GBrainError(
'INVALID_HOLDER',
'get_calibration_profile.holder must be a non-empty string',
'pass holder="<slug>" or omit to default to the owner holder (config emotional_weight.user_holder, else "self")',
'pass holder="<slug>" or omit to default to "garry"',
);
}
const scope = sourceScopeOpts(ctx);
+2 -8
View File
@@ -28,7 +28,6 @@ import type { DbUrlSource } from '../core/config.ts';
import { gbrainPath, loadConfig } from '../core/config.ts';
import { reflexEnabled } from '../core/context/reflex.ts';
import { resolveSocketPath } from '../core/context/resolve-ipc.ts';
import { resolveOwnerHolder } from '../core/owner-holder.ts';
import { homedir } from 'os';
import { dirname, isAbsolute, join, resolve as resolvePath } from 'path';
import { fileURLToPath } from 'url';
@@ -1288,19 +1287,14 @@ export async function checkAbandonedThreads(engine: BrainEngine): Promise<Check>
/**
* calibration_freshness: warns when the active calibration profile is
* older than 7 days (configurable). Default holder resolves via resolveOwnerHolder
* (config emotional_weight.user_holder, else 'self'). Multi-source
* older than 7 days (configurable). Default holder 'garry'. Multi-source
* brains see one row per source; this check uses the most recent across
* all sources.
*/
export async function checkCalibrationFreshness(engine: BrainEngine): Promise<Check> {
try {
const ownerHolder = resolveOwnerHolder({
configValue: await engine.getConfig('emotional_weight.user_holder'),
});
const rows = await engine.executeRaw<{ generated_at: Date | null }>(
`SELECT MAX(generated_at) AS generated_at FROM calibration_profiles WHERE holder = $1`,
[ownerHolder],
`SELECT MAX(generated_at) AS generated_at FROM calibration_profiles WHERE holder = 'garry'`,
);
const generated = rows[0]?.generated_at;
if (!generated) {
+3 -4
View File
@@ -45,7 +45,6 @@ import {
type IngestionContentType,
type IngestionEvent,
} from '../core/ingestion/types.ts';
import { resolveOwnerHolder } from '../core/owner-holder.ts';
/**
* /health endpoint timeout. 3s rather than 5s: Fly.io's default
@@ -1191,7 +1190,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
app.get('/admin/api/calibration/pattern/:id', requireAdmin, async (req: Request, res: Response) => {
try {
const { getLatestProfile } = await import('./calibration.ts');
const holder = resolveOwnerHolder({ override: (req.query.holder as string) || undefined, configValue: await engine.getConfig('emotional_weight.user_holder') });
const holder = (req.query.holder as string) || 'garry';
const profile = await getLatestProfile(engine, { holder });
if (!profile) {
res.status(404).json({ error: 'no_profile' });
@@ -1241,7 +1240,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
app.get('/admin/api/calibration/profile', requireAdmin, async (req: Request, res: Response) => {
try {
const { getLatestProfile } = await import('./calibration.ts');
const holder = resolveOwnerHolder({ override: (req.query.holder as string) || undefined, configValue: await engine.getConfig('emotional_weight.user_holder') });
const holder = (req.query.holder as string) || 'garry';
const profile = await getLatestProfile(engine, { holder });
res.json(profile);
} catch (err) {
@@ -1258,7 +1257,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption
renderAbandonedThreadsCard,
renderPatternStatementsCard,
} = await import('../core/calibration/svg-renderer.ts');
const holder = resolveOwnerHolder({ override: (req.query.holder as string) || undefined, configValue: await engine.getConfig('emotional_weight.user_holder') });
const holder = (req.query.holder as string) || 'garry';
const type = req.params.type;
const profile = await getLatestProfile(engine, { holder });
+1 -2
View File
@@ -29,7 +29,6 @@ import {
} from '../core/takes-fence.ts';
import { withPageLock } from '../core/page-lock.ts';
import { resolveSourceId } from '../core/source-resolver.ts';
import { resolveOwnerHolder } from '../core/owner-holder.ts';
// --- Helpers ---
@@ -365,7 +364,7 @@ async function cmdResolve(engine: BrainEngine, args: string[], sourceId?: string
// --evidence is the v0.30.0 alias for --source on the resolve subcommand
// (semantic clarity: "what evidence resolved this bet?").
const source = flagValue(args, '--evidence') ?? flagValue(args, '--source');
const resolvedBy = flagValue(args, '--by') ?? resolveOwnerHolder({ configValue: await engine.getConfig('emotional_weight.user_holder') });
const resolvedBy = flagValue(args, '--by') ?? 'garry';
const dirArg = flagValue(args, '--dir');
const pageId = await getPageId(engine, slug, sourceId);
+2 -3
View File
@@ -68,7 +68,6 @@ import {
type BrainstormCheckpoint,
type CheckpointCross,
} from './checkpoint.ts';
import { resolveOwnerHolder } from '../owner-holder.ts';
export { BudgetExhausted };
@@ -140,7 +139,7 @@ export interface BrainstormOptions {
modelOverride?: string;
/** Skip the cost-preview TTY grace window. Required for non-interactive callers. */
skipCostPreview?: boolean;
/** When set, force the user holder for calibration profile lookup. Falls back to config (`emotional_weight.user_holder`) then `'self'`. */
/** When set, force the user holder for calibration profile lookup. Falls back to config (`emotional_weight.user_holder`) then `'garry'`. */
holderOverride?: string;
/** Source scope. */
sourceId?: string;
@@ -624,7 +623,7 @@ async function _runBrainstormInner(
}
// ---- Phase 3: calibration context (cold-start fallback) ----
const holder = resolveOwnerHolder({ override: opts.holderOverride, configValue: config.emotional_weight?.user_holder });
const holder = opts.holderOverride ?? config.emotional_weight?.user_holder ?? 'garry';
const calibContext = await loadCalibrationContext(engine, {
holder,
sourceId: opts.sourceId,
+1 -1
View File
@@ -25,7 +25,7 @@ import type { BrainEngine } from '../engine.ts';
export interface ABRunInput {
question: string;
/** Holder context for calibration. Resolves via resolveOwnerHolder (config emotional_weight.user_holder, else 'self'). */
/** Holder context for calibration. Default 'garry'. */
holder?: string;
/** Engine for DB write. */
engine: BrainEngine;
+2 -6
View File
@@ -26,7 +26,6 @@
*/
import { BaseCyclePhase, type ScopedReadOpts, type BasePhaseOpts } from './base-phase.ts';
import { resolveOwnerHolder } from '../owner-holder.ts';
import { chat as gatewayChat } from '../ai/gateway.ts';
import { TIER_DEFAULTS } from '../model-config.ts';
import { gateVoice, type VoiceGateGenerator, type VoiceGateJudge } from '../calibration/voice-gate.ts';
@@ -97,7 +96,7 @@ export type PatternStatementsGenerator = (input: {
export type BiasTagsGenerator = (patterns: string[]) => Promise<string[]>;
export interface CalibrationProfileOpts extends BasePhaseOpts {
/** Holder to generate the profile for. Default resolves via resolveOwnerHolder (config emotional_weight.user_holder, else 'self'). */
/** Holder to generate the profile for. Default 'garry'. */
holder?: string;
/** Inject the patterns generator (tests). */
patternsGenerator?: PatternStatementsGenerator;
@@ -228,10 +227,7 @@ class CalibrationProfilePhase extends BaseCyclePhase {
_ctx: OperationContext,
opts: CalibrationProfileOpts,
): Promise<{ summary: string; details: Record<string, unknown>; status?: PhaseStatus }> {
const holder = resolveOwnerHolder({
override: opts.holder,
configValue: await engine.getConfig('emotional_weight.user_holder'),
});
const holder = opts.holder ?? 'garry';
const promptVersion = opts.promptVersion ?? CALIBRATION_PROFILE_PROMPT_VERSION;
const modelId = opts.model ?? TIER_DEFAULTS.reasoning;
const gradeCompletion = opts.gradeCompletion ?? 1.0;
+4 -7
View File
@@ -14,8 +14,6 @@
* See `loadHighEmotionTags` for the resolution path.
*/
import { DEFAULT_OWNER_HOLDER } from '../owner-holder.ts';
/**
* Default high-emotion tag seed list. Pages with any tag in this set get the
* tag-emotion boost in the formula below. Override via config key
@@ -45,12 +43,11 @@ export const HIGH_EMOTION_TAGS: ReadonlySet<string> = new Set([
]);
/**
* Holder name treated as "the user" for the user-as-holder ratio. Configurable
* via the `emotional_weight.user_holder` config key; defaults to the canonical
* owner holder ('self', DEFAULT_OWNER_HOLDER) so it matches the consolidate
* facts→takes writer instead of a hardcoded name.
* Holder name treated as "the user" for the Garry-as-holder ratio. Configurable
* via the `emotional_weight.user_holder` config key (defaults to 'garry' to
* match the v0.28 schema's takes table convention).
*/
export const DEFAULT_USER_HOLDER = DEFAULT_OWNER_HOLDER;
export const DEFAULT_USER_HOLDER = 'garry';
export interface EmotionalWeightTake {
holder: string;
+1 -1
View File
@@ -3289,7 +3289,7 @@ const get_calibration_profile: Operation = {
holder: {
type: 'string',
description:
"Holder slug, e.g. 'self' or 'people/charlie-example'. Defaults to config emotional_weight.user_holder, else 'self', when omitted.",
"Holder slug, e.g. 'garry' or 'people/charlie-example'. Defaults to 'garry' when omitted.",
},
},
handler: async (ctx, p) => {
-24
View File
@@ -1,24 +0,0 @@
/**
* Canonical holder string for "the brain owner," resolved in ONE place so the
* calibration / think / doctor / emotional-weight defaults stop disagreeing.
*
* The default matches the consolidate factstakes writer
* (src/core/cycle/phases/consolidate.ts: holder:'self') and docs/takes-vs-facts.md.
* Do NOT introduce a fourth literal three already exist historically
* ('garry', 'system', 'self'); this is the source of truth.
*
* NORMALIZATION NOTE: the brain owner may also appear under other holder
* strings 'brain' (propose_takes when the author asserts a claim) and
* people/<owner> (extraction that names the owner). This resolver only selects
* the *default* canonical owner string for reads; it does NOT merge those other
* strings. Unifying them is owner-identity entity-resolution, tracked separately
* (see garrytan/gbrain#2465). Until then, historical owner takes
* under 'brain'/people-<owner> are not folded into the default profile.
*/
export const DEFAULT_OWNER_HOLDER = 'self';
export function resolveOwnerHolder(
opts: { override?: string | null; configValue?: string | null },
): string {
return opts.override ?? opts.configValue ?? DEFAULT_OWNER_HOLDER;
}
+3 -7
View File
@@ -23,7 +23,6 @@ import { runGather, renderPagesBlock, takesHitToTakeForPrompt } from './gather.t
import { renderTakesBlock } from './sanitize.ts';
import { buildThinkSystemPrompt, buildThinkUserMessage } from './prompt.ts';
import { resolveCitations, type ParsedCitation } from './cite-render.ts';
import { resolveOwnerHolder } from '../owner-holder.ts';
import { resolveModel } from '../model-config.ts';
import { chat as gatewayChat, probeChatModel, type ChatResult } from '../ai/gateway.ts';
import { AIConfigError } from '../ai/errors.ts';
@@ -77,8 +76,8 @@ export interface RunThinkOpts {
*/
withCalibration?: boolean;
/**
* Holder to retrieve the calibration profile for. Resolves via resolveOwnerHolder
* (config emotional_weight.user_holder, else 'self'). Only consulted when withCalibration=true.
* Holder to retrieve the calibration profile for. Default 'garry'. Only
* consulted when withCalibration=true.
*/
calibrationHolder?: string;
/**
@@ -309,10 +308,7 @@ export async function runThink(
try {
const { getLatestProfile } = await import('../../commands/calibration.ts');
const profile = await getLatestProfile(engine, {
holder: resolveOwnerHolder({
override: opts.calibrationHolder,
configValue: await engine.getConfig('emotional_weight.user_holder'),
}),
holder: opts.calibrationHolder ?? 'garry',
});
if (profile) {
calibrationBlockOpts = {
@@ -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');
}
});
});
+7 -13
View File
@@ -27,12 +27,6 @@ function buildMockEngine(opts: { rows: CalibrationProfileRow[] }): {
const capturedParams: unknown[][] = [];
const engine = {
kind: 'pglite',
// #2464: getCalibrationProfileOp resolves the owner holder via
// resolveOwnerHolder(config emotional_weight.user_holder, else 'self'), so the
// mock must implement getConfig. null = key unset → resolver falls back to 'self'.
async getConfig(): Promise<string | null> {
return null;
},
async executeRaw<T>(sql: string, params?: unknown[]): Promise<T[]> {
capturedSql.push(sql);
capturedParams.push(params ?? []);
@@ -217,17 +211,17 @@ describe('formatProfileText', () => {
// ─── getCalibrationProfileOp ────────────────────────────────────────
describe('getCalibrationProfileOp (MCP)', () => {
test('defaults holder to "self" when omitted (config emotional_weight.user_holder unset)', async () => {
const { engine } = buildMockEngine({ rows: [buildProfile({ holder: 'self' })] });
test('defaults holder to "garry" when omitted', async () => {
const { engine } = buildMockEngine({ rows: [buildProfile({ holder: 'garry' })] });
const ctx = buildCtx(engine);
const result = await getCalibrationProfileOp(ctx, {});
expect(result?.holder).toBe('self');
expect(result?.holder).toBe('garry');
});
test('routes through sourceScopeOpts: scalar source-bound client gets source-scoped result', async () => {
const rows = [
buildProfile({ holder: 'self', source_id: 'default' }),
buildProfile({ holder: 'self', source_id: 'tenant-b' }),
buildProfile({ holder: 'garry', source_id: 'default' }),
buildProfile({ holder: 'garry', source_id: 'tenant-b' }),
];
const { engine } = buildMockEngine({ rows });
const ctx = buildCtx(engine, { sourceId: 'tenant-b' });
@@ -237,8 +231,8 @@ describe('getCalibrationProfileOp (MCP)', () => {
test('federated read scope sees the union of allowed sources', async () => {
const rows = [
buildProfile({ holder: 'self', source_id: 'tenant-a' }),
buildProfile({ holder: 'self', source_id: 'tenant-z' }),
buildProfile({ holder: 'garry', source_id: 'tenant-a' }),
buildProfile({ holder: 'garry', source_id: 'tenant-z' }),
];
const { engine } = buildMockEngine({ rows });
const ctx = buildCtx(engine, { allowedSources: ['tenant-a', 'tenant-b'] });
+2 -26
View File
@@ -32,7 +32,7 @@ interface CapturedSql {
params: unknown[];
}
function buildMockEngine(opts: { scorecard: TakesScorecard; userHolder?: string | null }): {
function buildMockEngine(opts: { scorecard: TakesScorecard }): {
engine: BrainEngine;
captured: CapturedSql[];
} {
@@ -42,10 +42,6 @@ function buildMockEngine(opts: { scorecard: TakesScorecard; userHolder?: string
async getScorecard() {
return opts.scorecard;
},
async getConfig(key: string): Promise<string | null> {
if (key === 'emotional_weight.user_holder') return opts.userHolder ?? null;
return null;
},
async executeRaw<T>(sql: string, params?: unknown[]): Promise<T[]> {
captured.push({ sql, params: params ?? [] });
return [];
@@ -238,7 +234,7 @@ describe('runPhaseCalibrationProfile — phase integration', () => {
// grade_completion, domain_scorecards_json, patterns[], voice_passed, voice_attempts,
// bias_tags[], model_id
expect(insert!.params[0]).toBe('default'); // source_id
expect(insert!.params[1]).toBe('self'); // holder (resolved via resolveOwnerHolder, no override)
expect(insert!.params[1]).toBe('garry'); // holder
expect(insert!.params[2]).toBe(12); // total_resolved
expect(insert!.params[9]).toBe(true); // voice_gate_passed
expect(insert!.params[10]).toBe(1); // voice_gate_attempts
@@ -334,24 +330,4 @@ describe('runPhaseCalibrationProfile — phase integration', () => {
const insert = captured.find(c => c.sql.includes('INSERT INTO calibration_profiles'));
expect(insert!.params[0]).toBe('tenant-b');
});
test('cold-brain summary uses resolved owner holder self when user_holder unset', async () => {
const { engine } = buildMockEngine({
scorecard: { total_bets: 0, resolved: 0, correct: 0, incorrect: 0, partial: 0,
accuracy: null, brier: null, partial_rate: null, unresolvable_count: 0, unresolvable_rate: null },
});
const result = await runPhaseCalibrationProfile(buildCtx(engine), {});
expect(result.summary).toContain('holder=self');
expect(result.summary).not.toContain('holder=garry');
});
test('configured user_holder overrides the default in the cold-brain summary', async () => {
const { engine } = buildMockEngine({
scorecard: { total_bets: 0, resolved: 0, correct: 0, incorrect: 0, partial: 0,
accuracy: null, brier: null, partial_rate: null, unresolvable_count: 0, unresolvable_rate: null },
userHolder: 'people/charlie-example',
});
const result = await runPhaseCalibrationProfile(buildCtx(engine), {});
expect(result.summary).toContain('holder=people/charlie-example');
});
});
-6
View File
@@ -32,12 +32,6 @@ function buildMockEngine(opts: {
}): BrainEngine {
return {
kind: 'pglite',
// #2464: checkCalibrationFreshness resolves the owner holder via
// resolveOwnerHolder(config emotional_weight.user_holder, else 'self'), so the
// mock must implement getConfig. null = key unset → resolver falls back to 'self'.
async getConfig(): Promise<string | null> {
return null;
},
async executeRaw<T>(sql: string): Promise<T[]> {
if (opts.throwOn && opts.throwOn.test(sql)) {
throw new Error('mock engine error: ' + sql.slice(0, 50));
-4
View File
@@ -112,7 +112,3 @@ describe('computeEmotionalWeight', () => {
expect(HIGH_EMOTION_TAGS.has('mental-health')).toBe(true);
});
});
test('DEFAULT_USER_HOLDER is the canonical owner holder self', () => {
expect(DEFAULT_USER_HOLDER).toBe('self');
});
-27
View File
@@ -1,27 +0,0 @@
import { describe, test, expect } from 'bun:test';
import { resolveOwnerHolder, DEFAULT_OWNER_HOLDER } from '../src/core/owner-holder.ts';
describe('owner-holder', () => {
test('DEFAULT_OWNER_HOLDER is self', () => {
expect(DEFAULT_OWNER_HOLDER).toBe('self');
});
test('defaults to self when nothing provided', () => {
expect(resolveOwnerHolder({})).toBe('self');
});
test('null/undefined config falls back to self', () => {
expect(resolveOwnerHolder({ configValue: null })).toBe('self');
expect(resolveOwnerHolder({ configValue: undefined })).toBe('self');
});
test('uses config value when set and no override', () => {
expect(resolveOwnerHolder({ configValue: 'people/charlie-example' }))
.toBe('people/charlie-example');
});
test('override beats config and default', () => {
expect(resolveOwnerHolder({ override: 'world', configValue: 'people/charlie-example' }))
.toBe('world');
});
});