diff --git a/backend/src/agent/podman.ts b/backend/src/agent/podman.ts index c08f13d..b912cb5 100644 --- a/backend/src/agent/podman.ts +++ b/backend/src/agent/podman.ts @@ -9,11 +9,42 @@ import { recordIntervention, updateInterventionStatus, } from '../memory/store.js'; -import { getGitStates } from '../memory/db.js'; +import { getGitStates, type GitState } from '../memory/db.js'; import { recallSimilar } from '../memory/vectors.js'; import { shouldIntervene, preferredAction } from '../memory/policy.js'; import { publishHermesIntervention } from '../action/hermes.js'; +/** Strip a git-status prefix ("M ", "?? ") and reduce a path to its lowercased + * basename — matches comparableFile() in memory/store.ts so keys line up. */ +function comparableBasename(raw?: string): string { + return ( + (raw ?? '') + .trim() + .replace(/^(\?\?|[MADRCU!]{1,2})\s+/, '') + .split(/[\\/]/) + .pop() + ?.toLowerCase() ?? '' + ); +} + +/** Canonicalize an engineer name for case/whitespace-insensitive matching, so + * "Karti" and "karti" resolve to the same engineer's git state. */ +function canonicalName(raw?: string): string { + return (raw ?? '').trim().toLowerCase(); +} + +/** Git ground truth: do ALL involved engineers currently have the collided file + * in their changedFiles? Computed at detection time while git state is fresh. */ +function engineersOverlapOnFile(collision: Collision, gitStates: Map): boolean { + const target = comparableBasename(collision.file); + if (!target || collision.engineers.length < 2) return false; + const byCanon = new Map(); + for (const [name, st] of gitStates) byCanon.set(canonicalName(name), st.changedFiles); + return collision.engineers.every((e) => + (byCanon.get(canonicalName(e)) ?? []).some((f) => comparableBasename(f) === target), + ); +} + export class PodMan { private contexts = new Map(); /** @@ -75,6 +106,12 @@ export class PodMan { if (!current.has(key)) this.activeConflicts.delete(key); } + // Capture git ground-truth overlap now, while engineer_states are fresh, so + // the outcome-time verifier never depends on a stale sidecar or a late click. + for (const collision of collisions) { + collision.gitOverlap = engineersOverlapOnFile(collision, gitStates); + } + for (const collision of collisions) await this.handle(collision); } @@ -85,14 +122,7 @@ export class PodMan { * basename. */ private conflictKey(collision: Collision): string { - return ( - (collision.file ?? '') - .trim() - .replace(/^(\?\?|[MADRCU!]{1,2})\s+/, '') - .split(/[\\/]/) - .pop() - ?.toLowerCase() ?? '' - ); + return comparableBasename(collision.file); } private async handle(collision: Collision): Promise { diff --git a/backend/src/memory/store.ts b/backend/src/memory/store.ts index 61af96f..97bff45 100644 --- a/backend/src/memory/store.ts +++ b/backend/src/memory/store.ts @@ -96,15 +96,22 @@ export async function deriveWasRealCollision(outcome: InterventionOutcome): Prom try { const c = await collections(); const collision = await c.collisions.findOne({ id: outcome.collisionId }); - if (!collision || !Array.isArray(collision.engineers) || collision.engineers.length < 2) { - return false; - } + if (!collision) return false; + // Prefer the overlap evidence captured at detection time (fresh git state): + // immune to late clicks, stale sidecars, and the engineer_states TTL. + if (typeof collision.gitOverlap === 'boolean') return collision.gitOverlap; + // Fallback for collisions detected before gitOverlap was captured: re-derive + // from latest git state, matching engineers on case/whitespace-canonical names. + if (!Array.isArray(collision.engineers) || collision.engineers.length < 2) return false; const target = comparableFile(collision.file); if (!target) return false; - const gitStates = await getGitStates(outcome.podId); - const touchesTarget = (name: string): boolean => - (gitStates.get(name)?.changedFiles ?? []).some((f) => comparableFile(f) === target); - return collision.engineers.every(touchesTarget); + const byCanon = new Map(); + for (const [name, st] of await getGitStates(outcome.podId)) { + byCanon.set(name.trim().toLowerCase(), st.changedFiles); + } + return collision.engineers.every((e) => + (byCanon.get(e.trim().toLowerCase()) ?? []).some((f) => comparableFile(f) === target), + ); } catch (err) { console.error(`[memory] wasRealCollision verifier failed: ${(err as Error).message}`); return false; diff --git a/docs/PLAN.md b/docs/PLAN.md index bbbf83f..1a7893b 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -510,14 +510,19 @@ no schema change. Owner: RSI track. Independent of the MongoDB-cleanup handoff. kind), `plan.md:66` (second similar event behaves differently). 3. **Step 3 - derive `wasRealCollision` from git overlap (backend-authoritative)** ✅ - - `backend/src/memory/store.ts` `recordOutcome` now overrides the - client-supplied `wasRealCollision` with `deriveWasRealCollision()`: a - collision is real only if BOTH named engineers currently have the collided - file in their git `changedFiles` (`getGitStates`, 120s freshness TTL). - Conservative `false` when orphaned/stale. `frontend/.../useInterventions.ts` - stops sending a hardcoded `true` (now a backend-overridden placeholder). - - Restores the (accepted × wasReal) 2×2 the spec assumes. + - Overlap is captured AT detection time as `Collision.gitOverlap` + (`backend/src/agent/podman.ts`), while `engineer_states` are still fresh — + true only if ALL involved engineers have the collided file in their git + `changedFiles`, matched on case/whitespace-canonical names. + - `backend/src/memory/store.ts` `recordOutcome` overrides the client value + with `deriveWasRealCollision()`, which prefers the stored `gitOverlap` + (immune to late clicks / stale sidecars / the 120s TTL) and only falls back + to a live canonical-name re-derivation for pre-existing collisions. + `frontend/.../useInterventions.ts` stops sending hardcoded `true`. + - Restores the (accepted × wasReal) 2×2 the spec assumes; keeps `learned_from` + edges (`graph/live.ts:413`) from being silently zeroed on stage. - Spec: `docs/continual-learning/spec.md:98-108`, `policy.md:35-42`. + - Hardened per Codex review (name canonicalization + detection-time capture). Follow-ups (separate rungs, not in this change): Step 4-5 `strategy_versions` + Gemini-proposed `LearningProposal` slice; Step 6 durable `owns` write; seed a diff --git a/shared/src/collision.ts b/shared/src/collision.ts index 84ecbaf..7739b3d 100644 --- a/shared/src/collision.ts +++ b/shared/src/collision.ts @@ -16,6 +16,14 @@ export interface Collision { severity: CollisionSeverity; /** Snapshot of relevant GitHub state at detection time. */ githubState?: GithubStateSnapshot; + /** + * Git ground-truth overlap captured AT detection time, while engineer_states + * are still fresh: true when every involved engineer had `file` in their git + * changedFiles. Read as the authoritative wasRealCollision evidence at outcome + * time, so a late click, a stale sidecar, or the engineer_states freshness TTL + * cannot retroactively zero it out. + */ + gitOverlap?: boolean; detectedAt: string; }