From f138ead5bebf124f3b5ac0b2d338fb65d8ffba2e Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Tue, 12 May 2026 16:53:12 +0200 Subject: [PATCH] Enforce flat wiki keys --- packages/cli/src/agent.test.ts | 8 +-- packages/cli/src/index.test.ts | 14 ++--- packages/cli/src/knowledge.test.ts | 45 ++++++++++++--- .../src/ingest/adapters/notion/chunk.ts | 2 +- .../src/ingest/adapters/notion/cluster.ts | 2 +- .../adapters/notion/notion.adapter.test.ts | 3 + .../src/ingest/local-bundle-runtime.ts | 13 ++++- .../src/search/backend-conformance.test.ts | 16 +++--- packages/context/src/wiki/index.ts | 7 +++ packages/context/src/wiki/keys.ts | 31 +++++++++++ .../src/wiki/knowledge-wiki.service.test.ts | 55 ++++++++++++++++++- .../src/wiki/knowledge-wiki.service.ts | 23 ++++++-- .../context/src/wiki/local-knowledge.test.ts | 49 ++++++++++------- packages/context/src/wiki/local-knowledge.ts | 35 ++++++++---- .../src/wiki/tools/wiki-read.tool.test.ts | 19 +++++++ .../context/src/wiki/tools/wiki-read.tool.ts | 8 +++ .../src/wiki/tools/wiki-remove.tool.test.ts | 19 +++++++ .../src/wiki/tools/wiki-remove.tool.ts | 8 +++ .../src/wiki/tools/wiki-search.tool.test.ts | 10 ++-- .../src/wiki/tools/wiki-write.tool.test.ts | 21 ++++++- .../context/src/wiki/tools/wiki-write.tool.ts | 8 +++ 21 files changed, 323 insertions(+), 73 deletions(-) create mode 100644 packages/context/src/wiki/keys.ts diff --git a/packages/cli/src/agent.test.ts b/packages/cli/src/agent.test.ts index 043bdddb..d72814bf 100644 --- a/packages/cli/src/agent.test.ts +++ b/packages/cli/src/agent.test.ts @@ -185,8 +185,8 @@ describe('runKtxAgent', () => { search: vi.fn(async () => ({ results: [ { - key: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + key: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', scope: 'GLOBAL' as const, summary: 'Revenue metric definition', score: 0.02459016393442623, @@ -207,8 +207,8 @@ describe('runKtxAgent', () => { expect(JSON.parse(io.stdout())).toEqual({ results: [ expect.objectContaining({ - key: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + key: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', matchReasons: ['lexical', 'token'], }), ], diff --git a/packages/cli/src/index.test.ts b/packages/cli/src/index.test.ts index 87a0089f..83b2e820 100644 --- a/packages/cli/src/index.test.ts +++ b/packages/cli/src/index.test.ts @@ -1139,7 +1139,7 @@ describe('runKtxCli', () => { projectDir: tempDir, inputMode: 'disabled', cliVersion: '0.0.0-private', - anthropicApiKeyEnv: 'ANTHROPIC_API_KEY', + anthropicApiKeyEnv: 'ANTHROPIC_API_KEY', // pragma: allowlist secret anthropicModel: 'claude-sonnet-4-6', skipLlm: false, }), @@ -1200,7 +1200,7 @@ describe('runKtxCli', () => { inputMode: 'disabled', skipLlm: true, embeddingBackend: 'openai', - embeddingApiKeyEnv: 'OPENAI_API_KEY', + embeddingApiKeyEnv: 'OPENAI_API_KEY', // pragma: allowlist secret skipEmbeddings: false, }), setupIo.io, @@ -1301,7 +1301,7 @@ describe('runKtxCli', () => { source: 'metabase', sourceConnectionId: 'prod_metabase', sourceUrl: 'https://metabase.example.com', - sourceApiKeyRef: 'env:METABASE_API_KEY', + sourceApiKeyRef: 'env:METABASE_API_KEY', // pragma: allowlist secret sourceWarehouseConnectionId: 'warehouse', metabaseDatabaseId: 1, }), @@ -1727,8 +1727,8 @@ describe('runKtxCli', () => { { results: [ { - key: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + key: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', scope: 'GLOBAL', summary: 'Revenue metric definition', score: 0.02459016393442623, @@ -1754,8 +1754,8 @@ describe('runKtxCli', () => { expect(JSON.parse(io.stdout())).toEqual({ results: [ expect.objectContaining({ - key: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + key: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', matchReasons: ['lexical', 'token'], }), ], diff --git a/packages/cli/src/knowledge.test.ts b/packages/cli/src/knowledge.test.ts index 0e9ed1d5..d7e17605 100644 --- a/packages/cli/src/knowledge.test.ts +++ b/packages/cli/src/knowledge.test.ts @@ -61,7 +61,7 @@ describe('runKtxKnowledge', () => { { command: 'write', projectDir, - key: 'metrics/revenue', + key: 'metrics-revenue', scope: 'GLOBAL', userId: 'local', summary: 'Revenue', @@ -73,24 +73,53 @@ describe('runKtxKnowledge', () => { writeIo.io, ), ).resolves.toBe(0); - expect(writeIo.stdout()).toContain('Wrote knowledge/global/metrics/revenue.md'); + expect(writeIo.stdout()).toContain('Wrote knowledge/global/metrics-revenue.md'); const readIo = makeIo(); await expect( - runKtxKnowledge({ command: 'read', projectDir, key: 'metrics/revenue', userId: 'local' }, readIo.io), + runKtxKnowledge({ command: 'read', projectDir, key: 'metrics-revenue', userId: 'local' }, readIo.io), ).resolves.toBe(0); - expect(readIo.stdout()).toContain('# metrics/revenue'); + expect(readIo.stdout()).toContain('# metrics-revenue'); expect(readIo.stdout()).toContain('Revenue is paid order value.'); const listIo = makeIo(); await expect(runKtxKnowledge({ command: 'list', projectDir, userId: 'local' }, listIo.io)).resolves.toBe(0); - expect(listIo.stdout()).toContain('GLOBAL\tmetrics/revenue\tRevenue'); + expect(listIo.stdout()).toContain('GLOBAL\tmetrics-revenue\tRevenue'); const searchIo = makeIo(); await expect( runKtxKnowledge({ command: 'search', projectDir, query: 'paid order', userId: 'local' }, searchIo.io), ).resolves.toBe(0); - expect(searchIo.stdout()).toContain('metrics/revenue'); + expect(searchIo.stdout()).toContain('metrics-revenue'); + }); + + it('rejects slash-delimited write keys with a flat-key suggestion', async () => { + const projectDir = join(tempDir, 'project'); + await initKtxProject({ projectDir, projectName: 'warehouse' }); + + const writeIo = makeIo(); + await expect( + runKtxKnowledge( + { + command: 'write', + projectDir, + key: 'orbit/company-overview', + scope: 'GLOBAL', + userId: 'local', + summary: 'Orbit', + content: 'Orbit overview.', + tags: [], + refs: [], + slRefs: [], + }, + writeIo.io, + ), + ).resolves.toBe(1); + + expect(writeIo.stderr()).toContain( + 'Invalid wiki key "orbit/company-overview". Wiki keys must be flat; use "orbit-company-overview".', + ); + expect(writeIo.stdout()).toBe(''); }); it('explains empty search results for a project without wiki pages', async () => { @@ -116,7 +145,7 @@ describe('runKtxKnowledge', () => { { command: 'write', projectDir, - key: 'historic-sql/active-contract-arr-open-tickets', + key: 'active-contract-arr-open-tickets', scope: 'GLOBAL', userId: 'local', summary: 'Active Contract ARR Ranked by Open Support Ticket Count', @@ -138,7 +167,7 @@ describe('runKtxKnowledge', () => { ), ).resolves.toBe(0); - expect(searchIo.stdout()).toContain('historic-sql/active-contract-arr-open-tickets'); + expect(searchIo.stdout()).toContain('active-contract-arr-open-tickets'); expect(searchIo.stderr()).toBe(''); }); }); diff --git a/packages/context/src/ingest/adapters/notion/chunk.ts b/packages/context/src/ingest/adapters/notion/chunk.ts index 782b7e71..7d85fb76 100644 --- a/packages/context/src/ingest/adapters/notion/chunk.ts +++ b/packages/context/src/ingest/adapters/notion/chunk.ts @@ -8,7 +8,7 @@ const MAX_NOTION_WORK_UNIT_CHARS = 40_000; export const NOTION_ORG_KNOWLEDGE_WARNING = 'Anything accessible to this Notion integration can become organization knowledge.'; const NOTION_SL_WRITE_GUIDANCE = - 'Write wiki entries with wiki_write. Search existing wiki pages for the same tables or sl_refs before creating a new page. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. Notion dataSourceCount counts Notion databases/data sources only, not warehouse/dbt mappings. If a warehouse/dbt connection exists but the named table or source is absent, use reason no_physical_table rather than no_connection_mapping. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; + 'Write wiki entries with wiki_write. Wiki keys must be flat slugs like orbit-company-overview, not orbit/company-overview. Search existing wiki pages for the same tables or sl_refs before creating a new page. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. Notion dataSourceCount counts Notion databases/data sources only, not warehouse/dbt mappings. If a warehouse/dbt connection exists but the named table or source is absent, use reason no_physical_table rather than no_connection_mapping. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; async function walk(root: string): Promise { const entries = await readdir(root, { withFileTypes: true, recursive: true }); diff --git a/packages/context/src/ingest/adapters/notion/cluster.ts b/packages/context/src/ingest/adapters/notion/cluster.ts index 4aab3ea8..090ff4f7 100644 --- a/packages/context/src/ingest/adapters/notion/cluster.ts +++ b/packages/context/src/ingest/adapters/notion/cluster.ts @@ -9,7 +9,7 @@ export const MIN_PAGES_TO_CLUSTER = 5; const CLUSTER_TEXT_BODY_CHARS = 1024; const CLUSTER_SEED = 42; const NOTION_CLUSTER_SL_WRITE_GUIDANCE = - 'Write wiki entries directly with wiki_write. Search existing wiki pages for the same tables or sl_refs before creating a new page. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. Notion dataSourceCount counts Notion databases/data sources only, not warehouse/dbt mappings. If a warehouse/dbt connection exists but the named table or source is absent, use reason no_physical_table rather than no_connection_mapping. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; + 'Write wiki entries directly with wiki_write. Wiki keys must be flat slugs like orbit-company-overview, not orbit/company-overview. Search existing wiki pages for the same tables or sl_refs before creating a new page. Only write or edit SL sources after sl_discover/sl_read_source confirms a mapped non-Notion target source; if no mapped target exists, emit_unmapped_fallback and keep the fact wiki-only. Notion dataSourceCount counts Notion databases/data sources only, not warehouse/dbt mappings. If a warehouse/dbt connection exists but the named table or source is absent, use reason no_physical_table rather than no_connection_mapping. Do not create SL sources under the Notion connection just because a page mentions a warehouse table.'; interface ClusterNotionWorkUnitsArgs { workUnits: WorkUnit[]; diff --git a/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts b/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts index 7e752e9b..de989d02 100644 --- a/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts +++ b/packages/context/src/ingest/adapters/notion/notion.adapter.test.ts @@ -244,6 +244,9 @@ describe('NotionSourceAdapter', () => { expect(result.workUnits[0].notes).toContain('emit_unmapped_fallback'); expect(result.workUnits[0].notes).toContain('use reason no_physical_table rather than no_connection_mapping'); expect(result.workUnits[0].notes).toContain('Do not create SL sources under the Notion connection'); + expect(result.workUnits[0].notes).toContain( + 'Wiki keys must be flat slugs like orbit-company-overview, not orbit/company-overview', + ); expect(result.reconcileNotes).toEqual([ 'Notion maxKnowledgeCreatesPerRun=25', 'Notion maxKnowledgeUpdatesPerRun=20', diff --git a/packages/context/src/ingest/local-bundle-runtime.ts b/packages/context/src/ingest/local-bundle-runtime.ts index 67a65140..38e37b1d 100644 --- a/packages/context/src/ingest/local-bundle-runtime.ts +++ b/packages/context/src/ingest/local-bundle-runtime.ts @@ -429,10 +429,19 @@ class LocalKnowledgeIndex implements KnowledgeIndexPort { function parseKnowledgeIndexPath(file: string): { scope: 'GLOBAL' | 'USER'; pageKey: string } | null { const segments = file.split('/'); if (segments.length === 2 && segments[0] === 'global') { - return { scope: 'GLOBAL', pageKey: segments[1].replace(/\.md$/, '') }; + const pageKey = segments[1].replace(/\.md$/, ''); + return /^[a-zA-Z0-9][a-zA-Z0-9_-]*$/.test(pageKey) ? { scope: 'GLOBAL', pageKey } : null; + } + if (segments.length >= 3 && segments[0] === 'global' && segments[1] === 'historic-sql') { + const historicPath = segments.slice(2).join('/').replace(/\.md$/, ''); + if (historicPath.split('/').every((segment) => /^[a-zA-Z0-9_][a-zA-Z0-9_-]*$/.test(segment))) { + return { scope: 'GLOBAL', pageKey: `historic-sql/${historicPath}` }; + } + return null; } if (segments.length === 3 && segments[0] === 'user') { - return { scope: 'USER', pageKey: segments[2].replace(/\.md$/, '') }; + const pageKey = segments[2].replace(/\.md$/, ''); + return /^[a-zA-Z0-9][a-zA-Z0-9_-]*$/.test(pageKey) ? { scope: 'USER', pageKey } : null; } return null; } diff --git a/packages/context/src/search/backend-conformance.test.ts b/packages/context/src/search/backend-conformance.test.ts index e033082d..d2d8e3bf 100644 --- a/packages/context/src/search/backend-conformance.test.ts +++ b/packages/context/src/search/backend-conformance.test.ts @@ -146,7 +146,7 @@ async function seedSemanticLayerProject(project: KtxLocalProject): Promise async function seedWikiProject(project: KtxLocalProject): Promise { await writeLocalKnowledgePage(project, { - key: 'metrics/revenue', + key: 'metrics-revenue', scope: 'GLOBAL', summary: 'Semantic revenue definition', content: 'Revenue is recognized when an order is paid.', @@ -155,7 +155,7 @@ async function seedWikiProject(project: KtxLocalProject): Promise { slRefs: ['orders'], }); await writeLocalKnowledgePage(project, { - key: 'support/escalations', + key: 'support-escalations', scope: 'GLOBAL', summary: 'Support escalation process', content: 'Escalations move urgent support tickets to the operations queue.', @@ -338,9 +338,9 @@ describe('SQLite hybrid search backend conformance', () => { surface: 'wiki', caseName: 'lexical page ranking', results: lexical.map(toWikiConformanceResult), - expectedTopIds: ['metrics/revenue'], + expectedTopIds: ['metrics-revenue'], expectedReasonsById: { - 'metrics/revenue': ['lexical'], + 'metrics-revenue': ['lexical'], }, expectedLanes: { lexical: { status: 'available' }, @@ -359,9 +359,9 @@ describe('SQLite hybrid search backend conformance', () => { surface: 'wiki', caseName: 'semantic page ranking', results: semantic.map(toWikiConformanceResult), - expectedTopIds: ['metrics/revenue'], + expectedTopIds: ['metrics-revenue'], expectedReasonsById: { - 'metrics/revenue': ['semantic'], + 'metrics-revenue': ['semantic'], }, expectedLanes: { semantic: { status: 'available' }, @@ -378,9 +378,9 @@ describe('SQLite hybrid search backend conformance', () => { surface: 'wiki', caseName: 'token page fallback', results: token.map(toWikiConformanceResult), - expectedTopIds: ['metrics/revenue'], + expectedTopIds: ['metrics-revenue'], expectedReasonsById: { - 'metrics/revenue': ['token'], + 'metrics-revenue': ['token'], }, expectedLanes: { token: { status: 'available' }, diff --git a/packages/context/src/wiki/index.ts b/packages/context/src/wiki/index.ts index 892eff34..6eae10f0 100644 --- a/packages/context/src/wiki/index.ts +++ b/packages/context/src/wiki/index.ts @@ -1,4 +1,11 @@ export { buildKnowledgeSearchText } from './knowledge-search-text.js'; +export { + assertFlatWikiKey, + invalidFlatWikiKeyMessage, + isFlatWikiKey, + suggestFlatWikiKey, + validateFlatWikiKey, +} from './keys.js'; export { KnowledgeWikiService } from './knowledge-wiki.service.js'; export * from './local-knowledge.js'; export type { diff --git a/packages/context/src/wiki/keys.ts b/packages/context/src/wiki/keys.ts new file mode 100644 index 00000000..4bffae2e --- /dev/null +++ b/packages/context/src/wiki/keys.ts @@ -0,0 +1,31 @@ +const FLAT_WIKI_KEY_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9_-]*$/; + +export function suggestFlatWikiKey(key: string): string { + const suggested = key + .trim() + .replace(/[\\/]+/g, '-') + .replace(/[^a-zA-Z0-9_-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^[-_]+|[-_]+$/g, ''); + return suggested.length > 0 ? suggested : 'page-key'; +} + +export function invalidFlatWikiKeyMessage(key: string): string { + return `Invalid wiki key "${key}". Wiki keys must be flat; use "${suggestFlatWikiKey(key)}".`; +} + +export function isFlatWikiKey(key: string): boolean { + return FLAT_WIKI_KEY_PATTERN.test(key); +} + +export function validateFlatWikiKey(key: string): { ok: true; key: string } | { ok: false; error: string } { + return isFlatWikiKey(key) ? { ok: true, key } : { ok: false, error: invalidFlatWikiKeyMessage(key) }; +} + +export function assertFlatWikiKey(key: string): string { + const result = validateFlatWikiKey(key); + if (!result.ok) { + throw new Error(result.error); + } + return result.key; +} diff --git a/packages/context/src/wiki/knowledge-wiki.service.test.ts b/packages/context/src/wiki/knowledge-wiki.service.test.ts index 8fadee11..ecbf954a 100644 --- a/packages/context/src/wiki/knowledge-wiki.service.test.ts +++ b/packages/context/src/wiki/knowledge-wiki.service.test.ts @@ -33,13 +33,19 @@ function makeService() { diffNameStatus: vi.fn().mockResolvedValue([]), getFileAtCommit: vi.fn().mockResolvedValue(''), }; + const logger = { + log: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; const service = new KnowledgeWikiService( configService as any, embeddingService as any, pagesRepository as any, gitService as any, + logger as any, ); - return { service, pagesRepository, embeddingService, configService, gitService }; + return { service, pagesRepository, embeddingService, configService, gitService, logger }; } const fm: WikiFrontmatter = { summary: 'sum', usage_mode: 'auto' }; @@ -107,6 +113,53 @@ describe('KnowledgeWikiService.syncFromCommit', () => { expect(call.deletes).toEqual([{ scope: 'GLOBAL', scopeId: null, pageKey: 'gone-page' }]); }); + it('indexes historic-SQL nested pages but skips other nested wiki paths from commit sync', async () => { + const { service, pagesRepository, gitService, logger } = makeService(); + + gitService.diffNameStatus.mockResolvedValue([ + { status: 'A', path: 'knowledge/global/revenue-policy.md' }, + { status: 'A', path: 'knowledge/global/historic-sql/order-lifecycle.md' }, + { status: 'A', path: 'knowledge/global/historic-sql/_archived/retired-pattern.md' }, + { status: 'A', path: 'knowledge/global/orbit/company-overview.md' }, + ]); + gitService.getFileAtCommit.mockImplementation((path: string) => { + if (path.endsWith('revenue-policy.md')) { + return Promise.resolve('---\nsummary: revenue\nusage_mode: auto\n---\n\nbody-revenue\n'); + } + if (path.endsWith('order-lifecycle.md')) { + return Promise.resolve('---\nsummary: order lifecycle\nusage_mode: auto\n---\n\nbody-orders\n'); + } + if (path.endsWith('retired-pattern.md')) { + return Promise.resolve('---\nsummary: retired\nusage_mode: never\n---\n\nbody-retired\n'); + } + return Promise.reject(new Error(`unexpected getFileAtCommit path: ${path}`)); + }); + + await service.syncFromCommit('sha-before', 'sha-after', 'run-uuid'); + + expect(gitService.getFileAtCommit).not.toHaveBeenCalledWith('knowledge/global/orbit/company-overview.md', 'sha-after'); + expect(logger.warn).toHaveBeenCalledWith( + '[knowledge.sync] skipping unparseable path: knowledge/global/orbit/company-overview.md', + ); + const call = pagesRepository.applyDiffTransactional.mock.calls[0][0]; + expect(call.upserts).toEqual( + expect.arrayContaining([ + expect.objectContaining({ scope: 'GLOBAL', pageKey: 'revenue-policy', summary: 'revenue' }), + expect.objectContaining({ + scope: 'GLOBAL', + pageKey: 'historic-sql/order-lifecycle', + summary: 'order lifecycle', + }), + expect.objectContaining({ + scope: 'GLOBAL', + pageKey: 'historic-sql/_archived/retired-pattern', + summary: 'retired', + }), + ]), + ); + expect(call.upserts).toHaveLength(3); + }); + it('is a no-op when the diff between shas has no knowledge changes', async () => { const { service, pagesRepository, gitService } = makeService(); gitService.diffNameStatus.mockResolvedValue([]); diff --git a/packages/context/src/wiki/knowledge-wiki.service.ts b/packages/context/src/wiki/knowledge-wiki.service.ts index 6234c729..2ca32f79 100644 --- a/packages/context/src/wiki/knowledge-wiki.service.ts +++ b/packages/context/src/wiki/knowledge-wiki.service.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import YAML from 'yaml'; import type { KtxEmbeddingPort, KtxFileStorePort, KtxLogger } from '../core/index.js'; import { noopLogger } from '../core/index.js'; +import { assertFlatWikiKey, isFlatWikiKey } from './keys.js'; import { buildKnowledgeSearchText } from './knowledge-search-text.js'; import type { KnowledgeGitDiffPort, KnowledgeIndexPort, UpsertPageParams } from './ports.js'; import type { WikiFrontmatter, WikiPage, WikiPageWithScope } from './types.js'; @@ -10,6 +11,10 @@ const WIKI_PREFIX = 'knowledge'; export type { WikiFrontmatter }; +function isHistoricSqlPathSegment(segment: string): boolean { + return /^[a-zA-Z0-9_][a-zA-Z0-9_-]*$/.test(segment); +} + export class KnowledgeWikiService { private isWorktreeScoped = false; @@ -53,7 +58,7 @@ export class KnowledgeWikiService { } pagePath(scope: string, scopeId: string | null | undefined, pageKey: string): string { - return `${this.scopeDir(scope, scopeId)}/${pageKey}.md`; + return `${this.scopeDir(scope, scopeId)}/${assertFlatWikiKey(pageKey)}.md`; } // ── Parsing / serialization ─────────────────────────────────── @@ -140,7 +145,7 @@ export class KnowledgeWikiService { const name = f.replace(`${dir}/`, '').replace(/\.md$/, ''); return name; }) - .filter((name) => !name.includes('/')); + .filter(isFlatWikiKey); } catch { return []; } @@ -417,6 +422,7 @@ export class KnowledgeWikiService { * Parse a `knowledge//...` file path into its scope and page key. * `knowledge/global/foo.md` → { scope: 'GLOBAL', scopeId: null, pageKey: 'foo' } * `knowledge/user//bar.md` → { scope: 'USER', scopeId: '', pageKey: 'bar' } + * `knowledge/global/historic-sql/foo.md` → { scope: 'GLOBAL', scopeId: null, pageKey: 'historic-sql/foo' } */ function parseKnowledgePath(path: string): { scope: string; scopeId: string | null; pageKey: string } | null { if (!path.endsWith('.md')) { @@ -428,10 +434,19 @@ function parseKnowledgePath(path: string): { scope: string; scopeId: string | nu } const rest = segments.slice(1); if (rest.length === 2 && rest[0] === 'global') { - return { scope: 'GLOBAL', scopeId: null, pageKey: rest[1].replace(/\.md$/, '') }; + const pageKey = rest[1].replace(/\.md$/, ''); + return isFlatWikiKey(pageKey) ? { scope: 'GLOBAL', scopeId: null, pageKey } : null; + } + if (rest.length >= 3 && rest[0] === 'global' && rest[1] === 'historic-sql') { + const historicPath = rest.slice(2).join('/').replace(/\.md$/, ''); + if (historicPath.split('/').every(isHistoricSqlPathSegment)) { + return { scope: 'GLOBAL', scopeId: null, pageKey: `historic-sql/${historicPath}` }; + } + return null; } if (rest.length === 3 && rest[0] === 'user') { - return { scope: 'USER', scopeId: rest[1], pageKey: rest[2].replace(/\.md$/, '') }; + const pageKey = rest[2].replace(/\.md$/, ''); + return isFlatWikiKey(pageKey) ? { scope: 'USER', scopeId: rest[1], pageKey } : null; } return null; } diff --git a/packages/context/src/wiki/local-knowledge.test.ts b/packages/context/src/wiki/local-knowledge.test.ts index 1b979cef..78da841f 100644 --- a/packages/context/src/wiki/local-knowledge.test.ts +++ b/packages/context/src/wiki/local-knowledge.test.ts @@ -37,7 +37,7 @@ describe('local knowledge helpers', () => { it('writes, reads, lists, and searches global knowledge pages', async () => { const write = await writeLocalKnowledgePage(project, { - key: 'metrics/revenue', + key: 'metrics-revenue', scope: 'GLOBAL', summary: 'Revenue metric definition', content: 'Revenue is recognized when an order is paid.', @@ -46,11 +46,11 @@ describe('local knowledge helpers', () => { slRefs: ['orders'], }); - expect(write.path).toBe('knowledge/global/metrics/revenue.md'); + expect(write.path).toBe('knowledge/global/metrics-revenue.md'); expect(write.operation).toBe('write'); - await expect(readLocalKnowledgePage(project, { key: 'metrics/revenue', userId: 'local' })).resolves.toMatchObject({ - key: 'metrics/revenue', + await expect(readLocalKnowledgePage(project, { key: 'metrics-revenue', userId: 'local' })).resolves.toMatchObject({ + key: 'metrics-revenue', scope: 'GLOBAL', summary: 'Revenue metric definition', content: 'Revenue is recognized when an order is paid.', @@ -61,8 +61,8 @@ describe('local knowledge helpers', () => { await expect(listLocalKnowledgePages(project, { userId: 'local' })).resolves.toEqual([ { - key: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + key: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', scope: 'GLOBAL', summary: 'Revenue metric definition', }, @@ -71,8 +71,8 @@ describe('local knowledge helpers', () => { const search = await searchLocalKnowledgePages(project, { query: 'paid order', userId: 'local' }); expect(search).toEqual([ expect.objectContaining({ - key: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + key: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', scope: 'GLOBAL', score: expect.any(Number), matchReasons: expect.arrayContaining(['lexical']), @@ -85,7 +85,7 @@ describe('local knowledge helpers', () => { it('adds the token lane alongside lexical wiki matches', async () => { await writeLocalKnowledgePage(project, { - key: 'metrics/revenue', + key: 'metrics-revenue', scope: 'GLOBAL', summary: 'Revenue metric definition', content: 'Revenue is recognized when an order is paid.', @@ -95,7 +95,7 @@ describe('local knowledge helpers', () => { const search = await searchLocalKnowledgePages(project, { query: 'paid---', userId: 'local', limit: 5 }); expect(search[0]).toMatchObject({ - key: 'metrics/revenue', + key: 'metrics-revenue', matchReasons: expect.arrayContaining(['token']), lanes: expect.arrayContaining([expect.objectContaining({ lane: 'token', status: 'available' })]), }); @@ -103,14 +103,14 @@ describe('local knowledge helpers', () => { it('uses stored page embeddings when a wiki embedding backend is configured', async () => { await writeLocalKnowledgePage(project, { - key: 'metrics/revenue', + key: 'metrics-revenue', scope: 'GLOBAL', summary: 'Semantic revenue definition', content: 'Revenue search text.', tags: ['finance'], }); await writeLocalKnowledgePage(project, { - key: 'support/escalations', + key: 'support-escalations', scope: 'GLOBAL', summary: 'Support escalation process', content: 'Support search text.', @@ -125,7 +125,7 @@ describe('local knowledge helpers', () => { }); expect(search[0]).toMatchObject({ - key: 'metrics/revenue', + key: 'metrics-revenue', matchReasons: expect.arrayContaining(['semantic']), lanes: expect.arrayContaining([expect.objectContaining({ lane: 'semantic', status: 'available' })]), }); @@ -133,7 +133,7 @@ describe('local knowledge helpers', () => { it('reports semantic lane as skipped when wiki embeddings are not configured', async () => { await writeLocalKnowledgePage(project, { - key: 'metrics/revenue', + key: 'metrics-revenue', scope: 'GLOBAL', summary: 'Revenue metric definition', content: 'Revenue is recognized when an order is paid.', @@ -172,7 +172,7 @@ describe('local knowledge helpers', () => { it('serializes historic-SQL frontmatter fields for global pages', async () => { await writeLocalKnowledgePage(project, { - key: 'queries/monthly-paid-orders', + key: 'monthly-paid-orders', scope: 'GLOBAL', summary: 'Monthly paid orders', content: '## Monthly paid order count', @@ -195,7 +195,7 @@ describe('local knowledge helpers', () => { fingerprints: ['fp_paid_orders'], }); - const raw = await project.fileStore.readFile('knowledge/global/queries/monthly-paid-orders.md'); + const raw = await project.fileStore.readFile('knowledge/global/monthly-paid-orders.md'); expect(raw.content).toContain('source: historic-sql'); expect(raw.content).toContain('intent: Monthly paid order count'); expect(raw.content).toContain(['tables:', ' - analytics.orders'].join('\n')); @@ -207,7 +207,7 @@ describe('local knowledge helpers', () => { it('falls back to Markdown scanning when the config does not select sqlite-fts5', async () => { project.config.storage.search = 'postgres-hybrid'; await writeLocalKnowledgePage(project, { - key: 'metrics/revenue', + key: 'metrics-revenue', scope: 'GLOBAL', summary: 'Revenue metric definition', content: 'Revenue is recognized when an order is paid.', @@ -216,7 +216,7 @@ describe('local knowledge helpers', () => { await expect(searchLocalKnowledgePages(project, { query: 'paid order', userId: 'local' })).resolves.toEqual([ expect.objectContaining({ - key: 'metrics/revenue', + key: 'metrics-revenue', score: 3, matchReasons: ['token'], }), @@ -231,6 +231,17 @@ describe('local knowledge helpers', () => { summary: 'bad', content: 'bad', }), - ).rejects.toThrow('Unsafe knowledge key'); + ).rejects.toThrow('Invalid wiki key "../secret". Wiki keys must be flat; use "secret".'); + }); + + it('rejects slash-delimited knowledge keys with a flat-key suggestion', async () => { + await expect( + writeLocalKnowledgePage(project, { + key: 'orbit/company-overview', + scope: 'GLOBAL', + summary: 'bad', + content: 'bad', + }), + ).rejects.toThrow('Invalid wiki key "orbit/company-overview". Wiki keys must be flat; use "orbit-company-overview".'); }); }); diff --git a/packages/context/src/wiki/local-knowledge.ts b/packages/context/src/wiki/local-knowledge.ts index 11cc1cc1..007b006e 100644 --- a/packages/context/src/wiki/local-knowledge.ts +++ b/packages/context/src/wiki/local-knowledge.ts @@ -4,6 +4,7 @@ import type { KtxEmbeddingPort, KtxFileWriteResult } from '../core/index.js'; import type { KtxLocalProject } from '../project/index.js'; import { HybridSearchCore, type SearchCandidateGenerator } from '../search/index.js'; import { buildKnowledgeSearchText } from './knowledge-search-text.js'; +import { assertFlatWikiKey, isFlatWikiKey } from './keys.js'; import { SqliteKnowledgeIndex, type SqliteKnowledgeIndexPage } from './sqlite-knowledge-index.js'; import type { HistoricSqlWikiUsageFrontmatter, WikiSearchLaneSummary, WikiSearchMatchReason } from './types.js'; @@ -67,28 +68,39 @@ function assertSafePathToken(kind: string, value: string): string { return value; } -function assertSafeKnowledgeKey(key: string): string { - if (!/^[a-zA-Z0-9][a-zA-Z0-9_/-]*$/.test(key)) { - throw new Error(`Unsafe knowledge key: ${key}`); - } - return assertSafePathToken('knowledge key', key); -} - function stringArray(value: unknown): string[] { return Array.isArray(value) ? value.filter((item): item is string => typeof item === 'string') : []; } function knowledgePath(scope: LocalKnowledgeScope, userId: string | undefined, key: string): string { - const safeKey = assertSafeKnowledgeKey(key); + const safeKey = assertFlatWikiKey(key); if (scope === 'GLOBAL') { return `knowledge/global/${safeKey}.md`; } return `knowledge/user/${assertSafePathToken('user id', userId ?? 'local')}/${safeKey}.md`; } -function keyFromKnowledgePath(path: string, scope: LocalKnowledgeScope, userId: string): string { +function isHistoricSqlPathSegment(segment: string): boolean { + return /^[a-zA-Z0-9_][a-zA-Z0-9_-]*$/.test(segment); +} + +function keyFromKnowledgePath(path: string, scope: LocalKnowledgeScope, userId: string): string | null { const prefix = scope === 'GLOBAL' ? 'knowledge/global/' : `knowledge/user/${assertSafePathToken('user id', userId)}/`; - return path.slice(prefix.length).replace(/\.md$/, ''); + const key = path.slice(prefix.length).replace(/\.md$/, ''); + if (isFlatWikiKey(key)) { + return key; + } + if ( + scope === 'GLOBAL' && + key.startsWith('historic-sql/') && + key + .slice('historic-sql/'.length) + .split('/') + .every(isHistoricSqlPathSegment) + ) { + return key; + } + return null; } function parseKnowledgePage(key: string, path: string, scope: LocalKnowledgeScope, raw: string): LocalKnowledgePage { @@ -187,6 +199,9 @@ export async function listLocalKnowledgePages( const listed = await project.fileStore.listFiles(root); for (const path of listed.files.filter((file) => file.endsWith('.md')).sort()) { const key = keyFromKnowledgePath(path, scope, userId); + if (!key) { + continue; + } const page = await readPageAtPath(project, key, path, scope); if (page) { pages.push({ key, path, scope, summary: page.summary }); diff --git a/packages/context/src/wiki/tools/wiki-read.tool.test.ts b/packages/context/src/wiki/tools/wiki-read.tool.test.ts index b7ad3d1a..1457702c 100644 --- a/packages/context/src/wiki/tools/wiki-read.tool.test.ts +++ b/packages/context/src/wiki/tools/wiki-read.tool.test.ts @@ -38,6 +38,25 @@ describe('WikiReadTool', () => { expect(result.markdown).toContain('A page written earlier in the same ingest worktree.'); }); + it('rejects slash-delimited page keys with a flat-key suggestion', async () => { + const rootWikiService = { readPageForUser: vi.fn().mockResolvedValue(null) }; + const pagesRepository = { findPageByKey: vi.fn(), incrementUsageCount: vi.fn() }; + const tool = new WikiReadTool(rootWikiService as any, pagesRepository as any); + + const result = await tool.call({ key: 'orbit/company-overview' }, baseContext); + + expect(result.structured).toEqual({ + blockKey: 'orbit/company-overview', + content: '', + scope: '', + found: false, + }); + expect(result.markdown).toContain( + 'Invalid wiki key "orbit/company-overview". Wiki keys must be flat; use "orbit-company-overview".', + ); + expect(rootWikiService.readPageForUser).not.toHaveBeenCalled(); + }); + it('does not append derived refs to the editable markdown body', async () => { const rootWikiService = { readPageForUser: vi.fn().mockResolvedValue({ diff --git a/packages/context/src/wiki/tools/wiki-read.tool.ts b/packages/context/src/wiki/tools/wiki-read.tool.ts index 228acdf6..858e717e 100644 --- a/packages/context/src/wiki/tools/wiki-read.tool.ts +++ b/packages/context/src/wiki/tools/wiki-read.tool.ts @@ -1,6 +1,7 @@ import { z } from 'zod'; import type { KnowledgeIndexPort } from '../ports.js'; import { KnowledgeWikiService } from '../index.js'; +import { validateFlatWikiKey } from '../keys.js'; import { BaseTool, type ToolContext, type ToolOutput } from '../../tools/index.js'; const WikiReadInputSchema = z.object({ @@ -44,6 +45,13 @@ export class WikiReadTool extends BaseTool { } async call(input: WikiReadInput, context: ToolContext): Promise> { + const keyValidation = validateFlatWikiKey(input.key); + if (!keyValidation.ok) { + return { + markdown: keyValidation.error, + structured: { blockKey: input.key, content: '', scope: '', found: false }, + }; + } const wikiService = context.session?.wikiService ?? this.wikiService; const page = await wikiService.readPageForUser(context.userId, input.key); diff --git a/packages/context/src/wiki/tools/wiki-remove.tool.test.ts b/packages/context/src/wiki/tools/wiki-remove.tool.test.ts index 71abcd12..2cdb87e0 100644 --- a/packages/context/src/wiki/tools/wiki-remove.tool.test.ts +++ b/packages/context/src/wiki/tools/wiki-remove.tool.test.ts @@ -22,6 +22,25 @@ describe('WikiRemoveTool', () => { expect(result.markdown).toMatch(/removed/i); }); + it('rejects slash-delimited page keys with a flat-key suggestion', async () => { + const wikiService = { + deletePage: vi.fn().mockResolvedValue(undefined), + deleteFromIndex: vi.fn().mockResolvedValue(undefined), + }; + const pagesRepository = { findPageByKey: vi.fn().mockResolvedValue({ page_key: 'old' }) }; + const knowledgeRepository = { createEvent: vi.fn().mockResolvedValue(undefined) }; + const tool = new WikiRemoveTool(wikiService as any, pagesRepository as any, knowledgeRepository as any); + + const result = await tool.call({ key: 'orbit/company-overview' } as any, baseContext); + + expect(result.structured).toEqual({ success: false, key: 'orbit/company-overview' }); + expect(result.markdown).toContain( + 'Invalid wiki key "orbit/company-overview". Wiki keys must be flat; use "orbit-company-overview".', + ); + expect(pagesRepository.findPageByKey).not.toHaveBeenCalled(); + expect(wikiService.deletePage).not.toHaveBeenCalled(); + }); + it('skips deleteFromIndex when session is worktree-scoped', async () => { const wikiService = { readPage: vi.fn().mockResolvedValue({ pageKey: 'old', frontmatter: { summary: 'Old' }, content: 'body' }), diff --git a/packages/context/src/wiki/tools/wiki-remove.tool.ts b/packages/context/src/wiki/tools/wiki-remove.tool.ts index f128e47b..7cb56e7d 100644 --- a/packages/context/src/wiki/tools/wiki-remove.tool.ts +++ b/packages/context/src/wiki/tools/wiki-remove.tool.ts @@ -3,6 +3,7 @@ import type { KnowledgeIndexPort } from '../ports.js'; import type { KnowledgeEventPort } from '../ports.js'; type BlockScope = 'GLOBAL' | 'USER'; import { KnowledgeWikiService } from '../index.js'; +import { validateFlatWikiKey } from '../keys.js'; import { BaseTool, type ToolContext, type ToolOutput, validateActionRawPaths } from '../../tools/index.js'; const SYSTEM_AUTHOR = 'System User'; @@ -46,6 +47,13 @@ export class WikiRemoveTool extends BaseTool { const wikiService = context.session?.wikiService ?? this.wikiService; const writesGlobal = !!context.session; const skipIndex = context.session?.isWorktreeScoped === true; + const keyValidation = validateFlatWikiKey(input.key); + if (!keyValidation.ok) { + return { + markdown: keyValidation.error, + structured: { success: false, key: input.key }, + }; + } const rawPathValidation = validateActionRawPaths(context.session, input.rawPaths); if (!rawPathValidation.ok) { return { diff --git a/packages/context/src/wiki/tools/wiki-search.tool.test.ts b/packages/context/src/wiki/tools/wiki-search.tool.test.ts index a15a78df..33bd752b 100644 --- a/packages/context/src/wiki/tools/wiki-search.tool.test.ts +++ b/packages/context/src/wiki/tools/wiki-search.tool.test.ts @@ -6,8 +6,8 @@ describe('WikiSearchTool', () => { const search = vi.fn(async () => ({ results: [ { - key: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + key: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', scope: 'GLOBAL' as const, summary: 'Revenue metric definition', score: 0.02459016393442623, @@ -27,8 +27,8 @@ describe('WikiSearchTool', () => { expect(result.structured).toEqual({ results: [ { - blockKey: 'metrics/revenue', - path: 'knowledge/global/metrics/revenue.md', + blockKey: 'metrics-revenue', + path: 'knowledge/global/metrics-revenue.md', summary: 'Revenue metric definition', score: 0.02459016393442623, matchReasons: ['lexical', 'token'], @@ -36,6 +36,6 @@ describe('WikiSearchTool', () => { ], totalFound: 1, }); - expect(result.markdown).toContain('**metrics/revenue**'); + expect(result.markdown).toContain('**metrics-revenue**'); }); }); diff --git a/packages/context/src/wiki/tools/wiki-write.tool.test.ts b/packages/context/src/wiki/tools/wiki-write.tool.test.ts index 17c6144a..16d2abad 100644 --- a/packages/context/src/wiki/tools/wiki-write.tool.test.ts +++ b/packages/context/src/wiki/tools/wiki-write.tool.test.ts @@ -38,6 +38,21 @@ describe('WikiWriteTool', () => { expect(result.markdown).toMatch(/created/i); }); + it('rejects slash-delimited page keys with a flat-key suggestion', async () => { + const { tool, wikiService } = makeTool(); + const result = await tool.call( + { key: 'orbit/company-overview', summary: 'Company overview', content: '# Orbit' } as any, + baseContext, + ); + + expect(result.structured).toEqual({ success: false, key: 'orbit/company-overview' }); + expect(result.markdown).toContain( + 'Invalid wiki key "orbit/company-overview". Wiki keys must be flat; use "orbit-company-overview".', + ); + expect(wikiService.readPage).not.toHaveBeenCalled(); + expect(wikiService.writePage).not.toHaveBeenCalled(); + }); + it('normalizes accidentally escaped markdown newlines before writing', async () => { const { tool, wikiService } = makeTool(); @@ -150,7 +165,7 @@ describe('WikiWriteTool', () => { await tool.call( { - key: 'queries/monthly-paid-orders', + key: 'monthly-paid-orders', summary: 'Monthly paid orders', tags: ['historic-sql', 'query-pattern'], sl_refs: ['analytics.orders'], @@ -225,7 +240,7 @@ describe('WikiWriteTool', () => { const { tool, wikiService } = makeTool({ wikiService: { readPage: vi.fn().mockResolvedValue({ - pageKey: 'queries/monthly-paid-orders', + pageKey: 'monthly-paid-orders', frontmatter: existingFrontmatter, content: 'old body', }), @@ -234,7 +249,7 @@ describe('WikiWriteTool', () => { await tool.call( { - key: 'queries/monthly-paid-orders', + key: 'monthly-paid-orders', summary: 'Monthly paid orders updated', content: '## Monthly paid order count updated', } as any, diff --git a/packages/context/src/wiki/tools/wiki-write.tool.ts b/packages/context/src/wiki/tools/wiki-write.tool.ts index 1ca9f31d..edd34f8f 100644 --- a/packages/context/src/wiki/tools/wiki-write.tool.ts +++ b/packages/context/src/wiki/tools/wiki-write.tool.ts @@ -3,6 +3,7 @@ import type { KnowledgeIndexPort } from '../ports.js'; import type { KnowledgeEventPort } from '../ports.js'; type BlockScope = 'GLOBAL' | 'USER'; import { KnowledgeWikiService, type WikiFrontmatter } from '../index.js'; +import { validateFlatWikiKey } from '../keys.js'; import { applySqlEdits } from '../../tools/sql-edit-replacer.js'; import { BaseTool, type ToolContext, type ToolOutput, validateActionRawPaths } from '../../tools/index.js'; @@ -160,6 +161,13 @@ tags/refs/sl_refs use REPLACE semantics: omit to keep existing on update, [] to const wikiService = context.session?.wikiService ?? this.wikiService; const writesGlobal = !!context.session; const skipIndex = context.session?.isWorktreeScoped === true; + const keyValidation = validateFlatWikiKey(input.key); + if (!keyValidation.ok) { + return { + markdown: keyValidation.error, + structured: { success: false, key: input.key }, + }; + } const rawPathValidation = validateActionRawPaths(context.session, input.rawPaths); if (!rawPathValidation.ok) { return {