Fix deletion-only diff mapping; harden the API server and web tier - #2
Merged
Merged
Conversation
Correctness: - diff parser dropped pure-deletion hunks (only '-' and context lines), producing zero line ranges so the enclosing function was never flagged and its callers were never traversed — a silent empty blast radius for one of the highest-impact change types. Anchor a zero-width range at the deletion point; a following '+' (a modification) supersedes it, and consecutive deletions collapse to one range. Covered by two new tests and verified end-to-end via a deletion diff against ComputeBlastRadius. Security: - serve now binds 127.0.0.1 by default (was all interfaces with no auth while the banner claimed localhost); --host makes remote binding opt-in. - handleGitDiff passes --end-of-options before the user-supplied revision so a '-'-prefixed value can't be parsed as a git flag. Truthfulness / hygiene: - Drop the stale "WebSocket" claim from serve --help and the README; the feature was removed in d7a59cc. - CI blast-radius job now enforces the exit-code contract: block on HIGH (1) or tool failure (3), MEDIUM (2) stays advisory. The comment still posts. - gitignore cv.*, ideas.txt, web/.next/; allow docs/*.html. - Add docs/architecture-explorer.html: an offline, interactive four-layer architecture + LLD + SOTA explorer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BcVscS2c2WCee2ZcU353bF
The graph carried only a local RootPath, so analysis joined a diff to the
graph on file paths alone — with no way to tell a stale graph from a fresh
one, or (via the server) one repo's diff from another's. This is the gap
behind the dashboard's unsound "analyze any PR" feature and the documented
graph/diff revision-skew weakness.
- GraphData gains Commit (indexed HEAD) and RepoRemote (normalized origin).
- index stamps both at store time (best-effort; empty outside a git repo).
- analyze warns when the graph's commit != current HEAD, since a stale
graph drifts line numbers and silently mis-maps the diff.
- New GET /api/repo exposes {remote, commit, root_path} so the dashboard
can scope PR analysis to the served repo instead of an arbitrary one.
- normalizeRemote canonicalizes https/ssh/.git forms; covered by tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BcVscS2c2WCee2ZcU353bF
The dashboard analyzed any GitHub repo's diff against the local graph — no repo->graph binding — so it silently produced nonsense for any repo other than the served one. And four endpoints had no UI, one shelled out to git with a user-controlled revision, and the web app was in no CI job at all. - analyze page: scope the GitHub PR picker to the served repo via /api/repo (parse its remote); the diff and the graph now always describe the same code. Add a "Local changes" mode (analyze the served repo's uncommitted work tree) and keep Paste Diff. - server: drop the dead /api/why and /api/analyze/branches handlers, and reduce /api/analyze/diff to the working-tree diff against HEAD only — no query value reaches git's argument parser anymore. /api/reload stays (exercised by the concurrency test). - github proxy: remove the now-unused repos action. - api client: add getRepo(); drop the dead why/getBranches/reload clients; add analyzeLocal(). - auth: read BETTER_AUTH_SECRET from env so sessions survive restart, and document that production must be a GitHub App (installation tokens, real DB) — an OAuth app can't create check runs. - CI: new web job (npm ci, tsc --noEmit, lint, next build) so the dashboard can no longer break silently. Verified: go build + go test -race green; web tsc/lint/next build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BcVscS2c2WCee2ZcU353bF
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.
Summary
Three focused commits: a real correctness bug in the core product, the API-server security hardening, and the web-tier fixes — plus an interactive architecture explorer.
Correctness
-and context lines produced zero line ranges, so the enclosing function was never flagged and its callers were never traversed — a silent empty blast radius for one of the highest-impact change types. Fixed by anchoring a zero-width range at the deletion point (a following+supersedes it; consecutive deletions collapse). Covered by 2 new tests and verified end-to-end.Security / hardening
servebinds127.0.0.1by default (was all interfaces, no auth, while the banner claimed localhost);--hostmakes remote binding an explicit opt-in./api/analyze/diffis reduced to the working-tree diff against HEAD; the arbitrary base/head form is gone.GraphDatanow carriesCommit+RepoRemote, stamped at index time;analyzewarns on graph/HEAD skew. NewGET /api/repo.Web tier
/api/repo), fixing the flaw where any GitHub repo's diff was analyzed against the local graph and returned confident nonsense. Added a "Local changes" mode; kept Paste Diff./api/whyand/api/analyze/branchesendpoints and their unused clients.BETTER_AUTH_SECRETfrom env so sessions survive restart, with a documented note that production must be a GitHub App (an OAuth app can't create check runs).webjob (tsc --noEmit, lint,next build) so the dashboard can no longer break silently. The dogfooding workflow's risk gate now blocks on HIGH / tool-failure only (MEDIUM stays advisory).Docs
docs/architecture-explorer.html— an offline, interactive four-layer architecture + LLD + SOTA explorer.Verification
go build ./...andgo test ./... -racegreen across all packages (incl. the server concurrency test).tsc --noEmit,npm run lint, andnext buildall clean.ComputeBlastRadiusnow reports the function + its 17 dependents (previously zero).🤖 Generated with Claude Code