diff --git a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts index 5b9ce3504..93a282fb8 100644 --- a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts +++ b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts @@ -333,11 +333,16 @@ async function preserveScanDraft( )) .map((surface) => surface.candidateId), ].filter((value): value is string => typeof value === "string")); + const currentSurfaces = result.coverage.surfaces as JsonObject[]; + const resolvedSurfaces = resolvedCoverageSurfaces(currentSurfaces, sources); const resolvedFollowUpSurfaces = sources.flatMap((source) => { const pending = source.coverage.deferred as JsonObject[]; if (pending.length === 0 || pending.some((item) => { const candidateId = item.candidateId ?? item.id; - return typeof candidateId !== "string" || !resolvedCandidateIds.has(candidateId); + return ( + (typeof candidateId !== "string" || !resolvedCandidateIds.has(candidateId)) + && !deferredSurfacesResolved(item, resolvedSurfaces.ids) + ); })) return []; return (source.coverage.surfaces as JsonObject[]).filter( (surface) => surface.disposition === "needs_follow_up", @@ -401,6 +406,7 @@ async function preserveScanDraft( return ( (typeof candidateId !== "string" || !resolvedIds.has(candidateId)) && !coverageEntryPresent(result.coverage.deferred as unknown[], item) + && !deferredSurfacesResolved(item, resolvedSurfaces.ids) ); }), surfaces: (source.coverage.surfaces as JsonObject[]).filter((surface) => { @@ -410,7 +416,10 @@ async function preserveScanDraft( && !coverageEntryPresent(result.coverage.surfaces as unknown[], surface) && !( surface.disposition === "needs_follow_up" - && coverageEntryPresent(resolvedFollowUpSurfaces, surface) + && ( + coverageEntryPresent(resolvedFollowUpSurfaces, surface) + || coverageSurfaceResolved(surface, resolvedSurfaces) + ) ) ); }), @@ -851,6 +860,75 @@ function coverageEntryIdentities(entry: JsonObject): string[] { return []; } +function resolvedCoverageSurfaces( + current: JsonObject[], + sources: ScanDraftInput[], +): { ids: Set; semanticKeys: Set } { + // Surface IDs are workbench-owned, so a later semantic draft can omit them. + // Reconnect a terminal surface to history only when its label and risk area + // identify one historical ID; duplicate surfaces must stay distinct. + const historicalIds = new Map>(); + for (const source of sources) { + for (const surface of source.coverage.surfaces as JsonObject[]) { + const key = coverageSurfaceSemanticKey(surface); + if (key === undefined || typeof surface.id !== "string") continue; + const ids = historicalIds.get(key) ?? new Set(); + ids.add(surface.id); + historicalIds.set(key, ids); + } + } + const ids = new Set(); + const currentByKey = new Map(); + for (const surface of current) { + if (surface.disposition !== "needs_follow_up" && typeof surface.id === "string") { + ids.add(surface.id); + } + const key = coverageSurfaceSemanticKey(surface); + if (key === undefined) continue; + const surfaces = currentByKey.get(key) ?? []; + surfaces.push(surface); + currentByKey.set(key, surfaces); + } + const semanticKeys = new Set(); + for (const [key, surfaces] of currentByKey) { + if ( + surfaces.length !== 1 + || surfaces[0]!.disposition === "needs_follow_up" + ) continue; + const previousIds = historicalIds.get(key); + if (previousIds?.size !== 1) continue; + ids.add([...previousIds][0]!); + semanticKeys.add(key); + } + return { ids, semanticKeys }; +} + +function deferredSurfacesResolved( + deferred: JsonObject, + resolvedSurfaceIds: Set, +): boolean { + const surfaceIds = deferred.surfaceIds; + return Array.isArray(surfaceIds) + && surfaceIds.length > 0 + && surfaceIds.every((value) => ( + typeof value === "string" && resolvedSurfaceIds.has(value) + )); +} + +function coverageSurfaceResolved( + surface: JsonObject, + resolved: { ids: Set; semanticKeys: Set }, +): boolean { + if (typeof surface.id === "string") return resolved.ids.has(surface.id); + const key = coverageSurfaceSemanticKey(surface); + return key !== undefined && resolved.semanticKeys.has(key); +} + +function coverageSurfaceSemanticKey(surface: JsonObject): string | undefined { + if (typeof surface.label !== "string") return undefined; + return JSON.stringify([surface.riskArea ?? null, surface.label]); +} + /** Reducers cannot resolve source review by omitting its coverage records. */ export function preserveScanCoverage( coverage: JsonObject, diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs index 77ff8a93d..190861709 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs +++ b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft.mjs @@ -527,6 +527,157 @@ try { assert.deepEqual(resolvedRejection.coverage.deferred, []); assert.deepEqual(resolvedRejection.coverage.surfaces[0].candidate, candidate); + const resolvedSurfaceRoot = path.join(root, "resolved-checkpoint-surface"); + await mkdir(resolvedSurfaceRoot); + const resolvedSurfaceContext = { ...context, root: resolvedSurfaceRoot }; + const surfaceId = "surface_source-review"; + await recordCodexSecurityScanDraft(resolvedSurfaceContext, { + ...input, + complete: false, + findings: [], + coverage: { + ...coverage, + completeness: "partial", + surfaces: [{ label: "Source review", disposition: "needs_follow_up" }], + deferred: [{ + id: "checkpoint-source-review", + reason: "Source review was pending.", + surfaceIds: [surfaceId], + }], + openQuestions: ["Has source review completed?"], + }, + }); + await recordCodexSecurityScanDraft(resolvedSurfaceContext, { + ...input, + findings: [], + coverage: { + ...coverage, + surfaces: [{ label: "Source review", disposition: "no_issue_found" }], + openQuestions: [], + }, + }); + const resolvedSurfaceResult = JSON.parse( + await readFile(path.join(resolvedSurfaceRoot, "coverage.json"), "utf8"), + ); + assert.equal(resolvedSurfaceResult.completeness, "complete"); + assert.deepEqual(resolvedSurfaceResult.deferred, []); + assert.deepEqual(resolvedSurfaceResult.openQuestions, []); + const resolvedSurfaceHistory = await Promise.all( + (await readdir(path.join(resolvedSurfaceRoot, "checkpoints"))).map(async name => ( + JSON.parse(await readFile(path.join(resolvedSurfaceRoot, "checkpoints", name), "utf8")) + )), + ); + assert.equal(resolvedSurfaceHistory.some(checkpoint => ( + checkpoint.complete === false + && checkpoint.coverage.deferred.some(item => item.id === "checkpoint-source-review") + )), true); + + const unresolvedSurfaceRoot = path.join(root, "unresolved-checkpoint-surface"); + await mkdir(unresolvedSurfaceRoot); + const unresolvedSurfaceContext = { ...context, root: unresolvedSurfaceRoot }; + await recordCodexSecurityScanDraft(unresolvedSurfaceContext, { + ...input, + complete: false, + findings: [], + coverage: { + ...coverage, + completeness: "partial", + surfaces: [ + { label: "Source review", disposition: "needs_follow_up" }, + { label: "Source review", disposition: "needs_follow_up" }, + ], + deferred: [{ + id: "checkpoint-multiple-surfaces", + reason: "Two source review units were pending.", + surfaceIds: [surfaceId, `${surfaceId}-2`], + }], + }, + }); + await recordCodexSecurityScanDraft(unresolvedSurfaceContext, { + ...input, + findings: [], + coverage: { + ...coverage, + surfaces: [{ label: "Source review", disposition: "no_issue_found" }], + }, + }); + const unresolvedSurfaceResult = JSON.parse( + await readFile(path.join(unresolvedSurfaceRoot, "coverage.json"), "utf8"), + ); + assert.equal(unresolvedSurfaceResult.completeness, "partial"); + assert.deepEqual( + unresolvedSurfaceResult.deferred.map(item => item.id), + ["checkpoint-multiple-surfaces"], + ); + + const resolvedDuplicateRoot = path.join(root, "resolved-duplicate-surfaces"); + await mkdir(resolvedDuplicateRoot); + const resolvedDuplicateContext = { ...context, root: resolvedDuplicateRoot }; + await recordCodexSecurityScanDraft(resolvedDuplicateContext, { + ...input, + complete: false, + findings: [], + coverage: { + ...coverage, + completeness: "partial", + surfaces: [ + { label: "Source review", disposition: "needs_follow_up" }, + { label: "Source review", disposition: "needs_follow_up" }, + ], + deferred: [{ + id: "checkpoint-resolved-duplicates", + reason: "Two source review units were pending.", + surfaceIds: [surfaceId, `${surfaceId}-2`], + }], + }, + }); + const rawDuplicateHistory = await Promise.all( + (await readdir(path.join(resolvedDuplicateRoot, "checkpoints"))).map(async name => ( + JSON.parse(await readFile(path.join(resolvedDuplicateRoot, "checkpoints", name), "utf8")) + )), + ); + const rawDuplicateCheckpoint = rawDuplicateHistory.find(checkpoint => ( + checkpoint.complete === false + && checkpoint.coverage.deferred.some(item => item.id === "checkpoint-resolved-duplicates") + )); + assert.ok(rawDuplicateCheckpoint); + assert.equal(rawDuplicateCheckpoint.coverage.surfaces.length, 2); + assert.equal( + rawDuplicateCheckpoint.coverage.surfaces.every(surface => !("id" in surface)), + true, + ); + await recordCodexSecurityScanDraft(resolvedDuplicateContext, { + ...input, + findings: [], + coverage: { + ...coverage, + surfaces: [ + { + id: surfaceId, + label: "Source review", + disposition: "no_issue_found", + }, + { + id: `${surfaceId}-2`, + label: "Source review", + disposition: "no_issue_found", + }, + ], + }, + }); + const resolvedDuplicateResult = JSON.parse( + await readFile(path.join(resolvedDuplicateRoot, "coverage.json"), "utf8"), + ); + assert.equal(resolvedDuplicateResult.completeness, "complete"); + assert.deepEqual(resolvedDuplicateResult.deferred, []); + assert.deepEqual( + resolvedDuplicateResult.surfaces.map(surface => [surface.id, surface.disposition]), + [ + [surfaceId, "no_issue_found"], + [`${surfaceId}-2`, "no_issue_found"], + ], + ); + const undefinedCandidateRoot = path.join(root, "undefined-candidate-worker"); await mkdir(undefinedCandidateRoot); const undefinedCandidateContext = { ...workerContext, root: undefinedCandidateRoot };