fix(plugin): keep resolved checkpoint deferrals closed

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<string>; semanticKeys: Set<string> } {
+  // 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<string, Set<string>>();
+  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<string>();
+      ids.add(surface.id);
+      historicalIds.set(key, ids);
+    }
+  }
+  const ids = new Set<string>();
+  const currentByKey = new Map<string, JsonObject[]>();
+  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<string>();
+  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<string>,
+): 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<string>; semanticKeys: Set<string> },
+): 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,

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 };