Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions bin/knowledge-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -27900,6 +27900,26 @@ function pinnedTransportEnv(env, mode2) {
return { ...env, [KNOWLEDGE_MODE_ENV_KEYS[0]]: mode2 };
}

class HalfConfiguredKnowledgeClientError extends Error {
code = "knowledge_mode_unset_with_api_url";
constructor(urlKeysPresent) {
const canonical = KNOWLEDGE_MODE_ENV_KEYS[0];
super(`knowledge: ${urlKeysPresent.join(", ")} names an API store, but no mode variable says to use it, ` + "so this command would silently read and write the on-box store instead. " + `Set ${canonical}=cloud to use the API, or ${canonical}=local to confirm you want the on-box store. ` + `Run 'knowledge mode' to see the full resolution.`);
this.name = "HalfConfiguredKnowledgeClientError";
}
}
function assertKnowledgeModeSelected(env = process.env, options = {}) {
const resolution = resolveKnowledgeModeSelection(env);
if (options.storePathOverridden)
return resolution;
if (resolution.source.kind !== "default")
return resolution;
const urlKeysPresent = presentEnvNames(env, KNOWLEDGE_API_URL_ENV_KEYS);
if (urlKeysPresent.length === 0)
return resolution;
throw new HalfConfiguredKnowledgeClientError(urlKeysPresent);
}

// src/cloud-store.ts
function transportOverrides(env) {
return {
Expand Down Expand Up @@ -42406,6 +42426,7 @@ function resolveStorePath(storePath, scope) {
return defaultStorePath();
}
function itemStoreFor(storePath, scope) {
assertKnowledgeModeSelected(process.env, { storePathOverridden: Boolean(storePath) });
const resolved = resolveStorePath(storePath, scope);
return resolveItemStore({ storePath: resolved, storePathOverridden: Boolean(storePath) });
}
Expand Down
382 changes: 191 additions & 191 deletions bin/knowledge.js

Large diffs are not rendered by default.

31 changes: 31 additions & 0 deletions dist/knowledge-mode.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,37 @@ export declare function resolveKnowledgeModeSelection(env?: NodeJS.ProcessEnv):
* put the backend choice back in a second layer.
*/
export declare function pinnedTransportEnv(env: NodeJS.ProcessEnv, mode: KnowledgeMode): NodeJS.ProcessEnv;
/**
* Raised when the environment names a store but never says to use it.
*
* Carries a `code` so callers can branch on the condition without matching on
* message text.
*/
export declare class HalfConfiguredKnowledgeClientError extends Error {
readonly code = "knowledge_mode_unset_with_api_url";
constructor(urlKeysPresent: readonly string[]);
}
/**
* Gate a store-touching command on an UNAMBIGUOUS environment.
*
* Deliberately separate from {@link resolveKnowledgeModeSelection}, which stays
* total and non-throwing. The resolver has to keep answering `local` in exactly
* the environment this rejects, because `knowledge mode` — the command whose
* whole job is explaining the situation — resolves through it. A guard fused
* into the resolver would kill the diagnostic along with the defect.
*
* Fires on an API URL only, never on a key alone. A key with no URL points at
* no store, so there is nothing to be ambiguous about; erroring there would
* fire on machines that could never have routed anywhere, and a check that
* cries wolf is a check somebody turns off.
*
* `storePathOverridden` (an explicit `--store <path>`) is an explicit local
* choice and passes for the same reason `MODE=local` does: the operator said
* which store they meant.
*/
export declare function assertKnowledgeModeSelected(env?: NodeJS.ProcessEnv, options?: {
storePathOverridden?: boolean;
}): KnowledgeModeResolution;
export interface KnowledgeModeReport extends KnowledgeModeResolution {
/** `local` -> the on-box store; `api` -> the HTTP `/v1` transport. */
store_transport: 'local' | 'api';
Expand Down
11 changes: 11 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
KNOWLEDGE_API_KEY_ENV_KEYS,
KNOWLEDGE_API_URL_ENV_KEYS,
KNOWLEDGE_MODE_ENV_KEYS,
assertKnowledgeModeSelected,
knowledgeModeReport,
type KnowledgeModeReport,
} from './knowledge-mode';
Expand Down Expand Up @@ -897,6 +898,16 @@ async function run(argv: string[]): Promise<void> {
return;
}

// Refuse to guess. An API URL with no mode variable is a HALF-CONFIGURED
// client: the environment names a store and never says to use it, and every
// command below would answer from the on-box store with exit 0 — measured on
// station01 as 98 entries against the 869 in the store the URL names. A
// silent wrong answer is worse than no answer, so this stops here and names
// the variable. Placed AFTER `mode` on purpose: the diagnostic that explains
// this state has to keep working in it. `--store` is an explicit local
// choice and passes, exactly like an explicit mode=local.
assertKnowledgeModeSelected(process.env, { storePathOverridden: Boolean(flags.store) });

const serviceScope = command === 'project-panel' || command === 'app-wiki' ? (flags.scope ?? 'project') : flags.scope;
const service = createKnowledgeService({ scope: serviceScope });
if (command === 'storage') {
Expand Down
50 changes: 50 additions & 0 deletions src/knowledge-mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,56 @@ export function pinnedTransportEnv(env: NodeJS.ProcessEnv, mode: KnowledgeMode):
return { ...env, [KNOWLEDGE_MODE_ENV_KEYS[0]]: mode };
}

/**
* Raised when the environment names a store but never says to use it.
*
* Carries a `code` so callers can branch on the condition without matching on
* message text.
*/
export class HalfConfiguredKnowledgeClientError extends Error {
readonly code = 'knowledge_mode_unset_with_api_url';
constructor(urlKeysPresent: readonly string[]) {
const canonical = KNOWLEDGE_MODE_ENV_KEYS[0];
super(
`knowledge: ${urlKeysPresent.join(', ')} names an API store, but no mode variable says to use it, `
+ 'so this command would silently read and write the on-box store instead. '
+ `Set ${canonical}=cloud to use the API, or ${canonical}=local to confirm you want the on-box store. `
+ `Run 'knowledge mode' to see the full resolution.`,
);
this.name = 'HalfConfiguredKnowledgeClientError';
}
}

/**
* Gate a store-touching command on an UNAMBIGUOUS environment.
*
* Deliberately separate from {@link resolveKnowledgeModeSelection}, which stays
* total and non-throwing. The resolver has to keep answering `local` in exactly
* the environment this rejects, because `knowledge mode` — the command whose
* whole job is explaining the situation — resolves through it. A guard fused
* into the resolver would kill the diagnostic along with the defect.
*
* Fires on an API URL only, never on a key alone. A key with no URL points at
* no store, so there is nothing to be ambiguous about; erroring there would
* fire on machines that could never have routed anywhere, and a check that
* cries wolf is a check somebody turns off.
*
* `storePathOverridden` (an explicit `--store <path>`) is an explicit local
* choice and passes for the same reason `MODE=local` does: the operator said
* which store they meant.
*/
export function assertKnowledgeModeSelected(
env: NodeJS.ProcessEnv = process.env,
options: { storePathOverridden?: boolean } = {},
): KnowledgeModeResolution {
const resolution = resolveKnowledgeModeSelection(env);
if (options.storePathOverridden) return resolution;
if (resolution.source.kind !== 'default') return resolution;
const urlKeysPresent = presentEnvNames(env, KNOWLEDGE_API_URL_ENV_KEYS);
if (urlKeysPresent.length === 0) return resolution;
throw new HalfConfiguredKnowledgeClientError(urlKeysPresent);
}

export interface KnowledgeModeReport extends KnowledgeModeResolution {
/** `local` -> the on-box store; `api` -> the HTTP `/v1` transport. */
store_transport: 'local' | 'api';
Expand Down
7 changes: 7 additions & 0 deletions src/mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { migrateKnowledgeDb, openKnowledgeDb } from './knowledge-db.ts';
import { defaultStorePath } from './store.ts';
import { resolveItemStore } from './item-store.ts';
import { isKnowledgeApiMode } from './cloud-store.ts';
import { assertKnowledgeModeSelected } from './knowledge-mode.ts';
import { parseSourceRef } from './source-ref.ts';
import { createKnowledgeService } from './service.ts';
import { getStorageStatus as getDatabaseStorageStatus } from './storage.ts';
Expand Down Expand Up @@ -43,6 +44,12 @@ function resolveStorePath(storePath, scope) {
* Every MCP item tool routes through this Store — never the JSON file directly.
*/
function itemStoreFor(storePath, scope) {
// Same gate the CLI applies, and this surface needs it more: an agent calling
// an MCP item tool never sees a `knowledge mode` line, so a half-configured
// client hands it an empty result it reads as "the corpus is empty". Throwing
// here surfaces as an MCP tool error naming the variable rather than a
// plausible empty list. An explicit `store_path` is an explicit local choice.
assertKnowledgeModeSelected(process.env, { storePathOverridden: Boolean(storePath) });
const resolved = resolveStorePath(storePath, scope);
return resolveItemStore({ storePath: resolved, storePathOverridden: Boolean(storePath) });
}
Expand Down
127 changes: 126 additions & 1 deletion tests/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { tmpdir } from 'node:os';
import { delimiter, join, dirname, resolve } from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { migrateKnowledgeDb, openKnowledgeDb } from '../src/knowledge-db';
import { KNOWLEDGE_API_KEY_ENV_KEYS, KNOWLEDGE_API_URL_ENV_KEYS } from '../src/knowledge-mode';
import { KNOWLEDGE_API_KEY_ENV_KEYS, KNOWLEDGE_API_URL_ENV_KEYS, KNOWLEDGE_MODE_ENV_KEYS } from '../src/knowledge-mode';
import { createKnowledgeService } from '../src/service';
import { parseSourceRef } from '../src/source-ref';
import { recordStorageObjects } from '../src/storage-contract';
Expand Down Expand Up @@ -3598,3 +3598,128 @@ describe('knowledge cli', () => {
expect(items.find((entry) => entry.id === 'k_arch_1')!.archived).toBe(true);
});
});

/**
* The half-configured client, at the surface an operator actually touches.
*
* `knowledge mode` has reported this state correctly since the explicit-mode
* work landed. Every other command stayed silent about it, which is the half
* that matters: an agent runs `knowledge list --json`, gets
* `{"ok": true, "total": 0, "items": []}` and exit 0, and concludes the corpus
* is empty — on a machine whose HASNA_KNOWLEDGE_API_URL points at a store
* holding 869 entries.
*
* These spawn the CLI rather than calling the resolver, because the defect is
* the exit code and the stream contents, and only a real invocation measures
* those.
*/
describe('a half-configured CLI fails loudly instead of reading the wrong store', () => {
const decode = (buf: Uint8Array) => new TextDecoder().decode(buf);

/**
* The parent env minus BOTH pointer and mode vars, so the case under test is
* the one the overrides describe. `childEnv` only strips pointers, and this
* developer shell exports a mode var — inheriting it would silence the very
* error being asserted and the test would pass for the wrong reason.
*/
function runCliNoMode(args: string[], env: Record<string, string>) {
const inherited = { ...process.env } as Record<string, string>;
for (const key of [...KNOWLEDGE_API_URL_ENV_KEYS, ...KNOWLEDGE_API_KEY_ENV_KEYS, ...KNOWLEDGE_MODE_ENV_KEYS]) {
delete inherited[key];
}
return Bun.spawnSync(['bun', CLI, ...args], {
env: { ...inherited, ...env },
stdout: 'pipe',
stderr: 'pipe',
});
}

/** A syntactically valid pointer at nothing. The outbound guard blocks it regardless. */
const FAKE_API_URL = 'https://knowledge.invalid';

function sandboxHome(): Record<string, string> {
const home = mkdtempSync(join(tmpdir(), 'knowledge-halfconf-'));
return { HOME: home, USERPROFILE: home };
}

test('REGRESSION: `list --json` with an API URL and no mode var exits non-zero', () => {
const result = runCliNoMode(['list', '--json'], {
...sandboxHome(),
HASNA_KNOWLEDGE_API_URL: FAKE_API_URL,
HASNA_KNOWLEDGE_API_KEY: 'k_fake_test_key',
});
const stdout = decode(result.stdout);
const stderr = decode(result.stderr);

// On 0.2.92 this was exit 0 with a successful-looking empty page.
expect(result.exitCode).not.toBe(0);
expect(stderr).toContain('HASNA_KNOWLEDGE_API_URL');
expect(stderr).toContain('HASNA_KNOWLEDGE_STORAGE_MODE');

// The JSON contract must report the failure too. A consumer parsing stdout
// sees `ok: false` rather than an empty page it would read as "no entries".
const payload = JSON.parse(stdout) as { ok: boolean; error?: string };
expect(payload.ok).toBe(false);
expect(payload.error).toContain('HASNA_KNOWLEDGE_STORAGE_MODE');
// Positive control on the defect itself: no empty success page anywhere.
expect(stdout).not.toContain('"total": 0');
});

test('REGRESSION: the error names no pointer VALUE, only variable names', () => {
const secretish = 'k_super_secret_value';
const result = runCliNoMode(['list', '--json'], {
...sandboxHome(),
HASNA_KNOWLEDGE_API_URL: FAKE_API_URL,
HASNA_KNOWLEDGE_API_KEY: secretish,
});
const combined = decode(result.stdout) + decode(result.stderr);
expect(combined).not.toContain(secretish);
expect(combined).not.toContain(FAKE_API_URL);
});

test('`mode` still answers in the environment the guard rejects', () => {
// The command whose entire job is explaining this state must survive it,
// or the error message points at a diagnostic that also fails.
const result = runCliNoMode(['mode', '--json'], {
...sandboxHome(),
HASNA_KNOWLEDGE_API_URL: FAKE_API_URL,
});
expect(result.exitCode).toBe(0);
const report = JSON.parse(decode(result.stdout)) as { ok: boolean; mode: string; pointer_ignored: boolean };
expect(report.ok).toBe(true);
expect(report.mode).toBe('local');
expect(report.pointer_ignored).toBe(true);
});

test('an explicit local mode alongside the URL still lists, and stays local', () => {
const result = runCliNoMode(['list', '--json'], {
...sandboxHome(),
HASNA_KNOWLEDGE_API_URL: FAKE_API_URL,
HASNA_KNOWLEDGE_STORAGE_MODE: 'local',
});
expect(result.exitCode).toBe(0);
const payload = JSON.parse(decode(result.stdout)) as { ok: boolean; total: number };
expect(payload.ok).toBe(true);
expect(payload.total).toBe(0);
});

test('an explicit --store override is an explicit local choice and still lists', () => {
const dir = mkdtempSync(join(tmpdir(), 'knowledge-halfconf-store-'));
const store = join(dir, 'db.json');
writeFileSync(store, JSON.stringify({ items: [] }));
const result = runCliNoMode(['list', '--store', store, '--json'], {
...sandboxHome(),
HASNA_KNOWLEDGE_API_URL: FAKE_API_URL,
});
expect(result.exitCode).toBe(0);
expect((JSON.parse(decode(result.stdout)) as { ok: boolean }).ok).toBe(true);
});

test('a clean environment with no pointer at all is untouched', () => {
// Positive control: the guard must not fire on the ordinary local install,
// which is the overwhelming majority of invocations.
const result = runCliNoMode(['list', '--json'], sandboxHome());
expect(result.exitCode).toBe(0);
expect((JSON.parse(decode(result.stdout)) as { ok: boolean }).ok).toBe(true);
});
});
Loading
Loading