chore: phase 1 hygiene + phase 2 internals#2
Merged
Conversation
Runs on push and pull_request. A build job runs `npm ci`, `npm run typecheck`, and `npm test` on a Node 20.x/22.x matrix. A separate secrets-scan job runs scripts/secrets_scan.py as the CI tripwire its own docstring advertises, failing the job on non-zero exit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The server reads no environment variables (grep for process.env in src/ finds only a smell-detector pattern and a tool description, never a read), and the README states no API keys are required. The ANTHROPIC_API_KEY entry was leftover from the LLM-backed revenue- diagnosis tool removed in 2.0.0. Reduce the file to a comment making the no-env contract explicit, so .env.example, the code, and the README agree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add engines.node ">=18" (code relies on NodeNext ESM and node:fs/promises). - Add tsconfig.typecheck.json (extends base, rootDir ".", includes src + tests, excludes the synthetic auth/dep fixtures which import uninstalled packages by design) and point `npm run typecheck` at it. Build output is unchanged: build still uses tsconfig.build.json (src only). - Align tests/v2_1.test.ts to the .js import convention used by the other three suites (was importing ../src/server.ts, both static and dynamic). No exports map added: see commit notes / report — index.js boots a stdio server on import, so exposing it as the package import entry would be a footgun, and the package is consumed via `bin`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scripts/secrets_scan.py and scripts/install_hooks.ps1 were untracked while the README's "Secrets & API keys" section (also pending) pointed at them, so the public README referenced files not in the repo. Commit the scripts together with that README section so the docs are accurate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove fileCache, readFirstLinesCached, and clearFileCache from src/analyzer.ts. Grep confirmed they were referenced nowhere outside their own definitions (no imports in src/, tests/, or scripts/). Decision: delete rather than wire in. The server runs over stdio as a short-lived, one-repo-per-session process, so a long-lived mutable file cache adds stale-content risk and invalidation complexity the usage pattern does not justify. readFirstLines and streamFileForKeywords are live and kept unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The five copies of the IGNORED_DIRS-aware recursive readdir
(runScanProjectStructure, runCollectTsAndJsonFiles, runCollectTsFiles,
the inline walk in runFindLikelyConfigFiles, and collectFiles) are
replaced by a single async-generator walkTree(). Each call site keeps
its exact filter and sort, so tool output is unchanged.
Two pre-existing per-site quirks are deliberately PRESERVED, not
flattened (both annotated with comments):
- runScanProjectStructure has no isFile() guard (any non-directory
entry counts as a file); the other four require entry.isFile().
- runFindLikelyConfigFiles collects native-separator paths and sorts
before POSIX-normalizing; the others sort POSIX-normalized paths.
Verified byte-for-byte identical output across all 8 tools on a fixed
external corpus (ignored dirs, nested dirs, mixed extensions, config
files, smells) — before/after SHA256 matched. 25/25 tests pass,
typecheck green. Net -44 lines across the two refactor commits.
The execution-plan re-read path (runExecutionPlanForTask runs three
scanners in parallel, then loadShallowFileContext re-reads heads) is
left functionally as-is: deduping it cleanly would require either a
mutable cache (explicitly rejected this phase) or threading read
results through three independently-scoring functions, which risks
output drift for a micro-optimization. Output stability wins here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI, env/engines/typecheck consistency, committed scripts (phase 1); dead-cache deletion + walk unification (phase 2). Both audited independently, output byte-for-byte stable, 25/25 tests, CI green on both branches.