diff --git a/docs/specs/2026-09-24-session-population-duplicate-identity.md b/docs/specs/2026-09-24-session-population-duplicate-identity.md new file mode 100644 index 00000000..1404e051 --- /dev/null +++ b/docs/specs/2026-09-24-session-population-duplicate-identity.md @@ -0,0 +1,82 @@ +# Session population freeze must keep one entry per distinct session identity + +## Traceability + +- Spec ID: 2026-09-24-session-population-duplicate-identity +- Story: related to QoderAI/better-harness#164 (same failure family) +- Status: Implemented + +## Intent + +The frozen Session population binding counts eligible Sessions by **distinct +trimmed sessionId** (`sessionIds()`), but `freezeSessionPopulation` stored the +raw prepared inventory in `population.sessions`. A provider whose discovery can +emit two entries with the same content-derived sessionId therefore freezes an +inventory whose raw length disagrees with its own binding count: the Session +facts lane (all-eligible `scope.eligibleSessions`), `selectSessions`, and every +other raw-count consumer then contradict the binding and the bundle fails with +`SESSION_POPULATION_BINDING_MISMATCH` — no report can be produced for that +workspace at all. + +Static review of the platform adapters shows the reachable trigger is Copilot: +session identity comes from file content (`workspace.yaml` `id:` and the +`session.start` event's `data.sessionId`, `copilot.mjs`) rather than a unique +filesystem location, and the discovery collector is a plain array. Two +session-state directories that resolve to one content id — copied, re-synced, +or resumed session state — reproduce the failure deterministically. Augment +shares the content-derived identity pattern but is not an evidence-bundle +provider, so its duplicates cannot reach the freeze today. + +The freeze owns the invariant its binding already declares: the frozen +inventory holds at most one entry per distinct trimmed session identity, and +dropped duplicate or empty-identity entries are recorded explicitly in the +binding's omission block rather than silently disappearing. + +## Acceptance scenarios + +- AC-1: `freezeSessionPopulation` drops duplicate and empty-trimmed-id entries + from the frozen inventory (`first-wins` in the given array order) so that + `population.sessions.length === population.binding.eligible.count` always + holds. The workspace-CWD candidate map follows the same first-wins rule so + the surviving entry inherits its own candidates (or none), never a dropped + duplicate sibling's. +- AC-2: the binding's `omission` block gains an additive + `duplicateIdentitySessions` count; existing omission fields, fingerprints, + schema versions, and `samePopulation` comparisons are unchanged. +- AC-3: a Copilot fixture home with two session-state directories sharing one + content id freezes a one-entry population, reports + `omission.duplicateIdentitySessions: 1`, and the Session facts lane stays + available with `eligibleSessions === selectedSessions === 1` instead of + throwing `SESSION_POPULATION_BINDING_MISMATCH`. + +## Non-goals + +- Changing any platform adapter's discovery or identity semantics (Copilot's + content-derived identity stays as is; merging duplicate entries' source refs + is future adapter work if ever needed). +- Changing selection strategies, count semantics of `selectSessions`, lane + envelopes, or the fail-closed binding validation itself. +- Renaming or versioning the population binding schema. + +## Plan and tasks + +1. Deduplicate the prepared inventory inside `freezeSessionPopulation` before + freezing, keyed by trimmed sessionId, counting dropped entries. +2. Add `duplicateIdentitySessions` to the binding omission block. +3. Regression tests: a unit test over the freeze (duplicate id + empty id + + unique id) and a real-fixture Copilot evidence-bundle test following the + existing Claude population/facts template. + +## Test and review evidence + +- New tests verified red on the pre-fix implementation (fix stashed: + 2 failed; the CWD-inheritance test separately verified red against the + pre-fix last-wins candidate map) and green after it (`npx vitest run + test/sessions/session-population.test.mjs + test/reporting/better-harness-evidence-bundle.test.mjs` — 45 passed). +- Full root suite: 1774 passed, 6 skipped, 0 failed; `npm run pack:verify` + passed (the freeze module ships in the npm package). +- End-to-end driver on a synthetic Copilot home: bundle goes from + `failed` with both lanes `SESSION_POPULATION_BINDING_MISMATCH` to a bound + population with both lanes available; the Claude end-to-end driver's healthy + and lead-failure paths are unchanged. diff --git a/scripts/session-analysis/session-population.mjs b/scripts/session-analysis/session-population.mjs index 5776070f..f5755e36 100644 --- a/scripts/session-analysis/session-population.mjs +++ b/scripts/session-analysis/session-population.mjs @@ -72,12 +72,33 @@ export function freezeSessionPopulation({ _factsStartedAt: startedAt, }, platform, providerSessionId); const sourceSessions = rows(sessions); - const workspaceCwdsBySessionId = new Map(sourceSessions.flatMap((session) => { + // First occurrence wins so the frozen (first-wins) entry inherits its own + // workspace CWD candidates instead of a later duplicate's. + const workspaceCwdsBySessionId = new Map(); + for (const session of sourceSessions) { const sessionId = String(session?.sessionId ?? "").trim(); - return sessionId ? [[sessionId, sessionWorkspaceCwds(session)]] : []; - })); + if (sessionId && !workspaceCwdsBySessionId.has(sessionId)) { + workspaceCwdsBySessionId.set(sessionId, sessionWorkspaceCwds(session)); + } + } const prepared = prepareFactsSessionInventory(sourceSessions, factsContext); - const frozenSessions = Object.freeze(prepared.sessions.map((session) => { + // The binding counts eligible Sessions by distinct trimmed id, so the frozen + // inventory must hold the same set: a duplicate or empty-id entry would make + // every raw-count consumer (facts scope, selectSessions) disagree with the + // binding and fail the whole bundle with SESSION_POPULATION_BINDING_MISMATCH. + const dedupedSessions = []; + const seenIds = new Set(); + let duplicateIdentitySessions = 0; + for (const session of prepared.sessions) { + const sessionId = String(session?.sessionId ?? "").trim(); + if (!sessionId || seenIds.has(sessionId)) { + duplicateIdentitySessions += 1; + continue; + } + seenIds.add(sessionId); + dedupedSessions.push(session); + } + const frozenSessions = Object.freeze(dedupedSessions.map((session) => { const sessionId = String(session?.sessionId ?? "").trim(); return freezeSession(session, sessionId ? workspaceCwdsBySessionId.get(sessionId) : []); })); @@ -103,6 +124,7 @@ export function freezeSessionPopulation({ exactIdentityAvailable: Boolean(factsContext.excludedSessionId), activeSessions: count(prepared.omitted.activeSessions), homeSessionOnly: count(prepared.omitted.homeSessionOnly), + duplicateIdentitySessions: count(duplicateIdentitySessions), recencyInference: suppliedUntil ? "disabled-frozen-until" : "enabled-unfrozen-until", }), eligible: Object.freeze({ diff --git a/test/reporting/better-harness-evidence-bundle.test.mjs b/test/reporting/better-harness-evidence-bundle.test.mjs index 3252fd4b..3d417b27 100644 --- a/test/reporting/better-harness-evidence-bundle.test.mjs +++ b/test/reporting/better-harness-evidence-bundle.test.mjs @@ -1128,6 +1128,49 @@ test("Claude population freeze and Session facts agree under one frozen topology } }); +test("Copilot population freeze keeps one identity per duplicated session-state directory", async () => { + const fixture = await realpath(await mkdtemp(path.join(os.tmpdir(), "evidence-bundle-copilot-duplicate-"))); + try { + const workspace = path.join(fixture, "workspace"); + const home = path.join(fixture, ".copilot"); + const sharedId = "831cdd03-a101-4246-8809-5d7a80dd48be"; + await mkdir(workspace, { recursive: true }); + for (const dirName of ["session-a", "session-b"]) { + const sessionDir = path.join(home, "session-state", dirName); + await mkdir(sessionDir, { recursive: true }); + await writeFile(path.join(sessionDir, "workspace.yaml"), `id: ${sharedId}\ncwd: ${workspace}\n`); + await writeFile(path.join(sessionDir, "events.jsonl"), [ + { type: "session.start", id: "e1", timestamp: "2026-07-20T01:00:00.000Z", data: { sessionId: sharedId, selectedModel: "test-model", context: { cwd: workspace } } }, + { type: "user.message", id: "e2", timestamp: "2026-07-20T01:00:01.000Z", data: { content: "run the tests" } }, + { type: "assistant.message", id: "e3", timestamp: "2026-07-20T01:00:04.000Z", data: { model: "test-model", content: "done", messageId: "m1" } }, + ].map((row) => JSON.stringify(row)).join("\n") + "\n"); + } + const resolution = topologyResolution(workspace); + const context = freezeEvidenceBundleContext({ + workspace, + platform: "copilot", + depth: "normal", + since: "2026-07-01T00:00:00.000Z", + until: "2026-07-24T08:00:00.000Z", + topology: resolution.topology, + analysisScope: resolution.analysisScope, + }, NOW); + const options = { "copilot-home": home }; + + const population = await collectSessionPopulation(context, options); + assert.equal(population.sessions.length, 1); + assert.equal(population.binding.eligible.count, 1); + assert.equal(population.binding.omission.duplicateIdentitySessions, 1); + + const lane = await collectSessionEvidence(context, options, { sessionPopulation: population }); + assert.equal(lane.status, "available"); + assert.equal(lane.data.scope.eligibleSessions, population.binding.eligible.count); + assert.equal(lane.data.scope.selectedSessions, population.binding.eligible.count); + } finally { + await rm(fixture, { recursive: true, force: true }); + } +}); + test("zero-signal Episode admission remains valid inside one bound population", async () => { const result = await collectEvidenceBundle({ workspace: ".", platform: "codex" }, dependencies()); diff --git a/test/sessions/session-population.test.mjs b/test/sessions/session-population.test.mjs index 74c9ab4b..4a24cffb 100644 --- a/test/sessions/session-population.test.mjs +++ b/test/sessions/session-population.test.mjs @@ -114,3 +114,43 @@ test("binding validation preserves zero-signal lead admission and reconciles Ses lead: { population: population.binding, selection: leadSelection, admission: leadAdmission }, }), []); }); + +test("frozen population keeps one entry per distinct session identity", async () => { + const { freezeSessionPopulation } = await populationModule(); + const population = freezeSessionPopulation({ + scope: { + platform: "copilot", + workspace: "/private/workspace", + since: "2026-09-17T00:00:00.000Z", + until: "2026-09-24T00:00:00.000Z", + }, + sessions: [ + { sessionId: "shared-private", sourceRefs: [{ kind: "session-state", path: "/private/a/events.jsonl" }] }, + { sessionId: "shared-private", sourceRefs: [{ kind: "session-state", path: "/private/b/events.jsonl" }] }, + { sessionId: " ", sourceRefs: [{ kind: "session-state", path: "/private/c/events.jsonl" }] }, + { sessionId: "unique-private", sourceRefs: [{ kind: "session-state", path: "/private/d/events.jsonl" }] }, + ], + suppliedUntil: true, + }); + + assert.deepEqual(population.sessions.map((session) => session.sessionId), ["shared-private", "unique-private"]); + assert.equal(population.binding.eligible.count, 2); + assert.equal(population.binding.omission.duplicateIdentitySessions, 2); + assert.doesNotMatch(JSON.stringify(population.binding), /\/private\//u); +}); + +test("frozen population keeps the surviving duplicate entry own workspace CWD candidates", async () => { + const { freezeSessionPopulation } = await populationModule(); + const { bindSessionWorkspaceCwds, sessionWorkspaceCwds } = await workspaceModule(); + const population = freezeSessionPopulation({ + scope: { platform: "copilot", workspace: "/workspace", until: "2026-09-24T00:00:00.000Z" }, + sessions: [ + { sessionId: "shared" }, + bindSessionWorkspaceCwds({ sessionId: "shared" }, ["/elsewhere"]), + ], + suppliedUntil: true, + }); + + assert.equal(population.sessions.length, 1); + assert.deepEqual(sessionWorkspaceCwds(population.sessions[0]), []); +});