fix: detect same-file collisions reliably via basename + git overlap
Live observations showed two engineers both dirty on README.md but zero
collisions firing. Two root causes:
1. normalize() only lowercased and prepended src/, so the same file at
different path depths ("agent.ts" vs "backend/src/agent.ts") never
matched. Replace with lowercased-basename matching.
2. Git ground truth (engineer_states.changedFiles) was only used to set
the unpushed flag, never to match the file. The strongest signal was
wasted.
Detector now fuses vision currentFile AND git changedFiles into one
file->engineers map, so a collision fires when two people share a dirty
file even if both screens aren't on it at the same instant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AaCFWMkYQmTcuPsxaaACft
This commit is contained in:
@@ -60,7 +60,7 @@ export class PodMan {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const github = await getGithubState(); // cached
|
const github = await getGithubState(); // cached
|
||||||
const collisions = detectCollisions([...this.contexts.values()], github);
|
const collisions = detectCollisions([...this.contexts.values()], github, gitStates);
|
||||||
for (const collision of collisions) await this.handle(collision);
|
for (const collision of collisions) await this.handle(collision);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,34 +1,84 @@
|
|||||||
import type { EngineerContext, Collision, GithubStateSnapshot } from '@podman/shared';
|
import type { EngineerContext, Collision, GithubStateSnapshot } from '@podman/shared';
|
||||||
|
import type { GitState } from '../memory/db.js';
|
||||||
|
|
||||||
function normalize(path?: string): string | undefined {
|
/**
|
||||||
if (!path) return undefined;
|
* Collapse any path-ish string to a comparable file key.
|
||||||
return path.replace(/^\.?\/?(src\/)?/, 'src/').toLowerCase();
|
*
|
||||||
|
* Vision reads paths at inconsistent depths ("agent.ts" vs
|
||||||
|
* "backend/src/agent.ts"), and git status lines carry a status prefix
|
||||||
|
* ("M README.md", "?? test.txt"). Reduce both to a lowercased basename so the
|
||||||
|
* same file matches regardless of how it was observed. Basename matching can
|
||||||
|
* over-group two same-named files in different dirs, but for live coordination
|
||||||
|
* that bias toward firing is the right trade.
|
||||||
|
*/
|
||||||
|
function fileKey(raw?: string): string | undefined {
|
||||||
|
if (!raw) return undefined;
|
||||||
|
const stripped = raw.trim().replace(/^(\?\?|[MADRCU!]{1,2})\s+/, ''); // drop git status prefix
|
||||||
|
const base = stripped.split(/[\\/]/).pop()?.trim();
|
||||||
|
if (!base) return undefined;
|
||||||
|
return base.toLowerCase();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface Touch {
|
||||||
|
engineerId: string;
|
||||||
|
unpushed: boolean;
|
||||||
|
display: string; // original path/name to show in the card
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Detect same-file collisions from two fused signals:
|
||||||
|
* 1. Vision — what each engineer currently has on screen.
|
||||||
|
* 2. Git ground truth — each engineer's dirty/unpushed `changedFiles`.
|
||||||
|
*
|
||||||
|
* Git overlap is deterministic and does not require both engineers to have the
|
||||||
|
* file on screen at the same instant, so it is the reliable demo path.
|
||||||
|
*/
|
||||||
export function detectCollisions(
|
export function detectCollisions(
|
||||||
contexts: EngineerContext[],
|
contexts: EngineerContext[],
|
||||||
github: GithubStateSnapshot,
|
github: GithubStateSnapshot,
|
||||||
|
gitStates?: Map<string, GitState>,
|
||||||
): Collision[] {
|
): Collision[] {
|
||||||
const byFile = new Map<string, EngineerContext[]>();
|
const byFile = new Map<string, Touch[]>();
|
||||||
|
const add = (key: string | undefined, touch: Touch): void => {
|
||||||
|
if (!key) return;
|
||||||
|
(byFile.get(key) ?? byFile.set(key, []).get(key)!).push(touch);
|
||||||
|
};
|
||||||
|
|
||||||
|
// Signal 1: live vision context.
|
||||||
for (const c of contexts) {
|
for (const c of contexts) {
|
||||||
const f = normalize(c.currentFile);
|
add(fileKey(c.currentFile), {
|
||||||
if (!f) continue;
|
engineerId: c.engineerId,
|
||||||
(byFile.get(f) ?? byFile.set(f, []).get(f)!).push(c);
|
unpushed: c.hasUnpushedChanges === true,
|
||||||
|
display: c.currentFile ?? '',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// Signal 2: git ground truth (a dirty changed file is unpushed by definition).
|
||||||
|
if (gitStates) {
|
||||||
|
for (const [engineerId, git] of gitStates) {
|
||||||
|
for (const changed of git.changedFiles) {
|
||||||
|
add(fileKey(changed), { engineerId, unpushed: true, display: changed });
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const out: Collision[] = [];
|
const out: Collision[] = [];
|
||||||
for (const [file, group] of byFile) {
|
for (const [, touches] of byFile) {
|
||||||
const engineers = [...new Set(group.map((g) => g.engineerId))];
|
const engineers = [...new Set(touches.map((t) => t.engineerId))];
|
||||||
if (engineers.length < 2) continue;
|
if (engineers.length < 2) continue; // need two distinct people on one file
|
||||||
|
|
||||||
const anyUnpushed = group.some((g) => g.hasUnpushedChanges) || github.unpushed === true;
|
const anyUnpushed = touches.some((t) => t.unpushed) || github.unpushed === true;
|
||||||
if (!anyUnpushed) continue; // the crux GitHub alone cannot answer
|
if (!anyUnpushed) continue; // the crux GitHub alone cannot answer
|
||||||
|
|
||||||
|
// Show the most specific path we saw for this file.
|
||||||
|
const display =
|
||||||
|
touches.map((t) => t.display).sort((a, b) => b.length - a.length)[0] ?? touches[0]!.display;
|
||||||
|
|
||||||
out.push({
|
out.push({
|
||||||
id: `col_${file}_${Date.now()}`,
|
id: `col_${fileKey(display)}_${Date.now()}`,
|
||||||
podId: group[0]!.podId,
|
podId: contexts[0]?.podId ?? 'demo-pod',
|
||||||
file,
|
file: display,
|
||||||
symbol: group.find((g) => g.currentSymbol)?.currentSymbol,
|
symbol: contexts.find((c) => c.currentSymbol)?.currentSymbol,
|
||||||
engineers,
|
engineers,
|
||||||
severity: 'warn',
|
severity: 'warn',
|
||||||
githubState: { ...github, unpushed: anyUnpushed },
|
githubState: { ...github, unpushed: anyUnpushed },
|
||||||
|
|||||||
Reference in New Issue
Block a user