Skip to content

PLA-52: one owner for knowledge/ - #114

Merged
hristo2612 merged 2 commits into
mainfrom
simplify/PLA-52-one-knowledge-owner
Aug 3, 2026
Merged

PLA-52: one owner for knowledge/#114
hristo2612 merged 2 commits into
mainfrom
simplify/PLA-52-one-knowledge-owner

Conversation

@hristo2612

Copy link
Copy Markdown
Owner

Selected area

<JINN_HOME>/knowledge/ had two independent owners:

  • packages/jinn/src/knowledge/store.ts (278 lines) — flat, non-recursive searchKnowledge over knowledge/ + docs/, gated by SEARCH_REL_PATH_PATTERN (exactly one /), plus readKnowledgeFile.
  • packages/jinn/src/notes/store.ts (581 lines) — recursive walker over the same directory, with its own validator, its own containment check, and its own byte caps.

Each module carried a full parallel implementation of the same primitives: containment (isInsideReal vs isRealpathContained), file-size caps (SEARCH_FILE_MAX_BYTES vs NOTE_FILE_MAX_BYTES — the same 2 MB value declared twice), and path validation.

Evidence of blended concerns

  1. A user-visible defect falls straight out of the split. create_note { folder: "product/research" } writes knowledge/product/research/<f>.md. search_knowledge — the verb every session is instructed to use (sessions/context.ts:926-929) — could never return that file, because the flat search's path gate admitted exactly one /. Notes could write where search could not read.
  2. The duplication is on record as intentional-but-temporary: docs/superpowers/plans/2026-07-14-notes-over-knowledge.md:178.
  3. Shipped doctrine forbids it: packages/jinn/template/docs/company-doctrine.md:25 rules out parallel concepts for one thing.

Fixed constraint budget

Frozen during CONSTRAIN, before any implementation (PLAN.md):

Field Cap
netLineDelta -80
filesTouched 8
newFiles 0
maxFileLines 7584

Sub-caps: notes/store.ts ≤ 800 final lines; gateway/api.ts ≤ 7584 (must not grow). No new dependencies, no new config options, no new single-caller abstractions, no public exports beyond the seven re-homed names.

Measured budget (verbatim)

netLineDelta=-80
filesTouched=5
newFiles=0
maxFileLines=7576

Sub-caps: notes/store.ts = 783 (≤ 800); gateway/api.ts = 7576 (≤ 7584, shrank by 8 lines).

What was deleted / clarified

notes/store.ts is now the only module that touches the directory.

Deleted outrightpackages/jinn/src/knowledge/ in full (502 lines: store.ts 278 + its test file 224).

Retired duplicate primitivesSEARCH_REL_PATH_PATTERN, SEARCH_FILE_MAX_BYTES, isInsideReal, realRootOf, rootDir, ROOT_LABELS. grep -rnw for those four names over packages/jinn/src now prints nothing.

Re-homed into notes/store.tsreadKnowledgeFile (moved verbatim; scope, caps, and error contract unchanged) and searchKnowledge (reimplemented on the existing notes walker, reusing its containment and its NOTE_FILE_MAX_BYTES cap).

Deliberately NOT collapsed (would triple the blast radius; deferred): the MCP tool families mcp/knowledge-tools.ts and mcp/note-tools.ts, the HTTP routes, and sessions/context.ts. All of packages/web is untouched. The public surface is unchanged in shape — every MCP verb and every route still registered and served.

Three intended behavior deltas

# Delta Rationale
D1 searchKnowledge now returns nested files (e.g. knowledge/product/research/x.md) This is the recorded defect being fixed
D2 docs/ subdirectories become searchable Same recursive regime for both roots
D3 In-root symlinked .md files are no longer returned by search Unifies on the stricter of the two regimes; Notes already enforces O_NOFOLLOW

Each has a test that fails at the base SHA and passes at HEAD. Everything else is behavior-preserved and still asserted by the moved tests: token-AND matching, case-insensitivity, filename matches, ≤20 hits sorted by matchCount desc then path, «»-marked snippets that never leak bodies, non-.md ignored, >2 MB skipped, escape-symlink content never leaked, and control-byte/oversized/empty queries degrading to empty results. Every it() title from the deleted test file survives in notes/__tests__/store.test.ts, except the D3 case, which appears inverted. readKnowledgeFile is behavior-preserved, full stop.

Test results

Run from the worktree after the final commit (7351082c):

pnpm typecheck

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    214ms >>> FULL TURBO

pnpm test

jinn-cli:test:  Test Files  308 passed (308)
jinn-cli:test:       Tests  3830 passed | 1 skipped (3831)

@jinn/web:test:  Test Files  122 passed (122)
@jinn/web:test:       Tests  1275 passed (1275)

 Tasks:    2 successful, 2 total

pnpm build

jinn-cli:build: build: compiled to dist/, copied 2 talk asset(s)

 Tasks:    2 successful, 2 total
  Time:    338ms >>> FULL TURBO

synced packages/web/out -> packages/jinn/dist/web

Privacy leak-grep over the packages/** diff: clean.

Follow-ups recorded, not done here

  1. Rename the instance-wide read primitive. readKnowledgeFile reads any regular file in the instance, not just knowledge; the name understates its blast radius. Renaming touches two MCP tool modules and the web file viewer, so it is its own Todo.
  2. Note verbs in the context manifest. sessions/context.ts still advertises only search_knowledge/read_knowledge; adding the Note verbs re-pins the manifest-hash tests for zero line savings.
  3. listNotes strips only the FIRST control byte of a query (CONTROL_BYTES lacks the /g flag). Pre-existing; deliberately preserved.
  4. Two stale comments now point at the deleted module, both in files this Todo was not allowed to touch: mcp/knowledge-tools.ts:11 and gateway/__tests__/knowledge-route.test.ts:12-13. Comment-only.
  5. Flaky test observed: workflows/__tests__/workflow-vertical.test.ts (the attemptInterruptionCause: "user-message" assertion) failed twice under a loaded full test run mid-implementation. It then passed 4/4 full runs at HEAD, 5/5 runs of that file alone, and 3/3 at the base SHA. It imports none of the moved code — pre-existing load-sensitive flake, worth its own Todo.

notes/store.ts absorbs searchKnowledge and readKnowledgeFile and
packages/jinn/src/knowledge/ is deleted, so a single module owns the
directory instead of two with independent walkers, validators, and caps.

Search is reimplemented on the Notes walker, which listNotes and
searchKnowledge now share. Three intended behavior deltas, each covered by
a test that fails at the base SHA:

  D1 nested knowledge/** files are searchable (the recorded defect:
     create_note({ folder }) wrote files search could never return)
  D2 nested docs/** files are searchable (same regime for both roots)
  D3 in-root symlinked .md files are no longer searchable, unifying on the
     stricter of the two symlink regimes

readKnowledgeFile moves unchanged. Every other search semantic is preserved
and every surviving test from the deleted store test moved across.
#113 relocated stripControlChars/hasControlBytes out of sessions/registry.ts
into shared/sanitize.ts and left no re-export, so the absorbed knowledge code
in notes/store.ts must import from the new owner. Also retarget the stale
knowledge/store.ts reference in the mcp/knowledge-tools.ts header.
@hristo2612
hristo2612 force-pushed the simplify/PLA-52-one-knowledge-owner branch from 7351082 to da56169 Compare August 3, 2026 10:10
@hristo2612
hristo2612 merged commit f9ef719 into main Aug 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant