From 6f116164b6ce9d5f4be1a3deab45788e5e206bb5 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 10 Oct 2026 23:38:27 +0000 Subject: [PATCH 1/8] refactor: share persisted finding normalization --- CONTRIBUTING.md | 4 + .../mcp-app/src/artifact-scan-draft.ts | 170 +-------------- .../mcp-app/src/finding-normalization.ts | 193 +++++++++++++++++ .../test_artifact_scan_draft_recovery.ts | 61 ++++++ sdk/typescript/scripts/check-package.mjs | 1 + sdk/typescript/scripts/generate-models.cjs | 31 ++- sdk/typescript/src/contract.ts | 140 +------------ sdk/typescript/src/finding-normalization.ts | 194 ++++++++++++++++++ 8 files changed, 479 insertions(+), 315 deletions(-) create mode 100644 plugins/codex-security/mcp-app/src/finding-normalization.ts create mode 100644 sdk/typescript/src/finding-normalization.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 83335097d..50ac46466 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,6 +15,10 @@ packages and releases. Do not edit or commit files in that directory. See the [SDK testing guide](sdk/typescript/TESTING.md) for the generation and validation commands. +`pnpm --dir sdk/typescript run generate:models` also copies shared finding +normalization from the plugin source into the SDK. Edit the plugin helper, then +regenerate its SDK copy. `pnpm --dir sdk/typescript run types` checks freshness. + Model-based evaluations live in [`evals/`](evals/README.md), outside the plugin source. Deterministic triage checks and the MCP reducer IPC regression remain part of normal CI. 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 01d011a04..41c13b101 100644 --- a/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts +++ b/plugins/codex-security/mcp-app/src/artifact-scan-draft.ts @@ -1,5 +1,6 @@ +import { normalizePersistedFindingDetails } from "./finding-normalization.js"; import type { JsonObject } from "./types.js"; -import { isRecord as isObject, isNonEmptyString } from "./record.js"; +import { isRecord as isObject } from "./record.js"; import { createHash, randomUUID } from "node:crypto"; import { promises as fs } from "node:fs"; import { dirname, join, sep } from "node:path"; @@ -1765,7 +1766,7 @@ export function parsePersistedScanDraft( } for (const finding of compatible.findings) { if (!isObject(finding)) continue; - normalizePersistedFindingDetails(finding); + normalizePersistedFindingDetails(finding, "draft"); } return parseScanDraftDocument(compatible); } @@ -1803,171 +1804,6 @@ function parsePersistedCheckpoint( return parsePersistedScanDraft(compatible); } -function normalizePersistedFindingDetails(finding: JsonObject): void { - const canonicalEvidence = Array.isArray(finding.codeEvidence) - ? finding.codeEvidence - : []; - const evidenceIds = new Set( - canonicalEvidence.flatMap((evidence) => { - if (!isObject(evidence)) return []; - const id = evidence.id; - return isNonEmptyString(id) ? [id] : []; - }), - ); - if (Array.isArray(finding.code_evidence)) { - const compatibleEvidence: JsonObject[] = []; - for (const evidence of finding.code_evidence) { - if (!isObject(evidence)) continue; - const id = evidence.id; - const code = evidence.code; - if ( - !isNonEmptyString(id) || - !isNonEmptyString(code) || - evidenceIds.has(id) - ) { - continue; - } - evidenceIds.add(id); - compatibleEvidence.push(evidence); - } - finding.code_evidence = compatibleEvidence; - } else if ("code_evidence" in finding) { - delete finding.code_evidence; - } - - for (const [sectionName, listFields] of [ - ["rootCause", ["evidenceRefs", "evidence_refs"]], - ["root_cause", ["evidenceRefs", "evidence_refs"]], - [ - "validation", - [ - "assertions", - "counterEvidence", - "evidence", - "evidenceRefs", - "evidence_refs", - "limitations", - ], - ], - [ - "attackPath", - [ - "assumptions", - "blindspots", - "controls", - "evidenceRefs", - "evidence_refs", - "limitations", - "preconditions", - "steps", - ], - ], - ] satisfies Array<[string, string[]]>) { - const section = finding[sectionName]; - if (!isObject(section)) continue; - normalizePersistedStringLists(section, listFields); - } - - const rootCause = finding.rootCause; - if (isObject(rootCause)) { - if (!isNonEmptyString(rootCause.summary)) { - delete finding.rootCause; - } else { - removeUnsupportedPersistedStrings(rootCause, ["code", "language"]); - } - } - const legacyRootCause = finding.root_cause; - if (isObject(legacyRootCause)) { - removeUnsupportedPersistedStrings(legacyRootCause, [ - "summary", - "code", - "language", - ]); - } else if ("root_cause" in finding && !isNonEmptyString(legacyRootCause)) { - delete finding.root_cause; - } - - const validation = finding.validation; - if (isObject(validation)) { - removeUnsupportedPersistedStrings(validation, [ - "method", - "status", - "summary", - "disposition", - "result", - ]); - } - - const attackPath = finding.attackPath; - if (!isObject(attackPath)) return; - removeUnsupportedPersistedStrings(attackPath, ["summary"]); - for (const field of ["dataFlow", "data_flow", "dataflow", "reachability"]) { - const detail = attackPath[field]; - if (!isObject(detail)) { - if (!isNonEmptyString(detail)) delete attackPath[field]; - continue; - } - removeUnsupportedPersistedStrings(detail, [ - "summary", - "source", - "sink", - "outcome", - ...(field === "reachability" ? ["attacker", "entrypoint"] : []), - ]); - normalizePersistedStringLists(detail, [ - "evidenceRefs", - "evidence_refs", - "transformations", - ...(field === "reachability" ? ["preconditions"] : []), - ]); - } - for (const field of ["impact", "likelihood"]) { - const detail = attackPath[field]; - if (isObject(detail)) { - removeUnsupportedPersistedStrings(detail, ["level", "rationale", "why"]); - } else if ( - detail !== undefined && - detail !== null && - !isNonEmptyString(detail) - ) { - delete attackPath[field]; - } - } - - function normalizePersistedStringLists( - section: JsonObject, - fields: string[], - ): void { - for (const field of fields) { - if (!(field in section)) continue; - const value = section[field]; - const normalized = isNonEmptyString(value) - ? [value] - : Array.isArray(value) - ? value.filter(isNonEmptyString) - : []; - if (normalized.length > 0) section[field] = normalized; - else delete section[field]; - } - for (const field of ["evidenceRefs", "evidence_refs"]) { - const refs = section[field]; - if (!Array.isArray(refs)) continue; - section[field] = refs.filter((ref) => evidenceIds.has(ref)); - } - } -} - -function removeUnsupportedPersistedStrings( - section: JsonObject, - fields: string[], -): void { - for (const field of fields) { - if (field in section && !isNonEmptyString(section[field])) { - delete section[field]; - } - } -} - function requireBoundScan( context: ArtifactContext, input: CompletedScanInput, diff --git a/plugins/codex-security/mcp-app/src/finding-normalization.ts b/plugins/codex-security/mcp-app/src/finding-normalization.ts new file mode 100644 index 000000000..381a4b32d --- /dev/null +++ b/plugins/codex-security/mcp-app/src/finding-normalization.ts @@ -0,0 +1,193 @@ +import { isRecord as isObject } from "./record.js"; + +type JsonObject = Record; + +export function normalizePersistedFindingDetails( + finding: JsonObject, + profile: "draft" | "canonical", +): void { + const canonicalEvidence = + profile === "draft" && Array.isArray(finding["codeEvidence"]) + ? finding["codeEvidence"] + : []; + const evidenceIds = new Set( + canonicalEvidence.flatMap((evidence) => { + if (!isObject(evidence)) return []; + const id = evidence["id"]; + return isNonEmptyString(id) ? [id] : []; + }), + ); + if (Array.isArray(finding["code_evidence"])) { + const compatibleEvidence: JsonObject[] = []; + for (const evidence of finding["code_evidence"]) { + if (!isObject(evidence)) continue; + const id = evidence["id"]; + const code = evidence["code"]; + if ( + !isNonEmptyString(id) || + !isNonEmptyString(code) || + (profile === "draft" && evidenceIds.has(id)) + ) { + continue; + } + if (profile === "draft") evidenceIds.add(id); + compatibleEvidence.push(evidence); + } + finding["code_evidence"] = compatibleEvidence; + } else if (profile === "draft" || finding["code_evidence"] !== null) { + delete finding["code_evidence"]; + } + + for (const [sectionName, listFields] of [ + ["rootCause", ["evidenceRefs", "evidence_refs"]], + ["root_cause", ["evidenceRefs", "evidence_refs"]], + [ + "validation", + [ + "assertions", + "counterEvidence", + "evidence", + "evidenceRefs", + "evidence_refs", + "limitations", + ], + ], + [ + "attackPath", + [ + "assumptions", + "blindspots", + "controls", + "evidenceRefs", + "evidence_refs", + "limitations", + "preconditions", + "steps", + ], + ], + ] satisfies Array<[string, string[]]>) { + const section = finding[sectionName]; + if (!isObject(section)) continue; + normalizePersistedStringLists(section, listFields); + } + + const rootCause = finding["rootCause"]; + if (profile === "draft" && isObject(rootCause)) { + if (!isNonEmptyString(rootCause["summary"])) { + delete finding["rootCause"]; + } else { + removeUnsupportedPersistedStrings(rootCause, ["code", "language"]); + } + } + const legacyRootCause = finding["root_cause"]; + if (isObject(legacyRootCause)) { + removeUnsupportedPersistedStrings(legacyRootCause, [ + "summary", + "code", + "language", + ]); + } else if ( + profile === "draft" + ? !isNonEmptyString(legacyRootCause) + : legacyRootCause !== null && typeof legacyRootCause !== "string" + ) { + delete finding["root_cause"]; + } + + const validation = finding["validation"]; + if (isObject(validation)) { + removeUnsupportedPersistedStrings( + validation, + ["method", "status", "summary", "disposition", "result"].filter( + (field) => + profile === "draft" || + !["status", "disposition", "result"].includes(field) || + validation[field] !== null, + ), + ); + } + + const attackPath = finding["attackPath"]; + if (!isObject(attackPath)) return; + removeUnsupportedPersistedStrings(attackPath, ["summary"]); + for (const field of ["dataFlow", "data_flow", "dataflow", "reachability"]) { + const detail = attackPath[field]; + if (!isObject(detail)) { + if (!isNonEmptyString(detail)) delete attackPath[field]; + continue; + } + removeUnsupportedPersistedStrings(detail, [ + "summary", + "source", + "sink", + "outcome", + ...(field === "reachability" ? ["attacker", "entrypoint"] : []), + ]); + normalizePersistedStringLists(detail, [ + "evidenceRefs", + "evidence_refs", + "transformations", + ...(field === "reachability" ? ["preconditions"] : []), + ]); + } + for (const field of ["impact", "likelihood"]) { + const detail = attackPath[field]; + if (isObject(detail)) { + removeUnsupportedPersistedStrings(detail, ["level", "rationale", "why"]); + } else if ( + detail !== undefined && + detail !== null && + !isNonEmptyString(detail) + ) { + delete attackPath[field]; + } + } + + function normalizePersistedStringLists( + section: JsonObject, + fields: string[], + ): void { + for (const field of fields) { + const value = section[field]; + if ( + profile === "canonical" && + field === "evidence" && + isNonEmptyString(value) + ) + continue; + const normalized = isNonEmptyString(value) + ? [value] + : Array.isArray(value) + ? value.filter(isNonEmptyString) + : undefined; + if ( + normalized !== undefined && + (profile === "canonical" || normalized.length > 0) + ) + section[field] = normalized; + else delete section[field]; + } + if (profile === "canonical") return; + for (const field of ["evidenceRefs", "evidence_refs"]) { + const refs = section[field]; + if (!Array.isArray(refs)) continue; + section[field] = refs.filter((ref) => evidenceIds.has(ref)); + } + } + + function removeUnsupportedPersistedStrings( + section: JsonObject, + fields: string[], + ): void { + for (const field of fields) { + if (!isNonEmptyString(section[field])) delete section[field]; + } + } + + function isNonEmptyString(value: unknown): value is string { + return ( + typeof value === "string" && + (profile === "canonical" ? value.length > 0 : value.trim().length > 0) + ); + } +} diff --git a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft_recovery.ts b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft_recovery.ts index 548b29c53..c173582f1 100644 --- a/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft_recovery.ts +++ b/plugins/codex-security/mcp-app/tests/test_artifact_scan_draft_recovery.ts @@ -41,6 +41,67 @@ const findingFor = (candidateId: string) => ({ provenance: { source: "local_plugin", candidateId }, }); +test("persisted draft recovery preserves evidence while removing unsupported optional details", async (t) => { + const f = await fixture(t, "worker"); + const evidence = { + id: "canonical-source", + label: "Synthetic evidence", + path: "src/example.py", + startLine: 1, + code: "example()", + explanation: "Synthetic recovery fixture.", + }; + const legacyEvidence = { id: "legacy-source", code: "legacy_example()" }; + const input = { + ...f.draft(), + findings: [ + { + ...findingFor("candidate-1"), + codeEvidence: [evidence], + code_evidence: [ + evidence, + legacyEvidence, + legacyEvidence, + { id: " ", code: " " }, + null, + ], + rootCause: { summary: " " }, + root_cause: null, + validation: { + evidence: "single proof", + counterEvidence: [], + status: null, + evidenceRefs: ["canonical-source", "legacy-source", "missing"], + }, + attackPath: { + steps: [" ", "Check the entrypoint", null], + reachability: { + attacker: false, + preconditions: [], + evidence_refs: ["missing"], + }, + }, + }, + ], + }; + const original = structuredClone(input); + const recovered = draftApi.parsePersistedScanDraft(input).findings[0]; + + assert.deepEqual(recovered.codeEvidence, [evidence]); + assert.deepEqual(recovered.code_evidence, [legacyEvidence]); + assert.equal(recovered.rootCause, undefined); + assert.equal(recovered.root_cause, undefined); + assert.deepEqual(recovered.validation, { + evidence: ["single proof"], + evidenceRefs: ["canonical-source", "legacy-source"], + }); + assert.deepEqual(recovered.attackPath, { + steps: ["Check the entrypoint"], + reachability: { evidence_refs: [] }, + }); + assert.deepEqual(input, original); +}); + for (const observation of ["checkpoint head", "worker result"]) { test(`worker: reopening survives replacement of the ${observation} during a read`, async (t) => { const f = await fixture(t, "worker"); diff --git a/sdk/typescript/scripts/check-package.mjs b/sdk/typescript/scripts/check-package.mjs index 6d2d01090..d152c8675 100644 --- a/sdk/typescript/scripts/check-package.mjs +++ b/sdk/typescript/scripts/check-package.mjs @@ -213,6 +213,7 @@ const allowedFiles = new Set([ "deduplication/refusal", "deduplication/retry", "deduplication/deduplication", + "finding-normalization", "finding-retrieval", "finding-workflow", "findings-client", diff --git a/sdk/typescript/scripts/generate-models.cjs b/sdk/typescript/scripts/generate-models.cjs index bcb23faf7..0411b4ff1 100644 --- a/sdk/typescript/scripts/generate-models.cjs +++ b/sdk/typescript/scripts/generate-models.cjs @@ -88,15 +88,26 @@ async function generate() { } generate().then((models) => { - const output = join(packageRoot, "src", "models.ts"); - if (process.argv.includes("--check")) { - if (readFileSync(output, "utf8").replaceAll("\r\n", "\n") !== models) { - console.error( - "src/models.ts is out of date. Run `pnpm generate:models`.", - ); - process.exitCode = 1; - } - return; + const shared = ["finding-normalization"].map((name) => [ + `${name}.ts`, + `/* Generated from plugin ${name}.ts. Run \`pnpm generate:models\`. */\n` + + readFileSync( + join( + packageRoot, + `../../plugins/codex-security/mcp-app/src/${name}.ts`, + ), + "utf8", + ), + ]); + for (const [name, content] of [["models.ts", models], ...shared]) { + const output = join(packageRoot, "src", name); + if (process.argv.includes("--check")) { + if (readFileSync(output, "utf8").replaceAll("\r\n", "\n") !== content) { + console.error( + `src/${name} is out of date. Run \`pnpm generate:models\`.`, + ); + process.exitCode = 1; + } + } else writeFileSync(output, content); } - writeFileSync(output, models); }); diff --git a/sdk/typescript/src/contract.ts b/sdk/typescript/src/contract.ts index 3c40f830d..448d07c18 100644 --- a/sdk/typescript/src/contract.ts +++ b/sdk/typescript/src/contract.ts @@ -1,6 +1,6 @@ import { isDeepStrictEqual as sameArray } from "node:util"; import { createHash, hash } from "node:crypto"; -import { isNonEmptyString } from "./value.js"; +import { normalizePersistedFindingDetails } from "./finding-normalization.js"; import { constants, type BigIntStats, type Stats } from "node:fs"; import { lstat, @@ -15,7 +15,6 @@ import { regexes } from "zod"; import { ContractValidationError, abortReason } from "./errors.js"; import { isRecord } from "./record.js"; import type { - ContractObject as JsonRecord, CoverageDocument, FindingsDocument, ScanManifest, @@ -192,146 +191,11 @@ export function normalizePersistedFindings(payload: unknown): unknown { } for (const finding of compatible["findings"]) { if (!isRecord(finding)) continue; - const legacyEvidence = finding["code_evidence"]; - if (Array.isArray(legacyEvidence)) { - finding["code_evidence"] = legacyEvidence.filter( - (evidence) => - isRecord(evidence) && - isNonEmptyString(evidence["id"]) && - isNonEmptyString(evidence["code"]), - ); - } else if ("code_evidence" in finding && legacyEvidence !== null) { - delete finding["code_evidence"]; - } - - for (const [sectionName, listFields] of [ - ["rootCause", ["evidenceRefs", "evidence_refs"]], - ["root_cause", ["evidenceRefs", "evidence_refs"]], - [ - "validation", - [ - "assertions", - "counterEvidence", - "evidenceRefs", - "evidence_refs", - "limitations", - ], - ], - [ - "attackPath", - [ - "assumptions", - "blindspots", - "controls", - "evidenceRefs", - "evidence_refs", - "limitations", - "preconditions", - "steps", - ], - ], - ] satisfies Array<[string, string[]]>) { - const section = finding[sectionName]; - if (!isRecord(section)) continue; - normalizeLegacyStringLists(section, listFields); - } - - const legacyRootCause = finding["root_cause"]; - if (isRecord(legacyRootCause)) { - removeUnsupportedLegacyStrings(legacyRootCause, [ - "summary", - "code", - "language", - ]); - } else if ( - "root_cause" in finding && - legacyRootCause !== null && - typeof legacyRootCause !== "string" - ) { - delete finding["root_cause"]; - } - - const validation = finding["validation"]; - if (isRecord(validation)) { - if (!isNonEmptyString(validation["evidence"])) { - normalizeLegacyStringLists(validation, ["evidence"]); - } - removeUnsupportedLegacyStrings(validation, ["method", "summary"]); - removeUnsupportedLegacyStrings( - validation, - ["status", "disposition", "result"].filter( - (field) => validation[field] !== null, - ), - ); - } - - const attackPath = finding["attackPath"]; - if (!isRecord(attackPath)) continue; - removeUnsupportedLegacyStrings(attackPath, ["summary"]); - for (const field of ["dataFlow", "data_flow", "dataflow", "reachability"]) { - const detail = attackPath[field]; - if (!isRecord(detail)) { - if (!isNonEmptyString(detail)) delete attackPath[field]; - continue; - } - removeUnsupportedLegacyStrings(detail, [ - "summary", - "source", - "sink", - "outcome", - ...(field === "reachability" ? ["attacker", "entrypoint"] : []), - ]); - normalizeLegacyStringLists(detail, [ - "evidenceRefs", - "evidence_refs", - "transformations", - ...(field === "reachability" ? ["preconditions"] : []), - ]); - } - for (const field of ["impact", "likelihood"]) { - const detail = attackPath[field]; - if (isRecord(detail)) { - removeUnsupportedLegacyStrings(detail, ["level", "rationale", "why"]); - } else if ( - detail !== undefined && - detail !== null && - !isNonEmptyString(detail) - ) { - delete attackPath[field]; - } - } + normalizePersistedFindingDetails(finding, "canonical"); } return compatible; } -function normalizeLegacyStringLists( - section: JsonRecord, - fields: string[], -): void { - for (const field of fields) { - if (!(field in section)) continue; - const value = section[field]; - if (Array.isArray(value)) { - section[field] = value.filter(isNonEmptyString); - } else if (isNonEmptyString(value)) { - section[field] = [value]; - } else { - delete section[field]; - } - } -} - -function removeUnsupportedLegacyStrings( - section: JsonRecord, - fields: string[], -): void { - for (const field of fields) { - if (field in section && !isNonEmptyString(section[field])) { - delete section[field]; - } - } -} - function validateCanonicalContract( manifest: ScanManifest, findings: FindingsDocument, diff --git a/sdk/typescript/src/finding-normalization.ts b/sdk/typescript/src/finding-normalization.ts new file mode 100644 index 000000000..6663124ea --- /dev/null +++ b/sdk/typescript/src/finding-normalization.ts @@ -0,0 +1,194 @@ +/* Generated from plugin finding-normalization.ts. Run `pnpm generate:models`. */ +import { isRecord as isObject } from "./record.js"; + +type JsonObject = Record; + +export function normalizePersistedFindingDetails( + finding: JsonObject, + profile: "draft" | "canonical", +): void { + const canonicalEvidence = + profile === "draft" && Array.isArray(finding["codeEvidence"]) + ? finding["codeEvidence"] + : []; + const evidenceIds = new Set( + canonicalEvidence.flatMap((evidence) => { + if (!isObject(evidence)) return []; + const id = evidence["id"]; + return isNonEmptyString(id) ? [id] : []; + }), + ); + if (Array.isArray(finding["code_evidence"])) { + const compatibleEvidence: JsonObject[] = []; + for (const evidence of finding["code_evidence"]) { + if (!isObject(evidence)) continue; + const id = evidence["id"]; + const code = evidence["code"]; + if ( + !isNonEmptyString(id) || + !isNonEmptyString(code) || + (profile === "draft" && evidenceIds.has(id)) + ) { + continue; + } + if (profile === "draft") evidenceIds.add(id); + compatibleEvidence.push(evidence); + } + finding["code_evidence"] = compatibleEvidence; + } else if (profile === "draft" || finding["code_evidence"] !== null) { + delete finding["code_evidence"]; + } + + for (const [sectionName, listFields] of [ + ["rootCause", ["evidenceRefs", "evidence_refs"]], + ["root_cause", ["evidenceRefs", "evidence_refs"]], + [ + "validation", + [ + "assertions", + "counterEvidence", + "evidence", + "evidenceRefs", + "evidence_refs", + "limitations", + ], + ], + [ + "attackPath", + [ + "assumptions", + "blindspots", + "controls", + "evidenceRefs", + "evidence_refs", + "limitations", + "preconditions", + "steps", + ], + ], + ] satisfies Array<[string, string[]]>) { + const section = finding[sectionName]; + if (!isObject(section)) continue; + normalizePersistedStringLists(section, listFields); + } + + const rootCause = finding["rootCause"]; + if (profile === "draft" && isObject(rootCause)) { + if (!isNonEmptyString(rootCause["summary"])) { + delete finding["rootCause"]; + } else { + removeUnsupportedPersistedStrings(rootCause, ["code", "language"]); + } + } + const legacyRootCause = finding["root_cause"]; + if (isObject(legacyRootCause)) { + removeUnsupportedPersistedStrings(legacyRootCause, [ + "summary", + "code", + "language", + ]); + } else if ( + profile === "draft" + ? !isNonEmptyString(legacyRootCause) + : legacyRootCause !== null && typeof legacyRootCause !== "string" + ) { + delete finding["root_cause"]; + } + + const validation = finding["validation"]; + if (isObject(validation)) { + removeUnsupportedPersistedStrings( + validation, + ["method", "status", "summary", "disposition", "result"].filter( + (field) => + profile === "draft" || + !["status", "disposition", "result"].includes(field) || + validation[field] !== null, + ), + ); + } + + const attackPath = finding["attackPath"]; + if (!isObject(attackPath)) return; + removeUnsupportedPersistedStrings(attackPath, ["summary"]); + for (const field of ["dataFlow", "data_flow", "dataflow", "reachability"]) { + const detail = attackPath[field]; + if (!isObject(detail)) { + if (!isNonEmptyString(detail)) delete attackPath[field]; + continue; + } + removeUnsupportedPersistedStrings(detail, [ + "summary", + "source", + "sink", + "outcome", + ...(field === "reachability" ? ["attacker", "entrypoint"] : []), + ]); + normalizePersistedStringLists(detail, [ + "evidenceRefs", + "evidence_refs", + "transformations", + ...(field === "reachability" ? ["preconditions"] : []), + ]); + } + for (const field of ["impact", "likelihood"]) { + const detail = attackPath[field]; + if (isObject(detail)) { + removeUnsupportedPersistedStrings(detail, ["level", "rationale", "why"]); + } else if ( + detail !== undefined && + detail !== null && + !isNonEmptyString(detail) + ) { + delete attackPath[field]; + } + } + + function normalizePersistedStringLists( + section: JsonObject, + fields: string[], + ): void { + for (const field of fields) { + const value = section[field]; + if ( + profile === "canonical" && + field === "evidence" && + isNonEmptyString(value) + ) + continue; + const normalized = isNonEmptyString(value) + ? [value] + : Array.isArray(value) + ? value.filter(isNonEmptyString) + : undefined; + if ( + normalized !== undefined && + (profile === "canonical" || normalized.length > 0) + ) + section[field] = normalized; + else delete section[field]; + } + if (profile === "canonical") return; + for (const field of ["evidenceRefs", "evidence_refs"]) { + const refs = section[field]; + if (!Array.isArray(refs)) continue; + section[field] = refs.filter((ref) => evidenceIds.has(ref)); + } + } + + function removeUnsupportedPersistedStrings( + section: JsonObject, + fields: string[], + ): void { + for (const field of fields) { + if (!isNonEmptyString(section[field])) delete section[field]; + } + } + + function isNonEmptyString(value: unknown): value is string { + return ( + typeof value === "string" && + (profile === "canonical" ? value.length > 0 : value.trim().length > 0) + ); + } +} From 3432ed4cdc6f6a604b3d460f1e65b2ff155ae735 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sat, 10 Oct 2026 23:59:19 +0000 Subject: [PATCH 2/8] refactor: share workbench argument encoding --- CONTRIBUTING.md | 5 +- plugins/codex-security/mcp-app/server.ts | 55 +++---- .../mcp-app/src/deep-scan/store.ts | 46 +++--- .../mcp-app/src/server/handoff-tools.ts | 17 +-- .../mcp-app/src/workbench-arguments.ts | 20 +++ sdk/typescript/scripts/check-package.mjs | 1 + sdk/typescript/scripts/generate-models.cjs | 22 +-- sdk/typescript/src/api.ts | 88 +++++------ sdk/typescript/src/artifact-export.ts | 19 ++- sdk/typescript/src/cli.ts | 141 +++++++----------- sdk/typescript/src/import-scan.ts | 25 ++-- sdk/typescript/src/saved-scan.ts | 22 ++- sdk/typescript/src/scan-comparison.ts | 14 +- sdk/typescript/src/security-policy.ts | 13 +- sdk/typescript/src/workbench-arguments.ts | 21 +++ .../tests-ts/workbench-arguments.test.ts | 35 +++++ 16 files changed, 273 insertions(+), 271 deletions(-) create mode 100644 plugins/codex-security/mcp-app/src/workbench-arguments.ts create mode 100644 sdk/typescript/src/workbench-arguments.ts create mode 100644 sdk/typescript/tests-ts/workbench-arguments.test.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 50ac46466..7cfa72afe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,8 +16,9 @@ packages and releases. Do not edit or commit files in that directory. See the commands. `pnpm --dir sdk/typescript run generate:models` also copies shared finding -normalization from the plugin source into the SDK. Edit the plugin helper, then -regenerate its SDK copy. `pnpm --dir sdk/typescript run types` checks freshness. +normalization and workbench argument encoding from the plugin source into the +SDK. Edit the plugin helper, then regenerate its SDK copy. +`pnpm --dir sdk/typescript run types` checks freshness. Model-based evaluations live in [`evals/`](evals/README.md), outside the plugin source. Deterministic triage checks and the MCP reducer IPC regression remain diff --git a/plugins/codex-security/mcp-app/server.ts b/plugins/codex-security/mcp-app/server.ts index c3b80b6ea..9684e239e 100644 --- a/plugins/codex-security/mcp-app/server.ts +++ b/plugins/codex-security/mcp-app/server.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "./src/workbench-arguments.js"; import { readingAnnotations, writingAnnotations, @@ -845,11 +846,9 @@ export function createCodexSecurityServer(): McpServer { _meta: appMeta, }, async ({ targetPath }) => { - const target = await runWorkbench([ - "inspect-target", - "--target-path", - targetPath, - ]); + const target = await runWorkbench( + workbenchArgs("inspect-target", { targetPath }), + ); return scanActionResult( { target }, "Validated the local Codex Security target.", @@ -1085,7 +1084,7 @@ export function createCodexSecurityServer(): McpServer { }, ); if (workspace === undefined && canceledLocally) { - workspace = (await runWorkbench(["get-scan", "--scan-id", scanId]))[ + workspace = (await runWorkbench(workbenchArgs("get-scan", { scanId })))[ "workspace" ] as JsonObject; } @@ -1146,7 +1145,7 @@ export function createCodexSecurityServer(): McpServer { }, async ({ scanId }) => scanActionResult( - await runWorkbench(["recover-scan-results", "--scan-id", scanId]), + await runWorkbench(workbenchArgs("recover-scan-results", { scanId })), "Recovered retained Codex Security scan results.", ), ); @@ -1389,7 +1388,7 @@ export function createCodexSecurityServer(): McpServer { _meta: appMeta, }, async ({ scanId, userContext }) => { - const current = await runWorkbench(["get-scan", "--scan-id", scanId]); + const current = await runWorkbench(workbenchArgs("get-scan", { scanId })); const workspace = isJsonObject(current.workspace) ? current.workspace : undefined; @@ -1433,7 +1432,7 @@ export function createCodexSecurityServer(): McpServer { extra, ) => { const modelSettings = codexModelSettingsFromExtra(extra); - const current = await runWorkbench(["get-scan", "--scan-id", scanId]); + const current = await runWorkbench(workbenchArgs("get-scan", { scanId })); const scan = isJsonObject(current.scan) ? current.scan : undefined; const progress = scan && isJsonObject(scan.progress) ? scan.progress : undefined; @@ -1611,15 +1610,13 @@ export function createCodexSecurityServer(): McpServer { }, async ({ occurrenceId, requestId, actionToken }) => scanActionResult( - await runWorkbench([ - "request-finding-remediation", - "--occurrence-id", - occurrenceId, - "--request-id", - requestId, - "--action-token", - actionToken, - ]), + await runWorkbench( + workbenchArgs("request-finding-remediation", { + occurrenceId, + requestId, + actionToken, + }), + ), "Queued the local Codex Security finding remediation request.", ), ); @@ -1636,19 +1633,15 @@ export function createCodexSecurityServer(): McpServer { }, async ({ occurrenceId, requestId, expectedVersion, action, actionToken }) => scanActionResult( - await runWorkbench([ - "request-finding-remediation-action", - "--occurrence-id", - occurrenceId, - "--request-id", - requestId, - "--expected-version", - String(expectedVersion), - "--action", - action, - "--action-token", - actionToken, - ]), + await runWorkbench( + workbenchArgs("request-finding-remediation-action", { + occurrenceId, + requestId, + expectedVersion: String(expectedVersion), + action, + actionToken, + }), + ), `Queued the local Codex Security finding remediation ${action} request.`, ), ); diff --git a/plugins/codex-security/mcp-app/src/deep-scan/store.ts b/plugins/codex-security/mcp-app/src/deep-scan/store.ts index f39eb28bc..891b2e37f 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/store.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/store.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "../workbench-arguments.js"; import { availableParallelism } from "node:os"; import { join } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; @@ -135,28 +136,19 @@ export class WorkbenchDeepScanStore { }): Promise { const userContext = input.userContext; const result = await this.enqueueWrite( - [ - "begin-deep-scan", - "--thread-id", - input.threadId, - ...(input.scanId ? ["--scan-id", input.scanId] : []), - ...(input.targetPath ? ["--target-path", input.targetPath] : []), - ...(input.scope ? ["--scope", input.scope] : []), - ...(userContext ? ["--user-context-stdin"] : []), - ...(input.handoffClaimToken - ? ["--claim-token", input.handoffClaimToken] - : []), - ...(input.model ? ["--model", input.model] : []), - ...(input.reasoningEffort - ? ["--reasoning-effort", input.reasoningEffort] - : []), - "--scan-root", - input.scanRoot, - "--available-parallelism", - String(availableParallelism()), - "--workflow-version", - WORKFLOW_VERSION, - ], + workbenchArgs("begin-deep-scan", { + threadId: input.threadId, + scanId: input.scanId || undefined, + targetPath: input.targetPath || undefined, + scope: input.scope || undefined, + userContextStdin: Boolean(userContext), + claimToken: input.handoffClaimToken || undefined, + model: input.model || undefined, + reasoningEffort: input.reasoningEffort || undefined, + scanRoot: input.scanRoot, + availableParallelism: String(availableParallelism()), + workflowVersion: WORKFLOW_VERSION, + }), false, userContext, ); @@ -172,13 +164,9 @@ export class WorkbenchDeepScanStore { async get(scanId: string, threadId: string): Promise { const run = parseDeepScan( - await this.runWorkbench([ - "get-deep-scan", - "--scan-id", - scanId, - "--thread-id", - threadId, - ]), + await this.runWorkbench( + workbenchArgs("get-deep-scan", { scanId, threadId }), + ), ); const generation = this.coordinatorLeases.get(scanId)?.run.coordinatorGeneration; diff --git a/plugins/codex-security/mcp-app/src/server/handoff-tools.ts b/plugins/codex-security/mcp-app/src/server/handoff-tools.ts index 1e41ca01c..f5d6f945d 100644 --- a/plugins/codex-security/mcp-app/src/server/handoff-tools.ts +++ b/plugins/codex-security/mcp-app/src/server/handoff-tools.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "../workbench-arguments.js"; import { writingAnnotations } from "./tool-annotations.js"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -86,16 +87,12 @@ export function registerScanHandoffTools( _meta: appMeta, }, async (input: z.infer) => { - const arguments_ = [ - command, - "--scan-id", - input.scanId, - "--claim-token", - input.claimToken, - ]; - if ("takeOverStale" in input && input.takeOverStale) - arguments_.push("--take-over-stale"); - if ("threadId" in input) arguments_.push("--thread-id", input.threadId); + const arguments_ = workbenchArgs(command, { + scanId: input.scanId, + claimToken: input.claimToken, + takeOverStale: "takeOverStale" in input && input.takeOverStale, + threadId: "threadId" in input ? input.threadId : undefined, + }); return workspaceResult( (await runWorkbench(arguments_)) as HandoffWorkspaceState, ); diff --git a/plugins/codex-security/mcp-app/src/workbench-arguments.ts b/plugins/codex-security/mcp-app/src/workbench-arguments.ts new file mode 100644 index 000000000..d4e14276c --- /dev/null +++ b/plugins/codex-security/mcp-app/src/workbench-arguments.ts @@ -0,0 +1,20 @@ +export type WorkbenchFields = Record< + string, + string | number | boolean | readonly string[] | undefined +>; + +export function workbenchArgs( + command: string, + fields: WorkbenchFields, +): string[] { + const args = [command]; + for (const [name, value] of Object.entries(fields)) { + if (value === undefined || value === false) continue; + const flag = `--${name.replace(/[A-Z]/g, (letter) => `-${letter.toLowerCase()}`)}`; + (Array.isArray(value) ? value : [value]).forEach((item) => { + args.push(flag); + if (item !== true) args.push(String(item)); + }); + } + return args; +} diff --git a/sdk/typescript/scripts/check-package.mjs b/sdk/typescript/scripts/check-package.mjs index d152c8675..953fc2188 100644 --- a/sdk/typescript/scripts/check-package.mjs +++ b/sdk/typescript/scripts/check-package.mjs @@ -214,6 +214,7 @@ const allowedFiles = new Set([ "deduplication/retry", "deduplication/deduplication", "finding-normalization", + "workbench-arguments", "finding-retrieval", "finding-workflow", "findings-client", diff --git a/sdk/typescript/scripts/generate-models.cjs b/sdk/typescript/scripts/generate-models.cjs index 0411b4ff1..60d04a8e1 100644 --- a/sdk/typescript/scripts/generate-models.cjs +++ b/sdk/typescript/scripts/generate-models.cjs @@ -88,17 +88,19 @@ async function generate() { } generate().then((models) => { - const shared = ["finding-normalization"].map((name) => [ - `${name}.ts`, - `/* Generated from plugin ${name}.ts. Run \`pnpm generate:models\`. */\n` + - readFileSync( - join( - packageRoot, - `../../plugins/codex-security/mcp-app/src/${name}.ts`, + const shared = ["finding-normalization", "workbench-arguments"].map( + (name) => [ + `${name}.ts`, + `/* Generated from plugin ${name}.ts. Run \`pnpm generate:models\`. */\n` + + readFileSync( + join( + packageRoot, + `../../plugins/codex-security/mcp-app/src/${name}.ts`, + ), + "utf8", ), - "utf8", - ), - ]); + ], + ); for (const [name, content] of [["models.ts", models], ...shared]) { const output = join(packageRoot, "src", name); if (process.argv.includes("--check")) { diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 801006670..73441c690 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -1,4 +1,5 @@ /// +import { workbenchArgs } from "./workbench-arguments.js"; import { isSafeNonNegativeInteger as safeInteger } from "./value.js"; import { @@ -1641,13 +1642,10 @@ export class CodexSecurity { } await workbench( { ...activeScan.options, signal: budgetSignal }, - [ - "set-scan-cost-limit", - "--scan-id", - activeScan.id, - "--max-cost-usd", - String(next), - ], + workbenchArgs("set-scan-cost-limit", { + scanId: activeScan.id, + maxCostUsd: String(next), + }), ); if (budgetSignal.aborted) return; maxCostUsd = next; @@ -1714,20 +1712,13 @@ export class CodexSecurity { ]) : await workbench( workbenchOptions, - [ - "register-cli-scan", - "--repository", - repo, - "--scan-dir", + workbenchArgs("register-cli-scan", { + repository: repo, scanDir, - "--registration-json-stdin", - ...(options.archiveExisting === true - ? ["--archive-existing"] - : []), - ...(options.parentScanId === undefined - ? [] - : ["--parent-scan-id", options.parentScanId]), - ], + registrationJsonStdin: true, + archiveExisting: options.archiveExisting === true, + parentScanId: options.parentScanId, + }), JSON.stringify({ recipe, userContext: options.scanPrompt, @@ -1878,7 +1869,7 @@ export class CodexSecurity { ...workbenchOptions, signal: AbortSignal.any([signal, progressSignal]), }, - ["get-scan", "--scan-id", scanId], + workbenchArgs("get-scan", { scanId }), ), onProgress: (progress) => notifyObserver(options, "onDeepProgress")(progress), @@ -2118,13 +2109,10 @@ export class CodexSecurity { if (budgetRecovery !== null) budgetRecovery.threadId = threadId; tracker.start(threadId); try { - await workbench(workbenchOptions, [ - "set-scan-thread", - "--scan-id", - scanId, - "--thread-id", - threadId, - ]); + await workbench( + workbenchOptions, + workbenchArgs("set-scan-thread", { scanId, threadId }), + ); } catch (error) { notifyObserver( options, @@ -2206,17 +2194,15 @@ export class CodexSecurity { completionCost = snapshot.cost; let preparation: JsonObject; try { - preparation = await workbench(workbenchOptions, [ - "prepare-scan-completion", - "--scan-id", - scanId, - ]); + preparation = await workbench( + workbenchOptions, + workbenchArgs("prepare-scan-completion", { scanId }), + ); } catch (error) { - const saved = await workbench(workbenchOptions, [ - "get-scan", - "--scan-id", - scanId, - ]).catch(() => null); + const saved = await workbench( + workbenchOptions, + workbenchArgs("get-scan", { scanId }), + ).catch(() => null); const savedScan = isRecord(saved) ? saved["scan"] : undefined; const progress = isRecord(savedScan) ? savedScan["progress"] @@ -3381,18 +3367,13 @@ export class CodexSecurity { throwIfAborted(signal, scanDir); const registration = await workbench( workbenchOptions, - [ - "register-cli-scan", - "--repository", - local.repository, - "--scan-dir", + workbenchArgs("register-cli-scan", { + repository: local.repository, scanDir, - "--registration-json-stdin", - ...(options.archiveExisting ? ["--archive-existing"] : []), - ...(options.parentScanId === undefined - ? [] - : ["--parent-scan-id", options.parentScanId]), - ], + registrationJsonStdin: true, + archiveExisting: Boolean(options.archiveExisting), + parentScanId: options.parentScanId, + }), JSON.stringify({ recipe: { ...scanRecipe({ @@ -3460,11 +3441,10 @@ export class CodexSecurity { output_tokens: 0, }; const cost = estimateScanCost(model, usage); - await workbench(workbenchOptions, [ - "prepare-scan-completion", - "--scan-id", - scanId, - ]); + await workbench( + workbenchOptions, + workbenchArgs("prepare-scan-completion", { scanId }), + ); const completion = await workbench(workbenchOptions, [ "complete-scan", "--scan-id", diff --git a/sdk/typescript/src/artifact-export.ts b/sdk/typescript/src/artifact-export.ts index fb0b97919..3925b33ea 100644 --- a/sdk/typescript/src/artifact-export.ts +++ b/sdk/typescript/src/artifact-export.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "./workbench-arguments.js"; import { spawn } from "node:child_process"; import { lstat, realpath } from "node:fs/promises"; import { basename, dirname, join, relative, resolve } from "node:path"; @@ -309,16 +310,14 @@ export async function resolveSavedArtifactDirectory( ): Promise { // Reuse the workbench's recorded digest and scan binding checks without // requiring a writable saved-scan directory for stdout or external exports. - const context = await workbench([ - "export-findings", - "--scan-id", - scanId, - "--artifact", - artifact, - "--format", - format, - "--validate-only", - ]); + const context = await workbench( + workbenchArgs("export-findings", { + scanId, + artifact, + format, + validateOnly: true, + }), + ); const scan = context["scan"] as JsonObject | undefined; if (typeof scan?.["scanDir"] !== "string") throw new CodexSecurityError( diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index f03cf43d1..89ffa19ae 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -1,4 +1,5 @@ #!/usr/bin/env node +import { workbenchArgs } from "./workbench-arguments.js"; import { listenForAbort } from "./cli-signals.js"; import { isNonEmptyString, parseJson } from "./value.js"; @@ -1821,14 +1822,11 @@ export async function main( pythonPath?: string, ): Promise => { const result = await history( - [ - "list-scans", - "--repository", - dependencies.currentDirectory(), - ...(status === "complete" ? ["--status", "complete"] : []), - "--limit", - String(count), - ], + workbenchArgs("list-scans", { + repository: dependencies.currentDirectory(), + status: status === "complete" ? "complete" : undefined, + limit: String(count), + }), (value) => { if ((value["scans"] as SavedScan[]).length < count) { const kind = status === "complete" ? "completed" : "saved"; @@ -1900,14 +1898,11 @@ export async function main( runMatching(async (options) => { const { matchingCached, matchingInputs, ...comparison } = await dependencies.runWorkbench( - [ - "compare-scans", - "--before-scan-id", - beforeId, - "--after-scan-id", - afterId, - "--include-matching-inputs", - ], + workbenchArgs("compare-scans", { + beforeScanId: beforeId, + afterScanId: afterId, + includeMatchingInputs: true, + }), undefined, options.signal, ); @@ -1916,14 +1911,11 @@ export async function main( const matching = await dependencies.matchFindings(input, options); options.signal?.throwIfAborted(); return await dependencies.runWorkbench( - [ - "save-scan-comparison", - "--before-scan-id", - beforeId, - "--after-scan-id", - afterId, - "--matches-json-stdin", - ], + workbenchArgs("save-scan-comparison", { + beforeScanId: beforeId, + afterScanId: afterId, + matchesJsonStdin: true, + }), JSON.stringify(matching), options.signal, ); @@ -2110,11 +2102,12 @@ export async function main( ? undefined : resolveCliPath(directory, args.repository ?? directory); return presentHistory( - await history([ - "list-scans", - ...(repository === undefined ? [] : ["--repository", repository]), - ...(scanRoot === undefined ? [] : ["--scan-root", scanRoot]), - ]), + await history( + workbenchArgs("list-scans", { + repository, + scanRoot, + }), + ), "list", format, { @@ -2275,11 +2268,9 @@ export async function main( async run({ args, error: incurError, format, options }) { let scanArguments: ScanArguments; try { - const saved = await dependencies.runWorkbench([ - "get-cli-scan-resume", - "--scan-id", - args.scanId, - ]); + const saved = await dependencies.runWorkbench( + workbenchArgs("get-cli-scan-resume", { scanId: args.scanId }), + ); if ( typeof saved["scanId"] !== "string" || typeof saved["scanDir"] !== "string" @@ -2391,11 +2382,9 @@ export async function main( let scanArguments: ScanArguments; try { const { recipe, scanId: resolvedScanId } = - await dependencies.runWorkbench([ - "get-scan-recipe", - "--scan-id", - scanId, - ]); + await dependencies.runWorkbench( + workbenchArgs("get-scan-recipe", { scanId }), + ); const parentScanId = resolvedScanId as string; if ( isJsonObject(recipe) && @@ -2861,11 +2850,9 @@ export async function main( `Interactive scan selection requires a terminal. Select a saved scan: codex-security publish scan --scan SCAN_ID --to ${options.to}${options.to === "linear" ? " --linear-team TEAM_ID" : ""}.`, ); } - const saved = await scanDependencies.runWorkbench([ - "list-scans", - "--status", - "complete", - ]); + const saved = await scanDependencies.runWorkbench( + workbenchArgs("list-scans", { status: "complete" }), + ); const listedScans = saved["scans"]; if (!Array.isArray(listedScans)) { throw new CodexSecurityError( @@ -4629,7 +4616,7 @@ export async function main( ? { recoverScan: async (scanDir, prompts) => { const history = await dependencies.runWorkbench( - ["list-scans", "--scan-root", scanDir], + workbenchArgs("list-scans", { scanRoot: scanDir }), undefined, controller.signal, ); @@ -4655,12 +4642,10 @@ export async function main( if (status !== "running" || scan["mode"] !== "deep") return undefined; const saved = await dependencies.runWorkbench( - [ - "get-cli-scan-resume", - "--scan-id", - scan.scanId, - "--allow-unavailable", - ], + workbenchArgs("get-cli-scan-resume", { + scanId: scan.scanId, + allowUnavailable: true, + }), undefined, controller.signal, ); @@ -5694,11 +5679,11 @@ export async function main( async run({ args, options }) { let scanId = args.scanId; if (scanId === undefined) { - const value = await history([ - "list-scans", - "--repository", - dependencies.currentDirectory(), - ]); + const value = await history( + workbenchArgs("list-scans", { + repository: dependencies.currentDirectory(), + }), + ); const scans = value["scans"] as { scanId: string; startedAt: string; @@ -6526,12 +6511,10 @@ async function matchAllScans( options: ScanComparisonOptions = {}, ): Promise { const result = (await dependencies.runWorkbench( - [ - "list-unmatched-scan-pairs", - "--repository", - dependencies.currentDirectory(), - ...(force ? ["--force"] : []), - ], + workbenchArgs("list-unmatched-scan-pairs", { + repository: dependencies.currentDirectory(), + force: Boolean(force), + }), undefined, options.signal, )) as MatchingPlan; @@ -6574,14 +6557,11 @@ async function matchAllScans( for (const { scanId, comparison } of comparisons) { options.signal?.throwIfAborted(); await dependencies.runWorkbench( - [ - "save-scan-comparison", - "--before-scan-id", - scanId, - "--after-scan-id", + workbenchArgs("save-scan-comparison", { + beforeScanId: scanId, afterScanId, - "--matches-json-stdin", - ], + matchesJsonStdin: true, + }), JSON.stringify(comparison), options.signal, ); @@ -6663,13 +6643,9 @@ async function selectSavedFindings( let scanId = requestedScanId; if (scanId === "latest") { const repository = resolve(dependencies.currentDirectory()); - const history = await dependencies.runWorkbench([ - "list-scans", - "--repository", - repository, - "--status", - "complete", - ]); + const history = await dependencies.runWorkbench( + workbenchArgs("list-scans", { repository, status: "complete" }), + ); const latest = (history["scans"] as { scanId?: string }[] | undefined)?.[0] ?.scanId; if (typeof latest !== "string") { @@ -9304,13 +9280,12 @@ async function readDeepScanStop( nextStep: "To scan further, rerun with a higher --max-cost.", }; } - const response = await runWorkbench([ - "get-deep-scan", - "--scan-id", - result.manifest.scan.id, - "--thread-id", - result.threadId, - ]); + const response = await runWorkbench( + workbenchArgs("get-deep-scan", { + scanId: result.manifest.scan.id, + threadId: result.threadId, + }), + ); const state = response["deepScan"] as | { terminalReason: string; diff --git a/sdk/typescript/src/import-scan.ts b/sdk/typescript/src/import-scan.ts index 611fca277..b14125c04 100644 --- a/sdk/typescript/src/import-scan.ts +++ b/sdk/typescript/src/import-scan.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "./workbench-arguments.js"; import { createHash } from "node:crypto"; import { constants } from "node:fs"; import { @@ -203,18 +204,13 @@ export async function importScan( signal?.throwIfAborted(); const registration = await workbench( workbenchOptions, - [ - "register-cli-scan", - "--repository", + workbenchArgs("register-cli-scan", { repository, - "--scan-dir", scanDir, - "--registration-json-stdin", - ...(options.archiveExisting ? ["--archive-existing"] : []), - ...(options.parentScanId === undefined - ? [] - : ["--parent-scan-id", options.parentScanId]), - ], + registrationJsonStdin: true, + archiveExisting: Boolean(options.archiveExisting), + parentScanId: options.parentScanId, + }), JSON.stringify({ recipe: { repository, @@ -292,11 +288,10 @@ export async function importScan( { flag: "wx", signal }, ); } - await workbench(workbenchOptions, [ - "prepare-scan-completion", - "--scan-id", - scanId, - ]); + await workbench( + workbenchOptions, + workbenchArgs("prepare-scan-completion", { scanId }), + ); // Finalization normally recovers partial output. An import must retain every supplied row. const contract = await loadContract(scanDir, { pluginRoot, diff --git a/sdk/typescript/src/saved-scan.ts b/sdk/typescript/src/saved-scan.ts index 655b8d0cb..1253e342c 100644 --- a/sdk/typescript/src/saved-scan.ts +++ b/sdk/typescript/src/saved-scan.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "./workbench-arguments.js"; import { isNonEmptyString } from "./value.js"; import { lstat } from "node:fs/promises"; import { resolve } from "node:path"; @@ -44,13 +45,12 @@ export async function resolveCompletedScan( ): Promise { let scanId = requestedId; if (scanId === "latest") { - const history = await dependencies.runWorkbench([ - "list-scans", - "--repository", - resolve(dependencies.currentDirectory()), - "--status", - "complete", - ]); + const history = await dependencies.runWorkbench( + workbenchArgs("list-scans", { + repository: resolve(dependencies.currentDirectory()), + status: "complete", + }), + ); const scans = history["scans"]; const latest = Array.isArray(scans) ? scans[0] : undefined; if (!isJsonObject(latest) || typeof latest["scanId"] !== "string") { @@ -60,11 +60,9 @@ export async function resolveCompletedScan( } scanId = latest["scanId"]; } - const context = await dependencies.runWorkbench([ - "get-scan", - "--scan-id", - scanId, - ]); + const context = await dependencies.runWorkbench( + workbenchArgs("get-scan", { scanId }), + ); const scan = context["scan"]; if (!isJsonObject(scan) || typeof scan["scanId"] !== "string") { throw new CodexSecurityError(`Could not read saved scan ${scanId}.`); diff --git a/sdk/typescript/src/scan-comparison.ts b/sdk/typescript/src/scan-comparison.ts index 07813ada8..9b26b6c5f 100644 --- a/sdk/typescript/src/scan-comparison.ts +++ b/sdk/typescript/src/scan-comparison.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "./workbench-arguments.js"; import { existsSync } from "node:fs"; import { isRecord } from "./record.js"; import { dirname, join } from "node:path"; @@ -829,14 +830,11 @@ export async function matchCompletedScan( for (const { scanId, projected } of comparisons) { options.signal?.throwIfAborted(); await options.workbench( - [ - "save-scan-comparison", - "--before-scan-id", - scanId, - "--after-scan-id", - options.scanId, - "--matches-json-stdin", - ], + workbenchArgs("save-scan-comparison", { + beforeScanId: scanId, + afterScanId: options.scanId, + matchesJsonStdin: true, + }), JSON.stringify(projected), ); } diff --git a/sdk/typescript/src/security-policy.ts b/sdk/typescript/src/security-policy.ts index a40da768b..ac8c430cb 100644 --- a/sdk/typescript/src/security-policy.ts +++ b/sdk/typescript/src/security-policy.ts @@ -1,3 +1,4 @@ +import { workbenchArgs } from "./workbench-arguments.js"; import { execFile } from "node:child_process"; import { randomUUID } from "node:crypto"; import { sha256Text as digest } from "./contract.js"; @@ -693,13 +694,11 @@ export async function resolveSecurityPolicyGuidance( process.execPath, [ join(pluginRoot, "mcp", "helpers.mjs"), - "resolve-security-md", - "--repo", - target.repository, - "--scope", - dirname(target.targetPath), - "--out", - "-", + ...workbenchArgs("resolve-security-md", { + repo: target.repository, + scope: dirname(target.targetPath), + out: "-", + }), ], { encoding: "utf8", maxBuffer: Infinity, env: environment, signal }, ); diff --git a/sdk/typescript/src/workbench-arguments.ts b/sdk/typescript/src/workbench-arguments.ts new file mode 100644 index 000000000..914be7cb6 --- /dev/null +++ b/sdk/typescript/src/workbench-arguments.ts @@ -0,0 +1,21 @@ +/* Generated from plugin workbench-arguments.ts. Run `pnpm generate:models`. */ +export type WorkbenchFields = Record< + string, + string | number | boolean | readonly string[] | undefined +>; + +export function workbenchArgs( + command: string, + fields: WorkbenchFields, +): string[] { + const args = [command]; + for (const [name, value] of Object.entries(fields)) { + if (value === undefined || value === false) continue; + const flag = `--${name.replace(/[A-Z]/g, (letter) => `-${letter.toLowerCase()}`)}`; + (Array.isArray(value) ? value : [value]).forEach((item) => { + args.push(flag); + if (item !== true) args.push(String(item)); + }); + } + return args; +} diff --git a/sdk/typescript/tests-ts/workbench-arguments.test.ts b/sdk/typescript/tests-ts/workbench-arguments.test.ts new file mode 100644 index 000000000..ea4f31022 --- /dev/null +++ b/sdk/typescript/tests-ts/workbench-arguments.test.ts @@ -0,0 +1,35 @@ +import { expect, test } from "bun:test"; +import { workbenchArgs } from "../src/workbench-arguments.js"; + +test("workbench arguments preserve field order, values, and repeated flags", () => { + expect( + workbenchArgs("fixture-command", { + scanId: "scan-example", + registrationJsonStdin: true, + archiveExisting: false, + parentScanId: undefined, + offset: 0, + name: "", + query: "--query\nUTF-8 λ\0", + inputWorkerId: ["worker-2", "worker-1", "worker-2"], + omittedWorkerId: [], + }), + ).toEqual([ + "fixture-command", + "--scan-id", + "scan-example", + "--registration-json-stdin", + "--offset", + "0", + "--name", + "", + "--query", + "--query\nUTF-8 λ\0", + "--input-worker-id", + "worker-2", + "--input-worker-id", + "worker-1", + "--input-worker-id", + "worker-2", + ]); +}); From bb0ed9b0abd56e6e45a0c0af4616752a7831ddfe Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sun, 11 Oct 2026 00:14:30 +0000 Subject: [PATCH 3/8] refactor(deep-scan): derive persisted response types from schemas --- .../mcp-app/src/deep-scan/registry.ts | 8 +- .../mcp-app/src/deep-scan/response.ts | 157 ++++++ .../mcp-app/src/deep-scan/store.ts | 465 ++++-------------- .../mcp-app/src/deep-scan/types.ts | 93 +--- .../mcp-app/src/deep-scan/worker-runner.ts | 55 +-- .../mcp-app/tests/test_deep_scan_store.ts | 103 +++- sdk/typescript/src/deep-progress.ts | 38 +- sdk/typescript/src/worker-progress.ts | 92 ++-- 8 files changed, 420 insertions(+), 591 deletions(-) create mode 100644 plugins/codex-security/mcp-app/src/deep-scan/response.ts diff --git a/plugins/codex-security/mcp-app/src/deep-scan/registry.ts b/plugins/codex-security/mcp-app/src/deep-scan/registry.ts index 3b0d269d8..92c7564e2 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/registry.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/registry.ts @@ -185,11 +185,9 @@ export class DeepScanRemoteCoordinator { } } -export async function startOrJoinDeepScanCoordinator(input: { - run: DeepScanRunState; - registry: Pick; - options: Omit; -}): Promise<{ +export async function startOrJoinDeepScanCoordinator( + input: ConstructorParameters[0], +): Promise<{ coordinator: DeepScanCoordinator | DeepScanRemoteCoordinator; joined: boolean; }> { diff --git a/plugins/codex-security/mcp-app/src/deep-scan/response.ts b/plugins/codex-security/mcp-app/src/deep-scan/response.ts new file mode 100644 index 000000000..b40f565b1 --- /dev/null +++ b/plugins/codex-security/mcp-app/src/deep-scan/response.ts @@ -0,0 +1,157 @@ +import { isRecord } from "../record.js"; +import { z } from "zod"; + +const requiredString = z.string().min(1); +// The workbench accepts integer numbers outside JavaScript's safe integer range. +const nonNegativeInteger = z.number().refine(Number.isInteger).nonnegative(); +const positiveInteger = z.number().refine(Number.isInteger).positive(); +const optionalValue = (schema: T) => + schema.optional().catch(undefined); +// Keep absent optional fields as own properties, matching persisted run/worker records. +const responseObject = (shape: T) => + z.object(shape).transform((value) => { + for (const key of Object.keys(shape)) { + if (!(key in value)) Object.assign(value, { [key]: undefined }); + } + return value; + }); +const optionalString = optionalValue(requiredString); + +export const workerSchema = responseObject({ + id: requiredString, + kind: z.enum(["setup", "discovery", "dedup"]), + status: z.enum(["queued", "running", "succeeded", "failed", "canceled"]), + mergeState: z.enum(["none", "buffered", "merging", "merged"]), + promptPath: requiredString, + artifactDir: requiredString, + attempt: nonNegativeInteger, + sdkThreadId: optionalString, + resultManifestPath: optionalString, + completionSequence: optionalValue(positiveInteger), + error: optionalString, +}).transform( + ({ sdkThreadId, ...worker }): typeof worker & { threadId?: string } => ({ + ...worker, + threadId: sdkThreadId, + }), +); + +export const dedupInputSchema = z.object({ + dedupWorkerId: requiredString, + discoveryWorkerId: requiredString, + inputOrder: nonNegativeInteger, +}); + +export const configSchema = z + .object({ + stopAfterNoNew: positiveInteger, + workers: positiveInteger, + subagents: nonNegativeInteger, + stopAfterConsecutiveErrors: positiveInteger.nullish(), + maxDiscoveryRuns: positiveInteger, + maxTimeHours: z.number().positive().max(96).optional(), + }) + .transform(({ stopAfterConsecutiveErrors, maxTimeHours, ...config }) => ({ + ...config, + stopAfterConsecutiveErrors: + stopAfterConsecutiveErrors ?? config.stopAfterNoNew, + ...(maxTimeHours === undefined ? {} : { maxTimeHours }), + })); + +// Check the config object before status, then its fields before the run fields. +export const runSchema = z + .looseObject({ config: z.looseObject({}) }) + .pipe( + responseObject({ + status: z.enum( + ["running", "succeeded", "canceled", "failed", "interrupted"], + { + error: ({ input }) => + typeof input === "string" && input + ? `Unsupported Deep Scan status: ${input}` + : undefined, + }, + ), + config: configSchema, + scanId: requiredString, + coordinatorGeneration: optionalValue(positiveInteger), + createdAt: optionalString, + updatedAt: optionalString, + targetPath: requiredString, + scope: requiredString, + userContext: optionalString, + scanDir: requiredString, + dispatchedCount: nonNegativeInteger, + noNewStreak: nonNegativeInteger, + consecutiveErrors: nonNegativeInteger + .nullish() + .transform((value) => value ?? 0), + manifestPath: optionalString, + terminalReason: optionalValue(z.enum(["saturated", "capped"])), + error: optionalString, + workers: z + .array(workerSchema) + .nullish() + .transform((value) => value ?? []), + dedupInputs: z + .array(dedupInputSchema) + .nullish() + .transform((value) => value ?? []), + }), + ) + .transform(({ workers, dedupInputs, ...run }) => ({ + ...run, + persistedWorkers: workers, + persistedDedupInputs: dedupInputs, + })); + +function parseResponse( + schema: T, + value: unknown, + field = "deepScan", +): z.output { + const result = schema.safeParse(value); + if (result.success) return result.data; + const issue = result.error.issues[0]; + const path = [field, ...issue.path] + .join(".") + .replace(/workers\.\d+/g, "worker") + .replace(/dedupInputs\.\d+/g, "dedupInput"); + throw new Error( + issue.message.startsWith("Unsupported Deep Scan status:") + ? issue.message + : `Codex Security workbench ${issue.code === "invalid_type" && issue.expected === "object" ? "did not return" : "returned invalid"} ${path}.`, + ); +} + +export function parseDeepScan(result: Record) { + return parseResponse(runSchema, result.deepScan); +} + +export function parseWorker(result: Record, workerId: string) { + const deepScan = parseResponse(z.looseObject({}), result.deepScan); + if (!Array.isArray(deepScan.workers)) { + throw new Error( + "Codex Security workbench did not return deepScan.workers.", + ); + } + const value = deepScan.workers.find( + (candidate) => isRecord(candidate) && candidate.id === workerId, + ); + if (!value) { + throw new Error( + `Codex Security workbench did not return Deep Scan worker ${workerId}.`, + ); + } + const worker = parseResponse(workerSchema, value, "deepScan.worker"); + return deepScan.consecutiveErrors === undefined + ? worker + : { + ...worker, + consecutiveErrors: parseResponse( + nonNegativeInteger, + deepScan.consecutiveErrors, + "deepScan.consecutiveErrors", + ), + }; +} diff --git a/plugins/codex-security/mcp-app/src/deep-scan/store.ts b/plugins/codex-security/mcp-app/src/deep-scan/store.ts index 891b2e37f..1eeb9a3fa 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/store.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/store.ts @@ -3,6 +3,9 @@ import { availableParallelism } from "node:os"; import { join } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; import { writeJsonAtomic } from "./artifacts.js"; +import { parseDeepScan, parseWorker } from "./response.js"; + +export { parseDeepScan }; import { boundedDeepScanErrorMessage, DeepScanNonRetryableError, @@ -13,15 +16,9 @@ import type { DedupCommit, DeepScanCoordinatorClaim, DeepScanCoordinatorLeaseInput, - DeepScanConfig, - DeepScanMergeState, DeepScanRunState, - DeepScanRunStatus, DeepScanTerminalReason, - DeepScanWorkerKind, DeepScanWorkerMutation, - DeepScanWorkerStatus, - PersistedDeepScanDedupInput, PersistedDeepScanWorker, } from "./types.js"; @@ -111,6 +108,14 @@ class DeepScanPersistenceError extends Error { } } +const RETRYABLE_PERSISTENCE_COMMANDS = new Set([ + "upsert-deep-scan-worker", + "claim-deep-scan-dedup", + "commit-deep-scan-dedup", + "finish-deep-scan", + "record-deep-scan-publication-failure", +]); + export class WorkbenchDeepScanStore { private writeTail: Promise = Promise.resolve(); private readonly coordinatorLeases = new Map< @@ -149,7 +154,6 @@ export class WorkbenchDeepScanStore { availableParallelism: String(availableParallelism()), workflowVersion: WORKFLOW_VERSION, }), - false, userContext, ); const run = parseDeepScan(result); @@ -251,46 +255,33 @@ export class WorkbenchDeepScanStore { async updateWorker( update: DeepScanWorkerMutation, ): Promise { - const result = await this.enqueueWrite( - [ - "upsert-deep-scan-worker", - "--scan-id", - update.scanId, - "--worker-id", - update.id, - "--kind", - update.kind, - "--status", - update.status, - "--prompt-path", - update.promptPath, - "--artifact-dir", - update.artifactDir, - "--attempt", - String(update.attempt), - ...this.coordinatorLeaseArgs(update.scanId), - ...(update.resultManifestPath - ? ["--result-manifest-path", update.resultManifestPath] - : []), - ...(update.threadId ? ["--sdk-thread-id", update.threadId] : []), - ...(update.error ? [`--error-message=${update.error}`] : []), - ...(update.replaceableFailureKind - ? ["--replaceable-failure-kind", update.replaceableFailureKind] - : []), - ], - true, - ); - const worker = parseWorker(result, update.id); - const state = objectValue(result.deepScan, "deepScan"); - return state.consecutiveErrors === undefined - ? worker - : { - ...worker, - consecutiveErrors: nonNegativeInteger( - state.consecutiveErrors, - "deepScan.consecutiveErrors", - ), - }; + const result = await this.enqueueWrite([ + "upsert-deep-scan-worker", + "--scan-id", + update.scanId, + "--worker-id", + update.id, + "--kind", + update.kind, + "--status", + update.status, + "--prompt-path", + update.promptPath, + "--artifact-dir", + update.artifactDir, + "--attempt", + String(update.attempt), + ...this.coordinatorLeaseArgs(update.scanId), + ...(update.resultManifestPath + ? ["--result-manifest-path", update.resultManifestPath] + : []), + ...(update.threadId ? ["--sdk-thread-id", update.threadId] : []), + ...(update.error ? [`--error-message=${update.error}`] : []), + ...(update.replaceableFailureKind + ? ["--replaceable-failure-kind", update.replaceableFailureKind] + : []), + ]); + return parseWorker(result, update.id); } async claimDedup(input: { @@ -300,47 +291,38 @@ export class WorkbenchDeepScanStore { promptPath: string; artifactDir: string; }): Promise { - await this.enqueueWrite( - [ - "claim-deep-scan-dedup", - "--scan-id", - input.scanId, - "--worker-id", - input.id, - "--prompt-path", - input.promptPath, - "--artifact-dir", - input.artifactDir, - ...this.coordinatorLeaseArgs(input.scanId), - ...input.workerIds.flatMap((workerId) => [ - "--input-worker-id", - workerId, - ]), - ], - true, - ); + await this.enqueueWrite([ + "claim-deep-scan-dedup", + "--scan-id", + input.scanId, + "--worker-id", + input.id, + "--prompt-path", + input.promptPath, + "--artifact-dir", + input.artifactDir, + ...this.coordinatorLeaseArgs(input.scanId), + ...input.workerIds.flatMap((workerId) => ["--input-worker-id", workerId]), + ]); } async commitDedup(commit: DedupCommit): Promise { return parseDeepScan( - await this.enqueueWrite( - [ - "commit-deep-scan-dedup", - "--scan-id", - commit.scanId, - "--worker-id", - commit.id, - ...this.coordinatorLeaseArgs(commit.scanId), - "--result-manifest-path", - commit.resultManifestPath, - ...(commit.candidateLedgerPath - ? ["--candidate-ledger-path", commit.candidateLedgerPath] - : []), - "--new-findings-count", - String(commit.newFindings), - ], - true, - ), + await this.enqueueWrite([ + "commit-deep-scan-dedup", + "--scan-id", + commit.scanId, + "--worker-id", + commit.id, + ...this.coordinatorLeaseArgs(commit.scanId), + "--result-manifest-path", + commit.resultManifestPath, + ...(commit.candidateLedgerPath + ? ["--candidate-ledger-path", commit.candidateLedgerPath] + : []), + "--new-findings-count", + String(commit.newFindings), + ]), ); } @@ -352,26 +334,23 @@ export class WorkbenchDeepScanStore { omittedWorkerIds: string[]; }): Promise { return parseDeepScan( - await this.enqueueWrite( - [ - "finish-deep-scan", - "--scan-id", - input.scanId, - ...this.coordinatorLeaseArgs(input.scanId), - "--terminal-reason", - input.reason, - "--manifest-path", - input.manifestPath, - ...(input.stagedManifestPath - ? ["--staged-manifest-path", input.stagedManifestPath] - : []), - ...input.omittedWorkerIds.flatMap((workerId) => [ - "--omitted-worker-id", - workerId, - ]), - ], - true, - ), + await this.enqueueWrite([ + "finish-deep-scan", + "--scan-id", + input.scanId, + ...this.coordinatorLeaseArgs(input.scanId), + "--terminal-reason", + input.reason, + "--manifest-path", + input.manifestPath, + ...(input.stagedManifestPath + ? ["--staged-manifest-path", input.stagedManifestPath] + : []), + ...input.omittedWorkerIds.flatMap((workerId) => [ + "--omitted-worker-id", + workerId, + ]), + ]), ); } @@ -404,18 +383,15 @@ export class WorkbenchDeepScanStore { coordinatorGeneration?: number, ): Promise { return parseDeepScan( - await this.enqueueWrite( - [ - "record-deep-scan-publication-failure", - "--scan-id", - scanId, - ...(coordinatorGeneration === undefined - ? this.coordinatorLeaseArgs(scanId) - : ["--coordinator-generation", String(coordinatorGeneration)]), - `--message=${message}`, - ], - true, - ), + await this.enqueueWrite([ + "record-deep-scan-publication-failure", + "--scan-id", + scanId, + ...(coordinatorGeneration === undefined + ? this.coordinatorLeaseArgs(scanId) + : ["--coordinator-generation", String(coordinatorGeneration)]), + `--message=${message}`, + ]), ); } @@ -465,14 +441,10 @@ export class WorkbenchDeepScanStore { : ["--coordinator-generation", String(generation)]; } - private enqueueWrite( - args: string[], - retryTransientFailure = false, - input?: string, - ): Promise { + private enqueueWrite(args: string[], input?: string): Promise { const operation = this.writeTail.then(async () => { try { - return retryTransientFailure + return RETRYABLE_PERSISTENCE_COMMANDS.has(args[0]) ? await this.runIdempotentPersistence(args) : await this.runWorkbench(args, input); } catch (error) { @@ -639,252 +611,3 @@ function argumentValue(args: string[], flag: string): string | undefined { const index = args.indexOf(flag); return index < 0 ? undefined : args[index + 1]; } - -export function parseDeepScan(result: JsonObject): DeepScanRunState { - const value = objectValue(result.deepScan, "deepScan"); - const configValue = objectValue(value.config, "deepScan.config"); - const status = requiredString( - value.status, - "deepScan.status", - ) as DeepScanRunStatus; - if ( - !["running", "succeeded", "canceled", "failed", "interrupted"].includes( - status, - ) - ) { - throw new Error(`Unsupported Deep Scan status: ${status}`); - } - const stopAfterNoNew = positiveInteger( - configValue.stopAfterNoNew, - "deepScan.config.stopAfterNoNew", - ); - const config: DeepScanConfig = { - workers: positiveInteger(configValue.workers, "deepScan.config.workers"), - subagents: nonNegativeInteger( - configValue.subagents, - "deepScan.config.subagents", - ), - stopAfterNoNew, - stopAfterConsecutiveErrors: positiveInteger( - configValue.stopAfterConsecutiveErrors ?? stopAfterNoNew, - "deepScan.config.stopAfterConsecutiveErrors", - ), - maxDiscoveryRuns: positiveInteger( - configValue.maxDiscoveryRuns, - "deepScan.config.maxDiscoveryRuns", - ), - ...(configValue.maxTimeHours === undefined - ? {} - : { - maxTimeHours: boundedPositiveNumber( - configValue.maxTimeHours, - "deepScan.config.maxTimeHours", - 96, - ), - }), - }; - return { - scanId: requiredString(value.scanId, "deepScan.scanId"), - status, - coordinatorGeneration: optionalPositiveInteger(value.coordinatorGeneration), - createdAt: optionalString(value.createdAt), - updatedAt: optionalString(value.updatedAt), - targetPath: requiredString(value.targetPath, "deepScan.targetPath"), - scope: requiredString(value.scope, "deepScan.scope"), - userContext: optionalString(value.userContext), - scanDir: requiredString(value.scanDir, "deepScan.scanDir"), - config, - dispatchedCount: nonNegativeInteger( - value.dispatchedCount, - "deepScan.dispatchedCount", - ), - noNewStreak: nonNegativeInteger(value.noNewStreak, "deepScan.noNewStreak"), - consecutiveErrors: nonNegativeInteger( - value.consecutiveErrors ?? 0, - "deepScan.consecutiveErrors", - ), - manifestPath: optionalString(value.manifestPath), - terminalReason: - value.terminalReason === "saturated" || value.terminalReason === "capped" - ? value.terminalReason - : undefined, - error: optionalString(value.error), - persistedWorkers: parsePersistedWorkers(value.workers), - persistedDedupInputs: parsePersistedDedupInputs(value.dedupInputs), - }; -} - -function parsePersistedDedupInputs( - value: unknown, -): PersistedDeepScanDedupInput[] { - if (value === undefined || value === null) return []; - if (!Array.isArray(value)) { - throw new Error( - "Codex Security workbench returned invalid deepScan.dedupInputs.", - ); - } - return value.map((candidate) => { - const input = objectValue(candidate, "deepScan.dedupInput"); - return { - dedupWorkerId: requiredString( - input.dedupWorkerId, - "deepScan.dedupInput.dedupWorkerId", - ), - discoveryWorkerId: requiredString( - input.discoveryWorkerId, - "deepScan.dedupInput.discoveryWorkerId", - ), - inputOrder: nonNegativeInteger( - input.inputOrder, - "deepScan.dedupInput.inputOrder", - ), - }; - }); -} - -function parsePersistedWorkers(value: unknown): PersistedDeepScanWorker[] { - if (value === undefined || value === null) return []; - if (!Array.isArray(value)) { - throw new Error( - "Codex Security workbench returned invalid deepScan.workers.", - ); - } - return value.map((candidate) => - parsePersistedWorker(objectValue(candidate, "deepScan.worker")), - ); -} - -function parseWorker( - result: JsonObject, - workerId: string, -): PersistedDeepScanWorker { - const deepScan = objectValue(result.deepScan, "deepScan"); - if (!Array.isArray(deepScan.workers)) { - throw new Error( - "Codex Security workbench did not return deepScan.workers.", - ); - } - const value = deepScan.workers.find( - (candidate) => - candidate && - typeof candidate === "object" && - !Array.isArray(candidate) && - (candidate as JsonObject).id === workerId, - ) as JsonObject | undefined; - if (!value) { - throw new Error( - `Codex Security workbench did not return Deep Scan worker ${workerId}.`, - ); - } - return parsePersistedWorker(value); -} - -function parsePersistedWorker(value: JsonObject): PersistedDeepScanWorker { - return { - id: requiredString(value.id, "deepScan.worker.id"), - kind: workerKind(value.kind), - status: workerStatus(value.status), - mergeState: mergeState(value.mergeState), - promptPath: requiredString(value.promptPath, "deepScan.worker.promptPath"), - artifactDir: requiredString( - value.artifactDir, - "deepScan.worker.artifactDir", - ), - attempt: nonNegativeInteger(value.attempt, "deepScan.worker.attempt"), - threadId: optionalString(value.sdkThreadId), - resultManifestPath: optionalString(value.resultManifestPath), - completionSequence: optionalPositiveInteger(value.completionSequence), - error: optionalString(value.error), - }; -} - -function mergeState(value: unknown): DeepScanMergeState { - if ( - value === "none" || - value === "buffered" || - value === "merging" || - value === "merged" - ) { - return value; - } - throw new Error( - "Codex Security workbench returned invalid deepScan.worker.mergeState.", - ); -} - -function workerKind(value: unknown): DeepScanWorkerKind { - if (value === "setup" || value === "discovery" || value === "dedup") - return value; - throw new Error( - "Codex Security workbench returned invalid deepScan.worker.kind.", - ); -} - -function workerStatus(value: unknown): DeepScanWorkerStatus { - if ( - value === "queued" || - value === "running" || - value === "succeeded" || - value === "failed" || - value === "canceled" - ) { - return value; - } - throw new Error( - "Codex Security workbench returned invalid deepScan.worker.status.", - ); -} - -function objectValue(value: unknown, field: string): JsonObject { - if (!value || typeof value !== "object" || Array.isArray(value)) { - throw new Error(`Codex Security workbench did not return ${field}.`); - } - return value as JsonObject; -} - -function requiredString(value: unknown, field: string): string { - if (typeof value !== "string" || !value) { - throw new Error(`Codex Security workbench returned invalid ${field}.`); - } - return value; -} - -function optionalString(value: unknown): string | undefined { - return typeof value === "string" && value ? value : undefined; -} - -function positiveInteger(value: unknown, field: string): number { - if (!Number.isInteger(value) || Number(value) < 1) { - throw new Error(`Codex Security workbench returned invalid ${field}.`); - } - return Number(value); -} - -function boundedPositiveNumber( - value: unknown, - field: string, - maximum: number, -): number { - if ( - typeof value !== "number" || - !Number.isFinite(value) || - value <= 0 || - value > maximum - ) { - throw new Error(`Codex Security workbench returned invalid ${field}.`); - } - return value; -} - -function optionalPositiveInteger(value: unknown): number | undefined { - return Number.isInteger(value) && Number(value) >= 1 - ? Number(value) - : undefined; -} - -function nonNegativeInteger(value: unknown, field: string): number { - if (!Number.isInteger(value) || Number(value) < 0) { - throw new Error(`Codex Security workbench returned invalid ${field}.`); - } - return Number(value); -} diff --git a/plugins/codex-security/mcp-app/src/deep-scan/types.ts b/plugins/codex-security/mcp-app/src/deep-scan/types.ts index bafd2af0f..d91160e11 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/types.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/types.ts @@ -1,54 +1,31 @@ import type { DeepReducerContext } from "../artifact-io.js"; import type { WorkbenchDeepScanStore } from "./store.js"; - -export type DeepScanTerminalReason = "saturated" | "capped"; - -export type DeepScanRunStatus = - "running" | "succeeded" | "canceled" | "failed" | "interrupted"; - -export type DeepScanWorkerKind = "setup" | "discovery" | "dedup"; - -export type DeepScanWorkerStatus = - "queued" | "running" | "succeeded" | "failed" | "canceled"; - -export type DeepScanMergeState = "none" | "buffered" | "merging" | "merged"; +import type { z } from "zod"; +import type { + configSchema, + dedupInputSchema, + runSchema, + workerSchema, +} from "./response.js"; /** Effective configuration and durable run state returned by the workbench. */ -export interface DeepScanConfig { - workers: number; - subagents: number; - stopAfterNoNew: number; - stopAfterConsecutiveErrors: number; - maxDiscoveryRuns: number; - maxTimeHours?: number; -} - -export interface DeepScanRunState { - scanId: string; - status: DeepScanRunStatus; - coordinatorGeneration?: number; - createdAt?: string; - updatedAt?: string; - targetPath: string; - scope: string; - userContext?: string; - scanDir: string; - config: DeepScanConfig; - dispatchedCount: number; - noNewStreak: number; - consecutiveErrors: number; - manifestPath?: string; - terminalReason?: DeepScanTerminalReason; - error?: string; +export type DeepScanConfig = z.output; +export type DeepScanRunState = Omit< + z.output, + "persistedWorkers" | "persistedDedupInputs" +> & { persistedWorkers?: PersistedDeepScanWorker[]; persistedDedupInputs?: PersistedDeepScanDedupInput[]; -} - -export interface PersistedDeepScanDedupInput { - dedupWorkerId: string; - discoveryWorkerId: string; - inputOrder: number; -} +}; +export type DeepScanTerminalReason = NonNullable< + DeepScanRunState["terminalReason"] +>; +export type PersistedDeepScanDedupInput = z.output; +/** The authoritative worker record returned after SQLite commits the change. */ +export type PersistedDeepScanWorker = z.output & { + consecutiveErrors?: number; +}; +export type DeepScanWorkerKind = PersistedDeepScanWorker["kind"]; export interface DeepScanCoordinatorClaim { run: DeepScanRunState; @@ -62,33 +39,17 @@ export interface DeepScanCoordinatorLeaseInput { } /** Fields supplied when the coordinator changes one worker. */ -export interface DeepScanWorkerMutation { - id: string; +export type DeepScanWorkerMutation = Omit< + PersistedDeepScanWorker, + "completionSequence" | "consecutiveErrors" | "mergeState" +> & { scanId: string; - kind: DeepScanWorkerKind; - status: DeepScanWorkerStatus; - promptPath: string; - artifactDir: string; - attempt: number; - threadId?: string; - resultManifestPath?: string; - error?: string; replaceableFailureKind?: DeepScanReplaceableFailureKind; -} +}; export type DeepScanReplaceableFailureKind = "policy_refusal" | "transient_error" | "invalid_discovery_artifacts"; -/** The authoritative worker record returned after SQLite commits the change. */ -export interface PersistedDeepScanWorker extends Omit< - DeepScanWorkerMutation, - "scanId" | "replaceableFailureKind" -> { - completionSequence?: number; - consecutiveErrors?: number; - mergeState: DeepScanMergeState; -} - /** Inputs committed atomically when a reducer finishes. */ export interface DedupCommit { id: string; diff --git a/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts b/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts index a3125b771..cf4733a41 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/worker-runner.ts @@ -43,14 +43,10 @@ export interface AcceptedDiscovery { export type DiscoveryOutcome = | { type: "discovery"; status: "succeeded"; worker: AcceptedDiscovery } | { type: "discovery"; status: "canceled"; workerId: string } - | { + | (Extract & { type: "discovery"; - status: "failed"; workerId: string; - error: Error; - replaceableFailureKind?: DeepScanReplaceableFailureKind; - consecutiveErrors?: number; - }; + }); export interface SuccessfulDedupOutcome { type: "dedup"; @@ -161,7 +157,7 @@ export class DeepScanWorkerRunner { }, }); if (outcome.status === "succeeded" && this.options.signal.aborted) { - await this.persistWorkerCancellation( + outcome = await this.cancelAttempt( { workerId, kind: "discovery", @@ -171,24 +167,13 @@ export class DeepScanWorkerRunner { outcome.attempt, outcome.threadId, ); - outcome = { status: "canceled" }; } if (!discoveryValidated) { await fs.rm(resultPath, { force: true }); } if (outcome.status === "failed") { - return { - type: "discovery", - status: "failed", - workerId, - error: outcome.error, - ...(outcome.replaceableFailureKind - ? { replaceableFailureKind: outcome.replaceableFailureKind } - : {}), - ...(outcome.consecutiveErrors === undefined - ? {} - : { consecutiveErrors: outcome.consecutiveErrors }), - }; + const { status, ...failure } = outcome; + return { type: "discovery", status, workerId, ...failure }; } if (outcome.status === "canceled" || this.options.signal.aborted) { return { type: "discovery", status: "canceled", workerId }; @@ -309,7 +294,7 @@ export class DeepScanWorkerRunner { }, }); if (outcome.status === "succeeded" && this.options.signal.aborted) { - await this.persistWorkerCancellation( + outcome = await this.cancelAttempt( { workerId: reducerId, kind: "dedup", @@ -319,7 +304,6 @@ export class DeepScanWorkerRunner { outcome.attempt, outcome.threadId, ); - outcome = { status: "canceled" }; } if (outcome.status === "failed") { if (outcome.error instanceof DeepScanNonRetryableError) @@ -571,16 +555,14 @@ export class DeepScanWorkerRunner { ); } - private async persistWorkerCancellation( - input: { - workerId: string; - kind: "discovery" | "dedup"; - promptPath: string; - artifactDir: string; - }, + private async cancelAttempt( + input: Pick< + Parameters[0], + "workerId" | "kind" | "promptPath" | "artifactDir" + >, attempt: number, threadId: string | undefined, - ): Promise { + ): Promise { const coordinatorShutdown = this.options.signal.reason === "mcp_transport_closed"; await this.options.store.updateWorker({ @@ -596,19 +578,6 @@ export class DeepScanWorkerRunner { ? { error: "coordinator_shutdown: mcp_transport_closed" } : {}), }); - } - - private async cancelAttempt( - input: { - workerId: string; - kind: "discovery" | "dedup"; - promptPath: string; - artifactDir: string; - }, - attempt: number, - threadId: string | undefined, - ): Promise { - await this.persistWorkerCancellation(input, attempt, threadId); return { status: "canceled" }; } } diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_store.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_store.ts index f153cd0d5..a7530489c 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_store.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_store.ts @@ -16,6 +16,7 @@ await testBeginProtocolAndParsing(); await testCanonicalCommitProtocol(); await testTerminalProtocol(); testRunErrorParsing(); +testLegacyResponseNormalization(); testConfiguredMaximumDurationParsing(); await testWriteSerializationAndRecovery(); await testBeginUsesTheWriteQueue(); @@ -312,27 +313,34 @@ async function testWorkerResponseParsing() { attempt: 1, resultManifestPath: "/fixture/output/result.json", }; - const worker = await new WorkbenchDeepScanStore(async () => - stateResult(scanId, { - deepScan: { - workers: [ - { - id: workerId, - kind: "discovery", - status: "succeeded", - mergeState: "buffered", - promptPath: mutation.promptPath, - artifactDir: mutation.artifactDir, - resultManifestPath: mutation.resultManifestPath, - attempt: 1, - sdkThreadId: "thread-worker", - completionSequence: 3, - error: "worker was rate limited", - }, - ], - }, - }), - ).updateWorker(mutation); + const response = stateResult(scanId, { + deepScan: { + workers: [ + null, + { id: "unrelated-malformed-worker" }, + { + ...mutation, + mergeState: "buffered", + sdkThreadId: "thread-worker", + completionSequence: 3, + error: "worker was rate limited", + }, + ], + }, + }); + const store = new WorkbenchDeepScanStore(async () => response); + const worker = await store.updateWorker(mutation); + const state: Record = response.deepScan; + delete state.consecutiveErrors; + assert.equal( + Object.hasOwn(await store.updateWorker(mutation), "consecutiveErrors"), + false, + ); + state.consecutiveErrors = null; + await assert.rejects(store.updateWorker(mutation), { + message: + "Codex Security workbench returned invalid deepScan.consecutiveErrors.", + }); assert.equal(worker.completionSequence, 3); assert.equal(worker.mergeState, "buffered"); @@ -934,6 +942,59 @@ function testRunErrorParsing() { assert.equal(state.error, "discovery retries exhausted"); } +function testLegacyResponseNormalization() { + for (const ignored of [undefined, null, "", false, [], {}]) { + const result = stateResult(" "); + const state: Record = result.deepScan; + Object.assign(state, { + userContext: ignored, + coordinatorGeneration: ignored, + terminalReason: ignored, + consecutiveErrors: null, + workers: null, + dedupInputs: null, + extra: "discard", + }); + Object.assign(result.deepScan.config, { + workers: Number.MAX_SAFE_INTEGER + 1, + stopAfterConsecutiveErrors: null, + }); + const parsed = parseDeepScan(result); + assert.equal(parsed.scanId, " "); + assert.equal(parsed.config.workers, Number.MAX_SAFE_INTEGER + 1); + assert.equal(parsed.config.stopAfterConsecutiveErrors, 6); + assert.equal(parsed.consecutiveErrors, 0); + assert.deepEqual(parsed.persistedWorkers, []); + assert.deepEqual(parsed.persistedDedupInputs, []); + for (const field of [ + "userContext", + "coordinatorGeneration", + "terminalReason", + ]) { + assert.equal(Object.hasOwn(parsed, field), true); + assert.equal(parsed[field], undefined); + } + assert.equal(Object.hasOwn(parsed, "extra"), false); + } + for (const [deepScan, diagnostic] of [ + [null, "did not return deepScan."], + [{ status: "unknown" }, "did not return deepScan.config."], + [ + { status: "unknown", config: {} }, + "Unsupported Deep Scan status: unknown", + ], + [ + { status: "running", config: {} }, + "returned invalid deepScan.config.stopAfterNoNew.", + ], + ] as const) { + assert.throws( + () => parseDeepScan({ deepScan }), + (error: Error) => error.message.endsWith(diagnostic), + ); + } +} + function stateResult( scanId: string, overrides: { diff --git a/sdk/typescript/src/deep-progress.ts b/sdk/typescript/src/deep-progress.ts index 0c6821c96..81c79372c 100644 --- a/sdk/typescript/src/deep-progress.ts +++ b/sdk/typescript/src/deep-progress.ts @@ -1,10 +1,19 @@ import { isRecord } from "./record.js"; -import { isSafeNonNegativeInteger } from "./value.js"; +import { z } from "zod"; -export interface DeepScanProgress { - completed: number; - active: number; - maximum: number; +const progressSchema = z + .object({ + completed: z.int().nonnegative(), + active: z.int().nonnegative(), + maximum: z.int().positive(), + consolidating: z.boolean().optional(), + }) + .transform(({ consolidating, ...counts }) => ({ + ...counts, + ...(consolidating === undefined ? {} : { consolidating }), + })); + +export interface DeepScanProgress extends z.output { /** Whether the coordinator is reducing results or has finished. */ consolidating?: boolean; } @@ -89,23 +98,8 @@ export function deepScanProgressFromWorkbench( if (!isRecord(progress)) return null; const independentReviews = progress["independentReviews"]; if (independentReviews === undefined) return null; - if (isRecord(independentReviews)) { - const { completed, active, maximum, consolidating } = independentReviews; - if ( - isSafeNonNegativeInteger(completed) && - isSafeNonNegativeInteger(active) && - isSafeNonNegativeInteger(maximum) && - maximum > 0 && - (consolidating === undefined || typeof consolidating === "boolean") - ) { - return { - completed, - active, - maximum, - ...(typeof consolidating === "boolean" ? { consolidating } : {}), - }; - } - } + const parsed = progressSchema.safeParse(independentReviews); + if (parsed.success) return parsed.data; throw new Error( "Codex Security workbench returned invalid Deep Scan progress.", ); diff --git a/sdk/typescript/src/worker-progress.ts b/sdk/typescript/src/worker-progress.ts index 453fb28ac..53d2a4dd5 100644 --- a/sdk/typescript/src/worker-progress.ts +++ b/sdk/typescript/src/worker-progress.ts @@ -1,32 +1,38 @@ import { isRecord } from "./record.js"; import { isSafeNonNegativeInteger, parseJson } from "./value.js"; +import { z } from "zod"; const WORKER_STATUS_PREFIX = "CODEX_SECURITY_WORKER_STATUS "; const SCAN_PROGRESS_PREFIX = "CODEX_SECURITY_SCAN_PROGRESS "; const PREFLIGHT_COMMAND = /(?:^|[\\/])config_preflight\.py(?=$|["'\s])/u; -const WORKER_PHASES = new Set([ +const workerPhaseSchema = z.enum([ "ranking", "file_review", "validation", "attack_path", ]); -export type ScanWorkerPhase = - "ranking" | "file_review" | "validation" | "attack_path"; +export type ScanWorkerPhase = z.infer; -export type ScanPhase = - | "preflight" - | "threat_model" - | "discovery" - | "validation" - | "attack_path" - | "reporting"; +const scanPhaseSchema = z.enum([ + "preflight", + "threat_model", + "discovery", + "validation", + "attack_path", + "reporting", +]); +export type ScanPhase = z.infer; -export interface ScanProgress { - phase: ScanPhase; - filesCompleted: number; - filesTotal: number; -} +const count = z.number().int().nonnegative(); +const scanProgressSchema = z + .object({ phase: scanPhaseSchema, filesCompleted: count, filesTotal: count }) + .refine(({ filesCompleted, filesTotal }) => filesCompleted <= filesTotal); +const dispatchSchema = z + .object({ phase: workerPhaseSchema, planned: count, started: count }) + .refine(({ started, planned }) => started <= planned); + +export interface ScanProgress extends z.infer {} export function scanPhaseLabel(value: ScanWorkerPhase | ScanPhase): string { return { @@ -48,12 +54,7 @@ export type ScanWorkerStatus = delegation: "available" | "unavailable" | "unknown"; configuredSlots: number | null; } - | { - kind: "dispatch"; - phase: ScanWorkerPhase; - planned: number; - started: number; - }; + | ({ kind: "dispatch" } & z.infer); export function workerStatusFromEvent( event: Readonly>, @@ -118,21 +119,10 @@ function linesOutsideFences(text: string): string[] { } function scanProgressFromMarker(marker: string): ScanProgress | null { - const payload = parseJson(() => marker.slice(SCAN_PROGRESS_PREFIX.length)); - if ( - !isRecord(payload) || - !isScanPhase(payload["phase"]) || - !isSafeNonNegativeInteger(payload["filesCompleted"]) || - !isSafeNonNegativeInteger(payload["filesTotal"]) || - payload["filesCompleted"] > payload["filesTotal"] - ) { - return null; - } - return { - phase: payload["phase"], - filesCompleted: payload["filesCompleted"], - filesTotal: payload["filesTotal"], - }; + const parsed = scanProgressSchema.safeParse( + parseJson(() => marker.slice(SCAN_PROGRESS_PREFIX.length)), + ); + return parsed.success ? parsed.data : null; } function preflightStatus( @@ -195,32 +185,8 @@ function dispatchStatus( ); const marker = markers[0]; if (markers.length !== 1 || marker === undefined) return null; - const payload = parseJson(() => marker.slice(WORKER_STATUS_PREFIX.length)); - if ( - !isRecord(payload) || - typeof payload["phase"] !== "string" || - !WORKER_PHASES.has(payload["phase"]) || - !isSafeNonNegativeInteger(payload["planned"]) || - !isSafeNonNegativeInteger(payload["started"]) || - payload["started"] > payload["planned"] - ) { - return null; - } - return { - kind: "dispatch", - phase: payload["phase"] as ScanWorkerPhase, - planned: payload["planned"], - started: payload["started"], - }; -} - -function isScanPhase(value: unknown): value is ScanPhase { - return ( - value === "preflight" || - value === "threat_model" || - value === "discovery" || - value === "validation" || - value === "attack_path" || - value === "reporting" + const parsed = dispatchSchema.safeParse( + parseJson(() => marker.slice(WORKER_STATUS_PREFIX.length)), ); + return parsed.success ? { kind: "dispatch", ...parsed.data } : null; } From 133018f3c435975b3a554a9f51ce9cb68e36458f Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sun, 11 Oct 2026 00:35:34 +0000 Subject: [PATCH 4/8] refactor(mcp): derive artifact and input contracts --- plugins/codex-security/mcp-app/server.ts | 141 +++++++----------- .../codex-security/mcp-app/src/artifact-io.ts | 33 ++-- .../mcp-app/src/deep-scan/artifacts.ts | 9 +- .../src/helpers/normalize-candidates.ts | 34 ++--- .../src/server/compact-artifact-tools.ts | 21 ++- 5 files changed, 85 insertions(+), 153 deletions(-) diff --git a/plugins/codex-security/mcp-app/server.ts b/plugins/codex-security/mcp-app/server.ts index 9684e239e..6b3708609 100644 --- a/plugins/codex-security/mcp-app/server.ts +++ b/plugins/codex-security/mcp-app/server.ts @@ -43,8 +43,10 @@ const CONFIGURED_SCAN_ROOT = process.env.CODEX_SECURITY_SCAN_ROOT?.trim(); const PLUGIN_ROOT = process.env.CODEX_SECURITY_PLUGIN_ROOT || resolve(__dirname, ".."); const USER_INPUT_WAIT_TIMEOUT_MS = 14 * 60 * 1000; -const userContextSchema = z.string().trim().min(1); -const editableUserContextSchema = z.string().trim(); +const trimmedText = z.string().trim(); +const requiredText = trimmedText.min(1); +const nonnegativeInteger = z.number().int().nonnegative(); +const positiveInteger = z.number().int().positive(); const pathSchema = z .string() .min(1) @@ -108,24 +110,24 @@ async function scanRoot(): Promise { const diffTargetSchema = z.discriminatedUnion("kind", [ z.strictObject({ kind: z.literal("working_tree"), - baseRevision: z.string().trim().min(1).max(512).optional(), - contentDigest: z.string().trim().min(1).max(128).optional(), - headRevision: z.string().trim().min(1).max(512).optional(), + baseRevision: requiredText.max(512).optional(), + contentDigest: requiredText.max(128).optional(), + headRevision: requiredText.max(512).optional(), }), z.strictObject({ kind: z.literal("commit"), - baseRevision: z.string().trim().min(1).max(512).optional(), - headRevision: z.string().trim().min(1).max(512), + baseRevision: requiredText.max(512).optional(), + headRevision: requiredText.max(512), }), z.strictObject({ kind: z.literal("range"), - baseRevision: z.string().trim().min(1).max(512), - headRevision: z.string().trim().min(1).max(512), + baseRevision: requiredText.max(512), + headRevision: requiredText.max(512), }), ]); const currentScanPreflightCheckSchema = z.strictObject({ - capability: z.string().trim().min(1).max(128), - reason: z.string().trim().min(1).max(1200), + capability: requiredText.max(128), + reason: requiredText.max(1200), severity: z.enum(["block", "warn", "suggest"]), status: z.enum(["pass", "fail", "unknown"]), }); @@ -152,29 +154,23 @@ const openSchema = { targetPath: pathSchema .optional() .describe("Optional resolved local target path."), - targetSummary: z - .string() - .trim() - .min(1) + targetSummary: requiredText .max(2400) .optional() .describe("Optional bounded target/security context."), - targetTitle: z - .string() - .trim() - .min(1) + targetTitle: requiredText .max(200) .optional() .describe("Optional human-readable target name."), - userContext: userContextSchema + userContext: requiredText .optional() .describe("Optional security focus supplied by the user."), }; const sessionSchema = { sessionId: z.string().uuid() }; const startScanSchema = { ...sessionSchema, - model: z.string().trim().min(1).max(200).optional(), - reasoningEffort: z.string().trim().min(1).max(32).optional(), + model: requiredText.max(200).optional(), + reasoningEffort: requiredText.max(32).optional(), }; const startPromptOnlyScanSchema = { diffTarget: diffTargetSchema @@ -189,14 +185,11 @@ const startPromptOnlyScanSchema = { "Directory inside targetPath. Use '.' for the whole target.", ), targetPath: pathSchema.describe("Resolved local target path."), - targetSummary: z - .string() - .trim() - .min(1) + targetSummary: requiredText .max(2400) .optional() .describe("Optional bounded target or change-set context."), - userContext: userContextSchema + userContext: requiredText .optional() .describe("Optional security focus supplied by the user."), }; @@ -207,31 +200,24 @@ const startHeadlessStandardScanSchema = { .describe( "Optional directory inside targetPath. Omit it or use '.' for the whole target.", ), - targetSummary: z - .string() - .trim() + targetSummary: trimmedText .max(2400) .optional() .describe("Optional bounded target context."), - userContext: editableUserContextSchema + userContext: trimmedText .optional() .describe("Optional security focus supplied by the user."), }; -type PromptOnlyScanInput = { - diffTarget?: z.output; - mode: "diff" | "standard"; - scope: string; - targetPath: string; - targetSummary?: string; - userContext?: string; -}; +type PromptOnlyScanInput = z.output< + z.ZodObject +>; const userInputOptionSchema = z.strictObject({ - description: z.string().trim().min(1).max(1200), - label: z.string().trim().min(1).max(200), + description: requiredText.max(1200), + label: requiredText.max(200), }); const userInputQuestionSchema = z .strictObject({ - header: z.string().trim().min(1).max(64), + header: requiredText.max(64), id: z .string() .regex( @@ -243,7 +229,7 @@ const userInputQuestionSchema = z "Question IDs must not use reserved object property names.", ), options: z.array(userInputOptionSchema).min(2).max(3), - question: z.string().trim().min(1).max(1200), + question: requiredText.max(1200), }) .superRefine((question, context) => { const labels = new Set(); @@ -289,8 +275,8 @@ const submissionSchema = { scope: pathSchema, sessionId: z.string().uuid(), targetPath: pathSchema, - targetSummary: z.string().trim().max(2400).optional(), - userContext: editableUserContextSchema.optional(), + targetSummary: trimmedText.max(2400).optional(), + userContext: trimmedText.optional(), }; const setupInspectionSchema = { diffTarget: diffTargetSchema.optional(), @@ -315,7 +301,7 @@ const startDeepScanSchema = { .describe( "Scope inside targetPath. Deep Scan currently requires the whole target.", ), - userContext: editableUserContextSchema + userContext: trimmedText .optional() .describe("Optional security focus supplied by the user."), handoffClaimToken: handoffClaimTokenSchema @@ -334,13 +320,13 @@ const continuationMutationClaimSchema = { const scanContextUpdateSchema = { ...scanSchema, ...continuationMutationClaimSchema, - userContext: editableUserContextSchema.describe( + userContext: trimmedText.describe( "Complete replacement context for the running scan. Pass an empty string to clear it.", ), }; const appScanContextUpdateSchema = { ...scanSchema, - userContext: editableUserContextSchema, + userContext: trimmedText, }; const phaseProgressUnitSchema = z.enum([ "checks", @@ -351,10 +337,7 @@ const phaseProgressUnitSchema = z.enum([ "report_artifacts", ]); const progressSchema = { - deepReviewPass: z - .number() - .int() - .positive() + deepReviewPass: positiveInteger .optional() .describe( "Current Deep Scan discovery pass. Send it when starting each pass together with that pass's total and zero completed items.", @@ -372,18 +355,12 @@ const progressSchema = { .describe( "Current workflow phase. Send it immediately when the scan enters a new phase so persisted progress advances.", ), - phaseItemsCompleted: z - .number() - .int() - .nonnegative() + phaseItemsCompleted: nonnegativeInteger .optional() .describe( "Completed authoritative coverage, receipts, or artifacts for the current phase. Increase it only after the corresponding work product exists.", ), - phaseItemsTotal: z - .number() - .int() - .nonnegative() + phaseItemsTotal: nonnegativeInteger .optional() .describe( "Expected authoritative coverage, receipts, or artifacts for the current phase. Increase it before newly discovered work begins.", @@ -400,19 +377,13 @@ const progressSchema = { .describe( "Current standard or diff scan preflight results. Project every helper results entry to capability, reason, severity, and status only. The server derives the item counts and visible block/warn attention items.", ), - reportableFindingsCount: z.number().int().nonnegative().optional(), - reviewItemsCompleted: z - .number() - .int() - .nonnegative() + reportableFindingsCount: nonnegativeInteger.optional(), + reviewItemsCompleted: nonnegativeInteger .optional() .describe( "Cumulative completed reviews or coverage surfaces in the current discovery pass. Increment only after the corresponding review is complete.", ), - reviewItemsTotal: z - .number() - .int() - .nonnegative() + reviewItemsTotal: nonnegativeInteger .optional() .describe( "Expected reviews or coverage surfaces in the current discovery pass. Increase it before assigning newly discovered work.", @@ -422,14 +393,14 @@ const progressSchema = { }; const failSchema = { ...continuationMutationClaimSchema, - message: z.string().trim().min(1).max(2400), + message: requiredText.max(2400), scanId: z.string().uuid(), }; const completeScanSchema = { ...scanSchema, ...continuationMutationClaimSchema, }; -const occurrenceIdSchema = z.string().trim().min(1).max(256); +const occurrenceIdSchema = requiredText.max(256); const scanReadSchema = { ...scanSchema, occurrenceId: occurrenceIdSchema @@ -450,14 +421,14 @@ const findingTriageSchema = { closeReason: z .enum(["already_fixed", "wont_fix", "false_positive"]) .optional(), - note: z.string().trim().max(2400).optional(), + note: trimmedText.max(2400).optional(), occurrenceId: occurrenceIdSchema, status: z.enum(["open", "closed"]), }; const findingRemediationSchema = { actionToken: z.string().uuid(), - baseRevision: z.string().trim().min(1).max(512).optional(), - expectedVersion: z.number().int().positive(), + baseRevision: requiredText.max(512).optional(), + expectedVersion: positiveInteger, occurrenceId: occurrenceIdSchema, patchDigest: z .string() @@ -466,13 +437,13 @@ const findingRemediationSchema = { patchPath: pathSchema.optional(), requestId: z.string().uuid(), state: z.enum(["generated", "applied", "verifying", "verified", "failed"]), - summary: z.string().trim().max(2400).optional(), - verificationSummary: z.string().trim().max(2400).optional(), + summary: trimmedText.max(2400).optional(), + verificationSummary: trimmedText.max(2400).optional(), }; const findingRemediationActionRequestSchema = { action: z.enum(["apply", "verify"]), actionToken: z.string().uuid(), - expectedVersion: z.number().int().positive(), + expectedVersion: positiveInteger, occurrenceId: occurrenceIdSchema, requestId: z.string().uuid(), }; @@ -487,19 +458,19 @@ const findingsExportSchema = { scanId: z.string().uuid(), }; const collectionPageSchema = { - limit: z.number().int().positive().max(50).optional(), - offset: z.number().int().nonnegative().optional(), + limit: positiveInteger.max(50).optional(), + offset: nonnegativeInteger.optional(), }; const findingCollectionFiltersSchema = { - query: z.string().trim().max(512).optional(), + query: trimmedText.max(512).optional(), severity: z .enum(["critical", "high", "medium", "low", "informational"]) .optional(), status: z.enum(["open", "closed"]).optional(), }; const targetCollectionFiltersSchema = { - query: z.string().trim().max(512).optional(), - targetId: z.string().trim().min(1).max(256).optional(), + query: trimmedText.max(512).optional(), + targetId: requiredText.max(256).optional(), }; const findingsPageSchema = { ...collectionPageSchema, @@ -508,9 +479,9 @@ const findingsPageSchema = { }; const globalFindingsPageSchema = { ...findingCollectionFiltersSchema, - limit: z.number().int().positive().max(20).optional(), - offset: z.number().int().nonnegative().optional(), - targetId: z.string().trim().min(1).max(256).optional(), + limit: positiveInteger.max(20).optional(), + offset: nonnegativeInteger.optional(), + targetId: requiredText.max(256).optional(), }; const scanListSchema = { ...collectionPageSchema, diff --git a/plugins/codex-security/mcp-app/src/artifact-io.ts b/plugins/codex-security/mcp-app/src/artifact-io.ts index 9d163d45e..a1db3177c 100644 --- a/plugins/codex-security/mcp-app/src/artifact-io.ts +++ b/plugins/codex-security/mcp-app/src/artifact-io.ts @@ -1,5 +1,5 @@ import { canonicalDirectory } from "./artifact-context.js"; -import { isRecord } from "./record.js"; +import type { ZodType, ZodError } from "zod/v4"; import { randomUUID } from "node:crypto"; import { promises as fs } from "node:fs"; import { dirname, isAbsolute, join, sep } from "node:path"; @@ -39,17 +39,6 @@ export interface ArtifactPage { limit?: number; } -export interface ArtifactPageResult { - rows: Row[]; - nextCursor?: string; -} - -export interface ArtifactRowSchema { - safeParse( - value: unknown, - ): { success: true; data: Row } | { success: false; error?: unknown }; -} - /** * Components are fixed, internal operation constants, not model-facing paths. */ @@ -146,7 +135,7 @@ export async function readArtifactJsonl( context: ArtifactContext, components: readonly string[], label: string, - rowSchema: ArtifactRowSchema, + rowSchema: ZodType, ): Promise { const source = await readArtifactText(context, components, label); const rows: Row[] = []; @@ -183,7 +172,7 @@ export function paginateArtifactRows( rows: readonly Row[], page: ArtifactPage, label: string, -): ArtifactPageResult { +) { // The MCP tool schemas validate cursor syntax and limit bounds. const start = Number(page.cursor ?? "0"); if (!Number.isSafeInteger(start) || start > rows.length) { @@ -326,16 +315,12 @@ async function inspectOptionalPath( } } -function formatRowSchemaError(error: unknown): string { - if (!isRecord(error) || !Array.isArray(error.issues)) return "."; - const issue: unknown = error.issues[0]; - if (!isRecord(issue)) return "."; - const issuePath = Array.isArray(issue.path) - ? issue.path.map(String).join(".") - : ""; - const message = typeof issue.message === "string" ? issue.message : ""; - if (!issuePath && !message) return "."; - return ": " + (issuePath ? issuePath + ": " : "") + message; +function formatRowSchemaError(error: ZodError): string { + const issue = error.issues[0]; + if (!issue) return "."; + const issuePath = issue.path.map(String).join("."); + if (!issuePath && !issue.message) return "."; + return ": " + (issuePath ? issuePath + ": " : "") + issue.message; } function isNodeError(error: unknown): error is NodeJS.ErrnoException { diff --git a/plugins/codex-security/mcp-app/src/deep-scan/artifacts.ts b/plugins/codex-security/mcp-app/src/deep-scan/artifacts.ts index 569dd6646..8f9e69dfb 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/artifacts.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/artifacts.ts @@ -2,14 +2,9 @@ import { randomUUID } from "node:crypto"; import { promises as fs } from "node:fs"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { errorMessage } from "./errors.js"; -export interface DeepScanArtifacts { - scanDir: string; - deepRoot: string; - workersRoot: string; - dedupRoot: string; -} +export type DeepScanArtifacts = ReturnType; -export function createDeepScanArtifacts(scanDir: string): DeepScanArtifacts { +export function createDeepScanArtifacts(scanDir: string) { const deepRoot = join(scanDir, "artifacts", "deep_discovery"); return { scanDir, diff --git a/plugins/codex-security/mcp-app/src/helpers/normalize-candidates.ts b/plugins/codex-security/mcp-app/src/helpers/normalize-candidates.ts index 731458293..41b8c9ecd 100644 --- a/plugins/codex-security/mcp-app/src/helpers/normalize-candidates.ts +++ b/plugins/codex-security/mcp-app/src/helpers/normalize-candidates.ts @@ -1,3 +1,6 @@ +import type { z } from "zod"; +import type { candidateSchemaV1 } from "../deep-scan/artifact-contracts.js"; +import discoverySchema from "../../../schemas/definitions/discovery-candidate.schema.json" with { type: "json" }; import { isRecord as object } from "../record.js"; import { decodeUtf8 } from "./utf8"; import { createHash, randomBytes } from "node:crypto"; @@ -31,32 +34,13 @@ const roles = [ "concrete_implementation", "evidence", ]; -const fields = new Set([ - "candidate_id", - "cwe_ids", - "locations", - "summary", - "evidence", - "context", - "instance", -]); -const locationFields = new Set(["path", "start_line", "end_line", "role"]); +const { discoveryCandidate, rawDiscoveryLocation } = discoverySchema.$defs; +const fields = new Set(Object.keys(discoveryCandidate.properties)); +const locationFields = new Set(Object.keys(rawDiscoveryLocation.properties)); const jsonFields = [...fields, ...locationFields].sort(); type Row = Record; -interface Location { - path: string; - start_line: number; - end_line: number; - role: string; -} -interface Candidate { - cwe_ids: string[]; - locations: Location[]; - summary: string; - evidence: string; - context?: string; - instance?: string; -} +type Candidate = Omit, "candidate_id">; +type Location = Candidate["locations"][number]; function compare(left: string, right: string): number { return left < right ? -1 : left > right ? 1 : 0; @@ -259,7 +243,7 @@ function normalizeLocations( path: name, start_line: start, end_line: end, - role: item.role, + role: item.role as Location["role"], }; normalized.set(stableJson(location), location); } diff --git a/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts b/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts index 6a03e7687..164056900 100644 --- a/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts +++ b/plugins/codex-security/mcp-app/src/server/compact-artifact-tools.ts @@ -1,5 +1,8 @@ import { readingAnnotations, writingAnnotations } from "./tool-annotations.js"; -import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import type { + McpServer, + ToolCallback, +} from "@modelcontextprotocol/sdk/server/mcp.js"; import type { ZodType } from "zod/v4"; import type { JsonObject as JsonRecord } from "../types.js"; import { @@ -65,15 +68,6 @@ const modelOnlyMeta = { ui: { visibility: ["model"] as const }, }; -function signalFromRequestContext( - requestContext: unknown, -): AbortSignal | undefined { - if (typeof requestContext !== "object" || requestContext === null) - return undefined; - const signal = Reflect.get(requestContext, "signal"); - return signal instanceof AbortSignal ? signal : undefined; -} - export function registerCompactArtifactTools( server: McpServer, options: CompactArtifactToolOptions, @@ -178,7 +172,7 @@ export function registerCompactArtifactTools( await scanContext(input, options, true, requestContext), input, options.runWorkbench, - signalFromRequestContext(requestContext), + requestContext.signal, ); }, }); @@ -316,7 +310,10 @@ interface CompactToolRegistration { description: string; inputSchema: ZodType; readOnly: boolean; - handler: (value: Input, requestContext: unknown) => Promise; + handler: ( + value: Input, + requestContext: Parameters>>[1], + ) => Promise; } function registerCompactTool( From 8705f07f30a9d17f59377b76ef67349ca45aeebf Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sun, 11 Oct 2026 01:10:07 +0000 Subject: [PATCH 5/8] test: share saved-log bundle across Node capability cases --- .../tests-ts/scan-logs-node.test.ts | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/sdk/typescript/tests-ts/scan-logs-node.test.ts b/sdk/typescript/tests-ts/scan-logs-node.test.ts index 7cc91ce75..596d2c8e6 100644 --- a/sdk/typescript/tests-ts/scan-logs-node.test.ts +++ b/sdk/typescript/tests-ts/scan-logs-node.test.ts @@ -4,7 +4,7 @@ import { join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { promisify } from "node:util"; import { zstdCompressSync } from "node:zlib"; -import { afterEach, expect, test } from "bun:test"; +import { afterEach, beforeAll, expect, test } from "bun:test"; import { createApiTestFixtures } from "./support/temporary-directories.js"; const { temporaryDirectory, cleanup } = createApiTestFixtures( @@ -13,6 +13,19 @@ const { temporaryDirectory, cleanup } = createApiTestFixtures( afterEach(cleanup); +let moduleSource: string; +beforeAll(async () => { + const built = await Bun.build({ + entrypoints: [ + fileURLToPath(new URL("../src/scan-logs.ts", import.meta.url)), + ], + target: "node", + format: "esm", + }); + expect(built.success).toBe(true); + moduleSource = await built.outputs[0]!.text(); +}); + test.each(["native", "without zstd"])( "reads saved logs with Node's %s capabilities", async (capabilities) => { @@ -37,16 +50,8 @@ test.each(["native", "without zstd"])( zstdCompressSync(`${JSON.stringify(worker)}\n`), ); - const built = await Bun.build({ - entrypoints: [ - fileURLToPath(new URL("../src/scan-logs.ts", import.meta.url)), - ], - target: "node", - format: "esm", - }); - expect(built.success).toBe(true); const module = join(root, "scan-logs.mjs"); - await writeFile(module, await built.outputs[0]!.text()); + await writeFile(module, moduleSource); const { stdout } = await promisify(execFile)( "node", [ From 15c810e48cba88e3f09371439d4fbf1012cd2f6c Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Sun, 11 Oct 2026 14:35:01 +0000 Subject: [PATCH 6/8] refactor: reuse shared argv for deep scan mutations --- .../mcp-app/src/deep-scan/store.ts | 183 +++++++----------- 1 file changed, 72 insertions(+), 111 deletions(-) diff --git a/plugins/codex-security/mcp-app/src/deep-scan/store.ts b/plugins/codex-security/mcp-app/src/deep-scan/store.ts index 891b2e37f..89dd2ac1d 100644 --- a/plugins/codex-security/mcp-app/src/deep-scan/store.ts +++ b/plugins/codex-security/mcp-app/src/deep-scan/store.ts @@ -146,7 +146,7 @@ export class WorkbenchDeepScanStore { model: input.model || undefined, reasoningEffort: input.reasoningEffort || undefined, scanRoot: input.scanRoot, - availableParallelism: String(availableParallelism()), + availableParallelism: availableParallelism(), workflowVersion: WORKFLOW_VERSION, }), false, @@ -184,17 +184,14 @@ export class WorkbenchDeepScanStore { async claimCoordinator( input: DeepScanCoordinatorLeaseInput, ): Promise { - const result = await this.enqueueWrite([ - "claim-deep-scan-coordinator", - "--scan-id", - input.scanId, - "--thread-id", - input.threadId, - ...this.coordinatorLeaseArgs(input.scanId), - ...(input.handoffClaimToken - ? ["--claim-token", input.handoffClaimToken] - : []), - ]); + const result = await this.enqueueWrite( + workbenchArgs("claim-deep-scan-coordinator", { + scanId: input.scanId, + threadId: input.threadId, + coordinatorGeneration: this.coordinatorGeneration(input.scanId), + claimToken: input.handoffClaimToken || undefined, + }), + ); const run = parseDeepScan(result); const disposition = result.coordinatorDisposition; if ( @@ -253,26 +250,18 @@ export class WorkbenchDeepScanStore { ): Promise { const result = await this.enqueueWrite( [ - "upsert-deep-scan-worker", - "--scan-id", - update.scanId, - "--worker-id", - update.id, - "--kind", - update.kind, - "--status", - update.status, - "--prompt-path", - update.promptPath, - "--artifact-dir", - update.artifactDir, - "--attempt", - String(update.attempt), - ...this.coordinatorLeaseArgs(update.scanId), - ...(update.resultManifestPath - ? ["--result-manifest-path", update.resultManifestPath] - : []), - ...(update.threadId ? ["--sdk-thread-id", update.threadId] : []), + ...workbenchArgs("upsert-deep-scan-worker", { + scanId: update.scanId, + workerId: update.id, + kind: update.kind, + status: update.status, + promptPath: update.promptPath, + artifactDir: update.artifactDir, + attempt: update.attempt, + coordinatorGeneration: this.coordinatorGeneration(update.scanId), + resultManifestPath: update.resultManifestPath || undefined, + sdkThreadId: update.threadId || undefined, + }), ...(update.error ? [`--error-message=${update.error}`] : []), ...(update.replaceableFailureKind ? ["--replaceable-failure-kind", update.replaceableFailureKind] @@ -301,22 +290,14 @@ export class WorkbenchDeepScanStore { artifactDir: string; }): Promise { await this.enqueueWrite( - [ - "claim-deep-scan-dedup", - "--scan-id", - input.scanId, - "--worker-id", - input.id, - "--prompt-path", - input.promptPath, - "--artifact-dir", - input.artifactDir, - ...this.coordinatorLeaseArgs(input.scanId), - ...input.workerIds.flatMap((workerId) => [ - "--input-worker-id", - workerId, - ]), - ], + workbenchArgs("claim-deep-scan-dedup", { + scanId: input.scanId, + workerId: input.id, + promptPath: input.promptPath, + artifactDir: input.artifactDir, + coordinatorGeneration: this.coordinatorGeneration(input.scanId), + inputWorkerId: input.workerIds, + }), true, ); } @@ -324,21 +305,14 @@ export class WorkbenchDeepScanStore { async commitDedup(commit: DedupCommit): Promise { return parseDeepScan( await this.enqueueWrite( - [ - "commit-deep-scan-dedup", - "--scan-id", - commit.scanId, - "--worker-id", - commit.id, - ...this.coordinatorLeaseArgs(commit.scanId), - "--result-manifest-path", - commit.resultManifestPath, - ...(commit.candidateLedgerPath - ? ["--candidate-ledger-path", commit.candidateLedgerPath] - : []), - "--new-findings-count", - String(commit.newFindings), - ], + workbenchArgs("commit-deep-scan-dedup", { + scanId: commit.scanId, + workerId: commit.id, + coordinatorGeneration: this.coordinatorGeneration(commit.scanId), + resultManifestPath: commit.resultManifestPath, + candidateLedgerPath: commit.candidateLedgerPath || undefined, + newFindingsCount: commit.newFindings, + }), true, ), ); @@ -353,23 +327,14 @@ export class WorkbenchDeepScanStore { }): Promise { return parseDeepScan( await this.enqueueWrite( - [ - "finish-deep-scan", - "--scan-id", - input.scanId, - ...this.coordinatorLeaseArgs(input.scanId), - "--terminal-reason", - input.reason, - "--manifest-path", - input.manifestPath, - ...(input.stagedManifestPath - ? ["--staged-manifest-path", input.stagedManifestPath] - : []), - ...input.omittedWorkerIds.flatMap((workerId) => [ - "--omitted-worker-id", - workerId, - ]), - ], + workbenchArgs("finish-deep-scan", { + scanId: input.scanId, + coordinatorGeneration: this.coordinatorGeneration(input.scanId), + terminalReason: input.reason, + manifestPath: input.manifestPath, + stagedManifestPath: input.stagedManifestPath || undefined, + omittedWorkerId: input.omittedWorkerIds, + }), true, ), ); @@ -384,10 +349,10 @@ export class WorkbenchDeepScanStore { ): Promise { return parseDeepScan( await this.enqueueWrite([ - "fail-deep-scan", - "--scan-id", - scanId, - ...this.coordinatorLeaseArgs(scanId), + ...workbenchArgs("fail-deep-scan", { + scanId, + coordinatorGeneration: this.coordinatorGeneration(scanId), + }), `--message=${message}`, ...(manifestPath ? ["--manifest-path", manifestPath] : []), ...(stagedManifestPath @@ -406,12 +371,13 @@ export class WorkbenchDeepScanStore { return parseDeepScan( await this.enqueueWrite( [ - "record-deep-scan-publication-failure", - "--scan-id", - scanId, - ...(coordinatorGeneration === undefined - ? this.coordinatorLeaseArgs(scanId) - : ["--coordinator-generation", String(coordinatorGeneration)]), + ...workbenchArgs("record-deep-scan-publication-failure", { + scanId, + coordinatorGeneration: + coordinatorGeneration === undefined + ? this.coordinatorGeneration(scanId) + : coordinatorGeneration, + }), `--message=${message}`, ], true, @@ -427,25 +393,21 @@ export class WorkbenchDeepScanStore { reviewItemsTotal?: number; reviewItemsCompleted?: number; }): Promise { - await this.enqueueWrite([ - "update-progress", - "--scan-id", - input.scanId, - ...this.coordinatorLeaseArgs(input.scanId), - ...(input.handoffClaimToken - ? ["--claim-token", input.handoffClaimToken] - : []), - ...(input.phase ? ["--phase", input.phase] : []), - ...(input.deepReviewPass === undefined - ? [] - : ["--deep-review-pass", String(input.deepReviewPass)]), - ...(input.reviewItemsTotal === undefined - ? [] - : ["--review-items-total", String(input.reviewItemsTotal)]), - ...(input.reviewItemsCompleted === undefined - ? [] - : ["--review-items-completed", String(input.reviewItemsCompleted)]), - ]); + await this.enqueueWrite( + workbenchArgs("update-progress", { + scanId: input.scanId, + coordinatorGeneration: this.coordinatorGeneration(input.scanId), + claimToken: input.handoffClaimToken || undefined, + phase: input.phase || undefined, + deepReviewPass: input.deepReviewPass, + reviewItemsTotal: input.reviewItemsTotal, + reviewItemsCompleted: input.reviewItemsCompleted, + }), + ); + } + + private coordinatorGeneration(scanId: string): number | undefined { + return this.coordinatorLeases.get(scanId)?.run.coordinatorGeneration; } /** @@ -458,8 +420,7 @@ export class WorkbenchDeepScanStore { * state before or after each mutation. */ coordinatorLeaseArgs(scanId: string): string[] { - const generation = - this.coordinatorLeases.get(scanId)?.run.coordinatorGeneration; + const generation = this.coordinatorGeneration(scanId); return generation === undefined ? [] : ["--coordinator-generation", String(generation)]; From 7e575d505b96d98653ec62798420bb304176e42c Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 13 Oct 2026 02:08:22 +0000 Subject: [PATCH 7/8] test(deep-scan): forward publication identity in terminal races --- .../mcp-app/tests/test_deep_scan_terminal_race.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/plugins/codex-security/mcp-app/tests/test_deep_scan_terminal_race.ts b/plugins/codex-security/mcp-app/tests/test_deep_scan_terminal_race.ts index 3ab9376bf..2db12a766 100644 --- a/plugins/codex-security/mcp-app/tests/test_deep_scan_terminal_race.ts +++ b/plugins/codex-security/mcp-app/tests/test_deep_scan_terminal_race.ts @@ -5,7 +5,10 @@ import { join } from "node:path"; import { test } from "node:test"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; -import type { ScanDraftInput } from "../src/artifact-scan-draft.js"; +import type { + DeepScanPublication, + ScanDraftInput, +} from "../src/artifact-scan-draft.js"; import type { DeepScanRunState } from "../src/deep-scan/types.js"; import { importModule } from "./import-module.ts"; import { temporaryDirectory } from "./support/temporary-directories.ts"; @@ -202,7 +205,11 @@ for (const ordering of [ pluginRoot, threadId: "fixture-owner", heartbeatIntervalMs: 60_000, - onComplete: async (draft: ScanDraftInput) => { + onComplete: async ( + draft: ScanDraftInput, + signal: AbortSignal, + publication: DeepScanPublication, + ) => { const context = await createScanArtifactContext( run.scanId, runWorkbench, @@ -212,6 +219,8 @@ for (const ordering of [ context, draft, runWorkbench, + signal, + publication, ); }, onStopped: async (stopped: DeepScanRunState) => { From 58dfff48b0478fd9474a1d3e977c6910c003480b Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 16 Oct 2026 04:30:12 +0000 Subject: [PATCH 8/8] test: isolate scan log bundling from coverage state --- sdk/typescript/tests-ts/scan-logs-node.test.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sdk/typescript/tests-ts/scan-logs-node.test.ts b/sdk/typescript/tests-ts/scan-logs-node.test.ts index 596d2c8e6..7b2deca11 100644 --- a/sdk/typescript/tests-ts/scan-logs-node.test.ts +++ b/sdk/typescript/tests-ts/scan-logs-node.test.ts @@ -15,15 +15,17 @@ afterEach(cleanup); let moduleSource: string; beforeAll(async () => { - const built = await Bun.build({ - entrypoints: [ + const { stdout } = await promisify(execFile)( + process.execPath, + [ + "build", fileURLToPath(new URL("../src/scan-logs.ts", import.meta.url)), + "--target=node", + "--format=esm", ], - target: "node", - format: "esm", - }); - expect(built.success).toBe(true); - moduleSource = await built.outputs[0]!.text(); + { encoding: "utf8" }, + ); + moduleSource = stdout; }); test.each(["native", "without zstd"])(