mirror of
https://github.com/Kaelio/ktx.git
synced 2026-07-25 12:01:03 +02:00
refactor(mongo-query): consolidate query types and drop unsafe cast
Route the Mongo read path through one canonical request/result shape and remove the `as unknown as` narrowing flagged in review. - Add optional executeQuery to KtxScanConnector and narrow by capability in the runner, replacing the double cast with a driver + method guard. - Move KtxMongoQueryInput/KtxMongoQueryResult into context/scan/types.ts; delete the duplicate runner request type, the inline MCP-port input, and the KtxMongoQueryResponse alias. KtxMongoQueryArgs now extends the base. - Inline the pass-through executeMongoQuery wrapper. - Correct the --output default in the mongo-query and sql CLI docs: it is TTY/CI/KTX_OUTPUT-derived, not unconditionally pretty. - Add tests: database override, empty result, parseLimitOption bounds, bigint/plural-row/empty-header rendering, and mongo_query_completed telemetry fields on both outcomes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga2tvQ2YCEphmY8HM5Guzq
This commit is contained in:
parent
a236ec9d91
commit
0e2e6c7d90
14 changed files with 139 additions and 65 deletions
|
|
@ -26,7 +26,7 @@ ktx mongo-query '<pipeline-json>' --connection <id> --collection <name> [options
|
|||
| `--collection <name>` | Collection to query. Required. | - |
|
||||
| `--database <name>` | Database name. | Connection's first configured database |
|
||||
| `--limit <n>` | Maximum documents to return. Must be between `1` and `10000`. | `1000` |
|
||||
| `--output <mode>` | Output mode: `pretty`, `plain` (TSV), or `json`. | `pretty` |
|
||||
| `--output <mode>` | Output mode: `pretty`, `plain` (TSV), or `json`. | `pretty` on a terminal, `plain` when piped or in CI (override with `KTX_OUTPUT`) |
|
||||
| `--json` | Shortcut for `--output=json` (overrides `--output`). | `false` |
|
||||
|
||||
## Examples
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ JSON.
|
|||
|------|-------------|---------|
|
||||
| `-c`, `--connection <id>` | **ktx** database connection id. Required. | - |
|
||||
| `--max-rows <n>` | Maximum rows to return. Must be between `1` and `10000`. | `1000` |
|
||||
| `--output <mode>` | Output mode: `pretty`, `plain` (TSV), or `json`. | `pretty` |
|
||||
| `--output <mode>` | Output mode: `pretty`, `plain` (TSV), or `json`. | `pretty` on a terminal, `plain` when piped or in CI (override with `KTX_OUTPUT`) |
|
||||
| `--json` | Shortcut for `--output=json` (overrides `--output`). | `false` |
|
||||
|
||||
## Examples
|
||||
|
|
|
|||
|
|
@ -7,7 +7,8 @@ profileMark('module:commands/mongo-query-commands');
|
|||
const DEFAULT_LIMIT = 1000;
|
||||
const LIMIT_CAP = 10_000;
|
||||
|
||||
function parseLimitOption(value: string): number {
|
||||
/** @internal exported only for unit testing */
|
||||
export function parseLimitOption(value: string): number {
|
||||
const parsed = Number(value);
|
||||
if (!Number.isInteger(parsed) || parsed < 1 || parsed > LIMIT_CAP) {
|
||||
throw new InvalidArgumentError(`must be an integer between 1 and ${LIMIT_CAP}`);
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ import {
|
|||
type KtxColumnSampleInput,
|
||||
type KtxColumnSampleResult,
|
||||
type KtxConnectorTestResult,
|
||||
type KtxMongoQueryInput,
|
||||
type KtxMongoQueryResult,
|
||||
type KtxScanConnector,
|
||||
type KtxScanContext,
|
||||
type KtxScanInput,
|
||||
|
|
@ -39,20 +41,6 @@ export interface KtxMongoListedCollection {
|
|||
type?: string;
|
||||
}
|
||||
|
||||
export interface KtxMongoQueryInput {
|
||||
connectionId: string;
|
||||
collection: string;
|
||||
database?: string;
|
||||
pipeline: Record<string, unknown>[];
|
||||
limit: number;
|
||||
}
|
||||
|
||||
export interface KtxMongoQueryResult {
|
||||
headers: string[];
|
||||
rows: unknown[][];
|
||||
rowCount: number;
|
||||
}
|
||||
|
||||
interface KtxMongoFindOptions {
|
||||
sort: Record<string, 1 | -1>;
|
||||
limit: number;
|
||||
|
|
@ -67,7 +55,7 @@ export interface KtxMongoClient {
|
|||
aggregate(
|
||||
databaseName: string,
|
||||
collectionName: string,
|
||||
pipeline: Record<string, unknown>[],
|
||||
pipeline: readonly Record<string, unknown>[],
|
||||
options: { limit: number },
|
||||
): Promise<KtxMongoDocument[]>;
|
||||
ping(databaseName: string): Promise<void>;
|
||||
|
|
@ -129,7 +117,7 @@ class DefaultMongoClient implements KtxMongoClient {
|
|||
async aggregate(
|
||||
databaseName: string,
|
||||
collectionName: string,
|
||||
pipeline: Record<string, unknown>[],
|
||||
pipeline: readonly Record<string, unknown>[],
|
||||
options: { limit: number },
|
||||
): Promise<KtxMongoDocument[]> {
|
||||
const client = await this.connectedClient();
|
||||
|
|
@ -223,7 +211,7 @@ function unionDocumentKeys(documents: readonly KtxMongoDocument[]): string[] {
|
|||
}
|
||||
|
||||
// MongoDB aggregation write stages ($out/$merge) persist results; mongo_query is read-only.
|
||||
function assertReadOnlyPipeline(pipeline: Record<string, unknown>[]): void {
|
||||
function assertReadOnlyPipeline(pipeline: readonly Record<string, unknown>[]): void {
|
||||
for (const stage of pipeline) {
|
||||
if ('$out' in stage || '$merge' in stage) {
|
||||
throw new Error('mongo_query pipelines must be read-only; $out and $merge stages are not allowed.');
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@ import { KtxDaemonComputeError, type KtxSemanticLayerComputePort } from '../../c
|
|||
import type { KtxLocalProject } from '../../context/project/project.js';
|
||||
import { createKtxEntityDetailsService } from '../../context/scan/entity-details.js';
|
||||
import type { LocalScanMcpOptions } from '../../context/scan/local-scan.js';
|
||||
import type { KtxScanConnector } from '../../context/scan/types.js';
|
||||
import { createKtxDiscoverDataService } from '../../context/search/discover.js';
|
||||
import { sqlAnalysisDialectForDriver } from '../../context/sql-analysis/dialect.js';
|
||||
import type { SqlAnalysisPort } from '../../context/sql-analysis/ports.js';
|
||||
|
|
@ -26,7 +25,7 @@ import { assertSafeConnectionId } from '../../context/sl/source-files.js';
|
|||
import { assertConfiguredConnectionId } from '../../context/connections/configured-connections.js';
|
||||
import { readLocalKnowledgePage, searchLocalKnowledgePages } from '../wiki/local-knowledge.js';
|
||||
import { runMongoQuery } from './mongo-query-runner.js';
|
||||
import type { KtxMcpContextPorts, KtxMcpProgressCallback, KtxMongoQueryResponse, KtxSqlExecutionResponse } from './types.js';
|
||||
import type { KtxMcpContextPorts, KtxMcpProgressCallback, KtxSqlExecutionResponse } from './types.js';
|
||||
|
||||
interface CreateLocalProjectMcpContextPortsOptions {
|
||||
semanticLayerCompute?: KtxSemanticLayerComputePort;
|
||||
|
|
@ -62,13 +61,6 @@ function projectHasMongoConnection(project: KtxLocalProject): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
async function executeMongoQuery(
|
||||
createConnector: (connectionId: string) => Promise<KtxScanConnector> | KtxScanConnector,
|
||||
input: { connectionId: string; collection: string; database?: string; pipeline: Record<string, unknown>[]; limit: number },
|
||||
): Promise<KtxMongoQueryResponse> {
|
||||
return runMongoQuery(createConnector, input);
|
||||
}
|
||||
|
||||
async function executeValidatedReadOnlySql(
|
||||
project: KtxLocalProject,
|
||||
options: CreateLocalProjectMcpContextPortsOptions,
|
||||
|
|
@ -259,7 +251,7 @@ export function createLocalProjectMcpContextPorts(
|
|||
ports.mongoQuery = {
|
||||
async execute(input) {
|
||||
try {
|
||||
return await executeMongoQuery(mongoCreateConnector, input);
|
||||
return await runMongoQuery(mongoCreateConnector, input);
|
||||
} catch (error) {
|
||||
throwClassifiedQueryError(error);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,34 +1,22 @@
|
|||
import type { KtxMongoDbScanConnector, KtxMongoQueryResult } from '../../connectors/mongodb/connector.js';
|
||||
import { KtxExpectedError } from '../../errors.js';
|
||||
import { assertSafeConnectionId } from '../sl/source-files.js';
|
||||
import type { KtxScanConnector } from '../scan/types.js';
|
||||
|
||||
export interface KtxMongoQueryRequest {
|
||||
connectionId: string;
|
||||
collection: string;
|
||||
database?: string;
|
||||
pipeline: Record<string, unknown>[];
|
||||
limit: number;
|
||||
}
|
||||
import type { KtxMongoQueryInput, KtxMongoQueryResult, KtxScanConnector } from '../scan/types.js';
|
||||
|
||||
/** Single Mongo-read execution seam: resolve the connector, guard the driver, run the pipeline. */
|
||||
export async function runMongoQuery(
|
||||
createConnector: (connectionId: string) => Promise<KtxScanConnector> | KtxScanConnector,
|
||||
input: KtxMongoQueryRequest,
|
||||
input: KtxMongoQueryInput,
|
||||
): Promise<KtxMongoQueryResult> {
|
||||
const connectionId = assertSafeConnectionId(input.connectionId);
|
||||
let connector: KtxScanConnector | null = null;
|
||||
try {
|
||||
connector = await createConnector(connectionId);
|
||||
if (connector.driver !== 'mongodb') {
|
||||
if (connector.driver !== 'mongodb' || typeof connector.executeQuery !== 'function') {
|
||||
throw new KtxExpectedError(
|
||||
`Connection "${connectionId}" driver "${connector.driver}" is not a MongoDB connection; mongo_query serves mongodb connections only.`,
|
||||
);
|
||||
}
|
||||
return await (connector as unknown as KtxMongoDbScanConnector).executeQuery(
|
||||
{ connectionId, collection: input.collection, database: input.database, pipeline: input.pipeline, limit: input.limit },
|
||||
{ runId: 'mongo-query' },
|
||||
);
|
||||
return await connector.executeQuery({ ...input, connectionId }, { runId: 'mongo-query' });
|
||||
} finally {
|
||||
await connector?.cleanup?.();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import type { KtxEntityDetailsInput, KtxEntityDetailsResponse } from '../scan/en
|
|||
import type { KtxDiscoverDataInput, KtxDiscoverDataResponse } from '../../context/search/discover.js';
|
||||
import type { KtxDictionarySearchInput, KtxDictionarySearchResponse } from '../../context/sl/dictionary-search.js';
|
||||
import type { SemanticLayerQueryInput } from '../../context/sl/types.js';
|
||||
import type { KtxMongoQueryResult } from '../../connectors/mongodb/connector.js';
|
||||
import type { KtxMongoQueryInput, KtxMongoQueryResult } from '../scan/types.js';
|
||||
import type { WikiSearchLaneSummary, WikiSearchMatchReason } from '../../context/wiki/types.js';
|
||||
|
||||
interface KtxMcpTextContent {
|
||||
|
|
@ -181,14 +181,9 @@ export interface KtxSqlExecutionMcpPort {
|
|||
): Promise<KtxSqlExecutionResponse>;
|
||||
}
|
||||
|
||||
export type KtxMongoQueryResponse = KtxMongoQueryResult;
|
||||
|
||||
/** @internal */
|
||||
export interface KtxMongoQueryMcpPort {
|
||||
execute(
|
||||
input: { connectionId: string; collection: string; database?: string; pipeline: Record<string, unknown>[]; limit: number },
|
||||
options?: { onProgress?: KtxMcpProgressCallback },
|
||||
): Promise<KtxMongoQueryResponse>;
|
||||
execute(input: KtxMongoQueryInput, options?: { onProgress?: KtxMcpProgressCallback }): Promise<KtxMongoQueryResult>;
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
|
|
|
|||
|
|
@ -293,6 +293,20 @@ export interface KtxReadOnlyQueryInput {
|
|||
maxRows?: number;
|
||||
}
|
||||
|
||||
export interface KtxMongoQueryInput {
|
||||
connectionId: string;
|
||||
collection: string;
|
||||
database?: string;
|
||||
pipeline: readonly Record<string, unknown>[];
|
||||
limit: number;
|
||||
}
|
||||
|
||||
export interface KtxMongoQueryResult {
|
||||
headers: string[];
|
||||
rows: unknown[][];
|
||||
rowCount: number;
|
||||
}
|
||||
|
||||
export interface KtxQueryResult {
|
||||
headers: string[];
|
||||
headerTypes?: string[];
|
||||
|
|
@ -346,6 +360,7 @@ export interface KtxScanConnector {
|
|||
sampleTable?(input: KtxTableSampleInput, ctx: KtxScanContext): Promise<KtxTableSampleResult>;
|
||||
columnStats?(input: KtxColumnStatsInput, ctx: KtxScanContext): Promise<KtxColumnStatsResult | null>;
|
||||
executeReadOnly?(input: KtxReadOnlyQueryInput, ctx: KtxScanContext): Promise<KtxQueryResult>;
|
||||
executeQuery?(input: KtxMongoQueryInput, ctx: KtxScanContext): Promise<KtxMongoQueryResult>;
|
||||
cleanup?(): Promise<void>;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { runMongoQuery } from './context/mcp/mongo-query-runner.js';
|
||||
import { resolveConfiguredConnection } from './context/connections/resolve-connection.js';
|
||||
import { loadKtxProject, type KtxLocalProject } from './context/project/project.js';
|
||||
import type { KtxMongoQueryInput } from './context/scan/types.js';
|
||||
import type { KtxCliIo } from './cli-runtime.js';
|
||||
import { type KtxOutputMode, resolveOutputMode } from './io/mode.js';
|
||||
import { type KtxResultTable, printResultTable } from './io/result-table.js';
|
||||
|
|
@ -13,13 +14,8 @@ import { scrubErrorClass } from './telemetry/scrubber.js';
|
|||
|
||||
profileMark('module:mongo-query');
|
||||
|
||||
export type KtxMongoQueryArgs = {
|
||||
export type KtxMongoQueryArgs = KtxMongoQueryInput & {
|
||||
projectDir: string;
|
||||
connectionId: string;
|
||||
collection: string;
|
||||
database?: string;
|
||||
pipeline: Record<string, unknown>[];
|
||||
limit: number;
|
||||
output?: KtxOutputMode;
|
||||
json?: boolean;
|
||||
cliVersion: string;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { InvalidArgumentError } from '@commander-js/extra-typings';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { parsePipelineArgument } from '../../src/commands/mongo-query-commands.js';
|
||||
import { parseLimitOption, parsePipelineArgument } from '../../src/commands/mongo-query-commands.js';
|
||||
|
||||
describe('parsePipelineArgument', () => {
|
||||
it('parses a valid JSON array of pipeline-stage objects', () => {
|
||||
|
|
@ -34,3 +34,26 @@ describe('parsePipelineArgument', () => {
|
|||
expect(() => parsePipelineArgument('[null]')).toThrow(InvalidArgumentError);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseLimitOption', () => {
|
||||
it('accepts the boundary values 1 and 10000', () => {
|
||||
expect(parseLimitOption('1')).toBe(1);
|
||||
expect(parseLimitOption('10000')).toBe(10000);
|
||||
});
|
||||
|
||||
it('rejects a non-numeric value', () => {
|
||||
expect(() => parseLimitOption('abc')).toThrow(InvalidArgumentError);
|
||||
});
|
||||
|
||||
it('rejects a non-integer value', () => {
|
||||
expect(() => parseLimitOption('1.5')).toThrow(InvalidArgumentError);
|
||||
});
|
||||
|
||||
it('rejects a value below 1', () => {
|
||||
expect(() => parseLimitOption('0')).toThrow(InvalidArgumentError);
|
||||
});
|
||||
|
||||
it('rejects a value above the cap', () => {
|
||||
expect(() => parseLimitOption('10001')).toThrow(InvalidArgumentError);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -295,4 +295,23 @@ describe('KtxMongoDbScanConnector.executeQuery', () => {
|
|||
),
|
||||
).rejects.toThrow(/must be read-only/);
|
||||
});
|
||||
|
||||
it('runs against the requested database when input.database is given', async () => {
|
||||
const { factory, client } = fakeClientFactory();
|
||||
await connector(baseConnection, factory).executeQuery(
|
||||
{ connectionId: 'mongo-prod', collection: 'orders', database: 'analytics', pipeline: [], limit: 5 },
|
||||
{ runId: 't' },
|
||||
);
|
||||
expect(client.aggregate).toHaveBeenCalledWith('analytics', 'orders', [], { limit: 5 });
|
||||
});
|
||||
|
||||
it('returns an empty result with no headers when the pipeline matches no documents', async () => {
|
||||
const { factory, client } = fakeClientFactory();
|
||||
(client.aggregate as ReturnType<typeof vi.fn>).mockResolvedValueOnce([]);
|
||||
const result = await connector(baseConnection, factory).executeQuery(
|
||||
{ connectionId: 'mongo-prod', collection: 'users', pipeline: [{ $match: { age: { $gte: 999 } } }], limit: 50 },
|
||||
{ runId: 't' },
|
||||
);
|
||||
expect(result).toEqual({ headers: [], rows: [], rowCount: 0 });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -22,12 +22,12 @@ import type {
|
|||
KtxMcpContextPorts,
|
||||
KtxMcpToolHandlerContext,
|
||||
KtxMongoQueryMcpPort,
|
||||
KtxMongoQueryResponse,
|
||||
KtxSemanticLayerMcpPort,
|
||||
KtxSqlExecutionMcpPort,
|
||||
KtxSqlExecutionResponse,
|
||||
MemoryIngestPort,
|
||||
} from '../../../src/context/mcp/types.js';
|
||||
import type { KtxMongoQueryResult } from '../../../src/context/scan/types.js';
|
||||
|
||||
const reportExceptionMock = vi.hoisted(() => vi.fn(async () => {}));
|
||||
|
||||
|
|
@ -499,7 +499,7 @@ describe('createKtxMcpServer', () => {
|
|||
|
||||
it('registers mongo_query when the host provides a MongoDB query port', async () => {
|
||||
const fake = makeFakeServer();
|
||||
const response: KtxMongoQueryResponse = {
|
||||
const response: KtxMongoQueryResult = {
|
||||
headers: ['_id', 'city'],
|
||||
rows: [
|
||||
['a1', 'Indianapolis'],
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ describe('formatValue', () => {
|
|||
expect(formatValue('x')).toBe('x');
|
||||
expect(formatValue(42)).toBe('42');
|
||||
expect(formatValue(true)).toBe('true');
|
||||
expect(formatValue(10n)).toBe('10');
|
||||
expect(formatValue({ a: 1 })).toBe(JSON.stringify({ a: 1 }));
|
||||
});
|
||||
});
|
||||
|
|
@ -37,10 +38,33 @@ describe('printResultTable', () => {
|
|||
expect(out()).toBe('_id\tcity\na1\tNY\n');
|
||||
});
|
||||
|
||||
it('pretty mode prints a header rule and a row count', () => {
|
||||
it('pretty mode prints a header rule and a singular row count', () => {
|
||||
const { io, out } = captureIo();
|
||||
printResultTable(table, 'pretty', io);
|
||||
expect(out()).toContain('_id');
|
||||
expect(out()).toContain('1 row');
|
||||
expect(out()).toContain('1 row\n');
|
||||
});
|
||||
|
||||
it('pretty mode aligns multiple rows and pluralizes the row count', () => {
|
||||
const { io, out } = captureIo();
|
||||
const multi = {
|
||||
connectionId: 'mongo',
|
||||
headers: ['id', 'city'],
|
||||
rows: [
|
||||
['1', 'NY'],
|
||||
['1000', 'Indianapolis'],
|
||||
],
|
||||
rowCount: 2,
|
||||
};
|
||||
printResultTable(multi, 'pretty', io);
|
||||
// Column widened to the longest cell ("1000"), so "1" is right-padded to 4 chars.
|
||||
expect(out()).toContain('1 NY');
|
||||
expect(out()).toContain('2 rows\n');
|
||||
});
|
||||
|
||||
it('pretty mode omits the header rule when there are no headers', () => {
|
||||
const { io, out } = captureIo();
|
||||
printResultTable({ connectionId: 'mongo', headers: [], rows: [], rowCount: 0 }, 'pretty', io);
|
||||
expect(out()).toBe('\n0 rows\n');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { runKtxMongoQuery } from '../src/mongo-query.js';
|
||||
import { KtxMongoDbScanConnector } from '../src/connectors/mongodb/connector.js';
|
||||
import type { KtxCliIo } from '../src/cli-runtime.js';
|
||||
|
|
@ -67,3 +67,36 @@ describe('runKtxMongoQuery', () => {
|
|||
expect(err().length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('runKtxMongoQuery telemetry', () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
it('emits mongo_query_completed with outcome ok and the pipeline stage count on success', async () => {
|
||||
vi.stubEnv('KTX_TELEMETRY_DEBUG', '1');
|
||||
vi.stubEnv('CI', '');
|
||||
const { io, err } = captureIo();
|
||||
const code = await runKtxMongoQuery({ ...baseArgs, output: 'plain' }, io, {
|
||||
loadProject: async () => project,
|
||||
createScanConnector: (async () => mongoConnector()) as never,
|
||||
});
|
||||
expect(code).toBe(0);
|
||||
expect(err()).toContain('"event":"mongo_query_completed"');
|
||||
expect(err()).toContain('"outcome":"ok"');
|
||||
expect(err()).toContain('"stageCount":1');
|
||||
});
|
||||
|
||||
it('emits mongo_query_completed with outcome error on failure', async () => {
|
||||
vi.stubEnv('KTX_TELEMETRY_DEBUG', '1');
|
||||
vi.stubEnv('CI', '');
|
||||
const { io, err } = captureIo();
|
||||
const code = await runKtxMongoQuery({ ...baseArgs, connectionId: 'nope' }, io, {
|
||||
loadProject: async () => project,
|
||||
createScanConnector: (async () => mongoConnector()) as never,
|
||||
});
|
||||
expect(code).toBe(1);
|
||||
expect(err()).toContain('"event":"mongo_query_completed"');
|
||||
expect(err()).toContain('"outcome":"error"');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue