feat: expandable diff context in code review (#247)

* feat: expandable diff context in code review (closes #243)

Switch from PatchDiff to FileDiff component from @pierre/diffs to enable
GitHub-style "show more lines" buttons between hunks. The library handles
all expansion UI when provided full file contents via oldLines/newLines.

- Add /api/file-content endpoint to serve old/new file content per diff type
- Add getFileContentsForDiff() helper with ref mapping for all diff types
- Parse patch client-side via getSingularPatch(), augment with file contents
- Update @pierre/diffs from 1.0.4 to 1.0.11 (scroll sync fix, no breaking changes)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: upgrade @pierre/diffs to beta, re-parse patch with full file contents

The previous approach spread file content arrays onto a partial-mode
FileDiffMetadata, causing hunk index mismatches and Shiki decoration
errors. Now uses processFile() to re-parse the patch with oldFile/newFile
so hunk indices are computed against the full file (isPartial: false),
which is required for expansion to work correctly.

Also fixes a flash error on file switch by tagging fileContents with the
filePath they were fetched for, preventing stale contents from being
paired with the wrong patch during the render before useEffect fires.

- Upgrade @pierre/diffs from ^1.0.x to ^1.1.0-beta.19 in all workspaces
- Add test fixtures for disjoint hunks, deleted/renamed/new files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: abort stale file-content fetches, validate path params

- Add AbortController to file-content fetch effect to cancel in-flight
  requests when switching files
- Reject path traversal (.. or absolute paths) on /api/file-content
- Document /api/file-content endpoint in CLAUDE.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Michael Ramos
2026-03-08 14:00:43 -07:00
committed by GitHub
parent 47bfc4bfd4
commit 472e17be3d
17 changed files with 852 additions and 35 deletions
+1
View File
@@ -176,6 +176,7 @@ Send Annotations → feedback sent to agent session
| Endpoint | Method | Purpose |
| --------------------- | ------ | ------------------------------------------ |
| `/api/diff` | GET | Returns `{ rawPatch, gitRef, origin }` |
| `/api/file-content` | GET | Returns `{ oldContent, newContent }` for expandable diff context |
| `/api/feedback` | POST | Submit review (body: feedback, annotations, agentSwitch) |
| `/api/image` | GET | Serve image by path query param |
| `/api/upload` | POST | Upload image, returns `{ path, originalName }` |
+1 -1
View File
@@ -12,7 +12,7 @@
"@plannotator/review-editor": "workspace:*",
"@plannotator/server": "workspace:*",
"@plannotator/ui": "workspace:*",
"@pierre/diffs": "^1.0.4",
"@pierre/diffs": "^1.1.0-beta.19",
"react": "^19.2.3",
"react-dom": "^19.2.3",
"tailwindcss": "^4.1.18",
+9 -9
View File
@@ -4,7 +4,7 @@
"": {
"name": "plannotator",
"dependencies": {
"@pierre/diffs": "^1.0.4",
"@pierre/diffs": "^1.1.0-beta.19",
"diff": "^8.0.3",
},
"devDependencies": {
@@ -53,7 +53,7 @@
},
"apps/opencode-plugin": {
"name": "@plannotator/opencode",
"version": "0.11.1",
"version": "0.11.2",
"dependencies": {
"@opencode-ai/plugin": "^1.1.10",
},
@@ -74,7 +74,7 @@
},
"apps/pi-extension": {
"name": "@plannotator/pi-extension",
"version": "0.11.1",
"version": "0.11.2",
"peerDependencies": {
"@mariozechner/pi-coding-agent": ">=0.53.0",
},
@@ -101,7 +101,7 @@
"name": "@plannotator/review",
"version": "0.0.1",
"dependencies": {
"@pierre/diffs": "^1.0.4",
"@pierre/diffs": "^1.1.0-beta.19",
"@plannotator/review-editor": "workspace:*",
"@plannotator/server": "workspace:*",
"@plannotator/ui": "workspace:*",
@@ -143,7 +143,7 @@
"name": "@plannotator/review-editor",
"version": "0.0.1",
"dependencies": {
"@pierre/diffs": "^1.0.4",
"@pierre/diffs": "^1.1.0-beta.19",
"@plannotator/ui": "workspace:*",
"highlight.js": "^11.11.1",
"react": "^19.2.3",
@@ -153,7 +153,7 @@
},
"packages/server": {
"name": "@plannotator/server",
"version": "0.11.1",
"version": "0.11.2",
"dependencies": {
"@plannotator/shared": "workspace:*",
},
@@ -572,7 +572,9 @@
"@oven/bun-windows-x64-baseline": ["@oven/bun-windows-x64-baseline@1.3.5", "", { "os": "win32", "cpu": "x64" }, "sha512-rtVQB9/1XK8FWJgFtsOthbPifRMYypgJwxu+pK3NHx8WvFKmq7HcPDqNr8xLzGULjQEO7eAo2aOZfONOwYz+5g=="],
"@pierre/diffs": ["@pierre/diffs@1.0.4", "", { "dependencies": { "@shikijs/core": "^3.0.0", "@shikijs/engine-javascript": "^3.0.0", "@shikijs/transformers": "^3.0.0", "diff": "8.0.2", "hast-util-to-html": "9.0.5", "lru_map": "0.4.1", "shiki": "^3.0.0" }, "peerDependencies": { "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0" } }, "sha512-jH0uzP3syMFcz0rp4uDDGTkV8sbMeh0W8Gq+e3P1rIiJ+gdGISfG+Qhwpe//W2W5Ac9G5vBAbMflywz+lLaJlQ=="],
"@pierre/diffs": ["@pierre/diffs@1.1.0-beta.19", "", { "dependencies": { "@pierre/theme": "0.0.22", "@shikijs/transformers": "^3.0.0", "diff": "8.0.3", "hast-util-to-html": "9.0.5", "lru_map": "0.4.1", "shiki": "^3.0.0" }, "peerDependencies": { "react": "^18.3.1 || ^19.0.0", "react-dom": "^18.3.1 || ^19.0.0" } }, "sha512-XxGPKkVW+1t2KJQfgjmSnS+93nI9+ACJl1XjhF3Lo4BdQJOxV3pHeyix31ySn/m/1llq6O/7bXucE0OYCK6Kog=="],
"@pierre/theme": ["@pierre/theme@0.0.22", "", {}, "sha512-ePUIdQRNGjrveELTU7fY89Xa7YGHHEy5Po5jQy/18lm32eRn96+tnYJEtFooGdffrx55KBUtOXfvVy/7LDFFhA=="],
"@pkgjs/parseargs": ["@pkgjs/parseargs@0.11.0", "", {}, "sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg=="],
@@ -2410,8 +2412,6 @@
"@opencode-ai/plugin/zod": ["zod@4.1.8", "", {}, "sha512-5R1P+WwQqmmMIEACyzSvo4JXHY5WiAFHRMg+zBZKgKS+Q1viRa0C1hmUKtHltoIFKtIdki3pRxkmpP74jnNYHQ=="],
"@pierre/diffs/diff": ["diff@8.0.2", "", {}, "sha512-sSuxWU5j5SR9QQji/o2qMvqRNYRDOcBTgsJ/DeCf4iSN4gW+gNMXM7wFIP+fdXZxoNiAnHUTGjCr+TSWXdRDKg=="],
"@rollup/pluginutils/estree-walker": ["estree-walker@2.0.2", "", {}, "sha512-Rfkk/Mp/DL7JVje3u18FxFujQlTNR2q6QfMSMB7AvCBx91NGj/ba3kCfza0f6dVDbw7YlRf/nDrn7pQrCCyQ/w=="],
"@tailwindcss/oxide-wasm32-wasi/@emnapi/core": ["@emnapi/core@1.7.1", "", { "dependencies": { "@emnapi/wasi-threads": "1.1.0", "tslib": "^2.4.0" }, "bundled": true }, "sha512-o1uhUASyo921r2XtHYOHy7gdkGLge8ghBEQHMWmyJFoXlpU58kIrhhN3w26lpQb6dspetweapMn2CSNwQ8I4wg=="],
+1 -1
View File
@@ -32,7 +32,7 @@
"test": "bun test"
},
"dependencies": {
"@pierre/diffs": "^1.0.4",
"@pierre/diffs": "^1.1.0-beta.19",
"diff": "^8.0.3"
},
"devDependencies": {
+1
View File
@@ -813,6 +813,7 @@ const ReviewApp: React.FC = () => {
<DiffViewer
patch={activeFile.patch}
filePath={activeFile.path}
oldPath={activeFile.oldPath}
diffStyle={diffStyle}
annotations={activeFileAnnotations}
selectedAnnotationId={selectedAnnotationId}
@@ -1,5 +1,6 @@
import React, { useMemo, useRef, useEffect, useCallback } from 'react';
import { PatchDiff } from '@pierre/diffs/react';
import React, { useMemo, useRef, useEffect, useCallback, useState } from 'react';
import { FileDiff } from '@pierre/diffs/react';
import { getSingularPatch, processFile } from '@pierre/diffs';
import { CodeAnnotation, CodeAnnotationType, SelectedLineRange, DiffAnnotationMetadata } from '@plannotator/ui/types';
import { useTheme } from '@plannotator/ui/components/ThemeProvider';
import { detectLanguage } from '../utils/detectLanguage';
@@ -12,6 +13,7 @@ import { SuggestionModal } from './SuggestionModal';
interface DiffViewerProps {
patch: string;
filePath: string;
oldPath?: string;
diffStyle: 'split' | 'unified';
annotations: CodeAnnotation[];
selectedAnnotationId: string | null;
@@ -28,6 +30,7 @@ interface DiffViewerProps {
export const DiffViewer: React.FC<DiffViewerProps> = ({
patch,
filePath,
oldPath,
diffStyle,
annotations,
selectedAnnotationId,
@@ -45,6 +48,39 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
const toolbar = useAnnotationToolbar({ patch, filePath, onLineSelection, onAddAnnotation, onEditAnnotation });
// Parse patch into FileDiffMetadata for @pierre/diffs FileDiff component
const fileDiff = useMemo(() => getSingularPatch(patch), [patch]);
// Fetch full file contents for expandable context
const [fileContents, setFileContents] = useState<{ forPath: string; old: string | null; new: string | null } | null>(null);
useEffect(() => {
const controller = new AbortController();
setFileContents(null);
const params = new URLSearchParams({ path: filePath });
if (oldPath) params.set('oldPath', oldPath);
fetch(`/api/file-content?${params}`, { signal: controller.signal })
.then(res => res.ok ? res.json() : null)
.then((data: { oldContent: string | null; newContent: string | null } | null) => {
if (data && (data.oldContent != null || data.newContent != null)) {
setFileContents({ forPath: filePath, old: data.oldContent, new: data.newContent });
}
})
.catch(() => {}); // Silent fallback — no expansion in demo mode
return () => controller.abort();
}, [filePath, oldPath]);
// Re-parse the patch with full file contents so hunk indices are computed
// against the complete file (isPartial: false), enabling expansion.
const augmentedDiff = useMemo(() => {
if (!fileContents || fileContents.forPath !== filePath || (fileContents.old == null && fileContents.new == null)) return fileDiff;
const result = processFile(patch, {
oldFile: fileContents.old != null ? { name: oldPath || filePath, contents: fileContents.old } : undefined,
newFile: fileContents.new != null ? { name: filePath, contents: fileContents.new } : undefined,
});
return result || fileDiff;
}, [patch, filePath, oldPath, fileContents, fileDiff]);
// Clear pending selection when file changes
const prevFilePathRef = useRef(filePath);
useEffect(() => {
@@ -147,14 +183,15 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
/>
<div className="p-4">
<PatchDiff
<FileDiff
key={filePath}
patch={patch}
fileDiff={augmentedDiff}
options={{
theme: pierreTheme,
themeType: 'dark',
diffStyle,
diffIndicators: 'bars',
hunkSeparators: 'line-info',
enableLineSelection: true,
enableHoverUtility: true,
onLineSelectionEnd: toolbar.handleLineSelectionEnd,
+1 -1
View File
@@ -8,7 +8,7 @@
},
"dependencies": {
"@plannotator/ui": "workspace:*",
"@pierre/diffs": "^1.0.4",
"@pierre/diffs": "^1.1.0-beta.19",
"highlight.js": "^11.11.1",
"react": "^19.2.3",
"react-dom": "^19.2.3",
+70 -1
View File
@@ -201,7 +201,7 @@ async function getUntrackedFileDiffs(srcPrefix = 'a/', dstPrefix = 'b/', cwd?: s
*/
const WORKTREE_SUB_TYPES = new Set(["uncommitted", "last-commit", "branch"]);
function parseWorktreeDiffType(diffType: string): { path: string; subType: string } | null {
export function parseWorktreeDiffType(diffType: string): { path: string; subType: string } | null {
if (!diffType.startsWith("worktree:")) return null;
const rest = diffType.slice("worktree:".length);
const lastColon = rest.lastIndexOf(":");
@@ -349,3 +349,72 @@ export async function runGitDiff(
return { patch, label };
}
/**
* Get old and new file contents for a given diff type.
* Used by the expandable context feature to feed full file content to @pierre/diffs.
*/
export async function getFileContentsForDiff(
diffType: DiffType,
defaultBranch: string,
filePath: string,
oldPath?: string,
cwd?: string,
): Promise<{ oldContent: string | null; newContent: string | null }> {
const oldFilePath = oldPath || filePath;
async function gitShow(ref: string, path: string): Promise<string | null> {
try {
const cmd = $`git show ${ref}:${path}`.quiet();
return (cwd ? await cmd.cwd(cwd) : await cmd).text();
} catch {
return null;
}
}
async function readWorkingTree(path: string): Promise<string | null> {
try {
const fullPath = cwd ? `${cwd}/${path}` : path;
return await Bun.file(fullPath).text();
} catch {
return null;
}
}
// Determine the effective diff type (handle worktree prefix)
let effectiveDiffType = diffType as string;
if (diffType.startsWith("worktree:")) {
const parsed = parseWorktreeDiffType(diffType);
if (!parsed) return { oldContent: null, newContent: null };
cwd = parsed.path;
effectiveDiffType = parsed.subType;
}
let oldContent: string | null = null;
let newContent: string | null = null;
switch (effectiveDiffType) {
case "uncommitted":
oldContent = await gitShow("HEAD", oldFilePath);
newContent = await readWorkingTree(filePath);
break;
case "staged":
oldContent = await gitShow("HEAD", oldFilePath);
newContent = await gitShow(":0", filePath);
break;
case "unstaged":
oldContent = await gitShow(":0", oldFilePath);
newContent = await readWorkingTree(filePath);
break;
case "last-commit":
oldContent = await gitShow("HEAD~1", oldFilePath);
newContent = await gitShow("HEAD", filePath);
break;
case "branch":
oldContent = await gitShow(defaultBranch, oldFilePath);
newContent = await gitShow("HEAD", filePath);
break;
}
return { oldContent, newContent };
}
+24 -1
View File
@@ -10,7 +10,7 @@
*/
import { isRemoteSession, getServerPort } from "./remote";
import { type DiffType, type GitContext, runGitDiff, getWorktreeDiffOptions } from "./git";
import { type DiffType, type GitContext, runGitDiff, getWorktreeDiffOptions, getFileContentsForDiff } from "./git";
import { getRepoInfo } from "./repo";
import { handleImage, handleUpload, handleAgents, handleServerReady, handleDraftSave, handleDraftLoad, handleDraftDelete, type OpencodeClient } from "./shared-handlers";
import { contentHash, deleteDraft } from "./draft";
@@ -198,6 +198,29 @@ export async function startReviewServer(
}
}
// API: Get file content for expandable diff context
if (url.pathname === "/api/file-content" && req.method === "GET") {
const filePath = url.searchParams.get("path");
if (!filePath) {
return Response.json({ error: "Missing path" }, { status: 400 });
}
if (filePath.includes("..") || filePath.startsWith("/")) {
return Response.json({ error: "Invalid path" }, { status: 400 });
}
const oldPath = url.searchParams.get("oldPath") || undefined;
if (oldPath && (oldPath.includes("..") || oldPath.startsWith("/"))) {
return Response.json({ error: "Invalid path" }, { status: 400 });
}
const defaultBranch = gitContext?.defaultBranch || "main";
const result = await getFileContentsForDiff(
currentDiffType,
defaultBranch,
filePath,
oldPath,
);
return Response.json(result);
}
// API: Serve images (local paths or temp uploads)
if (url.pathname === "/api/image") {
return handleImage(req);
@@ -0,0 +1,40 @@
/**
* @deprecated Use the new utility modules instead:
* - formatDate, formatBytes → src/utils/format.ts
* - slugify → src/utils/format.ts
* - parseJSON → src/utils/parser.ts
*
* This file is scheduled for removal in v2.0.
*/
export function formatDate(date: Date): string {
return date.toISOString().split('T')[0];
}
export function formatBytes(bytes: number): string {
if (bytes === 0) return '0 B';
const units = ['B', 'KB', 'MB', 'GB'];
const i = Math.floor(Math.log(bytes) / Math.log(1024));
return `${(bytes / Math.pow(1024, i)).toFixed(1)} ${units[i]}`;
}
export function slugify(text: string): string {
return text.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/(^-|-$)/g, '');
}
export function parseJSON<T>(input: string): T | null {
try {
return JSON.parse(input);
} catch {
return null;
}
}
export function capitalize(text: string): string {
return text.charAt(0).toUpperCase() + text.slice(1);
}
export function truncate(text: string, maxLength: number): string {
if (text.length <= maxLength) return text;
return text.slice(0, maxLength - 3) + '...';
}
+76
View File
@@ -0,0 +1,76 @@
/**
* Typed Event Emitter
*
* A minimal, type-safe event system for internal pub/sub communication.
* Used by services to broadcast lifecycle and domain events.
*/
export type EventHandler<T = unknown> = (data: T) => void | Promise<void>;
export interface EventSubscription {
unsubscribe(): void;
}
export class EventEmitter<Events extends Record<string, unknown>> {
private handlers = new Map<keyof Events, Set<EventHandler<any>>>();
/**
* Subscribe to an event. Returns a subscription object with unsubscribe().
*/
on<K extends keyof Events>(event: K, handler: EventHandler<Events[K]>): EventSubscription {
if (!this.handlers.has(event)) {
this.handlers.set(event, new Set());
}
this.handlers.get(event)!.add(handler);
return {
unsubscribe: () => {
this.handlers.get(event)?.delete(handler);
},
};
}
/**
* Subscribe to an event for a single emission only.
*/
once<K extends keyof Events>(event: K, handler: EventHandler<Events[K]>): EventSubscription {
const wrapper: EventHandler<Events[K]> = (data) => {
sub.unsubscribe();
return handler(data);
};
const sub = this.on(event, wrapper);
return sub;
}
/**
* Emit an event, calling all registered handlers.
* Handlers are called in registration order.
* Async handlers are awaited sequentially.
*/
async emit<K extends keyof Events>(event: K, data: Events[K]): Promise<void> {
const handlers = this.handlers.get(event);
if (!handlers) return;
for (const handler of handlers) {
await handler(data);
}
}
/**
* Remove all handlers for a specific event, or all events if no event specified.
*/
removeAllListeners(event?: keyof Events): void {
if (event) {
this.handlers.delete(event);
} else {
this.handlers.clear();
}
}
/**
* Get the number of handlers registered for a specific event.
*/
listenerCount(event: keyof Events): number {
return this.handlers.get(event)?.size ?? 0;
}
}
@@ -0,0 +1,216 @@
/**
* Service Registry
*
* Manages service lifecycle, dependency injection, and health monitoring
* for the Acme API platform.
*
* @module service-registry
* @since 2.0.0
*/
export interface ServiceConfig {
name: string;
version: string;
timeout: number;
retries: number;
enabled: boolean;
priority: number;
}
export interface ServiceHealth {
status: 'healthy' | 'degraded' | 'unhealthy';
latency: number;
lastCheck: Date;
uptime: number;
details?: string;
}
export interface Service {
config: ServiceConfig;
health: ServiceHealth;
start(): Promise<void>;
stop(): Promise<void>;
healthCheck(): Promise<ServiceHealth>;
}
type ServiceFactory = (config: ServiceConfig) => Service;
// Default configurations for well-known services
const DEFAULT_CONFIGS: Record<string, Partial<ServiceConfig>> = {
database: { timeout: 5000, retries: 3, priority: 1 },
cache: { timeout: 2000, retries: 1, priority: 2 },
queue: { timeout: 10000, retries: 5, priority: 3 },
search: { timeout: 3000, retries: 2, priority: 4 },
email: { timeout: 15000, retries: 3, priority: 5 },
storage: { timeout: 8000, retries: 2, priority: 4 },
metrics: { timeout: 1000, retries: 0, priority: 10 },
};
export class ServiceRegistry {
private services: Map<string, Service> = new Map();
private factories: Map<string, ServiceFactory> = new Map();
private startOrder: string[] = [];
private isShuttingDown = false;
/**
* Register a service factory for lazy instantiation.
* The factory is called when the service is first requested.
*/
registerFactory(name: string, factory: ServiceFactory): void {
if (this.factories.has(name)) {
throw new Error(`Factory already registered for service: ${name}`);
}
this.factories.set(name, factory);
}
/**
* Get a service by name. Creates it from the factory if not yet instantiated.
*/
async getService(name: string): Promise<Service> {
const existing = this.services.get(name);
if (existing) return existing;
const factory = this.factories.get(name);
if (!factory) {
throw new Error(`No factory registered for service: ${name}`);
}
const defaults = DEFAULT_CONFIGS[name] || {};
const config: ServiceConfig = {
name,
version: '1.0.0',
timeout: defaults.timeout ?? 5000,
retries: defaults.retries ?? 3,
enabled: true,
priority: defaults.priority ?? 99,
...defaults,
};
const service = factory(config);
this.services.set(name, service);
return service;
}
/**
* Start all registered services in priority order.
* Lower priority numbers start first (database before cache before app).
*/
async startAll(): Promise<void> {
const sorted = Array.from(this.services.entries())
.sort(([, a], [, b]) => a.config.priority - b.config.priority);
console.log(`Starting ${sorted.length} services in priority order...`);
for (const [name, service] of sorted) {
if (!service.config.enabled) {
console.log(`Skipping disabled service: ${name}`);
continue;
}
try {
const startTime = performance.now();
await service.start();
const elapsed = Math.round(performance.now() - startTime);
this.startOrder.push(name);
console.log(`Started: ${name} (v${service.config.version}) [${elapsed}ms]`);
} catch (err) {
console.error(`Failed to start ${name}:`, err);
// Stop already-started services before re-throwing
await this.stopAll();
throw err;
}
}
}
/**
* Stop all services in reverse startup order.
* Ensures graceful shutdown with proper resource cleanup.
*/
async stopAll(): Promise<void> {
if (this.isShuttingDown) return;
this.isShuttingDown = true;
console.log('Shutting down services...');
const reversed = [...this.startOrder].reverse();
for (const name of reversed) {
const service = this.services.get(name);
if (!service) continue;
try {
await service.stop();
console.log(`Stopped: ${name}`);
} catch (err) {
console.error(`Error stopping ${name}:`, err);
}
}
this.services.clear();
this.startOrder = [];
this.isShuttingDown = false;
}
/**
* Run health checks on all active services concurrently.
* Returns a summary of the overall system health.
*/
async healthCheckAll(): Promise<Map<string, ServiceHealth>> {
const results = new Map<string, ServiceHealth>();
const entries = Array.from(this.services.entries());
const checks = await Promise.allSettled(
entries.map(async ([name, service]) => {
const health = await service.healthCheck();
return { name, health };
}),
);
for (const result of checks) {
if (result.status === 'fulfilled') {
results.set(result.value.name, result.value.health);
} else {
// Find which service failed by matching the error
const failedName = entries.find(
([, s]) => !results.has(s.config.name),
)?.[0] ?? 'unknown';
results.set(failedName, {
status: 'unhealthy',
latency: -1,
lastCheck: new Date(),
uptime: 0,
details: result.reason instanceof Error ? result.reason.message : 'Unknown error',
});
}
}
return results;
}
/**
* List all registered service names and their current status.
*/
listServices(): Array<{ name: string; running: boolean; health?: ServiceHealth }> {
return Array.from(this.factories.keys()).map(name => {
const service = this.services.get(name);
return {
name,
running: !!service,
health: service?.health,
};
});
}
/**
* Get the count of currently running services.
*/
get runningCount(): number {
return this.services.size;
}
/**
* Check if a specific service is currently running.
*/
isRunning(name: string): boolean {
return this.services.has(name);
}
}
+190
View File
@@ -0,0 +1,190 @@
/**
* Service Registry
*
* Manages service lifecycle, dependency injection, and health monitoring
* for the Acme API platform.
*/
export interface ServiceConfig {
name: string;
version: string;
timeout: number;
retries: number;
enabled: boolean;
}
export interface ServiceHealth {
status: 'healthy' | 'degraded' | 'unhealthy';
latency: number;
lastCheck: Date;
details?: string;
}
export interface Service {
config: ServiceConfig;
health: ServiceHealth;
start(): Promise<void>;
stop(): Promise<void>;
healthCheck(): Promise<ServiceHealth>;
}
type ServiceFactory = (config: ServiceConfig) => Service;
// Default configurations for well-known services
const DEFAULT_CONFIGS: Record<string, Partial<ServiceConfig>> = {
database: { timeout: 5000, retries: 3 },
cache: { timeout: 2000, retries: 1 },
queue: { timeout: 10000, retries: 5 },
search: { timeout: 3000, retries: 2 },
email: { timeout: 15000, retries: 3 },
storage: { timeout: 8000, retries: 2 },
};
export class ServiceRegistry {
private services: Map<string, Service> = new Map();
private factories: Map<string, ServiceFactory> = new Map();
private startOrder: string[] = [];
private isShuttingDown = false;
/**
* Register a service factory for lazy instantiation.
* The factory is called when the service is first requested.
*/
registerFactory(name: string, factory: ServiceFactory): void {
if (this.factories.has(name)) {
throw new Error(`Factory already registered for service: ${name}`);
}
this.factories.set(name, factory);
}
/**
* Get a service by name. Creates it from the factory if not yet instantiated.
*/
async getService(name: string): Promise<Service> {
const existing = this.services.get(name);
if (existing) return existing;
const factory = this.factories.get(name);
if (!factory) {
throw new Error(`No factory registered for service: ${name}`);
}
const defaults = DEFAULT_CONFIGS[name] || {};
const config: ServiceConfig = {
name,
version: '1.0.0',
timeout: defaults.timeout ?? 5000,
retries: defaults.retries ?? 3,
enabled: true,
...defaults,
};
const service = factory(config);
this.services.set(name, service);
return service;
}
/**
* Start all registered services in dependency order.
* Services are started sequentially to respect dependencies.
*/
async startAll(): Promise<void> {
console.log(`Starting ${this.services.size} services...`);
for (const [name, service] of this.services) {
if (!service.config.enabled) {
console.log(`Skipping disabled service: ${name}`);
continue;
}
try {
await service.start();
this.startOrder.push(name);
console.log(`Started: ${name} (v${service.config.version})`);
} catch (err) {
console.error(`Failed to start ${name}:`, err);
throw err;
}
}
}
/**
* Stop all services in reverse startup order.
* Ensures graceful shutdown with proper resource cleanup.
*/
async stopAll(): Promise<void> {
if (this.isShuttingDown) return;
this.isShuttingDown = true;
console.log('Shutting down services...');
const reversed = [...this.startOrder].reverse();
for (const name of reversed) {
const service = this.services.get(name);
if (!service) continue;
try {
await service.stop();
console.log(`Stopped: ${name}`);
} catch (err) {
console.error(`Error stopping ${name}:`, err);
}
}
this.services.clear();
this.startOrder = [];
this.isShuttingDown = false;
}
/**
* Run health checks on all active services.
* Returns a summary of the overall system health.
*/
async healthCheckAll(): Promise<Map<string, ServiceHealth>> {
const results = new Map<string, ServiceHealth>();
for (const [name, service] of this.services) {
try {
const health = await service.healthCheck();
results.set(name, health);
} catch (err) {
results.set(name, {
status: 'unhealthy',
latency: -1,
lastCheck: new Date(),
details: err instanceof Error ? err.message : 'Unknown error',
});
}
}
return results;
}
/**
* List all registered service names and their current status.
*/
listServices(): Array<{ name: string; running: boolean; health?: ServiceHealth }> {
return Array.from(this.factories.keys()).map(name => {
const service = this.services.get(name);
return {
name,
running: !!service,
health: service?.health,
};
});
}
/**
* Get the count of currently running services.
*/
get runningCount(): number {
return this.services.size;
}
/**
* Check if a specific service is currently running.
*/
isRunning(name: string): boolean {
return this.services.has(name);
}
}
+44
View File
@@ -0,0 +1,44 @@
/**
* String utilities for the Acme API.
* Common text manipulation functions.
*/
export function capitalize(text: string): string {
if (!text) return text;
return text.charAt(0).toUpperCase() + text.slice(1);
}
export function camelToKebab(input: string): string {
return input.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
}
export function kebabToCamel(input: string): string {
return input.replace(/-([a-z])/g, (_, c) => c.toUpperCase());
}
export function truncate(text: string, maxLength: number, suffix = '...'): string {
if (text.length <= maxLength) return text;
return text.slice(0, maxLength - suffix.length) + suffix;
}
export function pluralize(word: string, count: number): string {
if (count === 1) return word;
if (word.endsWith('s') || word.endsWith('x') || word.endsWith('z')) return word + 'es';
if (word.endsWith('y')) return word.slice(0, -1) + 'ies';
return word + 's';
}
export function escapeHtml(text: string): string {
const map: Record<string, string> = {
'&': '&amp;',
'<': '&lt;',
'>': '&gt;',
'"': '&quot;',
"'": '&#039;',
};
return text.replace(/[&<>"']/g, c => map[c]);
}
export function template(str: string, vars: Record<string, string>): string {
return str.replace(/\{\{(\w+)\}\}/g, (_, key) => vars[key] ?? '');
}
+53
View File
@@ -0,0 +1,53 @@
/**
* Text utilities for the Acme API.
* Renamed from string-utils.ts — common text manipulation functions.
*/
export function capitalize(text: string): string {
if (!text) return text;
return text.charAt(0).toUpperCase() + text.slice(1);
}
export function camelToKebab(input: string): string {
return input.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
}
export function kebabToCamel(input: string): string {
return input.replace(/-([a-z])/g, (_, c) => c.toUpperCase());
}
export function truncate(text: string, maxLength: number, suffix = '...'): string {
if (text.length <= maxLength) return text;
return text.slice(0, maxLength - suffix.length) + suffix;
}
export function pluralize(word: string, count: number): string {
if (count === 1) return word;
if (word.endsWith('s') || word.endsWith('x') || word.endsWith('z')) return word + 'es';
if (word.endsWith('y')) return word.slice(0, -1) + 'ies';
return word + 's';
}
export function escapeHtml(text: string): string {
const map: Record<string, string> = {
'&': '&amp;',
'<': '&lt;',
'>': '&gt;',
'"': '&quot;',
"'": '&#039;',
};
return text.replace(/[&<>"']/g, c => map[c]);
}
export function template(str: string, vars: Record<string, string>): string {
return str.replace(/\{\{(\w+)\}\}/g, (_, key) => vars[key] ?? '');
}
export function wordCount(text: string): number {
return text.trim().split(/\s+/).filter(w => w.length > 0).length;
}
export function indent(text: string, spaces: number): string {
const pad = ' '.repeat(spaces);
return text.split('\n').map(line => pad + line).join('\n');
}
+6 -7
View File
@@ -1,5 +1,5 @@
#!/bin/bash
# Test script for worktree support in code review
# Test script for worktree support and expandable diff context in code review
#
# Usage:
# ./test-worktree-review.sh [--keep]
@@ -9,13 +9,12 @@
#
# What it does:
# 1. Builds the review app (ensures latest code)
# 2. Creates a temp git repo with 4 worktrees:
# - feature-auth: new file + modified file
# - fix-parser: modified file + untracked file
# - empty-branch: no changes (tests empty state)
# - detached HEAD: new file on detached HEAD
# 2. Creates a temp git repo with:
# - Main repo: 6 uncommitted changes including disjoint hunks, deleted file,
# renamed file, and new file (for expandable diff context testing)
# - 4 worktrees: feature-auth, fix-parser, empty-branch, detached HEAD
# 3. Launches review server — browser opens automatically
# 4. You test the worktree dropdown and diff switching
# 4. You test worktree dropdown, diff switching, and expandable context
# 5. Cleans up on exit (unless --keep)
set -e
+78 -10
View File
@@ -1,9 +1,9 @@
/**
* Test script for worktree support in Code Review
* Test script for worktree support and expandable diff context in Code Review
*
* Creates a temporary git repo with multiple worktrees, each with different
* kinds of changes, then launches the review server so you can manually test
* the worktree dropdown and diff switching.
* the worktree dropdown, diff switching, and expandable context features.
*
* Usage:
* bun run tests/manual/test-worktree-review.ts [--keep]
@@ -12,6 +12,8 @@
* --keep Don't clean up the temp repo on exit (for debugging)
*
* What to test:
*
* WORKTREE FEATURES:
* 1. Dropdown shows worktrees below a separator
* 2. Selecting a worktree enters "worktree mode" — dropdown shows
* Back to main / Uncommitted / Last commit / vs main
@@ -20,6 +22,22 @@
* 5. "Back to main repo" restores the original dropdown
* 6. Empty worktree shows appropriate empty state messages
* 7. Detached HEAD worktree uses directory name as label
*
* EXPANDABLE DIFF CONTEXT:
* 8. service-registry.ts — 4 disjoint hunks with gaps of 10-30 lines between them.
* Each gap shows "N unmodified lines" separator with expand up/down/both buttons.
* 9. Expand up/down reveals 100 lines at a time; small gaps (<100 lines) show
* a single "expand all" button instead.
* 10. Top of file (above first hunk) and bottom of file (below last hunk) are expandable.
* 11. deprecated-helper.ts — a deleted file. Should show expansion above hunks only
* (newContent is null, so newLines = []).
* 12. string-utils.ts → text-utils.ts — a renamed file. Expansion should use
* oldPath for old content and filePath for new content.
* 13. event-emitter.ts — a brand-new file with only additions. Expansion above
* the single hunk only (oldContent is null, so oldLines = []).
* 14. Switching diff types (uncommitted → last-commit → vs main) re-fetches
* file contents for expansion — verify separators appear in all modes.
* 15. Split and unified views both show expansion separators.
*/
import { $ } from "bun";
@@ -36,6 +54,10 @@ import html from "../../apps/review/dist/index.html" with { type: "text" };
const KEEP = process.argv.includes("--keep");
// --- Load fixtures ---
const FIXTURES_DIR = path.join(import.meta.dir, "fixtures");
const fixture = (name: string) => Bun.file(path.join(FIXTURES_DIR, name)).text();
// --- Setup temp repo with worktrees ---
const sandbox = path.join(tmpdir(), `plannotator-wt-test-${Date.now()}`);
@@ -52,6 +74,7 @@ await $`git init`.quiet().cwd(mainRepo);
await $`git checkout -b main`.quiet().cwd(mainRepo);
// Initial commit — a realistic small TypeScript project
// Large files loaded from fixtures, small ones inlined
const files: Record<string, string> = {
"src/index.ts": [
`import { App } from './app';`,
@@ -306,6 +329,10 @@ const files: Record<string, string> = {
`- \`src/utils/\` — Shared utilities (logging, parsing, formatting)`,
``,
].join("\n"),
// Large files from fixtures — test expandable diff context
"src/services/registry.ts": await fixture("service-registry.ts"),
"src/utils/deprecated-helper.ts": await fixture("deprecated-helper.ts"),
"src/utils/string-utils.ts": await fixture("string-utils.ts"),
};
for (const [filePath, content] of Object.entries(files)) {
@@ -365,7 +392,36 @@ await Bun.write(
].join("\n"),
);
console.error("Created main repo with uncommitted changes (2 files modified)");
// Disjoint hunks: service-registry.ts with 4 scattered edits (from fixture)
await Bun.write(
path.join(mainRepo, "src/services/registry.ts"),
await fixture("service-registry-modified.ts"),
);
// Deleted file: deprecated-helper.ts removed entirely
await $`git rm src/utils/deprecated-helper.ts`.quiet().cwd(mainRepo);
// Renamed file: string-utils.ts → text-utils.ts with additions
await $`git mv src/utils/string-utils.ts src/utils/text-utils.ts`.quiet().cwd(mainRepo);
await Bun.write(
path.join(mainRepo, "src/utils/text-utils.ts"),
await fixture("text-utils.ts"),
);
// New file (only additions): event-emitter.ts — brand new with no old version
await $`mkdir -p ${path.join(mainRepo, "src/events")}`.quiet();
await Bun.write(
path.join(mainRepo, "src/events/emitter.ts"),
await fixture("event-emitter.ts"),
);
console.error("Created main repo with uncommitted changes:")
console.error(" - src/index.ts — small edits (2 hunks)")
console.error(" - src/utils/format.ts — modified + new function")
console.error(" - src/services/registry.ts — 4 disjoint hunks (expansion test)")
console.error(" - src/utils/deprecated-helper.ts — deleted file")
console.error(" - src/utils/string-utils.ts → text-utils.ts — renamed + additions")
console.error(" - src/events/emitter.ts — brand new file");
// --- Worktree 1: feature-auth ---
// Tests the basic case: a new untracked file (auth.ts) and a modified tracked
@@ -1032,15 +1088,27 @@ console.error("");
console.error("Starting review server...");
console.error("Browser should open automatically.");
console.error("");
console.error("Things to test:");
console.error("=== WORKTREE TESTS ===");
console.error(" 1. Dropdown shows worktrees below a separator");
console.error(" 2. Select 'feature-auth' — dropdown switches to worktree mode");
console.error(" (Back to main / Uncommitted / Last commit / vs main)");
console.error(" 3. 'Uncommitted' shows the rate-limit.ts file");
console.error(" 4. 'Last commit' shows the committed auth module (5 files)");
console.error(" 2. Select 'feature-auth' → worktree mode (Back / Uncommitted / Last commit / vs main)");
console.error(" 3. 'Uncommitted' shows rate-limit.ts");
console.error(" 4. 'Last commit' shows committed auth module (5 files)");
console.error(" 5. 'Back to main repo' restores original dropdown");
console.error(" 6. Select 'empty-branch' — worktree mode, all options empty");
console.error(" 7. Select 'wt-detached' — label uses directory name, not branch");
console.error(" 6. 'empty-branch' → worktree mode, all options empty");
console.error(" 7. 'wt-detached' → label uses directory name, not branch");
console.error("");
console.error("=== EXPANDABLE DIFF CONTEXT TESTS ===");
console.error(" 8. registry.ts — 4 disjoint hunks. Between each pair, you should see");
console.error(" 'N unmodified lines' separators with expand up/down/both buttons.");
console.error(" 9. Click expand up/down — reveals lines incrementally.");
console.error(" Small gaps show a single 'expand all' button instead.");
console.error(" 10. Top-of-file (above first hunk) and bottom-of-file (below last hunk)");
console.error(" should also be expandable.");
console.error(" 11. deprecated-helper.ts — DELETED file. Only old content for expansion.");
console.error(" 12. string-utils.ts → text-utils.ts — RENAMED. Old path used for old side.");
console.error(" 13. events/emitter.ts — NEW file. Only new content, expansion above hunk.");
console.error(" 14. Switch diff types (uncommitted → last-commit → vs main) — expansion works in all.");
console.error(" 15. Toggle split/unified — expansion separators appear in both views.");
console.error("");
const server = await startReviewServer({