server

server — audit issues

Audited 2026-06-20. Repo: software/server. Criteria: completeness · tests · separation of concerns · verb-named functions · file size (<1000 lines) · organization.

Health summary

The server is small (≈850 lines across 7 files), cleanly layered (transport in main.cpp, protocol in Dispatch.cpp, domain handlers in Tools.cpp, message marshalling in Doc.h), builds clean with -Wall -Wextra, and ./test.sh passes all 9 checks. The pserver::Session rename (commit 4c3762c) is fully applied in source — no Document-wrapper collision remains and no stale prisma/prism identifiers leak outside legitimate .prisma/.prism extension handling. The main weaknesses are completeness gaps where mutating tools (connect/disconnect/rename/set_metadata) report success without validating their targets, thin test coverage (most of the 16 tools and the parse-error / missing-name paths are never exercised), and documentation drift (README tools table omits 5 tools and still names the MCP server prisma). No file exceeds 800 lines.

Issues

[MEDIUM] completeness — connect/disconnect report success without validating endpoints

• Where: src/Tools.cpp:290-301 (handlers); kinogaki-core/include/kinogaki/Document.h:51-52 (void connect/void disconnect)
• Problem: connect parses source/target only for syntactic validity, then calls s.connect(*src, *tgt) (a void) and unconditionally returns okText("connected …"). There is no check that either element/slot exists, so an agent connecting to a typo'd or nonexistent path gets a success message and a dangling connection. disconnect likewise always reports success even when no connection into target existed. The agent has no way to tell a real wiring from a no-op.
• Fix: Validate that the source and target elements exist (and ideally that the slot paths resolve) before calling connect; for disconnect, check s.connections() for an existing edge into target and report "no connection into …" when absent. Return a failed ToolResult otherwise.

[MEDIUM] completeness — rename_element reports success on a rejected/no-op rename

• Where: src/Tools.cpp:248-257
• Problem: Document::rename returns from unchanged when it rejects the move (e.g. moving an element under its own descendant) or no-ops (Document.h:48). The handler already guards !has(from), existing-to, and parentExists, but a self-descendant move slips past all three and is reported as "renamed to <from>" — a silent failure the agent reads as success.
• Fix: Compare the returned final path against *to; if they differ (and it wasn't an intentional uniquify), surface that the rename was rejected/adjusted rather than reporting plain success.

[MEDIUM] tests — protocol surface and most tools are untested

• Where: test.sh:13-39
• Problem: The smoke test covers initialize, tools/list, define_element, set_property, get_element, save, one failing get_element, and one unknown method. It never exercises: the JSON-RPC parse-error path (Dispatch.cpp:20-28, malformed JSON → -32700), the missing-method case (-32600, Dispatch.cpp:46), tools/call with no name (Dispatch.cpp:74-76), or 11 of the 16 tools (remove_element, rename_element, remove_property, set_metadata, connect, disconnect, load, search, compose, import, export). No auth/session model exists to test (stdio, single doc), but the request-validation and error-code surface is largely unverified.
• Fix: Add cases feeding a non-JSON line (assert -32700), a request with no method (assert -32600), a tools/call missing name (assert isError), and at least round-trip coverage for the structural mutators (define→connect→get_element shows the wiring; rename_element; remove_element subtree count) and one foreign-format import/export.

[LOW] completeness — no initialize-ordering enforcement

• Where: src/Dispatch.cpp:49-86
• Problem: The dispatcher will service tools/list and tools/call before any initialize handshake, and initialize can be sent more than once, with no state tracking. This is lenient rather than wrong for an stdio MCP server, but it means protocol-sequence violations are silently accepted.
• Fix: Optionally track an "initialized" flag and reject pre-initialize tools/* with a JSON-RPC error; at minimum note the intentional leniency in a comment.

[LOW] organization — README tools table omits 5 tools and example names the server prisma

• Where: README.md:19-33 (table), README.md:39 (.mcp.json example)
• Problem: The catalog in buildToolCatalog advertises 16 tools (Tools.cpp:167-201), but the README table documents only 11 — load, search, compose, import, export are missing. The .mcp.json snippet still registers the server under the key "prisma" (a pre-rename name), inconsistent with the kinogaki-server binary and serverInfo.name.
• Fix: Add the 5 missing tools to the table and rename the example server key to kinogaki (or prism). Also consider a one-line "no auth / single document / stdio" note for the protocol model.

[LOW] organization — stale binaries in the (gitignored) build dir

• Where: build/prism-server, build/prisma-mcp, build/prisma-server
• Problem: build/ is correctly gitignored and untracked, but it holds three obsolete binaries from earlier rename eras (prisma-mcp, prisma-server, prism-server) alongside the current kinogaki-server. Not committed, so harmless to the repo, but confusing local cruft that can mask which binary is current.
• Fix: rm build/prism-server build/prisma-mcp build/prisma-server (or rm -rf build and rebuild). No repo change needed.

[LOW] separation — clean

• Where: src/main.cpp (transport/stdio loop), src/Dispatch.cpp (JSON-RPC protocol), src/Tools.cpp (domain handlers), src/Doc.h (Reader/Builder message marshalling), src/Log.h (sink-swappable log)
• Problem: None. Transport, JSON-RPC protocol, domain tool dispatch, the "eat-our-own-food" Document message layer, and logging are each in their own unit, with dispatch(Session&, line) a deliberately reusable seam. No god files. (One stale comment: Dispatch.h:3 and Log.h:4 still reference an "editor calls it per HTTP request" / "Phase 3" integration that has no code here — cosmetic.)

[LOW] naming — clean

• Where: all src/
• Problem: None. Functions read as verbs: dispatch, callTool, buildToolCatalog, readFile/writeFile, parsePath, parentExists, childrenOf, coerce, valueText, endsWith, lowerExt, codecFor, addTool, logln. Reader/Builder/Session/ToolResult/Log are types (exempt). No noun-named or ambiguous function names.

[LOW] filesize — clean

• Where: all src/
• Problem: None. Largest file is src/Tools.cpp at 380 lines; no file exceeds 800 (let alone 1000). Line counts: Tools.cpp 380, Dispatch.cpp 91, Doc.h 74, main.cpp 70, Log.h 39, Tools.h 31, Dispatch.h 14.