feat: add in-memory diff cache with TTL and commit-hash invalidation
Adds DiffCache<T> module, extends BranchManager with getHeadCommitHash, and wires phase-level caching into getPhaseReviewDiff and getFileDiff. Cache is invalidated in ExecutionOrchestrator after each task merges into the phase branch, ensuring stale diffs are never served after new commits. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -48,6 +48,7 @@ function createMocks() {
|
|||||||
diffBranches: vi.fn().mockResolvedValue(''),
|
diffBranches: vi.fn().mockResolvedValue(''),
|
||||||
diffBranchesStat: vi.fn().mockResolvedValue([]),
|
diffBranchesStat: vi.fn().mockResolvedValue([]),
|
||||||
diffFileSingle: vi.fn().mockResolvedValue(''),
|
diffFileSingle: vi.fn().mockResolvedValue(''),
|
||||||
|
getHeadCommitHash: vi.fn().mockResolvedValue('deadbeef00000000000000000000000000000000'),
|
||||||
deleteBranch: vi.fn(),
|
deleteBranch: vi.fn(),
|
||||||
branchExists: vi.fn().mockResolvedValue(true),
|
branchExists: vi.fn().mockResolvedValue(true),
|
||||||
remoteBranchExists: vi.fn().mockResolvedValue(false),
|
remoteBranchExists: vi.fn().mockResolvedValue(false),
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import type { ConflictResolutionService } from '../coordination/conflict-resolut
|
|||||||
import { phaseBranchName, taskBranchName } from '../git/branch-naming.js';
|
import { phaseBranchName, taskBranchName } from '../git/branch-naming.js';
|
||||||
import { ensureProjectClone } from '../git/project-clones.js';
|
import { ensureProjectClone } from '../git/project-clones.js';
|
||||||
import { createModuleLogger } from '../logger/index.js';
|
import { createModuleLogger } from '../logger/index.js';
|
||||||
|
import { phaseMetaCache, fileDiffCache } from '../review/diff-cache.js';
|
||||||
|
|
||||||
const log = createModuleLogger('execution-orchestrator');
|
const log = createModuleLogger('execution-orchestrator');
|
||||||
|
|
||||||
@@ -249,6 +250,10 @@ export class ExecutionOrchestrator {
|
|||||||
log.info({ taskId, taskBranch, phaseBranch, project: project.name }, 'task branch merged into phase branch');
|
log.info({ taskId, taskBranch, phaseBranch, project: project.name }, 'task branch merged into phase branch');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Invalidate diff cache — phase branch HEAD has advanced after merges
|
||||||
|
phaseMetaCache.invalidateByPrefix(`${phaseId}:`);
|
||||||
|
fileDiffCache.invalidateByPrefix(`${phaseId}:`);
|
||||||
|
|
||||||
// Emit task:merged event
|
// Emit task:merged event
|
||||||
const mergedEvent: TaskMergedEvent = {
|
const mergedEvent: TaskMergedEvent = {
|
||||||
type: 'task:merged',
|
type: 'task:merged',
|
||||||
|
|||||||
@@ -45,6 +45,11 @@ export interface BranchManager {
|
|||||||
*/
|
*/
|
||||||
diffFileSingle(repoPath: string, baseBranch: string, headBranch: string, filePath: string): Promise<string>;
|
diffFileSingle(repoPath: string, baseBranch: string, headBranch: string, filePath: string): Promise<string>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns the current HEAD commit hash (40-char SHA) for the given branch in the repo.
|
||||||
|
*/
|
||||||
|
getHeadCommitHash(repoPath: string, branch: string): Promise<string>;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Delete a branch. No-op if the branch doesn't exist.
|
* Delete a branch. No-op if the branch doesn't exist.
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -267,6 +267,12 @@ export class SimpleGitBranchManager implements BranchManager {
|
|||||||
return result.trim();
|
return result.trim();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async getHeadCommitHash(repoPath: string, branch: string): Promise<string> {
|
||||||
|
const git = simpleGit(repoPath);
|
||||||
|
const result = await git.raw(['rev-parse', branch]);
|
||||||
|
return result.trim();
|
||||||
|
}
|
||||||
|
|
||||||
async pushBranch(repoPath: string, branch: string, remote = 'origin'): Promise<void> {
|
async pushBranch(repoPath: string, branch: string, remote = 'origin'): Promise<void> {
|
||||||
const git = simpleGit(repoPath);
|
const git = simpleGit(repoPath);
|
||||||
try {
|
try {
|
||||||
|
|||||||
67
apps/server/review/diff-cache.ts
Normal file
67
apps/server/review/diff-cache.ts
Normal file
@@ -0,0 +1,67 @@
|
|||||||
|
/**
|
||||||
|
* DiffCache — in-memory cache with TTL and prefix-based invalidation.
|
||||||
|
* Used to avoid re-running expensive git diff subprocesses on repeated requests.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import type { FileStatEntry } from '../git/types.js';
|
||||||
|
|
||||||
|
interface CacheEntry<T> {
|
||||||
|
value: T;
|
||||||
|
expiresAt: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
export class DiffCache<T> {
|
||||||
|
private store = new Map<string, CacheEntry<T>>();
|
||||||
|
private ttlMs: number;
|
||||||
|
|
||||||
|
constructor(ttlMs: number) {
|
||||||
|
this.ttlMs = ttlMs;
|
||||||
|
}
|
||||||
|
|
||||||
|
get(key: string): T | undefined {
|
||||||
|
const entry = this.store.get(key);
|
||||||
|
if (!entry) return undefined;
|
||||||
|
if (Date.now() > entry.expiresAt) {
|
||||||
|
this.store.delete(key);
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
return entry.value;
|
||||||
|
}
|
||||||
|
|
||||||
|
set(key: string, value: T): void {
|
||||||
|
this.store.set(key, { value, expiresAt: Date.now() + this.ttlMs });
|
||||||
|
}
|
||||||
|
|
||||||
|
invalidateByPrefix(prefix: string): void {
|
||||||
|
for (const key of this.store.keys()) {
|
||||||
|
if (key.startsWith(prefix)) this.store.delete(key);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Response shapes (mirror the return types of getPhaseReviewDiff / getFileDiff)
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
export interface PhaseMetaResponse {
|
||||||
|
phaseName: string;
|
||||||
|
sourceBranch: string;
|
||||||
|
targetBranch: string;
|
||||||
|
files: FileStatEntry[];
|
||||||
|
totalAdditions: number;
|
||||||
|
totalDeletions: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface FileDiffResponse {
|
||||||
|
binary: boolean;
|
||||||
|
rawDiff: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Singleton instances — TTL is read once at module load time
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
const TTL = parseInt(process.env.REVIEW_DIFF_CACHE_TTL_MS ?? '300000', 10);
|
||||||
|
|
||||||
|
export const phaseMetaCache = new DiffCache<PhaseMetaResponse>(TTL);
|
||||||
|
export const fileDiffCache = new DiffCache<FileDiffResponse>(TTL);
|
||||||
@@ -11,6 +11,7 @@ import { requirePhaseRepository, requireTaskRepository, requireBranchManager, re
|
|||||||
import { phaseBranchName } from '../../git/branch-naming.js';
|
import { phaseBranchName } from '../../git/branch-naming.js';
|
||||||
import { ensureProjectClone } from '../../git/project-clones.js';
|
import { ensureProjectClone } from '../../git/project-clones.js';
|
||||||
import type { FileStatEntry } from '../../git/types.js';
|
import type { FileStatEntry } from '../../git/types.js';
|
||||||
|
import { phaseMetaCache, fileDiffCache } from '../../review/diff-cache.js';
|
||||||
|
|
||||||
export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
||||||
return {
|
return {
|
||||||
@@ -237,6 +238,16 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
|||||||
const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId);
|
const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId);
|
||||||
const files: FileStatEntry[] = [];
|
const files: FileStatEntry[] = [];
|
||||||
|
|
||||||
|
if (projects.length === 0) {
|
||||||
|
return { phaseName: phase.name, sourceBranch: phBranch, targetBranch: initBranch, files: [], totalAdditions: 0, totalDeletions: 0 };
|
||||||
|
}
|
||||||
|
|
||||||
|
const firstClone = await ensureProjectClone(projects[0], ctx.workspaceRoot!);
|
||||||
|
const headHash = await branchManager.getHeadCommitHash(firstClone, phBranch);
|
||||||
|
const cacheKey = `${input.phaseId}:${headHash}`;
|
||||||
|
const cached = phaseMetaCache.get(cacheKey);
|
||||||
|
if (cached) return cached;
|
||||||
|
|
||||||
for (const project of projects) {
|
for (const project of projects) {
|
||||||
const clonePath = await ensureProjectClone(project, ctx.workspaceRoot!);
|
const clonePath = await ensureProjectClone(project, ctx.workspaceRoot!);
|
||||||
const entries = await branchManager.diffBranchesStat(clonePath, diffBase, phBranch);
|
const entries = await branchManager.diffBranchesStat(clonePath, diffBase, phBranch);
|
||||||
@@ -255,7 +266,7 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
|||||||
const totalAdditions = files.reduce((sum, f) => sum + f.additions, 0);
|
const totalAdditions = files.reduce((sum, f) => sum + f.additions, 0);
|
||||||
const totalDeletions = files.reduce((sum, f) => sum + f.deletions, 0);
|
const totalDeletions = files.reduce((sum, f) => sum + f.deletions, 0);
|
||||||
|
|
||||||
return {
|
const result = {
|
||||||
phaseName: phase.name,
|
phaseName: phase.name,
|
||||||
sourceBranch: phBranch,
|
sourceBranch: phBranch,
|
||||||
targetBranch: initBranch,
|
targetBranch: initBranch,
|
||||||
@@ -263,6 +274,8 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
|||||||
totalAdditions,
|
totalAdditions,
|
||||||
totalDeletions,
|
totalDeletions,
|
||||||
};
|
};
|
||||||
|
phaseMetaCache.set(cacheKey, result);
|
||||||
|
return result;
|
||||||
}),
|
}),
|
||||||
|
|
||||||
getFileDiff: publicProcedure
|
getFileDiff: publicProcedure
|
||||||
@@ -297,6 +310,13 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
|||||||
const decodedPath = decodeURIComponent(input.filePath);
|
const decodedPath = decodeURIComponent(input.filePath);
|
||||||
|
|
||||||
const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId);
|
const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId);
|
||||||
|
|
||||||
|
const firstClone = await ensureProjectClone(projects[0], ctx.workspaceRoot!);
|
||||||
|
const headHash = await branchManager.getHeadCommitHash(firstClone, phBranch);
|
||||||
|
const cacheKey = `${input.phaseId}:${headHash}:${input.filePath}`;
|
||||||
|
const cached = fileDiffCache.get(cacheKey);
|
||||||
|
if (cached) return cached;
|
||||||
|
|
||||||
let clonePath: string;
|
let clonePath: string;
|
||||||
if (input.projectId) {
|
if (input.projectId) {
|
||||||
const project = projects.find((p) => p.id === input.projectId);
|
const project = projects.find((p) => p.id === input.projectId);
|
||||||
@@ -305,18 +325,22 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
|||||||
}
|
}
|
||||||
clonePath = await ensureProjectClone(project, ctx.workspaceRoot!);
|
clonePath = await ensureProjectClone(project, ctx.workspaceRoot!);
|
||||||
} else {
|
} else {
|
||||||
clonePath = await ensureProjectClone(projects[0], ctx.workspaceRoot!);
|
clonePath = firstClone;
|
||||||
}
|
}
|
||||||
|
|
||||||
const git = simpleGit(clonePath);
|
const git = simpleGit(clonePath);
|
||||||
// Binary files appear as "-\t-\t<path>" in --numstat output
|
// Binary files appear as "-\t-\t<path>" in --numstat output
|
||||||
const numstatOut = await git.raw(['diff', '--numstat', `${diffBase}...${phBranch}`, '--', decodedPath]);
|
const numstatOut = await git.raw(['diff', '--numstat', `${diffBase}...${phBranch}`, '--', decodedPath]);
|
||||||
if (numstatOut.trim() && numstatOut.startsWith('-\t-\t')) {
|
if (numstatOut.trim() && numstatOut.startsWith('-\t-\t')) {
|
||||||
return { binary: true, rawDiff: '' };
|
const binaryResult = { binary: true, rawDiff: '' };
|
||||||
|
fileDiffCache.set(cacheKey, binaryResult);
|
||||||
|
return binaryResult;
|
||||||
}
|
}
|
||||||
|
|
||||||
const rawDiff = await branchManager.diffFileSingle(clonePath, diffBase, phBranch, decodedPath);
|
const rawDiff = await branchManager.diffFileSingle(clonePath, diffBase, phBranch, decodedPath);
|
||||||
return { binary: false, rawDiff };
|
const result = { binary: false, rawDiff };
|
||||||
|
fileDiffCache.set(cacheKey, result);
|
||||||
|
return result;
|
||||||
}),
|
}),
|
||||||
|
|
||||||
approvePhaseReview: publicProcedure
|
approvePhaseReview: publicProcedure
|
||||||
|
|||||||
Reference in New Issue
Block a user