Repository navigation
feat: server-side blast radius for PR reviews - #130
Conversation
LiveReview Pre-Commit Check: vouched (iter:1, coverage:0%)
LiveReview Pre-Commit Check: vouched (iter:1, coverage:0%)
… cache settings) LiveReview Pre-Commit Check: ran
Implement Server-Side Blast Radius CalculationOverviewThis change adds server-side blast radius scoring for code reviews. It introduces a new repository caching mechanism and integrates it with a background job queue. Users now receive automated blast radius scores directly within the review interface. Technical Highlights
Impact
|
| func MarkTooLarge(root string, c *Checkout, size int64) error { | ||
| entry := filepath.Dir(c.Dir) | ||
| removeEntry(root, entry) | ||
| repoLocks.Delete(c.Dir) |
There was a problem hiding this comment.
Severity: warning
There is a lock key mismatch in MarkTooLarge. The repoLocks.Delete function uses c.Dir instead of entry/repo.
Suggestions:
- Use consistent lock key construction matching removeIfIdle (filepath.Join(c.Dir, 'repo'))
| } | ||
| dir := filepath.Join(base, "repo") | ||
|
|
||
| mu, _ := repoLocks.LoadOrStore(dir, &sync.Mutex{}) |
There was a problem hiding this comment.
Severity: warning
The repoLocks sync.Map grows unboundedly, which will leak memory over time.
Suggestions:
- Implement a cleanup or LRU mechanism for repoLocks
- Use a bounded cache or lock striping strategy
| if err := os.MkdirAll(dir, 0o755); err != nil { | ||
| return err | ||
| } | ||
| return os.Setenv("CBM_CACHE_DIR", dir) |
There was a problem hiding this comment.
Severity: critical
os.Setenv mutates the global process environment. Concurrent requests may race on CBM_CACHE_DIR.
Suggestions:
- Pass the cache directory via command-line arguments or process execution environment instead of global os.Setenv
- Use a mutex or pool if process-wide env is unavoidable
| } | ||
| }) | ||
| .catch(() => { | ||
| if (!cancelled && ++attempts < 30) timer = setTimeout(load, 30000); |
There was a problem hiding this comment.
Severity: warning
There is an unhandled catch rejection if the async flow fails repeatedly. Consider adding an error boundary or state logging.
Suggestions:
- Capture and log polling errors after max attempts are exhausted
| min={settings.min_gb} | ||
| max={10000} | ||
| value={settings.max_gb} | ||
| onChange={(e) => setSettings({ ...settings, max_gb: parseInt(e.target.value, 10) || 0 })} |
There was a problem hiding this comment.
Severity: warning
parseInt is being used without a radix parameter. Please specify the radix 10 explicitly.
Suggestions:
- Pass radix 10 to parseInt
…drop schema.sql dump churn
Implement Server-Side Blast Radius ScoringOverviewThis change introduces server-side blast radius calculation for code reviews. It leverages a new background job system and a local repository cache. This enables automated risk assessment for UI and API-triggered reviews. Technical Highlights
Impact
|
| return 0, err | ||
| } | ||
| var st syscall.Statfs_t | ||
| if err := syscall.Statfs(root, &st); err != nil { |
There was a problem hiding this comment.
Severity: critical
The use of syscall.Statfs is platform-specific and will break compilation on Windows.
Suggestions:
- Use build tags to provide a non-Unix implementation or use a cross-platform filesystem stats library.
| // paths so they match the lock keys Prepare uses. | ||
| func entries(root string) []string { | ||
| root, _ = filepath.Abs(root) | ||
| matches, _ := filepath.Glob(filepath.Join(root, "*", "*", "*")) |
There was a problem hiding this comment.
Severity: warning
The glob pattern assumes a fixed 3-level directory depth, making it fragile if the directory structure changes.
Suggestions:
- Replace globbing with filepath.WalkDir to robustly discover cache entries regardless of depth.
|
|
||
| func dirSize(dir string) int64 { | ||
| var size int64 | ||
| _ = filepath.WalkDir(dir, func(_ string, d fs.DirEntry, err error) error { |
There was a problem hiding this comment.
Severity: critical
filepath.WalkDir follows symbolic links. A malicious repository could potentially escape the cache root.
Suggestions:
- Check path prefixes or avoid following symlinks when calculating directory size.
|
|
||
| // repoLocks: <entry>/repo -> *sync.Mutex. Bounded by the cached repos: a key is deleted | ||
| // when its repo leaves the cache (removeIfIdle, MarkTooLarge). | ||
| var repoLocks sync.Map |
There was a problem hiding this comment.
Severity: warning
Entries in repoLocks are never deleted, which will lead to unbounded memory growth over time.
Suggestions:
- Remove entries from repoLocks when repository cache is evicted or pruned
| if abs, err := filepath.Abs(indexDir); err == nil { | ||
| indexDir = abs | ||
| } | ||
| _ = os.Setenv("CBM_CACHE_DIR", indexDir) |
There was a problem hiding this comment.
Severity: warning
Calling os.Setenv within an init function can cause race conditions when running unit tests in parallel.
Suggestions:
- Pass cache directory configuration via options or function parameters instead of global env mutation
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
Implement Server-Side Blast Radius ScoringOverviewThis change introduces server-side blast radius scoring for Pull Request (PR) reviews. It shifts computation from client-side to a new asynchronous worker architecture. This includes a robust Git repository caching system and integration with an external blast radius engine. Technical Highlights
Impact
|
| } | ||
| defer mu.(*sync.Mutex).Unlock() | ||
| removeEntry(root, entry) | ||
| repoLocks.Delete(key) // the map only holds cached repos; a new job gets a fresh lock |
There was a problem hiding this comment.
Severity: critical
The lock is deleted from repoLocks before releasing the mutex. This allows for concurrent acquisition of a new mutex for the same path.
Suggestions:
- Remove repoLocks.Delete from removeIfIdle or protect map cleanup with a global mutex
|
|
||
| // repoLocks: <entry>/repo -> *sync.Mutex. Bounded by the cached repos: a key is deleted | ||
| // when its repo leaves the cache (removeIfIdle, MarkTooLarge). | ||
| var repoLocks sync.Map |
There was a problem hiding this comment.
Severity: critical
Multi-process workers require flock or an external locking mechanism. The current sync.Mutex only provides protection within a single process.
Suggestions:
- Implement
flockor another cross-process locking mechanism if multiple worker processes can access the same cache directory concurrently. - Document the current limitation and future plan for multi-process access.
| return err | ||
| } | ||
| // Rename detection in the graph engine's git log would lazily fetch old blobs one by one. | ||
| for _, kv := range [][2]string{{"diff.renames", "false"}, {"checkout.workers", "8"}} { |
There was a problem hiding this comment.
Severity: warning
Setting diff.renames=false conflicts with the Diff method, which uses git diff -M for rename detection.
Suggestions:
- Clarify intent: if
git diff -Mis used,diff.renamesshould typically betrueor left at default. If the graph engine handles renames, thisgitconfig might be irrelevant or intended to prevent Git from doing its own rename detection for other commands. Add a comment explaining this decision. - If
git diff -Mis meant to detect renames, remove{"diff.renames", "false"}.
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
Implement Server-Side Blast Radius Scoring and Repository CachingOverviewAdds a server-side background worker to compute code blast radius for pull requests. Integrates repository cloning, persistent disk caching, LRU eviction, and configuration APIs. Technical Highlights
Impact
|
| // UpdateBlastRadiusCacheSettings saves the switch and size limit; a lower limit evicts right away. | ||
| func (s *Server) UpdateBlastRadiusCacheSettings(c echo.Context) error { | ||
| // Shrinking the cache deletes cached repos for every org, so require a real session (CLAUDE.md session-only gating). | ||
| if authMethod, _ := c.Get(authMethodContextKey).(string); authMethod == authMethodAPIKey { |
There was a problem hiding this comment.
Severity: critical
The type assertion omits the 'ok' check, which means unauthenticated requests could bypass the API key block.
Suggestions:
- Check the comma-ok idiom on the type assertion:
authMethod, ok := c.Get(authMethodContextKey).(string); !ok || authMethod == authMethodAPIKey
| // removeOrphanIndexes deletes graph index files no cached repo points to (e.g. its repo | ||
| // was removed mid-job). ponytail: 1h age guard so a job still indexing keeps its files. | ||
| func removeOrphanIndexes(root string) { | ||
| used := map[string]bool{} |
There was a problem hiding this comment.
Severity: warning
The map is being used uninitialized, which could lead to a potential read/write concurrency race.
Suggestions:
- Initialize used map or protect with mutex during concurrent entry processing
| func indexFiles(root, entry string) []string { | ||
| project, err := os.ReadFile(filepath.Join(entry, ".index")) | ||
| name := strings.TrimSpace(string(project)) | ||
| if err != nil || name == "" || strings.ContainsAny(name, `/\`) { |
There was a problem hiding this comment.
Severity: warning
The unhandled read error is being ignored, and the function incorrectly returns nil on failure.
Suggestions:
- Check err explicitly instead of relying on empty name check
| } | ||
| dir := filepath.Join(base, "repo") | ||
|
|
||
| mu, _ := repoLocks.LoadOrStore(dir, &sync.Mutex{}) |
There was a problem hiding this comment.
Severity: critical
There is a potential race condition with LoadOrStore on a mutex pointer. The locked mutex can be returned to a concurrent caller.
Suggestions:
- Use a double-checked locking pattern or explicit locking around sync.Map
- Store pointers to structs containing a mutex and a reference count
| if _, err = c.git(ctx, "checkout", "--force", "--detach", c.HeadSHA); err != nil { | ||
| return nil, nil, err | ||
| } | ||
| if err = os.WriteFile(filepath.Join(base, ".lastused"), nil, 0o644); err != nil { |
There was a problem hiding this comment.
Severity: warning
There is a missing error check on WriteFile, which results in a silent failure when updating the lastused timestamp.
Suggestions:
- Check and return the error from os.WriteFile
LiveReview Pre-Commit Check: vouched (iter:1, coverage:0%)
Integrate Server-Side Blast Radius Scoring and Repository CachingOverviewThis change adds server-side blast radius calculation for webhooks and UI reviews using the Technical Highlights
Impact
|
|
|
||
| Let the server produce the same report the CLI produces, and store it through the path that already exists. Nothing downstream changes: blob storage, the Postgres mirror, Livi, and the diff viewer UI all keep working as-is. | ||
|
|
||
| The blast radius job runs **in parallel** with the AI review, the same way git-lrc does it locally — both start when the review is queued, neither waits for the other. |
There was a problem hiding this comment.
Severity: info
Parallel execution with AI review is beneficial as it avoids blocking the completion of the review process.
| review page load -> UI joins comments + blast-radius report (already does) | ||
| ``` | ||
|
|
||
| **No server-side merge step is needed.** The two results already get combined at read time: the diff viewer fetches the blast-radius artifact separately and joins it to hunks by `file:new_start:new_lines`, and the finding-severity blend (`blendRiskScore`) happens there too. Whichever job finishes first just lands first; the page shows blast radius once the artifact exists. |
There was a problem hiding this comment.
Severity: info
Explicitly stating that no server-side merge is required clarifies the design and helps to avoid unnecessary complexity.
|
|
||
| Cons: a small one-time change in git-lrc; LiveReview builds depend on git-lrc being reachable (or on the Go module proxy cache). | ||
|
|
||
| **Decision: Option B.** |
There was a problem hiding this comment.
Severity: info
Option B is the correct choice because it prevents code drift and simplifies maintenance.
| **Folder layout** (`./lrdata` is already the persistent volume in `docker-compose.yml`): | ||
|
|
||
| ``` | ||
| /app/lrdata/blastradius/ |
There was a problem hiding this comment.
Severity: info
Good tenant isolation is achieved by using org_id and connector_id in the cache path.
| | Flag | Effect | | ||
| |---|---| | ||
| | `--filter=blob:none` | Download commits + folder listings for history, but file contents only for the commit we check out | | ||
| | `--shallow-since="13 months ago"` | Drop history the engine never reads. 13, not 12, so the cut-off commit falls outside the engine's 1-year window (the cut-off commit looks like "every file added" and would fake co-change data) | |
There was a problem hiding this comment.
Severity: info
Using a 13-month period for shallow clones is a good defensive measure against the 1-year engine window.
| **Per job:** | ||
|
|
||
| ``` | ||
| lock <owner>__<repo> |
There was a problem hiding this comment.
Severity: info
Locking per repository prevents race conditions and ensures data integrity for the working tree.
| - **Commit, not branch.** We always check out the exact `head_sha` the review is about, in "detached" mode. No `git pull`, no branch switching: a pull merges and can conflict, and a branch name moves while the job runs. Fetching by SHA works on GitHub/GitLab. For PRs from forks, the SHA isn't on any branch of the base repo — fetch the PR ref instead (`refs/pull/<n>/head` on GitHub, `refs/merge-requests/<n>/head` on GitLab). | ||
| - **Why reuse is fast:** the fetch only downloads commits and files that changed since the last cached commit, and the graph index updates incrementally instead of re-parsing the whole repo. | ||
| - **One job per repo at a time** (the `.lock`): there is one working tree, so two reviews of the same repo run one after the other. Different repos run in parallel. | ||
| - **Self-healing:** if any git step fails (corrupt clone, force-pushed history, etc.), delete `repo/` and clone once more before giving up. |
There was a problem hiding this comment.
Severity: info
The self-healing mechanism, which involves deleting and re-cloning on failure, improves the system's robustness.
| | Gitea | `https://<user>:<token>@<host>/owner/repo.git` | | ||
| | Azure DevOps | `https://pat:<token>@dev.azure.com/org/project/_git/repo` | | ||
|
|
||
| Pass the token through a credential helper / env, never in a logged URL or in `repo/.git/config`. SSH is not used: it would need a key registered per repo (GitHub deploy keys can't be shared across repos) or a machine-user account, plus `known_hosts` per git host, while every connector already has a token. |
There was a problem hiding this comment.
Severity: info
Using HTTPS with connector tokens is a good choice as it avoids the complexity of SSH key management.
| Pass the token through a credential helper / env, never in a logged URL or in `repo/.git/config`. SSH is not used: it would need a key registered per repo (GitHub deploy keys can't be shared across repos) or a machine-user account, plus `known_hosts` per git host, while every connector already has a token. | ||
|
|
||
| 2. **Graph index** | ||
| - The Docker image's `codebase-memory-mcp` is pinned to **v0.9.0** (same as git-lrc's `graphengine.PinnedVersion`) and locked in `PINNED_DOCKER_DEPS`. v0.10.x prints `query_graph` results as text even with `--json`, which the engine can't parse (checked with v0.10.8). Bump only together with git-lrc. |
There was a problem hiding this comment.
Severity: info
Pinning the codebase-memory-mcp version is crucial for ensuring consistent scoring results.
| 3. **Scoring engine** — Option B above: `go get github.com/HexmosTech/git-lrc/blastradius@<commit>`, call `blastradius.ScoreHunks`. | ||
|
|
||
| 4. **Job** — `internal/jobqueue/blast_radius_worker.go` | ||
| - `BlastRadiusJobArgs{OrgID, ReviewID}`, `Kind() = "blast_radius"`, on its own `blast_radius` River queue with 1 worker, so indexing never takes a review worker. |
There was a problem hiding this comment.
Severity: info
A separate River queue and one worker for blast radius jobs prevent these jobs from starving the review workers.
| - Repo and PR come from the review row, all queries scoped by `org_id`: `pr_mr_url` gives the clone URL and PR number for every provider; the connector (`integration_tokens`) gives the token, using the same PAT-vs-OAuth choice as `buildProviderConfig`; `pull_requests` (when linked) gives the base and source branch. The token is only ever sent to the connector's own host. | ||
| - Diff source: `git diff -M <merge-base> <head>` from the cached checkout (repocache), no provider API. | ||
| - Save via `blobstore.SaveArtifact` + `storageblastradius.Store.ReplaceFromReport` (moved out of `internal/api`; the CLI upload handler uses it too). | ||
| - Best-effort: 15 min timeout, `MaxAttempts: 1`, every failure logged and the job returns success, never affects the review. |
There was a problem hiding this comment.
Severity: info
The best-effort approach, with a 15-minute timeout and a maximum of one attempt, prevents blast radius job failures from blocking reviews.
| - Diff source: `git diff -M <merge-base> <head>` from the cached checkout (repocache), no provider API. | ||
| - Save via `blobstore.SaveArtifact` + `storageblastradius.Store.ReplaceFromReport` (moved out of `internal/api`; the CLI upload handler uses it too). | ||
| - Best-effort: 15 min timeout, `MaxAttempts: 1`, every failure logged and the job returns success, never affects the review. | ||
| - **Repo too large for the cache** (phase 5, needs `max_gb`): 5 GB covers almost every repo without blobs. After the clone + index, if the repo alone is bigger than `max_gb`, delete it, write a `<owner>__<repo>.too_large` marker with the measured size, and save a small `blast-radius` artifact `{ "status": "skipped", "reason": "repo_too_large", "repo_gb": 7.2, "max_gb": 5 }` instead of a report. Later reviews of that repo skip straight away (no re-clone) until `max_gb` is raised above the recorded size. The skipped artifact ships together with the UI that renders it, since today's panel expects a full report. |
There was a problem hiding this comment.
Severity: info
Handling repo_too_large by skipping the artifact and providing UI feedback offers a good user experience.
| - The section shows current cache usage (used / max GB, number of cached repos) so admins can see when to raise the limit. | ||
| - Same access as the rest of the Storage tab: `adminOrOwnerGroup` (`/settings/storage/...` in `server.go`) — self-hosted owners and super admins; super-admin only in cloud. | ||
| - Mega menu: add a `link('Blast Radius Repo Cache', ..., '/settings#storage', ...)` next to the existing Storage / Log Compaction / Preloaded Changes Archival entries in `megaMenuData.ts`, with the same predicate as `Storage`. | ||
| - **Install requirement**: document "at least 5 GB free disk on the `lrdata` volume" in the self-hosted install docs. If free space on the volume is too low for a clone, the job is skipped with the same notice as a too-large repo (step 4). |
There was a problem hiding this comment.
Severity: info
Documenting the disk space requirements for self-hosted installations is critical for successful deployments.
| - **Install requirement**: document "at least 5 GB free disk on the `lrdata` volume" in the self-hosted install docs. If free space on the volume is too low for a clone, the job is skipped with the same notice as a too-large repo (step 4). | ||
|
|
||
| 6. **Cache eviction (least-recently-used, size-based)** — at the end of every job, plus right away when an admin saves a lower cache size. No periodic job: the cache only grows when a job runs. No new tables. | ||
| - Size of a cached repo = its folder + its graph index files (`index/<project>.db*`, named in the repo's `.index` file). While the whole cache is over `max_gb`, delete the repo with the oldest `.lastused`, together with its index. Repos that are reviewed often keep getting touched, so they stay; rarely used repos are removed first. |
There was a problem hiding this comment.
Severity: info
LRU eviction based on size is an effective strategy for cache management.
|
|
||
| 6. **Cache eviction (least-recently-used, size-based)** — at the end of every job, plus right away when an admin saves a lower cache size. No periodic job: the cache only grows when a job runs. No new tables. | ||
| - Size of a cached repo = its folder + its graph index files (`index/<project>.db*`, named in the repo's `.index` file). While the whole cache is over `max_gb`, delete the repo with the oldest `.lastused`, together with its index. Repos that are reviewed often keep getting touched, so they stay; rarely used repos are removed first. | ||
| - Never evict a repo a job is using right now (its in-process lock is held). |
There was a problem hiding this comment.
Severity: warning
Relying solely on job runs for eviction means the cache could temporarily exceed max_gb if jobs are infrequent. Consider a lightweight periodic sweep if this becomes an issue.
Suggestions:
- Add a lightweight periodic sweep if cache size becomes an issue during low job activity.
| ## Risks / open questions | ||
|
|
||
| - **Disk**: roughly one source tree + ~1 year of commit metadata + graph index per repo, capped by the admin-set `max_gb` (default 5 GB, min 1 GB, step 5). A repo bigger than `max_gb` is skipped and the review page says so, with a link to raise the limit (step 4). | ||
| - **Rename detection off**: with `diff.renames=false`, a renamed file shows up under both its old and new name in co-change data. Small effect; turn it back on if it matters (costs a one-time lazy download). |
There was a problem hiding this comment.
Severity: warning
The diff.renames=false flag might slightly impact the accuracy of co-change detection for renamed files. Re-evaluate if this becomes a problem.
Suggestions:
- Monitor impact of
diff.renames=falseon co-change signal accuracy; re-enable if necessary.
| - Internet connectivity for downloads | ||
| - Sufficient disk space for Docker images and data | ||
| - Sufficient disk space for Docker images and data, plus at least 5 GB free on the | ||
| `lrdata` volume for the blast radius repo cache (Settings → Storage) |
There was a problem hiding this comment.
Severity: info
Adding the disk space requirement to the installer checklist is important for users installing the system in a self-hosted environment.
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.33.0 // indirect | ||
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.55.0 // indirect | ||
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.55.0 // indirect | ||
| github.com/HexmosTech/git-lrc/blastradius v0.1.0 |
There was a problem hiding this comment.
Severity: info
A new dependency, github.com/HexmosTech/git-lrc/blastradius, has been added, as per the design document.
| // a cloned repo) are skipped explicitly, so nothing outside dir is ever counted. | ||
| func dirSize(dir string) int64 { | ||
| var size int64 | ||
| _ = filepath.WalkDir(dir, func(_ string, d fs.DirEntry, err error) error { |
There was a problem hiding this comment.
Severity: warning
Errors encountered during WalkDir are ignored, and a partial size is returned upon failure.
Suggestions:
- Return or log the WalkDir error to avoid silent calculation failures.
| const engineBinary = "codebase-memory-mcp" | ||
|
|
||
| // indexDir keeps graph indexes on the lrdata volume next to the repo cache, not in ~/.cache. | ||
| var indexDir = filepath.Join(repocache.DefaultRoot, "index") |
There was a problem hiding this comment.
Severity: critical
There is a compilation error because repocache.DefaultRoot is undefined.
Suggestions:
- Define DefaultRoot in repocache package or pass root parameter explicitly.
|
|
||
| // repoLocks: <entry>/repo -> *sync.Mutex. Bounded by the cached repos: a key is deleted | ||
| // when its repo leaves the cache (removeIfIdle, MarkTooLarge). | ||
| var repoLocks sync.Map |
There was a problem hiding this comment.
Severity: warning
The repoLocks sync.Map leaks mutexes due to missing cleanup logic.
Suggestions:
- Implement cache eviction and corresponding mutex deletion from repoLocks.
| } | ||
| switch { | ||
| case strings.HasPrefix(provider, "gitea"): | ||
| var p struct{ Pat, Username string } |
There was a problem hiding this comment.
Severity: warning
The patToken is unmarshaled as a gitea auth struct; consider using an explicit field or proper unmarshaling.
Suggestions:
- Define a dedicated struct for Gitea token JSON payload instead of reusing patToken string as JSON input
| } | ||
| unified, _ := difflib.GetUnifiedDiffString(diff) | ||
| hunks := parseHunksFromUnifiedDiff(unified) | ||
| hunks := diffutil.ParseUnifiedHunks(unified) |
There was a problem hiding this comment.
Severity: info
Consider using the shared diffutil.ParseUnifiedHunks function.
| } | ||
| var hunks []blastradius.HunkReport | ||
| for _, f := range report.Files { | ||
| hunks = append(hunks, f.Hunks...) |
There was a problem hiding this comment.
Severity: warning
There is a potential for many small appends; consider pre-allocating slice capacity.
Suggestions:
- Calculate total hunks across all files and pre-allocate
hunksslice capacity before the loop.
| * @returns Promise with the blast radius report | ||
| */ | ||
| export const getBlastRadiusReport = async (reviewId: number): Promise<BlastRadiusReport> => { | ||
| export const getBlastRadiusReport = async (reviewId: number): Promise<BlastRadiusReport | BlastRadiusSkipped> => { |
There was a problem hiding this comment.
Severity: info
The return type has been updated to include BlastRadiusSkipped.
| export const getBlastRadiusReport = async (reviewId: number): Promise<BlastRadiusReport | BlastRadiusSkipped> => { | ||
| try { | ||
| return await apiClient.get<BlastRadiusReport>(`/api/v1/diff-review/${reviewId}/artifacts/blast-radius`); | ||
| return await apiClient.get<BlastRadiusReport | BlastRadiusSkipped>(`/api/v1/diff-review/${reviewId}/artifacts/blast-radius`); |
There was a problem hiding this comment.
Severity: info
The API client type parameter has been updated to accommodate BlastRadiusSkipped.
2 participants
Summary
PR reviews started from LiveReview (UI, API, MCP, PR list) now get a blast-radius report, not just
git lrc reviewruns. A River job runs alongside the AI review and saves through the existing artifact path, so the diff viewer, Math Mode andblast_radius_hunks(Livi) work unchanged.Design:
docs/blast-radius-server-side-plan.md.How it works
internal/blastradius/repocache): blobless, ~13-month shallow clone per repo underlrdata/blastradius/<org>/<connector>/<owner>__<repo>. Later reviews only fetch the PR ref. HTTPS + the connector's token, passed as a header via env: never in argv,.git/configor logs. Token only sent to the connector's own host, no redirects. Diff is plaingit diff -M <merge-base> <head>, no provider API.internal/blastradius/serverscore):codebase-memory-mcpindex + git-lrc's engine imported as a Go module (github.com/HexmosTech/git-lrc/blastradius v0.1.0, blastradius: rename module to github.com/HexmosTech/git-lrc/blastradius git-lrc#145). Same engine as the CLI, so scores match.internal/jobqueue/blast_radius_worker.go): queued inQueueManualReviewJob, ownblast_radiusqueue (1 worker), 15 min timeout, best-effort, never affects the review. All queries scoped byorg_id. Skips CLI, scheduled and non-PR reviews.system_settingsrowblast_radius_cache; saving rejects API keys. Mega-menu entry added.Docker / deps
codebase-memory-mcppinned to v0.9.0 and locked inPINNED_DOCKER_DEPS. git-lrc's engine parses v0.9.0's JSON; v0.10.x prints text even with--json.gitadded to the runtime image (it was missing) and toverify_docker_deps.sh.go.mod+GOLANG_IMAGE_TAG=1.26.9-bookworm).Testing
LinceMathew/ragfolio): clone → index → score → badges on the review page; cache files, index and usage verified.gitlab.com/Amazing-Stardom/painlessMesh): full path works. That MR only touched.inofiles, which codebase-memory-mcp doesn't index, so its scores are 0.go build/go vet,npx tsc --noEmit,docker buildx build --checkon both Dockerfiles,check_docker_deps.py --check.Not yet verified
make verify-docker-deps.Deploy notes
Hosts running the worker need
gitandcodebase-memory-mcpv0.9.0 on the worker's PATH (Docker image has both). Otherwise jobs log and skip; reviews are unaffected.Also included:
db/schema.sqldump and.synced-commits.envupdates from local tooling.