Conversation
…g, path traversal Co-authored-by: Looted <6255880+Looted@users.noreply.github.com> Agent-Logs-Url: https://github.com/Looted/kibi/sessions/e93522d3-426b-4c57-a231-ecec98867131
fix: shared search ranking, path traversal in loadMarkdownBody, null syncedAt
Feature/discovery bundle
There was a problem hiding this comment.
Pull request overview
This PR expands Kibi’s “curated public surface” by adding a discovery/reporting bundle across MCP and CLI (search/status/gaps/coverage/graph), updates agent-facing guidance to be discovery-first (kb_search → kb_query), and refreshes docs/tests to match the new toolset and the default kibi init hook behavior.
Changes:
- Add new MCP tools (
kb_search,kb_status,kb_find_gaps,kb_coverage,kb_graph) plus supporting Prolog predicates (status.pl,discovery.pl) and shared MCP helpers. - Add new CLI commands (
kibi search/status/gaps/coverage/graph) and a shared table/JSON rendering helper. - Update OpenCode/VS Code guidance + repo docs/tests/changesets to reflect the curated tool surface and discovery-first workflow.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/populate-kb.ts | Update KB scenario title for kibi init |
| README.md | Document curated setup + discovery workflow |
| packages/vscode/README.md | Document dogfood + curated tool usage |
| packages/opencode/tests/prompt.test.ts | Update prompt guidance assertions |
| packages/opencode/tests/hook-contract.test.ts | Expand public tool assertions |
| packages/opencode/tests/agent-surface-policy.test.ts | Enforce curated MCP tool surface |
| packages/opencode/src/prompt.ts | Discovery-first agent guidance text |
| packages/opencode/README.md | Document discovery-first MCP guidance |
| packages/mcp/tsconfig.json | Add path alias for search-ranking |
| packages/mcp/tests/tools/status.test.ts | New unit tests for kb_status |
| packages/mcp/tests/tools/search.test.ts | New unit tests for kb_search |
| packages/mcp/tests/tools/graph.test.ts | New unit tests for kb_graph |
| packages/mcp/tests/tools/find-gaps.test.ts | New unit tests for kb_find_gaps |
| packages/mcp/tests/tools/coverage.test.ts | New unit tests for kb_coverage |
| packages/mcp/tests/tools/check-aggregated.test.ts | Assert richer kb_check text |
| packages/mcp/tests/server.test.ts | Validate expanded tool listing + status behavior |
| packages/mcp/src/tools/status.ts | MCP handler for kb_status |
| packages/mcp/src/tools/search.ts | MCP handler for kb_search |
| packages/mcp/src/tools/query.ts | Refactor to shared entity loader |
| packages/mcp/src/tools/graph.ts | MCP handler for kb_graph |
| packages/mcp/src/tools/find-gaps.ts | MCP handler for kb_find_gaps |
| packages/mcp/src/tools/entity-query.ts | Shared Prolog goal + entity loading |
| packages/mcp/src/tools/coverage.ts | MCP handler for kb_coverage |
| packages/mcp/src/tools/core-module.ts | Shared Prolog module JSON runner |
| packages/mcp/src/tools/check.ts | Improve human-readable violations output |
| packages/mcp/src/tools-config.ts | Register schemas for new tools |
| packages/mcp/src/server/tools.ts | Register new MCP tools + diagnostics fields |
| packages/mcp/src/server/docs.ts | Update MCP prompt/docs for curated surface |
| packages/mcp/src/diagnostics.ts | Derive additional usage-log fields |
| packages/core/src/status.pl | Prolog predicate for snapshot/freshness status |
| packages/core/src/discovery.pl | Prolog predicates for gaps/coverage/graph |
| packages/cli/tests/commands/status.test.ts | CLI status tests |
| packages/cli/tests/commands/search.test.ts | CLI search tests |
| packages/cli/tests/commands/graph.test.ts | CLI graph tests |
| packages/cli/tests/commands/gaps.test.ts | CLI gaps tests |
| packages/cli/tests/commands/coverage.test.ts | CLI coverage tests |
| packages/cli/src/search-ranking.ts | Deterministic search ranking + markdown loading |
| packages/cli/src/commands/status.ts | CLI status command |
| packages/cli/src/commands/search.ts | CLI search command |
| packages/cli/src/commands/graph.ts | CLI graph command |
| packages/cli/src/commands/gaps.ts | CLI gaps command |
| packages/cli/src/commands/doctor.ts | Replace init --hooks references |
| packages/cli/src/commands/discovery-shared.ts | Shared JSON/table rendering helpers |
| packages/cli/src/commands/coverage.ts | CLI coverage command |
| packages/cli/src/cli.ts | Wire new CLI subcommands |
| packages/cli/package.json | Export search-ranking entrypoint |
| documentation/tests/TEST-mcp-search-discovery.md | New discovery bundle verification test entity |
| documentation/tests/TEST-005.md | Update MCP toolset expectations |
| documentation/tests/e2e/packed/mcp.test.ts | Update tool list for packed E2E |
| documentation/tests/e2e/packed/mcp-protocol-regression.test.ts | Update tool list regression |
| documentation/tests/e2e/packed/issue-53-regression.test.ts | Update curated tool surface wording/assertions |
| documentation/tests/e2e/packed/discovery-bundle.test.ts | New packed E2E parity coverage |
| documentation/symbols.yaml | Refresh symbol coordinates |
| documentation/scenarios/SCEN-mcp-search-discovery.md | New discovery bundle scenario entity |
| documentation/scenarios/SCEN-002.md | Update init scenario title/updated_at |
| documentation/requirements/REQ-opencode-kibi-plugin-v1.md | Expand public tool references |
| documentation/requirements/REQ-opencode-agent-mcp-only.md | Update curated surface definition |
| documentation/requirements/REQ-mcp-search-discovery.md | New requirement for discovery/reporting tools |
| documentation/requirements/REQ-014.md | Update hook installation language |
| documentation/requirements/REQ-013.md | Clarify inference vs curated public surface |
| documentation/requirements/REQ-008.md | Update hook install language |
| documentation/requirements/REQ-002.md | Update MCP requirement for curated surface |
| documentation/facts/FACT-MCP-TOOLSET-CORE-6.md | Update fact to “curated surface” |
| documentation/facts/FACT-INFERENCE-TOOLS-CORE-3.md | Update inference surface wording |
| documentation/facts/FACT-INFERENCE-SURFACE.md | Update inference surface wording |
| documentation/adr/ADR-discovery-tools-public-surface.md | New ADR for discovery tools |
| docs/troubleshooting.md | Replace init --hooks docs |
| docs/superpowers/specs/2026-03-22-discovery-bundle-design.md | New spec document |
| docs/superpowers/plans/2026-03-22-discovery-bundle.md | New implementation plan |
| docs/prompts/llm-rules.md | Discovery-first agent rules |
| docs/mcp-reference.md | Document new MCP tools + workflows |
| docs/install.md | Dogfood workflow + discovery commands |
| docs/inference-rules.md | Curated surface vs inference clarification |
| docs/cli-reference.md | Document new CLI commands + updates |
| CONTRIBUTING.md | Update hook language (kibi init) |
| .changeset/docs-discovery-bundle-refresh.md | Changeset for docs/prompt refresh |
| .changeset/discovery-bundle-tools.md | Changeset for new discovery bundle tools |
| synced_at(DataFile, SyncedAt) :- | ||
| exists_file(DataFile), | ||
| !, | ||
| time_file(DataFile, Timestamp), | ||
| format_time(atom(SyncedAt), '%FT%TZ', Timestamp). | ||
| synced_at(_, @(null)). |
There was a problem hiding this comment.
syncedAt is formatted with '%FT%TZ' but format_time/3 uses local time by default, so the emitted string can be local-time while still being suffixed with Z (UTC). Consider either formatting with an explicit UTC option (e.g. format_time(..., [utc(true)])) or using an offset-aware format like %:z to avoid mislabeling the timezone.
| node_dict(Id, _{id: Id, type: Type, title: Title, status: Status}) :- | ||
| kb_entity(Id, Type, Props), | ||
| entity_title_status_source(Props, Title, Status, _Source). | ||
|
|
There was a problem hiding this comment.
node_dict/2 calls entity_title_status_source/4, which requires title and status props via memberchk/2. If the KB contains an incomplete entity (which can happen before kb_check runs), graph_expand_json/8 will fail the entire traversal when mapping nodes. Consider making node_dict/2 tolerant (e.g., defaulting missing title/status to empty values) so graph traversal still returns nodes even when some entities are malformed.
| // Resolve to absolute path; relative paths are resolved against workspaceRoot. | ||
| const resolved = path.resolve( | ||
| path.isAbsolute(normalizedSource) ? normalizedSource : path.join(workspaceRoot, normalizedSource), | ||
| ); | ||
|
|
||
| // Reject paths that escape the workspace root to prevent path traversal. | ||
| const normalizedRoot = path.resolve(workspaceRoot); | ||
| if (!resolved.startsWith(normalizedRoot + path.sep)) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The path traversal guard (resolved.startsWith(normalizedRoot + path.sep)) is a string-prefix check and doesn’t account for symlinks: a workspace-local symlinked markdown file can still point outside the workspace and be read successfully. If the goal is to ensure kb_search only reads markdown bodies from within the workspace, consider comparing fs.realpath() of both workspaceRoot and the candidate file (and/or explicitly rejecting symlinks) and using path.relative to enforce the file remains under the workspace root across platforms/case-insensitive FS.
No description provided.