feat: consolidate launch readiness improvements#114
Conversation
Greptile SummaryThis PR consolidates six launch-readiness workstreams: bounded/full codebase map modes, scoped memory, a Confidence Score: 5/5Safe to merge — all remaining findings are P2 style suggestions with no correctness or data-integrity impact. All three comments are P2: a project-style convention (CLI constants placement), a TypeScript safety preference (! assertions), and a minor UX edge case (limit=0 clamp). None affect runtime correctness, data integrity, or security. src/index.ts — CLI subcommand constants and !-asserted lazy vars worth addressing in a follow-up.
|
| Filename | Overview |
|---|---|
| src/index.ts | Major refactor: lazy-loads MCP runtime on non-CLI paths, adds idle-session STDIO timeout, full-context resource URIs, and defers server creation to ensureMcpRuntimeLoaded(). Two style concerns: CLI subcommand constants belong in src/cli.ts per AGENTS.md, and nine !-asserted lazy vars bypass TypeScript null safety. |
| src/health/derive.ts | New file: derives CodebaseHealthArtifact from indexed chunks and the file-import graph, scoring files by cycle participation, fan-in, hotspot rank, and cyclomatic complexity. |
| src/health/store.ts | New file: validates, normalises, reads, and indexes the health.json artifact. Defensive normalisation handles every field. |
| src/tools/get-codebase-health.ts | New tool exposing health.json to MCP clients. Minor: Math.max(1, Math.floor(limit)) silently clamps limit=0 to 1, and no minimum is declared in the schema. |
| src/tools/search-codebase.ts | Integrates health signals and scoped-memory boost into search results and the decision card. Unused _riskLevel renamed to riskLevel and now properly emitted. |
| src/memory/store.ts | Adds scoped-memory support. Scope is included in the dedup hash, query haystack, and sort-boost pipeline. |
| src/resources/uri.ts | Adds full-context URI variants. normalizeResourceUri simplified to a single indexOf slice — correctly handles all URI forms. |
| src/core/codebase-map.ts | Adds bounded/full map modes with per-section limits and test/generated file exclusion. |
| src/core/indexer.ts | Derives and writes health.json to activeContextDir (staging during full rebuild), consistent with other artifacts. |
| src/eval/edit-preflight-harness.ts | New eval harness for edit-preflight quality scoring. |
| src/tools/remember.ts | Adds optional scoped memory parameter, validated and included in dedup hash. |
Reviews (1): Last reviewed commit: "fix(ci): unblock functional tests" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const CLI_SUBCOMMANDS = [ | ||
| 'memory', | ||
| 'search', | ||
| 'metadata', | ||
| 'status', | ||
| 'reindex', | ||
| 'style-guide', | ||
| 'patterns', | ||
| 'refs', | ||
| 'cycles', | ||
| 'init', | ||
| 'map' | ||
| ]; | ||
|
|
||
| // Check if this module is the entry point. | ||
| const isDirectRun = | ||
| process.argv[1]?.replace(/\\/g, '/').endsWith('index.js') || | ||
| process.argv[1]?.replace(/\\/g, '/').endsWith('index.ts'); | ||
| const directSubcommand = process.argv[2]; | ||
| const isDirectCliSubcommand = | ||
| isDirectRun && | ||
| typeof directSubcommand === 'string' && |
There was a problem hiding this comment.
CLI_SUBCOMMANDS, isDirectRun, and isDirectCliSubcommand are CLI-routing concerns placed directly in src/index.ts. The project's AGENTS.md states: "CLI code belongs in src/cli.ts. Never in src/index.ts." These constants guard against loading the heavy MCP runtime on CLI paths, which is a valid optimization, but the implementation places CLI-dispatch logic in the wrong module.
Consider exporting an isCliSubcommandInvocation() helper from src/cli.ts and importing it here, keeping src/index.ts limited to routing/protocol concerns.
Context Used: AGENTS.md (source)
| let createServer!: typeof import('./server/factory.js').createServer; | ||
| let startHttpServer!: typeof import('./server/http.js').startHttpServer; | ||
| let loadServerConfig!: typeof import('./server/config.js').loadServerConfig; | ||
| let StdioServerTransport!: typeof import('@modelcontextprotocol/sdk/server/stdio.js').StdioServerTransport; | ||
| let CallToolRequestSchema!: typeof import('@modelcontextprotocol/sdk/types.js').CallToolRequestSchema; | ||
| let ListToolsRequestSchema!: typeof import('@modelcontextprotocol/sdk/types.js').ListToolsRequestSchema; | ||
| let ListResourcesRequestSchema!: typeof import('@modelcontextprotocol/sdk/types.js').ListResourcesRequestSchema; | ||
| let ReadResourceRequestSchema!: typeof import('@modelcontextprotocol/sdk/types.js').ReadResourceRequestSchema; | ||
| let RootsListChangedNotificationSchema!: typeof import('@modelcontextprotocol/sdk/types.js').RootsListChangedNotificationSchema; | ||
| let server!: Server; |
There was a problem hiding this comment.
!) on lazy-loaded module-level vars suppress TypeScript null safety
Nine module-level variables (server, createServer, StdioServerTransport, all five *Schema vars, etc.) use ! to bypass the TypeScript uninitialized-variable check. While the runtime sequence (ensureMcpRuntimeLoaded() is called before any of these are accessed) is correct, the assertions are invisible to the compiler — any future code path that reads them before initialization will produce a runtime TypeError with no compile-time warning.
A safer alternative is let server: Server | undefined with a narrow accessor (e.g., function getServer(): Server { if (!server) throw new Error('MCP runtime not loaded'); return server; }) so TypeScript enforces the invariant at each call site.
| const files = health.files | ||
| .filter((entry) => orderedLevels[entry.level] >= minLevel) | ||
| .slice(0, Math.max(1, Math.floor(limit))); |
There was a problem hiding this comment.
limit: 0 silently returns 1 result; schema has no minimum constraint
Math.max(1, Math.floor(limit)) clamps any non-positive limit to 1, so a caller that passes limit: 0 (intending "no files, summary only") receives one result instead. The JSON Schema definition does not declare a minimum, so this behaviour is undocumented and surprising.
| const files = health.files | |
| .filter((entry) => orderedLevels[entry.level] >= minLevel) | |
| .slice(0, Math.max(1, Math.floor(limit))); | |
| const files = health.files | |
| .filter((entry) => orderedLevels[entry.level] >= minLevel) | |
| .slice(0, Math.max(0, Math.floor(limit))); |
And add minimum: 1 to the limit property in the tool's inputSchema if the intent is that at least one file should always be returned, or minimum: 0 if "no files" is a valid response.
1 participant
Summary
Verification
Notes