From 2ebe041f0aa2d3d94873fd3e340eb6e1bd2f30c2 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Thu, 3 Sep 2026 15:39:08 -0400 Subject: [PATCH] refactor(core): classify artifact references in one place Artifact reference groups were derived twice: agent task run results matched exact kinds, and the public reference projection matched broader predicates. The rules had drifted, so the same bundle could group differently depending on which projection a consumer read. Add one classification module both projections compose their groups from. The difference between them is preserved and named: typed classification accepts only declared kinds because it feeds the workspace delta, whose patches a caller may apply, while discovery classification also infers from paths so partially typed bundles stay listable. Fix log extension matching while consolidating. The shared path helper compares trailing path segments, so the .log and .jsonl checks never matched a real filename. --- .../runtime-core/src/agent-task-run-result.ts | 15 +-- .../src/artifact-ref-classification.ts | 97 +++++++++++++++++++ .../runtime-core/src/artifact-references.ts | 36 ++----- tests/artifact-ref-classification.test.ts | 40 ++++++++ 4 files changed, 154 insertions(+), 34 deletions(-) create mode 100644 packages/runtime-core/src/artifact-ref-classification.ts create mode 100644 tests/artifact-ref-classification.test.ts diff --git a/packages/runtime-core/src/agent-task-run-result.ts b/packages/runtime-core/src/agent-task-run-result.ts index 174ee795b..dcdc01ff1 100644 --- a/packages/runtime-core/src/agent-task-run-result.ts +++ b/packages/runtime-core/src/agent-task-run-result.ts @@ -1,3 +1,4 @@ +import { isArtifactBundleRef, isChangedFilesArtifactRef, isEvidenceBundleArtifactRef, isLogArtifactRef, isPatchArtifactRef, isRuntimeArtifactRef, isTranscriptArtifactRef } from "./artifact-ref-classification.js" import { isPlainObject, numberValue, objectValue, stringValue, stripUndefined } from "./object-utils.js" import { normalizeAgentTerminalResult, type AgentTerminalResult } from "./agent-terminal-result.js" import { RUNTIME_ACCESS_SCHEMA, normalizeRuntimeAccess, type RuntimeAccess } from "./runtime-boundary-contracts.js" @@ -114,13 +115,13 @@ export function normalizeAgentTaskRunResult(raw: unknown, options: AgentTaskRunR summary: stringValue(result.summary) || stringValue(result.message) || stringValue(agentResult.summary) || defaultSummary(status), artifacts, refs: { - artifact_bundles: artifacts.filter((artifact) => artifact.kind === "artifact-bundle" || artifact.kind === "codebox-artifact-bundle"), - changed_files: artifacts.filter((artifact) => artifact.kind === "codebox-changed-files"), - patches: artifacts.filter((artifact) => artifact.kind === "codebox-patch"), - transcripts: artifacts.filter((artifact) => artifact.kind === "codebox-transcript"), - logs: artifacts.filter((artifact) => artifact.kind === "codebox-runtime-log" || artifact.kind === "codebox-command-log"), - runtimes: artifacts.filter((artifact) => artifact.kind === "codebox-runtime"), - evidence_bundles: artifacts.filter((artifact) => artifact.kind === "evidence-bundle" || artifact.kind === "codebox-evidence-bundle"), + artifact_bundles: artifacts.filter((artifact) => isArtifactBundleRef(artifact)), + changed_files: artifacts.filter((artifact) => isChangedFilesArtifactRef(artifact)), + patches: artifacts.filter((artifact) => isPatchArtifactRef(artifact)), + transcripts: artifacts.filter((artifact) => isTranscriptArtifactRef(artifact)), + logs: artifacts.filter((artifact) => isLogArtifactRef(artifact)), + runtimes: artifacts.filter((artifact) => isRuntimeArtifactRef(artifact)), + evidence_bundles: artifacts.filter((artifact) => isEvidenceBundleArtifactRef(artifact)), }, diagnostics: [...arrayObjects(result.diagnostics), ...(terminalResult?.diagnostics ?? [])], metadata: stripUndefined({ diff --git a/packages/runtime-core/src/artifact-ref-classification.ts b/packages/runtime-core/src/artifact-ref-classification.ts new file mode 100644 index 000000000..08fb72d72 --- /dev/null +++ b/packages/runtime-core/src/artifact-ref-classification.ts @@ -0,0 +1,97 @@ +/** + * Canonical artifact reference classification. + * + * Artifact reference kinds are classified in one place so every projection + * derives its groups from the same rules instead of restating them. + * + * Classification has two modes, and the difference is a trust boundary rather + * than an inconsistency: + * + * - `"typed"` accepts only explicitly declared artifact kinds. Projections that + * feed the workspace delta use this mode, because a reference classified as + * changed files or a patch describes changes a caller may apply. A path that + * merely looks like `files/patch.diff` must never earn that trust. + * - `"discovery"` also infers from artifact paths. Read-only projections that + * list, link, or display references use this mode so partially typed bundles + * stay discoverable. + * + * Callers compose their own group sets from these predicates; the group shape a + * surface publishes is its own concern, but what makes a reference a patch, a + * transcript, or a log is decided here. + */ + +export const CHANGED_FILES_ARTIFACT_PATH = "files/changed-files.json" as const +export const PATCH_ARTIFACT_PATH = "files/patch.diff" as const +export const ARTIFACT_MANIFEST_PATH = "manifest.json" as const + +export type ArtifactRefClassificationMode = "typed" | "discovery" + +export interface ClassifiableArtifactRef { + kind: string + path?: string +} + +export function isArtifactBundleRef(ref: ClassifiableArtifactRef): boolean { + return ref.kind === "artifact-bundle" || ref.kind === "codebox-artifact-bundle" +} + +export function isEvidenceBundleArtifactRef(ref: ClassifiableArtifactRef): boolean { + return ref.kind === "evidence-bundle" || ref.kind === "codebox-evidence-bundle" +} + +export function isRuntimeArtifactRef(ref: ClassifiableArtifactRef): boolean { + return ref.kind === "codebox-runtime" +} + +export function isChangedFilesArtifactRef(ref: ClassifiableArtifactRef, mode: ArtifactRefClassificationMode = "typed"): boolean { + if (ref.kind === "codebox-changed-files") return true + if (mode === "typed") return false + return ref.kind === "changed-files" || pathEndsWith(ref.path, CHANGED_FILES_ARTIFACT_PATH) +} + +export function isPatchArtifactRef(ref: ClassifiableArtifactRef, mode: ArtifactRefClassificationMode = "typed"): boolean { + if (ref.kind === "codebox-patch") return true + if (mode === "typed") return false + return ref.kind === "patch" || pathEndsWith(ref.path, PATCH_ARTIFACT_PATH) +} + +export function isTranscriptArtifactRef(ref: ClassifiableArtifactRef, mode: ArtifactRefClassificationMode = "typed"): boolean { + return mode === "typed" ? ref.kind === "codebox-transcript" : ref.kind.includes("transcript") +} + +export function isLogArtifactRef(ref: ClassifiableArtifactRef, mode: ArtifactRefClassificationMode = "typed"): boolean { + if (ref.kind === "codebox-runtime-log" || ref.kind === "codebox-command-log") return true + if (mode === "typed") return false + return ref.kind.includes("log") || pathHasExtension(ref.path, ".log") || pathHasExtension(ref.path, ".jsonl") +} + +export function isBrowserArtifactRef(ref: ClassifiableArtifactRef): boolean { + return ref.kind.startsWith("browser-") || pathIncludes(ref.path, "/browser/") +} + +/** + * Discovery-mode kind inference for references that arrive without a kind. + * Typed projections never call this; they require a declared kind. + */ +export function kindForArtifactPath(path: string | undefined): string | undefined { + if (pathEndsWith(path, CHANGED_FILES_ARTIFACT_PATH)) return "codebox-changed-files" + if (pathEndsWith(path, PATCH_ARTIFACT_PATH)) return "codebox-patch" + if (pathEndsWith(path, ARTIFACT_MANIFEST_PATH)) return "artifact-manifest" + return undefined +} + +/** Matches a trailing path segment, so `files/patch.diff` matches but `my-patch.diff` does not. */ +function pathEndsWith(path: string | undefined, suffix: string): boolean { + return typeof path === "string" && (path === suffix || path.endsWith(`/${suffix}`)) +} + +/** Matches a file extension on the final path segment. */ +function pathHasExtension(path: string | undefined, extension: string): boolean { + if (typeof path !== "string") return false + const name = path.slice(path.lastIndexOf("/") + 1) + return name.length > extension.length && name.endsWith(extension) +} + +function pathIncludes(path: string | undefined, fragment: string): boolean { + return typeof path === "string" && path.includes(fragment) +} diff --git a/packages/runtime-core/src/artifact-references.ts b/packages/runtime-core/src/artifact-references.ts index e271665dc..6bf8545c6 100644 --- a/packages/runtime-core/src/artifact-references.ts +++ b/packages/runtime-core/src/artifact-references.ts @@ -4,6 +4,10 @@ import type { RuntimeReferenceManifestArtifactBundleRef, RuntimeReferenceManifes import { redactJsonValue } from "./redaction.js" import { BROWSER_SESSION_PRODUCT_DTO_SCHEMA, normalizeRuntimeAccess, type RuntimeAccess } from "./runtime-boundary-contracts.js" +import { ARTIFACT_MANIFEST_PATH, CHANGED_FILES_ARTIFACT_PATH, PATCH_ARTIFACT_PATH, isArtifactBundleRef, isBrowserArtifactRef, isChangedFilesArtifactRef, isLogArtifactRef, isPatchArtifactRef, isTranscriptArtifactRef, kindForArtifactPath } from "./artifact-ref-classification.js" + +export { ARTIFACT_MANIFEST_PATH, CHANGED_FILES_ARTIFACT_PATH, PATCH_ARTIFACT_PATH } + export const METADATA_ARTIFACT_PATH = "metadata.json" as const export const REVIEW_ARTIFACT_PATH = "files/review.json" as const export const RUNTIME_EPISODE_TRACE_ARTIFACT_PATH = "files/runtime-episode-trace.json" as const @@ -11,9 +15,6 @@ export const RUNTIME_EPISODE_EVENTS_ARTIFACT_PATH = "files/runtime-episode.jsonl export const RUNTIME_REFERENCE_MANIFEST_ARTIFACT_PATH = "files/runtime-reference-manifest.json" as const export const RUNTIME_REPLAY_REFERENCE_INDEX_ARTIFACT_PATH = "files/runtime-replay-index.json" as const export const RUNTIME_SNAPSHOT_ARTIFACT_PATH = "files/runtime-snapshot.json" as const -export const CHANGED_FILES_ARTIFACT_PATH = "files/changed-files.json" as const -export const PATCH_ARTIFACT_PATH = "files/patch.diff" as const -export const ARTIFACT_MANIFEST_PATH = "manifest.json" as const export const PUBLIC_ARTIFACT_REF_DTO_SCHEMA = "wp-codebox/artifact-ref/v1" as const const RUNTIME_REFERENCE_MANIFEST_EXCLUDED_PATHS = new Set([ @@ -195,11 +196,11 @@ export function publicArtifactRefGroups(input: unknown): PublicArtifactRefGroups return { all, artifact_bundles: all.filter(isArtifactBundleRef), - changed_files: all.filter(isChangedFilesArtifactRef), - patches: all.filter(isPatchArtifactRef), - browser: all.filter((ref) => ref.kind.startsWith("browser-") || pathIncludes(ref.path, "/browser/")), - logs: all.filter((ref) => ref.kind.includes("log") || pathEndsWith(ref.path, ".log") || pathEndsWith(ref.path, ".jsonl")), - transcripts: all.filter((ref) => ref.kind.includes("transcript")), + changed_files: all.filter((ref) => isChangedFilesArtifactRef(ref, "discovery")), + patches: all.filter((ref) => isPatchArtifactRef(ref, "discovery")), + browser: all.filter(isBrowserArtifactRef), + logs: all.filter((ref) => isLogArtifactRef(ref, "discovery")), + transcripts: all.filter((ref) => isTranscriptArtifactRef(ref, "discovery")), } } @@ -496,25 +497,6 @@ function richerPublicArtifactRef(existing: PublicArtifactRefDTO, incoming: Publi }) } -function isArtifactBundleRef(ref: PublicArtifactRefDTO): boolean { - return ref.kind === "artifact-bundle" || ref.kind === "codebox-artifact-bundle" -} - -function isChangedFilesArtifactRef(ref: PublicArtifactRefDTO): boolean { - return ref.kind === "codebox-changed-files" || ref.kind === "changed-files" || pathEndsWith(ref.path, CHANGED_FILES_ARTIFACT_PATH) -} - -function isPatchArtifactRef(ref: PublicArtifactRefDTO): boolean { - return ref.kind === "codebox-patch" || ref.kind === "patch" || pathEndsWith(ref.path, PATCH_ARTIFACT_PATH) -} - -function kindForArtifactPath(path: string | undefined): string | undefined { - if (pathEndsWith(path, CHANGED_FILES_ARTIFACT_PATH)) return "codebox-changed-files" - if (pathEndsWith(path, PATCH_ARTIFACT_PATH)) return "codebox-patch" - if (pathEndsWith(path, ARTIFACT_MANIFEST_PATH)) return "artifact-manifest" - return undefined -} - function pathEndsWith(path: string | undefined, suffix: string): boolean { return typeof path === "string" && (path === suffix || path.endsWith(`/${suffix}`)) } diff --git a/tests/artifact-ref-classification.test.ts b/tests/artifact-ref-classification.test.ts new file mode 100644 index 000000000..2156076c0 --- /dev/null +++ b/tests/artifact-ref-classification.test.ts @@ -0,0 +1,40 @@ +import assert from "node:assert/strict" +import { isChangedFilesArtifactRef, isLogArtifactRef, isPatchArtifactRef, isTranscriptArtifactRef } from "../packages/runtime-core/src/artifact-ref-classification.js" +import { normalizeAgentTaskRunResult, publicArtifactRefGroups, workspaceDeltaFromAgentTaskRunResult } from "../packages/runtime-core/src/index.js" + +// A reference that only looks like a patch by path must never be trusted as one. +// Typed classification feeds the workspace delta, and a delta patch can be applied. +const untypedPatch = { kind: "artifact", path: "files/patch.diff" } +const untypedChangedFiles = { kind: "artifact", path: "files/changed-files.json" } + +assert.equal(isPatchArtifactRef(untypedPatch), false, "typed mode rejects path-inferred patches") +assert.equal(isChangedFilesArtifactRef(untypedChangedFiles), false, "typed mode rejects path-inferred changed files") +assert.equal(isPatchArtifactRef(untypedPatch, "discovery"), true, "discovery mode infers patches from path") +assert.equal(isChangedFilesArtifactRef(untypedChangedFiles, "discovery"), true, "discovery mode infers changed files from path") + +// Declared kinds are trusted in both modes. +for (const mode of ["typed", "discovery"] as const) { + assert.equal(isPatchArtifactRef({ kind: "codebox-patch" }, mode), true, `declared patch kind is classified in ${mode} mode`) + assert.equal(isChangedFilesArtifactRef({ kind: "codebox-changed-files" }, mode), true, `declared changed-files kind is classified in ${mode} mode`) + assert.equal(isTranscriptArtifactRef({ kind: "codebox-transcript" }, mode), true, `declared transcript kind is classified in ${mode} mode`) + assert.equal(isLogArtifactRef({ kind: "codebox-runtime-log" }, mode), true, `declared runtime log kind is classified in ${mode} mode`) +} + +// Loosely named kinds stay out of typed groups but remain discoverable. +assert.equal(isTranscriptArtifactRef({ kind: "agent-transcript" }), false, "typed mode requires the declared transcript kind") +assert.equal(isTranscriptArtifactRef({ kind: "agent-transcript" }, "discovery"), true, "discovery mode matches transcript-like kinds") +assert.equal(isLogArtifactRef({ kind: "artifact", path: "files/run.jsonl" }), false, "typed mode requires a declared log kind") +assert.equal(isLogArtifactRef({ kind: "artifact", path: "files/run.jsonl" }, "discovery"), true, "discovery mode infers logs from path") + +// The two consumers of the classifier keep their respective modes end to end. +const untypedArtifacts = [untypedChangedFiles, untypedPatch] +const runResult = normalizeAgentTaskRunResult({ success: true, artifacts: untypedArtifacts }) +assert.deepEqual(runResult.refs.patches, [], "agent task run refs stay typed") +assert.deepEqual(runResult.refs.changed_files, [], "agent task run refs reject inferred changed files") +assert.equal(workspaceDeltaFromAgentTaskRunResult(runResult).status, "unavailable", "workspace delta refuses untyped change evidence") + +const discovered = publicArtifactRefGroups({ artifacts: untypedArtifacts }) +assert.equal(discovered.patches.length, 1, "public discovery projection still surfaces the patch") +assert.equal(discovered.changed_files.length, 1, "public discovery projection still surfaces changed files") + +console.log("artifact ref classification ok")