diff --git a/agent-skills/quality/references/improve/assets/observation-sets.schema.json b/agent-skills/quality/references/improve/assets/observation-sets.schema.json deleted file mode 100644 index 1ed6172..0000000 --- a/agent-skills/quality/references/improve/assets/observation-sets.schema.json +++ /dev/null @@ -1,47 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "https://shiplight.dev/schemas/quality/observation-sets.schema.json", - "title": "Quality Observation Sets", - "type": "object", - "additionalProperties": false, - "required": ["observation_sets"], - "properties": { - "observation_sets": { - "type": "array", - "minItems": 1, - "items": { - "$ref": "#/definitions/observationSet" - } - } - }, - "definitions": { - "nonEmptyString": { - "type": "string", - "minLength": 1 - }, - "profileReference": { - "type": "object", - "additionalProperties": false, - "required": ["profile_id"], - "properties": { - "profile_id": { "$ref": "#/definitions/nonEmptyString" } - } - }, - "observationSet": { - "type": "object", - "additionalProperties": false, - "required": ["id", "name", "profiles"], - "properties": { - "id": { "$ref": "#/definitions/nonEmptyString" }, - "name": { "$ref": "#/definitions/nonEmptyString" }, - "description": { "$ref": "#/definitions/nonEmptyString" }, - "profiles": { - "type": "array", - "minItems": 1, - "uniqueItems": true, - "items": { "$ref": "#/definitions/profileReference" } - } - } - } - } -} diff --git a/agent-skills/quality/references/improve/assets/observation-sources.schema.json b/agent-skills/quality/references/improve/assets/observation-sources.schema.json deleted file mode 100644 index c87bdb1..0000000 --- a/agent-skills/quality/references/improve/assets/observation-sources.schema.json +++ /dev/null @@ -1,109 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$id": "https://shiplight.dev/schemas/quality/observation-sources.schema.json", - "title": "Quality Observation Source Profiles", - "type": "object", - "additionalProperties": false, - "required": ["profiles"], - "properties": { - "profiles": { - "type": "array", - "minItems": 1, - "items": { - "$ref": "#/definitions/profile" - } - } - }, - "definitions": { - "nonEmptyString": { - "type": "string", - "minLength": 1 - }, - "sourceRef": { - "type": "object", - "additionalProperties": false, - "minProperties": 1, - "properties": { - "path": { "$ref": "#/definitions/nonEmptyString" }, - "url": { "$ref": "#/definitions/nonEmptyString" }, - "label": { "$ref": "#/definitions/nonEmptyString" } - } - }, - "auth": { - "type": "object", - "additionalProperties": false, - "properties": { - "required_env": { - "type": "array", - "items": { "$ref": "#/definitions/nonEmptyString" } - } - } - }, - "github": { - "type": "object", - "additionalProperties": false, - "required": ["repo", "workflow", "artifact_names"], - "properties": { - "repo": { "$ref": "#/definitions/nonEmptyString" }, - "workflow": { "$ref": "#/definitions/nonEmptyString" }, - "artifact_names": { - "type": "array", - "minItems": 1, - "items": { "$ref": "#/definitions/nonEmptyString" } - }, - "branch": { "$ref": "#/definitions/nonEmptyString" } - } - }, - "localFolder": { - "type": "object", - "additionalProperties": false, - "required": ["path"], - "properties": { - "path": { "$ref": "#/definitions/nonEmptyString" } - } - }, - "profile": { - "type": "object", - "additionalProperties": false, - "required": ["id", "name", "transport", "observation_path"], - "properties": { - "id": { "$ref": "#/definitions/nonEmptyString" }, - "name": { "$ref": "#/definitions/nonEmptyString" }, - "description": { "$ref": "#/definitions/nonEmptyString" }, - "transport": { - "enum": ["github-actions", "local-folder"] - }, - "observation_path": { "$ref": "#/definitions/nonEmptyString" }, - "source_refs": { - "type": "array", - "items": { "$ref": "#/definitions/sourceRef" } - }, - "auth": { "$ref": "#/definitions/auth" }, - "github": { "$ref": "#/definitions/github" }, - "local_folder": { "$ref": "#/definitions/localFolder" } - }, - "allOf": [ - { - "if": { - "properties": { - "transport": { "const": "github-actions" } - } - }, - "then": { - "required": ["github"] - } - }, - { - "if": { - "properties": { - "transport": { "const": "local-folder" } - } - }, - "then": { - "required": ["local_folder"] - } - } - ] - } - } -} diff --git a/agent-skills/quality/references/improve/assets/observation-sources.template.yaml b/agent-skills/quality/references/improve/assets/observation-sources.template.yaml index c443e11..0115ba0 100644 --- a/agent-skills/quality/references/improve/assets/observation-sources.template.yaml +++ b/agent-skills/quality/references/improve/assets/observation-sources.template.yaml @@ -22,20 +22,40 @@ profiles: - "quality-observations-*" # branch: "main" - # For a local source: + # For a local source that reads a canonical file already on disk: # - change `transport` above to `"local-folder"` # - remove both `auth` and `github` # - add this block: # local_folder: # path: "artifacts/quality" + # For results the reading application fetches itself — a local test report, + # a platform that already holds the run — use the host transport. The + # provider name is resolved by whoever reads this repo, so which providers + # exist depends on the application, not on Quality: + # - change `transport` above to `"host"` + # - remove `observation_path`, `auth`, and `github` + # - add this block: + # host: + # provider: "local-reports" + # options: + # path: "playwright-report/report.json" + # report: "playwright-report/index.html" + # Authoring rules: -# - One profile represents one workflow or local folder. -# - observation_path is relative to the downloaded artifact or local_folder. +# - One profile represents one workflow, local folder, or host provider. +# - observation_path is relative to the downloaded artifact or local_folder. A +# host profile omits it: it addresses no file. # - GitHub artifact_names may select several uploaded archives from one run. # Every matching observation_path uses the same canonical contract; Quality # merges their observations. # - A local-folder profile reads exactly one observation_path. +# - A host profile names `host.provider`. An application that does not register +# that provider reports it as a diagnostic rather than reading nothing, so a +# profile written for one reader stays legible to another. +# - `local-reports` is the provider Quality ships: it reads a Playwright JSON +# report from the working tree and points each result at the HTML report a +# reviewer opens. It records no commit unless `options.commit` pins one. # - Raw JUnit, Playwright, telemetry, or custom reports are producer inputs. # Convert them in the workflow with `quality-tools observations`; source # configuration contains no parser or format selection. diff --git a/agent-skills/quality/references/improve/index.md b/agent-skills/quality/references/improve/index.md index 59326a8..a1a5593 100644 --- a/agent-skills/quality/references/improve/index.md +++ b/agent-skills/quality/references/improve/index.md @@ -94,7 +94,8 @@ Classify each gap before editing: 3. **Evidence strength:** a mapped method cannot establish the full claim or lacks the required execution context/gate. 4. **Source acquisition:** credentials, repository/workflow selection, artifact - names, or local-folder path prevent results from loading. + names, local-folder path, or an unregistered host provider prevent results + from loading. 5. **Artifact emission:** the workflow emits no canonical observation file or emits it at the wrong path. 6. **Producer format:** the canonical file has an invalid version, envelope, @@ -179,7 +180,7 @@ acquisition and resolution have been ruled out. ``` -- Observation config: compare with the configuration schemas in `assets/`, then +- Observation config: assess the project and read its `INVALID_*` diagnostics, then run the relevant assessment. Engine diagnostics verify acquisition and graph joins. - Implementation or verification-method changes: run their owning verification command before @@ -209,23 +210,43 @@ or requires a human decision. - `.quality/config/observation-sets.yaml` - `.quality/config/views.yaml` -Use the configuration templates and schemas under `assets/`. Use -`quality-observations.template.json` as the canonical output example; obtain its -current schema from `quality-tools observations schema`, not a bundled copy. +Use the configuration templates under `assets/`. Never vendor a copy of a +schema: a copy cannot be checked against the contract and drifts the moment the +contract moves. -### Sources +For the observation manifest, obtain the current schema from +`quality-tools observations schema`. -One profile represents one acquisition integration, such as one GitHub Actions -workflow or one local result folder. It answers only: +Configuration files — sources, sets, views — are validated by the engine itself +when you assess the project: an invalid profile, set, or view reports an +`INVALID_*` diagnostic naming the exact `yamlPath`. That is the authority, since +it is the parser that actually runs. Read the diagnostics rather than +pre-validating against a schema. + +Use `quality-observations.template.json` as the canonical output example. -- which transport fetches results: `github-actions` or `local-folder` -- which `observation_path` contains canonical `quality-observations.json` - content +### Sources -A source never selects a parser. Raw JUnit, Playwright, telemetry, or custom -gate output must be converted by its producer before the source reads it. Do -not create a source profile until the canonical file exists or its emit step is -being added in the same authorized change. +One profile represents one acquisition integration, such as one GitHub Actions +workflow, one local result folder, or one provider the reading application +supplies. It answers only: + +- which transport fetches results: `github-actions`, `local-folder`, or `host` +- for the two file transports, which `observation_path` contains canonical + `quality-observations.json` content +- for `host`, which `host.provider` the reading application resolves + +A file-based source never selects a parser. Raw JUnit, Playwright, telemetry, +or custom gate output must be converted by its producer before the source reads +it. Do not create a source profile until the canonical file exists or its emit +step is being added in the same authorized change. + +A `host` profile is the exception, and only because the reading application — +not this configuration — owns the fetch. Its provider may read a native report +directly. The engine still normalizes, resolves, and diagnoses every record it +returns, so a host provider gets no record past a check a canonical file must +pass. Which providers resolve depends on who reads the repo; one that is not +registered is reported as a diagnostic rather than read as nothing. A local-folder profile reads one file. A GitHub Actions profile may select several uploaded artifacts from one workflow run; every matching @@ -329,6 +350,7 @@ Follow this sequence. Do not ask the user to choose a parser or config shape. GitHub Actions metadata comes from `GITHUB_SHA`, `GITHUB_REF_NAME`, and `GITHUB_RUN_ID`. Outside GitHub Actions, supply `--commit`; `--branch`, `--run-id`, `--run-url`, and `--observed-at` are optional. + 4. **Schema-validate before upload.** ```bash @@ -346,9 +368,10 @@ Follow this sequence. Do not ask the user to choose a parser or config shape. every one uses the same contract. Raw native reports may remain alongside the canonical file for diagnosis; the quality engine never parses them. 6. **Configure the transport.** Copy - `assets/observation-sources.template.yaml`. Set `transport`, - `observation_path`, and either `github` or `local_folder`. Source - configuration contains no parser list or format selection. + `assets/observation-sources.template.yaml`. Set `transport`, then either + `observation_path` plus `github` or `local_folder` for a file transport, or + `host.provider` for a host transport. File-transport configuration contains + no parser list or format selection. 7. **Add the profile to an observation set.** 8. **Run `assess`.** Verify source acquisition first, then verify every observation resolves to the intended evidence identity. Use the engine's diff --git a/apps/explorer/src/app/api/quality-explorer/evidence-file/[...ref]/route.ts b/apps/explorer/src/app/api/quality-explorer/evidence-file/[...ref]/route.ts new file mode 100644 index 0000000..adcdcf7 --- /dev/null +++ b/apps/explorer/src/app/api/quality-explorer/evidence-file/[...ref]/route.ts @@ -0,0 +1,215 @@ +import { createReadStream, realpathSync, statSync } from "node:fs"; +import path from "node:path"; +import { Readable } from "node:stream"; +import { requireQcSession } from "@/lib/quality-explorer/require-session"; +import { problemResponse as problem } from "@/lib/quality-explorer/route-problem"; +import { qualityProjectRoot } from "@/lib/quality-explorer/project-root"; + +export const runtime = "nodejs"; + +// Serves a run-evidence file out of the opened project so a reviewer can open +// the report the producer already wrote. +// +// The path is in the URL rather than a query parameter on purpose. A Playwright +// HTML report inlines its own summary but fetches attachments from a sibling +// `data/` directory using RELATIVE urls, so the report only works if its own +// address has the same shape as its folder — `.../evidence-file/report/index.html` +// resolves `data/x.webm` to `.../evidence-file/report/data/x.webm`, while a +// `?ref=` query would resolve it against the route and 404. +// +// Trust boundary: this serves files from the local project the user themselves +// opened, on a loopback-bound single-user dev server, and only the extensions +// below. `requireQcSession` is a no-op in Quality Explorer, so LOOPBACK IS THE +// ONLY BOUNDARY: served behind `--hostname 0.0.0.0`, or port-forwarded out of a +// container, this becomes a read-the-project endpoint for anyone who can reach +// it. A host that binds anywhere else must put real authentication in front of +// this route. It is not a general static file server — an unlisted extension is +// refused rather than guessed at, which is what keeps a checked-in `.command` +// or `.sh` in a scanned repo from ever being handed to a browser. +const CONTENT_TYPES: Readonly> = { + ".html": "text/html; charset=utf-8", + ".htm": "text/html; charset=utf-8", + ".css": "text/css; charset=utf-8", + ".js": "text/javascript; charset=utf-8", + ".json": "application/json; charset=utf-8", + ".txt": "text/plain; charset=utf-8", + ".log": "text/plain; charset=utf-8", + ".md": "text/plain; charset=utf-8", + ".csv": "text/csv; charset=utf-8", + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".gif": "image/gif", + ".webp": "image/webp", + ".svg": "image/svg+xml", + ".ico": "image/x-icon", + ".webm": "video/webm", + ".mp4": "video/mp4", + ".mov": "video/quicktime", + ".ogg": "audio/ogg", + ".mp3": "audio/mpeg", + ".pdf": "application/pdf", + ".zip": "application/zip", + ".woff": "font/woff", + ".woff2": "font/woff2", + ".ttf": "font/ttf", +}; + +// `.js` and `.css` are here because a report is a web page that needs its own +// assets: Playwright's trace viewer ships as `trace/*.js` and `trace/*.css` +// beside the report, and dropping them would leave the trace — the most useful +// evidence a run produces — unopenable. +// +// The cost, accepted knowingly: any `.js` anywhere in the scanned project is +// reachable through this route as same-origin script, and the document CSP +// allows `script-src 'self'`, so a hostile report could load it. Serving them +// as downloads instead would break the viewer, and confining them to a report +// subdirectory would need configuration the profile does not carry. The +// boundary stays the one above — a project you would run tests from — and this +// is the sharpest edge of it. +const MAX_BYTES = 512 * 1024 * 1024; + +// Served HTML and SVG come from a scanned repo, which is not a trusted author, +// and they run on this application's own origin — so without a policy they +// could call its API and post what they read to a remote host. +// +// `sandbox` is NOT the answer here: an opaque origin turns the report's own +// sibling requests into cross-origin ones, breaking the trace and video that +// are the whole point, and the CORS header needed to restore them would open +// this local file server to every site the viewer has open. +// +// So the origin is kept and the cheap exfiltration routes are cut instead. +// `'self'` lets the report load its own assets and fetch its own `data/` +// directory, while every remote destination for fetch, XHR, beacon, image, +// script and form post is refused. `unsafe-inline`/`unsafe-eval` are required +// by the report's own bundle, and withholding them would only break honest +// reports. +// +// This is a reduction, NOT containment, and the difference matters: CSP cannot +// stop a top-level navigation, since the `navigate-to` directive that would +// have was dropped from the spec. A hostile page in a scanned repo can still +// read what this origin serves and then put it in a URL it navigates to. +// Closing that needs a separate origin for evidence, which one loopback dev +// server cannot provide — so the real boundary remains "only open a project you +// would run tests from". +const DOCUMENT_CSP = [ + "default-src 'self'", + "script-src 'self' 'unsafe-inline' 'unsafe-eval'", + "style-src 'self' 'unsafe-inline'", + "img-src 'self' data: blob:", + "media-src 'self' data: blob:", + "font-src 'self' data:", + "connect-src 'self'", + // `'self'`, not `'none'`: Playwright's trace viewer replays a captured page + // by framing it, so refusing frames outright would blank every snapshot pane + // in the viewer this route allows `.js` in order to serve. Remote framing + // stays blocked, which is the part that carries a URL somewhere else. + "frame-src 'self'", + "worker-src 'self'", + "form-action 'none'", + "frame-ancestors 'none'", + "base-uri 'none'", + "object-src 'none'" +].join("; "); + +const SCRIPTABLE_TYPES = new Set([".html", ".htm", ".svg"]); + +// `relative === ".."` or a leading `../` segment. A prefix test would also +// reject a contained directory literally named `..cache`, whose relative path +// starts with two dots but never leaves the root. +function escapesRoot(relative: string): boolean { + // `""` means the request resolved to the project root itself. Downstream the + // extension lookup and the isFile() check both refuse it, but containment + // should not depend on guards that follow it — reorder those and the hole + // opens silently. + return ( + relative === "" || + relative === ".." || + relative.startsWith(`..${path.sep}`) || + path.isAbsolute(relative) + ); +} + +export async function GET( + _request: Request, + context: { params: Promise<{ ref: readonly string[] }> }, +): Promise { + const unauthorized = await requireQcSession(); + if (unauthorized) return unauthorized; + + const { ref } = await context.params; + if (!Array.isArray(ref) || ref.length === 0) { + return problem(400, "invalid-evidence-ref", "An evidence file path is required."); + } + + const projectRoot = qualityProjectRoot(); + // Segments are used AS GIVEN. Next has already percent-decoded each one, so + // decoding again is not a no-op on a name that legitimately contains `%`: a + // Playwright artifact folder named after a test title like `100% progress` + // arrives correctly decoded and a second pass throws, while a file actually + // named `a%20b.png` silently resolves to `a b.png` — a different file. + const relative = ref.join("/"); + + // Containment, not sanitisation: resolve first, then require the result to sit + // under the project root. Stripping `..` textually misses encoded forms; + // comparing the resolved path does not. + // + // `path.resolve` is purely lexical, so it is not enough on its own: a repo + // containing `report -> /Users/someone` would pass a lexical check and then + // serve files from outside the project. Both the real path AND the project + // root are realpath'd before comparison, so a symlinked checkout still + // resolves against its own real location rather than failing spuriously. + const resolved = path.resolve(projectRoot, relative); + let realResolved: string; + let realRoot: string; + try { + realRoot = realpathSync(projectRoot); + realResolved = realpathSync(resolved); + } catch { + return problem(404, "evidence-ref-not-found", "That evidence file could not be read."); + } + + const contained = path.relative(realRoot, realResolved); + if (escapesRoot(contained)) { + return problem(403, "evidence-ref-outside-project", "That path is outside the opened project."); + } + + const contentType = CONTENT_TYPES[path.extname(realResolved).toLowerCase()]; + if (contentType === undefined) { + return problem( + 415, + "evidence-ref-unsupported-type", + "Quality Explorer does not serve that file type as run evidence.", + ); + } + + let size: number; + try { + const stats = statSync(realResolved); + if (!stats.isFile()) { + return problem(404, "evidence-ref-not-found", "That evidence file could not be read."); + } + size = stats.size; + } catch { + return problem(404, "evidence-ref-not-found", "That evidence file could not be read."); + } + + if (size > MAX_BYTES) { + return problem(413, "evidence-ref-too-large", "That evidence file is too large to serve."); + } + + const stream = Readable.toWeb(createReadStream(realResolved)) as ReadableStream; + return new Response(stream, { + headers: { + "content-type": contentType, + "content-length": String(size), + // The declared type is the one that was matched from the extension, so + // sniffing could only ever disagree with it. + "x-content-type-options": "nosniff", + "cache-control": "no-store", + ...(SCRIPTABLE_TYPES.has(path.extname(realResolved).toLowerCase()) + ? { "content-security-policy": DOCUMENT_CSP } + : {}), + }, + }); +} diff --git a/apps/explorer/src/app/api/quality-explorer/evidence-file/route.test.ts b/apps/explorer/src/app/api/quality-explorer/evidence-file/route.test.ts new file mode 100644 index 0000000..9b70df7 --- /dev/null +++ b/apps/explorer/src/app/api/quality-explorer/evidence-file/route.test.ts @@ -0,0 +1,139 @@ +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// The evidence-file route serves run-evidence out of the opened project. Its +// whole job is deciding what NOT to serve, so the cases below are mostly about +// refusal: a path that escapes the project, a type that is not display +// evidence, and names that percent-decoding can quietly turn into other files. + +let root: string; + +function projectFile(relative: string, contents: string): void { + const target = path.join(root, relative); + mkdirSync(path.dirname(target), { recursive: true }); + writeFileSync(target, contents, "utf8"); +} + +async function get(...segments: readonly string[]): Promise { + const { GET } = await import("./[...ref]/route"); + return GET(new Request("http://127.0.0.1/evidence"), { + params: Promise.resolve({ ref: segments }), + }); +} + +beforeEach(() => { + root = mkdtempSync(path.join(tmpdir(), "qc-evidence-route-")); + process.env.QUALITY_PROJECT_ROOT = root; + // project-root caches its resolution per module instance. + vi.resetModules(); +}); + +afterEach(() => { + rmSync(root, { force: true, recursive: true }); + delete process.env.QUALITY_PROJECT_ROOT; +}); + +describe("evidence-file route", () => { + it("serves a report with its content type and a document CSP", async () => { + projectFile("playwright-report/index.html", "

report

"); + + const response = await get("playwright-report", "index.html"); + + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toContain("text/html"); + expect(response.headers.get("x-content-type-options")).toBe("nosniff"); + // Served HTML comes from a scanned repo, which is not a trusted author. + expect(response.headers.get("content-security-policy")).toContain("default-src 'self'"); + expect(await response.text()).toBe("

report

"); + }); + + it("does not send a document CSP on a non-scriptable asset", async () => { + projectFile("playwright-report/data/video.webm", "binary"); + + const response = await get("playwright-report", "data", "video.webm"); + + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe("video/webm"); + expect(response.headers.get("content-security-policy")).toBeNull(); + }); + + it("uses path segments as given rather than decoding them a second time", async () => { + // Next has already percent-decoded each segment. Decoding again is not a + // no-op on a name that legitimately contains `%`: this file would resolve + // to `a b.txt` instead, and a name like `100% progress` would throw. + projectFile("probe/a%20b.txt", "literal-percent-name"); + projectFile("probe/a b.txt", "space-name"); + + expect(await (await get("probe", "a%20b.txt")).text()).toBe("literal-percent-name"); + expect(await (await get("probe", "a b.txt")).text()).toBe("space-name"); + }); + + it("serves a name containing a percent sign rather than failing to decode it", async () => { + // Playwright names artifact folders after the test title, so `%` in a title + // reaches the filesystem. + projectFile("probe/100% progress.txt", "percent-in-name"); + + const response = await get("probe", "100% progress.txt"); + + expect(response.status).toBe(200); + expect(await response.text()).toBe("percent-in-name"); + }); + + it("refuses a path that escapes the project root", async () => { + // Pointed at a file that really exists outside the root, so the refusal is + // the containment check rather than the target simply being absent. + const outside = mkdtempSync(path.join(tmpdir(), "qc-evidence-outside-")); + writeFileSync(path.join(outside, "secret.txt"), "secret", "utf8"); + + try { + const response = await get("..", path.basename(outside), "secret.txt"); + expect(response.status).toBe(403); + } finally { + rmSync(outside, { force: true, recursive: true }); + } + }); + + it("reports an escape to a path that does not exist as not found", async () => { + expect((await get("..", "..", "nope", "absent.txt")).status).toBe(404); + }); + + it("refuses a symlink that resolves outside the project root", async () => { + // Lexical containment cannot see this: `path.resolve` never follows links, + // so the check has to compare real paths. + const outside = mkdtempSync(path.join(tmpdir(), "qc-evidence-outside-")); + writeFileSync(path.join(outside, "secret.txt"), "secret", "utf8"); + symlinkSync(outside, path.join(root, "escape")); + + try { + expect((await get("escape", "secret.txt")).status).toBe(403); + } finally { + rmSync(outside, { force: true, recursive: true }); + } + }); + + it("refuses a file type that is not display evidence", async () => { + // An allowlist, so a checked-in `.command` or `.sh` in a scanned repo is + // never handed to a browser. + projectFile("scripts/run.sh", "#!/bin/sh\necho hi"); + + expect((await get("scripts", "run.sh")).status).toBe(415); + }); + + it("reports a missing file as not found", async () => { + expect((await get("playwright-report", "absent.html")).status).toBe(404); + }); + + it("refuses a request that resolves to the project root itself", async () => { + // `path.relative(root, root)` is "", which must be treated as an escape by + // the containment check rather than by the type and isFile guards after it. + expect((await get(".")).status).toBe(403); + }); + + it("refuses a directory", async () => { + mkdirSync(path.join(root, "playwright-report"), { recursive: true }); + + expect((await get("playwright-report")).status).toBe(415); + }); +}); diff --git a/apps/explorer/src/app/quality-explorer/layout.tsx b/apps/explorer/src/app/quality-explorer/layout.tsx index da90861..f325de1 100644 --- a/apps/explorer/src/app/quality-explorer/layout.tsx +++ b/apps/explorer/src/app/quality-explorer/layout.tsx @@ -15,6 +15,9 @@ const host: QcUiHost = { routeBase: "/quality-explorer", apiBase: "/api/quality-explorer", setProject: setQcProjectAction, + // This host reads the project from the local filesystem, so it can serve the + // report a run-evidence ref points at. See the evidence-file route. + servesEvidenceFiles: true, }; export default function QualityExplorerLayout({ diff --git a/apps/explorer/src/lib/quality-explorer/data-access/local-fs.ts b/apps/explorer/src/lib/quality-explorer/data-access/local-fs.ts index 6525857..646de17 100644 --- a/apps/explorer/src/lib/quality-explorer/data-access/local-fs.ts +++ b/apps/explorer/src/lib/quality-explorer/data-access/local-fs.ts @@ -7,9 +7,22 @@ import { readMarkdownArtifactOp, scanOp, } from "@shiplightai/quality-core/operations"; +import { + createLocalReportsTransport, + LOCAL_REPORTS_PROVIDER, +} from "@shiplightai/quality-core"; import type { QcDataAccess } from "./types"; import { qualityProjectRoot } from "../project-root"; +// The host transports this application serves. Registration is deliberately the +// host's act, not the engine's: a repo can declare any provider, and which ones +// actually resolve is a property of who is reading the repo. Quality Explorer +// serves the bundled local-reports provider and nothing else — a profile naming +// a platform provider gets an explicit diagnostic here rather than silence. +const hostTransports = { + [LOCAL_REPORTS_PROVIDER]: createLocalReportsTransport(), +}; + // Draft/preview/publish sync has no meaning in local `qc` mode (no origin, no PR flow) — // mirror the box's `qc-sync-unavailable` 400 so routes surface it consistently. const syncUnavailable = (): never => { @@ -35,8 +48,10 @@ export const localFsDataAccess: QcDataAccess = { saveObservationSets: () => readOnly(), saveObservationSources: () => readOnly(), saveViews: () => readOnly(), - executeObservationSet: (input) => executeObservationSetOp({ ...input, projectPath: qualityProjectRoot() }), - executeObservationSource: (input) => executeObservationSourceOp({ ...input, projectPath: qualityProjectRoot() }), + executeObservationSet: (input) => + executeObservationSetOp({ ...input, projectPath: qualityProjectRoot(), hostTransports }), + executeObservationSource: (input) => + executeObservationSourceOp({ ...input, projectPath: qualityProjectRoot(), hostTransports }), syncStatus: () => syncUnavailable(), pull: () => syncUnavailable(), publish: () => syncUnavailable(), diff --git a/docs/how-to/inspect-in-the-browser.md b/docs/how-to/inspect-in-the-browser.md index 235f64c..16a707d 100644 --- a/docs/how-to/inspect-in-the-browser.md +++ b/docs/how-to/inspect-in-the-browser.md @@ -36,6 +36,19 @@ Quality Explorer can: - Show matched, unmatched, and ambiguous runtime results. - Generate text instructions that you can give to a coding agent. +## Bind it to loopback only + +Quality Explorer has no sign-in: it is a single-user local tool, and the +loopback address is the whole access boundary. It serves run-evidence files out +of the opened project — reports, videos, traces — to any caller that can reach +it. + +Do not run it with `--hostname 0.0.0.0`, and do not port-forward it out of a +container or put it behind a reverse proxy. Doing either turns it into an +endpoint that hands the opened project's files to anyone on the network. A +deployment that needs to be reachable must put real authentication in front of +it. + ## Read-only boundaries Quality Explorer does not edit the selected repository. Actions such as diff --git a/docs/how-to/make-ci-results-count.md b/docs/how-to/make-ci-results-count.md index 8f45c8c..df9d86a 100644 --- a/docs/how-to/make-ci-results-count.md +++ b/docs/how-to/make-ci-results-count.md @@ -36,6 +36,40 @@ Without that permission, the agent should propose the workflow change and stop. This work serializes a result that already happened. It must not change the test command, retry behavior, gate, or status. +## Link the run evidence (optional) + +A result can carry a pointer to what the run left behind, so a reviewer opening +a check can see what the test actually did rather than only whether it passed: + +```json +{ + "path": "tests/e2e/checkout.test.yaml", + "test_case": "guest can pay", + "status": "pass", + "artifacts": [ + { "ref": "https://app.example.test/runs/8412?test=99231", "label": "Run 8412" } + ] +} +``` + +Point at the report a person already knows how to read — the run page, the HTML +report the runner wrote. Quality links to it and stops there; it is an index +from checks to evidence, not a viewer, and it will not enumerate videos and +screenshots for you. + +`ref` is opaque: Quality records and displays it, and never parses or resolves +it. An absolute `http(s)` ref is linked as it stands. Anything else is read as a +path inside the project, which only a reader holding that project can open — so +a local report path shows as a link in Quality Explorer and as plain text in a +hosted reader. + +This is additive. A result without it counts exactly the same; only the link is +missing. And a malformed pointer never costs you the result it was attached to: +it is reported and dropped, and the observed status stands. + +Do not confuse these with the workflow artifacts that carry the canonical file +itself. These are pointers inside it. + ## Decision that remains yours You decide whether the workflow may be edited and which result sources belong in diff --git a/packages/core/package.json b/packages/core/package.json index 354cee4..e7910fc 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -12,26 +12,95 @@ "import": "./dist/index.js", "default": "./dist/index.js" }, - "./analytics": { "types": "./dist/analytics.d.ts", "import": "./dist/analytics.js", "default": "./dist/analytics.js" }, - "./evidence-view": { "types": "./dist/evidence-view.d.ts", "import": "./dist/evidence-view.js", "default": "./dist/evidence-view.js" }, - "./fix-prompts": { "types": "./dist/fix-prompts.d.ts", "import": "./dist/fix-prompts.js", "default": "./dist/fix-prompts.js" }, - "./observation-sets": { "types": "./dist/observation-sets.d.ts", "import": "./dist/observation-sets.js", "default": "./dist/observation-sets.js" }, - "./gap-triage": { "types": "./dist/gap-triage.d.ts", "import": "./dist/gap-triage.js", "default": "./dist/gap-triage.js" }, - "./observation-sources": { "types": "./dist/observation-sources.d.ts", "import": "./dist/observation-sources.js", "default": "./dist/observation-sources.js" }, - "./observations": { "types": "./dist/observations.d.ts", "import": "./dist/observations.js", "default": "./dist/observations.js" }, - "./operations": { "types": "./dist/operations.d.ts", "import": "./dist/operations.js", "default": "./dist/operations.js" }, - "./recommendation-export": { "types": "./dist/recommendation-export.d.ts", "import": "./dist/recommendation-export.js", "default": "./dist/recommendation-export.js" }, - "./owner-view": { "types": "./dist/owner-view.d.ts", "import": "./dist/owner-view.js", "default": "./dist/owner-view.js" }, - "./project-map": { "types": "./dist/project-map.d.ts", "import": "./dist/project-map.js", "default": "./dist/project-map.js" }, - "./project-index": { "types": "./dist/project-index.d.ts", "import": "./dist/project-index.js", "default": "./dist/project-index.js" }, - "./priority": { "types": "./dist/priority.d.ts", "import": "./dist/priority.js", "default": "./dist/priority.js" }, - "./assessment": { "types": "./dist/assessment.d.ts", "import": "./dist/assessment.js", "default": "./dist/assessment.js" }, - "./views": { "types": "./dist/views.d.ts", "import": "./dist/views.js", "default": "./dist/views.js" }, - "./views/server": { "types": "./dist/views-server.d.ts", "import": "./dist/views-server.js", "default": "./dist/views-server.js" }, - "./workspace": { "types": "./dist/workspace.d.ts", "import": "./dist/workspace.js", "default": "./dist/workspace.js" }, + "./analytics": { + "types": "./dist/analytics.d.ts", + "import": "./dist/analytics.js", + "default": "./dist/analytics.js" + }, + "./evidence-view": { + "types": "./dist/evidence-view.d.ts", + "import": "./dist/evidence-view.js", + "default": "./dist/evidence-view.js" + }, + "./fix-prompts": { + "types": "./dist/fix-prompts.d.ts", + "import": "./dist/fix-prompts.js", + "default": "./dist/fix-prompts.js" + }, + "./observation-sets": { + "types": "./dist/observation-sets.d.ts", + "import": "./dist/observation-sets.js", + "default": "./dist/observation-sets.js" + }, + "./gap-triage": { + "types": "./dist/gap-triage.d.ts", + "import": "./dist/gap-triage.js", + "default": "./dist/gap-triage.js" + }, + "./observation-sources": { + "types": "./dist/observation-sources.d.ts", + "import": "./dist/observation-sources.js", + "default": "./dist/observation-sources.js" + }, + "./observations": { + "types": "./dist/observations.d.ts", + "import": "./dist/observations.js", + "default": "./dist/observations.js" + }, + "./operations": { + "types": "./dist/operations.d.ts", + "import": "./dist/operations.js", + "default": "./dist/operations.js" + }, + "./recommendation-export": { + "types": "./dist/recommendation-export.d.ts", + "import": "./dist/recommendation-export.js", + "default": "./dist/recommendation-export.js" + }, + "./owner-view": { + "types": "./dist/owner-view.d.ts", + "import": "./dist/owner-view.js", + "default": "./dist/owner-view.js" + }, + "./project-map": { + "types": "./dist/project-map.d.ts", + "import": "./dist/project-map.js", + "default": "./dist/project-map.js" + }, + "./project-index": { + "types": "./dist/project-index.d.ts", + "import": "./dist/project-index.js", + "default": "./dist/project-index.js" + }, + "./priority": { + "types": "./dist/priority.d.ts", + "import": "./dist/priority.js", + "default": "./dist/priority.js" + }, + "./assessment": { + "types": "./dist/assessment.d.ts", + "import": "./dist/assessment.js", + "default": "./dist/assessment.js" + }, + "./views": { + "types": "./dist/views.d.ts", + "import": "./dist/views.js", + "default": "./dist/views.js" + }, + "./views/server": { + "types": "./dist/views-server.d.ts", + "import": "./dist/views-server.js", + "default": "./dist/views-server.js" + }, + "./workspace": { + "types": "./dist/workspace.d.ts", + "import": "./dist/workspace.js", + "default": "./dist/workspace.js" + }, "./observation-sets.schema.json": "./dist/observation-sets.schema.json", "./observation-sources.schema.json": "./dist/observation-sources.schema.json", "./quality-observations.schema.json": "./dist/quality-observations.schema.json", + "./views.schema.json": "./dist/views.schema.json", "./package.json": "./package.json" }, "files": [ diff --git a/packages/core/src/observation-sets/execute.ts b/packages/core/src/observation-sets/execute.ts index 536631f..bbc4e45 100644 --- a/packages/core/src/observation-sets/execute.ts +++ b/packages/core/src/observation-sets/execute.ts @@ -1,6 +1,7 @@ import { createDiagnostic } from "../diagnostics/diagnostic"; import { executeObservationSourceProfile, + type HostObservationTransportRegistry, type ObservationSourceExecutionSelection, type ObservationSourceProfile } from "../observation-sources"; @@ -21,6 +22,7 @@ interface ExecuteObservationSetInput { readonly env?: NodeJS.ProcessEnv; readonly selection?: ObservationSetExecutionSelection; readonly fetchImpl?: typeof fetch; + readonly hostTransports?: HostObservationTransportRegistry; } function statusFor( @@ -148,6 +150,7 @@ export async function executeObservationSet( projectRoot: input.projectRoot, env: input.env, fetchImpl: input.fetchImpl, + hostTransports: input.hostTransports, selection: profileSelection({ globalSelection: input.selection, profileOverride: selectionMap.overrides.get(profile.id) @@ -177,7 +180,12 @@ export async function executeObservationSet( return { setId: input.observationSet.id, setName: input.observationSet.name, - status: statusFor(merged.observations.length, allDiagnostics.length), + // Same rule as a single source execution: info-severity notes are context, + // not a partial read. + status: statusFor( + merged.observations.length, + allDiagnostics.filter((entry) => entry.severity !== "info").length + ), profiles: profileResults, observations: merged.observations, diagnostics: allDiagnostics, diff --git a/packages/core/src/observation-sets/index.ts b/packages/core/src/observation-sets/index.ts index 93e592d..08b044d 100644 --- a/packages/core/src/observation-sets/index.ts +++ b/packages/core/src/observation-sets/index.ts @@ -1,4 +1,5 @@ export * from "./types"; export * from "./parse"; +export * from "./json-schema"; export * from "./serialize"; export * from "./execute"; diff --git a/packages/core/src/observation-sets/json-schema.ts b/packages/core/src/observation-sets/json-schema.ts new file mode 100644 index 0000000..9d8c25c --- /dev/null +++ b/packages/core/src/observation-sets/json-schema.ts @@ -0,0 +1,67 @@ +// Emitted from this package's own contract constants, like the observation +// manifest and source-profile schemas, so the published schema and the parser +// that enforces it cannot disagree. The checked-in JSON beside this module is +// generated from here and asserted equal in tests. +import { RESERVED_OBSERVATION_SET_ID } from "./types"; + +const nonEmptyString = { $ref: "#/definitions/nonEmptyString" } as const; + +// Matched case-insensitively because YAML authors write ids in whatever case +// they like, and the parser lowercases before comparing. +function caseInsensitivePattern(value: string): string { + return `^${[...value].map((c) => `[${c.toUpperCase()}${c.toLowerCase()}]`).join("")}$`; +} + +export function buildObservationSetsJsonSchema(): Record { + return { + $schema: "http://json-schema.org/draft-07/schema#", + $id: "https://shiplight.dev/schemas/quality/observation-sets.schema.json", + title: "Quality Observation Sets", + type: "object", + additionalProperties: false, + required: ["observation_sets"], + properties: { + observation_sets: { + type: "array", + minItems: 1, + items: { $ref: "#/definitions/observationSet" } + } + }, + definitions: { + nonEmptyString: { type: "string", minLength: 1 }, + profileReference: { + type: "object", + additionalProperties: false, + required: ["profile_id"], + properties: { profile_id: nonEmptyString } + }, + observationSet: { + type: "object", + additionalProperties: false, + required: ["id", "name", "profiles"], + properties: { + id: { + allOf: [ + nonEmptyString, + // `static` names the assessment that has no runtime observations + // at all, so a set claiming it would shadow that scope. + { not: { pattern: caseInsensitivePattern(RESERVED_OBSERVATION_SET_ID) } } + ] + }, + name: nonEmptyString, + description: nonEmptyString, + profiles: { + type: "array", + minItems: 1, + uniqueItems: true, + items: { $ref: "#/definitions/profileReference" } + } + } + } + } + }; +} + +export function serializeObservationSetsJsonSchema(): string { + return `${JSON.stringify(buildObservationSetsJsonSchema(), null, 2)}\n`; +} diff --git a/packages/core/src/observation-sets/observation-sets.schema.json b/packages/core/src/observation-sets/observation-sets.schema.json index 9f1b53d..5cacefa 100644 --- a/packages/core/src/observation-sets/observation-sets.schema.json +++ b/packages/core/src/observation-sets/observation-sets.schema.json @@ -4,7 +4,9 @@ "title": "Quality Observation Sets", "type": "object", "additionalProperties": false, - "required": ["observation_sets"], + "required": [ + "observation_sets" + ], "properties": { "observation_sets": { "type": "array", @@ -22,29 +24,49 @@ "profileReference": { "type": "object", "additionalProperties": false, - "required": ["profile_id"], + "required": [ + "profile_id" + ], "properties": { - "profile_id": { "$ref": "#/definitions/nonEmptyString" } + "profile_id": { + "$ref": "#/definitions/nonEmptyString" + } } }, "observationSet": { "type": "object", "additionalProperties": false, - "required": ["id", "name", "profiles"], + "required": [ + "id", + "name", + "profiles" + ], "properties": { "id": { "allOf": [ - { "$ref": "#/definitions/nonEmptyString" }, - { "not": { "pattern": "^[Ss][Tt][Aa][Tt][Ii][Cc]$" } } + { + "$ref": "#/definitions/nonEmptyString" + }, + { + "not": { + "pattern": "^[Ss][Tt][Aa][Tt][Ii][Cc]$" + } + } ] }, - "name": { "$ref": "#/definitions/nonEmptyString" }, - "description": { "$ref": "#/definitions/nonEmptyString" }, + "name": { + "$ref": "#/definitions/nonEmptyString" + }, + "description": { + "$ref": "#/definitions/nonEmptyString" + }, "profiles": { "type": "array", "minItems": 1, "uniqueItems": true, - "items": { "$ref": "#/definitions/profileReference" } + "items": { + "$ref": "#/definitions/profileReference" + } } } } diff --git a/packages/core/src/observation-sets/parse.ts b/packages/core/src/observation-sets/parse.ts index e7bc5ff..3833595 100644 --- a/packages/core/src/observation-sets/parse.ts +++ b/packages/core/src/observation-sets/parse.ts @@ -1,5 +1,6 @@ import { readFileSync } from "node:fs"; import { parseDocument } from "yaml"; +import { RESERVED_OBSERVATION_SET_ID } from "./types"; import type { ObservationSet, ObservationSetDiagnostic, @@ -128,12 +129,12 @@ function observationSetFrom( return undefined; } - if (id.toLowerCase() === "static") { + if (id.toLowerCase() === RESERVED_OBSERVATION_SET_ID) { diagnostics.push( diagnostic(source, { severity: "error", code: "RESERVED_OBSERVATION_SET_ID", - message: "Observation set id static is reserved for assessments without runtime observations.", + message: `Observation set id ${RESERVED_OBSERVATION_SET_ID} is reserved for assessments without runtime observations.`, yamlPath: `$.observation_sets[${index}].id` }) ); diff --git a/packages/core/src/observation-sets/types.ts b/packages/core/src/observation-sets/types.ts index b3ef489..4aa7016 100644 --- a/packages/core/src/observation-sets/types.ts +++ b/packages/core/src/observation-sets/types.ts @@ -1,3 +1,8 @@ +// The id `static` names the assessment that ran with no runtime observations, +// so no saved set may claim it. Shared with the JSON Schema so the parser and +// the published contract reserve exactly the same name. +export const RESERVED_OBSERVATION_SET_ID = "static"; + export type ObservationSetDiagnosticSeverity = "error" | "warning" | "info"; export type ObservationSetParseStatus = "parsed" | "invalid"; diff --git a/packages/core/src/observation-sources/execute.ts b/packages/core/src/observation-sources/execute.ts index 70b196b..1fb7084 100644 --- a/packages/core/src/observation-sources/execute.ts +++ b/packages/core/src/observation-sources/execute.ts @@ -4,8 +4,10 @@ import AdmZip from "adm-zip"; import { createDiagnostic } from "../diagnostics/diagnostic"; import type { ScanDiagnostic } from "../diagnostics/diagnostic"; import { + INTERNAL_OBSERVATION_CONTEXT, ingestObservationManifest, mergeObservationIngestionResults, + normalizeObservationBatches, qualityObservationIdentity, type ObservationIngestionResult } from "../observations"; @@ -13,6 +15,7 @@ import { evaluateObservationSourceProfileEnv } from "./env"; import type { ExecutedObservationSourceArtifact, ExecutedObservationSourceRun, + HostObservationTransportRegistry, ObservationSourceExecutionResult, ObservationSourceExecutionSelection, ObservationSourceProfile, @@ -25,6 +28,7 @@ interface ExecuteObservationSourceProfileInput { readonly env?: NodeJS.ProcessEnv; readonly selection?: ObservationSourceExecutionSelection; readonly fetchImpl?: typeof fetch; + readonly hostTransports?: HostObservationTransportRegistry; } interface DownloadedArtifactEntry { @@ -67,6 +71,14 @@ interface GitHubArtifactsResponse { readonly artifacts?: readonly GitHubArtifactResponse[]; } +// Counts only diagnostics that describe something going wrong. An `info` +// note — "this report records no commit", say — is context for the reader, not +// a degraded read, and reporting it as `partial` would say a run half-failed +// when every record arrived intact. +function countProblems(diagnostics: readonly ScanDiagnostic[]): number { + return diagnostics.filter((entry) => entry.severity !== "info").length; +} + function statusFor(observationCount: number, diagnosticsCount: number): ObservationIngestionResult["status"] { if (observationCount === 0 && diagnosticsCount > 0) { return "invalid"; @@ -128,7 +140,7 @@ function finalizeExecution(input: { profileId: input.profile.id, profileName: input.profile.name, transport: input.profile.transport, - status: statusFor(merged.observations.length, diagnostics.length), + status: statusFor(merged.observations.length, countProblems(diagnostics)), envStatus: input.envStatus, observations: merged.observations, diagnostics, @@ -137,6 +149,85 @@ function finalizeExecution(input: { }; } +async function executeHostProfile( + input: ExecuteObservationSourceProfileInput, + envStatus: ReturnType +): Promise { + const provider = input.profile.host?.provider; + if (provider === undefined) { + return finalizeExecution({ + profile: input.profile, + envStatus, + diagnostics: [ + createDiagnostic({ + severity: "error", + code: "INVALID_OBSERVATION_SOURCE", + message: `Observation source profile ${input.profile.id} uses transport host but declares no host.provider.` + }) + ] + }); + } + + const transport = input.hostTransports?.[provider]; + if (transport === undefined) { + // A repo can legitimately declare a provider this reader cannot serve — the + // OSS CLI reading a config written for the hosted app, say. Say which + // providers ARE registered so the reader can tell "wrong host" from "typo". + const registered = Object.keys(input.hostTransports ?? {}); + return finalizeExecution({ + profile: input.profile, + envStatus, + diagnostics: [ + createDiagnostic({ + severity: "error", + code: "INVALID_OBSERVATION_SOURCE", + message: `Observation source profile ${input.profile.id} needs host provider ${provider}, which this application does not register. Registered providers: ${joinedList(registered)}.` + }) + ] + }); + } + + let result: Awaited>; + try { + result = await transport({ + profile: input.profile, + selection: input.selection, + projectRoot: input.projectRoot, + env: input.env + }); + } catch (error) { + return finalizeExecution({ + profile: input.profile, + envStatus, + diagnostics: [ + createDiagnostic({ + severity: "error", + code: "INVALID_OBSERVATION_SOURCE", + message: `Observation source profile ${input.profile.id} host provider ${provider} failed: ${error instanceof Error ? error.message : String(error)}` + }) + ] + }); + } + + // The host handed us raw records; from here the engine owns everything. It + // normalizes and diagnoses them exactly as it would a parsed manifest, so a + // host transport cannot get a record past a check a file-based one must pass. + const ingestion = normalizeObservationBatches( + result.batches.map((batch) => ({ + ...batch, + context: batch.context ?? INTERNAL_OBSERVATION_CONTEXT + })) + ); + + return finalizeExecution({ + profile: input.profile, + envStatus, + ingestionResults: [ingestion], + diagnostics: result.diagnostics ?? [], + selectedRun: result.selectedRun + }); +} + function missingEnvDiagnostics(profile: ObservationSourceProfile, env: NodeJS.ProcessEnv): readonly ScanDiagnostic[] { const missing = profile.requiredEnv.filter((name) => { const value = env[name]; @@ -205,7 +296,8 @@ function joinedList(values: readonly string[]): string { async function executeLocalFolderProfile( input: ExecuteObservationSourceProfileInput, - envStatus: ReturnType + envStatus: ReturnType, + observationPath: string ): Promise { const diagnostics: ScanDiagnostic[] = []; const ingestionResults: ObservationIngestionResult[] = []; @@ -226,7 +318,7 @@ async function executeLocalFolderProfile( }); } - const resolvedPath = path.resolve(folderRoot, input.profile.observationPath); + const resolvedPath = path.resolve(folderRoot, observationPath); let rawText: string; try { rawText = await readFile(resolvedPath, "utf8"); @@ -235,7 +327,7 @@ async function executeLocalFolderProfile( createDiagnostic({ severity: "warning", code: "MISSING_OBSERVATION_ARTIFACT_MATCH", - message: `Observation source profile ${input.profile.id} could not read canonical observation file ${input.profile.observationPath} under ${folderRoot}: ${error instanceof Error ? error.message : String(error)}` + message: `Observation source profile ${input.profile.id} could not read canonical observation file ${observationPath} under ${folderRoot}: ${error instanceof Error ? error.message : String(error)}` }) ); @@ -249,7 +341,7 @@ async function executeLocalFolderProfile( } artifacts.push({ - declaredObservationPath: input.profile.observationPath, + declaredObservationPath: observationPath, sourcePath: resolvedPath }); ingestionResults.push( @@ -531,7 +623,8 @@ function duplicateObservationIdentityDetails( async function executeGitHubActionsProfile( input: ExecuteObservationSourceProfileInput, - envStatus: ReturnType + envStatus: ReturnType, + observationPath: string ): Promise { const diagnostics: ScanDiagnostic[] = []; const artifacts: ExecutedObservationSourceArtifact[] = []; @@ -562,7 +655,7 @@ async function executeGitHubActionsProfile( } const downloads = await downloadGitHubArtifacts(input.profile, runId, fetchImpl, token); - const matches = matchingDownloadedEntries(downloads, input.profile.observationPath); + const matches = matchingDownloadedEntries(downloads, observationPath); const ambiguityDetails = ambiguousDownloadedMatchDetails( matches, input.profile.github?.artifactNames ?? [] @@ -573,7 +666,7 @@ async function executeGitHubActionsProfile( severity: "warning", code: "MISSING_OBSERVATION_ARTIFACT_MATCH", message: [ - `Observation source profile ${input.profile.id} selected GitHub Actions run ${runId} for workflow ${input.profile.github?.workflow ?? "(unknown workflow)"}, but could not find canonical observation path ${input.profile.observationPath}.`, + `Observation source profile ${input.profile.id} selected GitHub Actions run ${runId} for workflow ${input.profile.github?.workflow ?? "(unknown workflow)"}, but could not find canonical observation path ${observationPath}.`, `Configured artifact_names: ${joinedList(input.profile.github?.artifactNames ?? [])}.`, `Downloaded matching artifacts: ${joinedList(downloads.map((download) => download.name))}.`, `Downloaded artifact entries: ${joinedList(downloads.flatMap((download) => download.entries.map((entry) => `${download.name}/${entry.path}`)))}.` @@ -585,7 +678,7 @@ async function executeGitHubActionsProfile( createDiagnostic({ severity: "warning", code: "AMBIGUOUS_OBSERVATION_ARTIFACT_MATCH", - message: `Observation source profile ${input.profile.id} matched canonical observation path ${input.profile.observationPath} ambiguously: ${ambiguityDetails.join("; ")}.` + message: `Observation source profile ${input.profile.id} matched canonical observation path ${observationPath} ambiguously: ${ambiguityDetails.join("; ")}.` }) ); } else { @@ -610,7 +703,7 @@ async function executeGitHubActionsProfile( for (const match of matches) { artifacts.push({ - declaredObservationPath: input.profile.observationPath, + declaredObservationPath: observationPath, matchedArtifactName: match.artifact.name, matchedObservationPath: match.entry.path }); @@ -702,9 +795,27 @@ export async function executeObservationSourceProfile( }); } + if (input.profile.transport === "host") { + return executeHostProfile(input, envStatus); + } + + if (input.profile.observationPath === undefined) { + return finalizeExecution({ + profile: input.profile, + envStatus, + diagnostics: [ + createDiagnostic({ + severity: "error", + code: "INVALID_OBSERVATION_SOURCE", + message: `Observation source profile ${input.profile.id} has no observation_path, which transport ${input.profile.transport} requires.` + }) + ] + }); + } + if (input.profile.transport === "local-folder") { - return executeLocalFolderProfile(input, envStatus); + return executeLocalFolderProfile(input, envStatus, input.profile.observationPath); } - return executeGitHubActionsProfile(input, envStatus); + return executeGitHubActionsProfile(input, envStatus, input.profile.observationPath); } diff --git a/packages/core/src/observation-sources/index.ts b/packages/core/src/observation-sources/index.ts index 589207e..ebee1fe 100644 --- a/packages/core/src/observation-sources/index.ts +++ b/packages/core/src/observation-sources/index.ts @@ -1,5 +1,7 @@ export * from "./types"; export * from "./parse"; +export * from "./json-schema"; export * from "./env"; export * from "./execute"; +export * from "./local-reports"; export * from "./serialize"; diff --git a/packages/core/src/observation-sources/json-schema.ts b/packages/core/src/observation-sources/json-schema.ts new file mode 100644 index 0000000..2d4abd8 --- /dev/null +++ b/packages/core/src/observation-sources/json-schema.ts @@ -0,0 +1,105 @@ +// Emitted from this package's own contract constants, exactly like the +// canonical observation manifest schema, so the published schema and the parser +// that enforces it cannot disagree. The checked-in JSON file beside this module +// is generated from here and asserted equal in tests. +import { OBSERVATION_SOURCE_TRANSPORTS } from "./types"; + +const nonEmptyString = { $ref: "#/definitions/nonEmptyString" } as const; + +function transportRequires( + transport: string, + required: readonly string[] +): Record { + return { + if: { properties: { transport: { const: transport } } }, + then: { required: [...required] } + }; +} + +export function buildObservationSourceProfilesJsonSchema(): Record { + return { + $schema: "http://json-schema.org/draft-07/schema#", + $id: "https://shiplight.dev/schemas/quality/observation-sources.schema.json", + title: "Quality Observation Source Profiles", + type: "object", + additionalProperties: false, + required: ["profiles"], + properties: { + profiles: { + type: "array", + minItems: 1, + items: { $ref: "#/definitions/profile" } + } + }, + definitions: { + nonEmptyString: { type: "string", minLength: 1 }, + sourceRef: { + type: "object", + additionalProperties: false, + minProperties: 1, + properties: { path: nonEmptyString, url: nonEmptyString, label: nonEmptyString } + }, + auth: { + type: "object", + additionalProperties: false, + properties: { required_env: { type: "array", items: nonEmptyString } } + }, + github: { + type: "object", + additionalProperties: false, + required: ["repo", "workflow", "artifact_names"], + properties: { + repo: nonEmptyString, + workflow: nonEmptyString, + artifact_names: { type: "array", minItems: 1, items: nonEmptyString }, + branch: nonEmptyString + } + }, + localFolder: { + type: "object", + additionalProperties: false, + required: ["path"], + properties: { path: nonEmptyString } + }, + host: { + type: "object", + additionalProperties: false, + required: ["provider"], + properties: { + provider: nonEmptyString, + // Values are strings so config stays a flat, reviewable block rather + // than a place to smuggle structure past a reader. + options: { type: "object", additionalProperties: nonEmptyString } + } + }, + profile: { + type: "object", + additionalProperties: false, + // `observation_path` is required per-transport below, not here: a host + // transport addresses no file. + required: ["id", "name", "transport"], + properties: { + id: nonEmptyString, + name: nonEmptyString, + description: nonEmptyString, + transport: { enum: [...OBSERVATION_SOURCE_TRANSPORTS] }, + observation_path: nonEmptyString, + source_refs: { type: "array", items: { $ref: "#/definitions/sourceRef" } }, + auth: { $ref: "#/definitions/auth" }, + github: { $ref: "#/definitions/github" }, + local_folder: { $ref: "#/definitions/localFolder" }, + host: { $ref: "#/definitions/host" } + }, + allOf: [ + transportRequires("github-actions", ["github", "observation_path"]), + transportRequires("local-folder", ["local_folder", "observation_path"]), + transportRequires("host", ["host"]) + ] + } + } + }; +} + +export function serializeObservationSourceProfilesJsonSchema(): string { + return `${JSON.stringify(buildObservationSourceProfilesJsonSchema(), null, 2)}\n`; +} diff --git a/packages/core/src/observation-sources/local-reports.ts b/packages/core/src/observation-sources/local-reports.ts new file mode 100644 index 0000000..8d699c1 --- /dev/null +++ b/packages/core/src/observation-sources/local-reports.ts @@ -0,0 +1,274 @@ +import { realpathSync } from "node:fs"; +import { readFile } from "node:fs/promises"; +import path from "node:path"; +import { createDiagnostic } from "../diagnostics/diagnostic"; +import type { ScanDiagnostic } from "../diagnostics/diagnostic"; +import { buildPlaywrightObservationBatch, buildShiplightObservationBatch } from "../observations"; +import type { ObservationEvidenceRefInput } from "../observations"; +import type { HostObservationTransport, ObservationSourceProfile } from "./types"; + +/** + * The bundled reference host transport, and the one that makes run evidence + * work with no platform at all: it reads a native test report from the working + * tree and hands the engine its results, pointing each one at the HTML report + * the runner already wrote. + * + * It deliberately does NOT enumerate videos, traces, or screenshots. Quality is + * an index from checks to evidence, not a viewer — the runner's report is + * already the viewer, the reviewer already knows it, and rebuilding a worse one + * inside the quality UI would earn nothing. So the evidence pointer is the + * report, and the report handles presentation. + * + * It is also the worked example every other host transport is written against, + * including the platform ones that live outside this repo. + * + * Registration is still the host's decision. This module only offers the + * factory; nothing here registers itself. + * + * Config: + * + * - id: local-playwright + * name: Local Playwright run + * transport: host + * host: + * provider: local-reports + * options: + * path: playwright-report/report.json # results to read + * report: playwright-report/index.html # what a reviewer opens + * format: playwright-json # optional; see below + * commit: # optional, see below + * + * `format` selects which report the run left behind: + * + * playwright-json Playwright's JSON reporter output. + * shiplight-report A Shiplight YAML run's `report-data.json`, which keys its + * results on the TRANSPILED spec; that adapter maps them + * back to the `.test.yaml` source a quality map can pin. + */ + +export const LOCAL_REPORTS_PROVIDER = "local-reports"; + +const SUPPORTED_FORMATS = new Set(["playwright-json", "shiplight-report"]); + +const REPORT_LABEL = "Test report"; + +function invalid(message: string): ScanDiagnostic { + return createDiagnostic({ severity: "error", code: "INVALID_OBSERVATION_SOURCE", message }); +} + +function missingReport(message: string): ScanDiagnostic { + return createDiagnostic({ + severity: "warning", + code: "MISSING_OBSERVATION_ARTIFACT_MATCH", + message + }); +} + +function resolveOption( + profile: ObservationSourceProfile, + option: "path" | "report", + projectRoot: string | undefined +): { readonly resolved?: string; readonly root?: string; readonly diagnostic?: ScanDiagnostic } { + const declared = profile.host?.options[option]; + if (declared === undefined || declared.length === 0) { + return { + diagnostic: invalid( + `Observation source profile ${profile.id} needs host.options.${option} naming a file under the project.` + ) + }; + } + + if (projectRoot === undefined) { + return { + diagnostic: invalid( + `Observation source profile ${profile.id} needs a project root to resolve the path ${declared}.` + ) + }; + } + + // Contained to the project root on purpose, and containment is checked for an + // ABSOLUTE declaration too. A profile is repo config: letting `/etc/hosts` + // through while refusing `../../etc/hosts` would enforce nothing, since a PR + // can write either. `path.resolve` returns an absolute declaration unchanged, + // so both forms reach the same comparison. + // + // Compared on REAL paths, for the same reason the evidence-file route does: + // `path.resolve` is lexical and never follows links, so a symlink committed + // inside the project — `playwright-report` pointing at a home directory — + // passes a purely textual check and is then read straight through. Both sides + // are realpath'd so a symlinked checkout still resolves against its own real + // location rather than being refused. + const escaped = (root: string, target: string): boolean => { + const relative = path.relative(root, target); + // Segment comparison, not a prefix test: a contained directory literally + // named `..cache` has a relative path starting with two dots and never + // leaves the root. + return relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative); + }; + const outside = (): { readonly diagnostic: ScanDiagnostic } => ({ + diagnostic: invalid( + `Observation source profile ${profile.id} resolves host.options.${option} ${declared} outside the project root.` + ) + }); + + // The lexical check runs ALWAYS, including when the target does not exist — + // `realpathSync` throws on a missing path, and treating that as "nothing to + // contain" would wave through every escape that merely points at no file. + const resolved = path.resolve(projectRoot, declared); + if (escaped(projectRoot, resolved)) { + return outside(); + } + + // Then the real-path check, for the case the lexical one structurally cannot + // see: a symlink committed inside the project — `playwright-report` pointing + // at a home directory — is textually contained and still reads through. Both + // sides are realpath'd so a symlinked checkout resolves against its own real + // location instead of being refused. Skipped when the target does not exist, + // which the lexical check above has already vouched for. + try { + const realRoot = realpathSync(projectRoot); + const realResolved = realpathSync(resolved); + if (escaped(realRoot, realResolved)) { + return outside(); + } + return { resolved: realResolved, root: realRoot }; + } catch { + // The target does not exist, so there is nothing to resolve and nothing to + // read. A dangling symlink whose text sits inside the root reaches here and + // keeps its lexically-checked path — safe, because the caller's `readFile` + // fails on it too, and reporting an unreadable file is a better message + // than a containment complaint about a path that resolves to nothing. + // + // For `report` this also becomes an evidence ref. A dangling symlink could + // later be completed to point outside the project, so the ref alone is not + // a containment guarantee — it is safe because the route that serves it + // re-checks containment against real paths at request time, rather than + // trusting a ref recorded earlier. + return { resolved, root: projectRoot }; + } +} + +// The one evidence pointer this transport produces: the report a reviewer +// opens. It is attached to every observation the run produced, because that is +// what it is — one report describing the whole run. Quality links to it and +// stops there; the report is the viewer. +function reportRef( + profile: ObservationSourceProfile, + projectRoot: string | undefined +): { readonly refs: readonly ObservationEvidenceRefInput[]; readonly diagnostics: readonly ScanDiagnostic[] } { + const declared = profile.host?.options.report; + if (declared === undefined || declared.length === 0) { + return { refs: [], diagnostics: [] }; + } + + if (/^https?:\/\//i.test(declared)) { + return { refs: [{ ref: declared, label: REPORT_LABEL }], diagnostics: [] }; + } + + const location = resolveOption(profile, "report", projectRoot); + if (location.resolved === undefined) { + return { refs: [], diagnostics: location.diagnostic === undefined ? [] : [location.diagnostic] }; + } + + // Relative to the ROOT `resolveOption` actually resolved against, not the one + // passed in. It realpaths the target, and relativizing a real path against a + // symlinked root yields an escaping `../../` chain — which then normalises + // away in the browser and 404s. Anywhere the root traverses a link (macOS + // `/var`, a symlinked checkout, a container workdir) this is not hypothetical. + const ref = path.relative(location.root ?? projectRoot ?? "", location.resolved); + return { refs: [{ ref: ref.replaceAll("\\", "/"), label: REPORT_LABEL }], diagnostics: [] }; +} + +export function createLocalReportsTransport(): HostObservationTransport { + return async ({ profile, projectRoot }) => { + const format = profile.host?.options.format ?? "playwright-json"; + if (!SUPPORTED_FORMATS.has(format)) { + return { + batches: [], + diagnostics: [ + invalid( + `Observation source profile ${profile.id} asks for report format ${format}; ${LOCAL_REPORTS_PROVIDER} supports ${[...SUPPORTED_FORMATS].join(", ")}.` + ) + ] + }; + } + + const location = resolveOption(profile, "path", projectRoot); + if (location.resolved === undefined) { + return { batches: [], diagnostics: location.diagnostic === undefined ? [] : [location.diagnostic] }; + } + + let reportJson: string; + try { + reportJson = await readFile(location.resolved, "utf8"); + } catch (error) { + // `warning`, not `error`, because "you have not run the suite yet" is an + // ordinary state for a local source and the message should read as + // guidance rather than a fault. + // + // The EXECUTION STATUS is still `invalid`, and deliberately so: a source + // that produced no observations read nothing, and `statusFor` is right to + // say so. Downgrading this to `info` to soften the status would report the + // same empty read as `valid`, which is a worse lie than a red badge. + return { + batches: [], + diagnostics: [ + missingReport( + `Observation source profile ${profile.id} could not read the report at ${location.resolved}: ${error instanceof Error ? error.message : String(error)}` + ) + ] + }; + } + + // A local report records no commit. Rather than stamp the working tree's + // HEAD onto results that may predate it — inventing provenance the report + // never claimed — the commit stays absent unless config pins one. The + // diagnostic says so, because an unpinned observation is skipped by any + // commit-scoped evaluation and that silence would otherwise look like a bug. + const commit = profile.host?.options.commit; + const diagnostics: ScanDiagnostic[] = + commit === undefined + ? [ + createDiagnostic({ + severity: "info", + code: "INVALID_OBSERVATION_SELECTION", + message: `Observation source profile ${profile.id} read a local report that records no commit, so a commit-scoped evaluation will not count it. Set host.options.commit to pin one.` + }) + ] + : []; + + const source = { + id: profile.id, + kind: LOCAL_REPORTS_PROVIDER, + label: profile.name + }; + const revision = commit === undefined ? {} : { revision: { commit } }; + const report = reportRef(profile, projectRoot); + + const built = + format === "shiplight-report" + ? buildShiplightObservationBatch({ + report_json: reportJson, + source, + ...revision, + evidence_refs: report.refs + }) + : buildPlaywrightObservationBatch({ report_json: reportJson, source, ...revision }); + + const batch = + built.batch === undefined || report.refs.length === 0 + ? built.batch + : { + ...built.batch, + observations: (built.batch.observations ?? []).map((observation) => ({ + ...observation, + evidence_refs: report.refs + })) + }; + + return { + batches: batch === undefined ? [] : [batch], + diagnostics: [...diagnostics, ...report.diagnostics, ...built.diagnostics] + }; + }; +} diff --git a/packages/core/src/observation-sources/observation-sources.schema.json b/packages/core/src/observation-sources/observation-sources.schema.json index c87bdb1..6457e37 100644 --- a/packages/core/src/observation-sources/observation-sources.schema.json +++ b/packages/core/src/observation-sources/observation-sources.schema.json @@ -4,7 +4,9 @@ "title": "Quality Observation Source Profiles", "type": "object", "additionalProperties": false, - "required": ["profiles"], + "required": [ + "profiles" + ], "properties": { "profiles": { "type": "array", @@ -24,9 +26,15 @@ "additionalProperties": false, "minProperties": 1, "properties": { - "path": { "$ref": "#/definitions/nonEmptyString" }, - "url": { "$ref": "#/definitions/nonEmptyString" }, - "label": { "$ref": "#/definitions/nonEmptyString" } + "path": { + "$ref": "#/definitions/nonEmptyString" + }, + "url": { + "$ref": "#/definitions/nonEmptyString" + }, + "label": { + "$ref": "#/definitions/nonEmptyString" + } } }, "auth": { @@ -35,72 +43,159 @@ "properties": { "required_env": { "type": "array", - "items": { "$ref": "#/definitions/nonEmptyString" } + "items": { + "$ref": "#/definitions/nonEmptyString" + } } } }, "github": { "type": "object", "additionalProperties": false, - "required": ["repo", "workflow", "artifact_names"], + "required": [ + "repo", + "workflow", + "artifact_names" + ], "properties": { - "repo": { "$ref": "#/definitions/nonEmptyString" }, - "workflow": { "$ref": "#/definitions/nonEmptyString" }, + "repo": { + "$ref": "#/definitions/nonEmptyString" + }, + "workflow": { + "$ref": "#/definitions/nonEmptyString" + }, "artifact_names": { "type": "array", "minItems": 1, - "items": { "$ref": "#/definitions/nonEmptyString" } + "items": { + "$ref": "#/definitions/nonEmptyString" + } }, - "branch": { "$ref": "#/definitions/nonEmptyString" } + "branch": { + "$ref": "#/definitions/nonEmptyString" + } } }, "localFolder": { "type": "object", "additionalProperties": false, - "required": ["path"], + "required": [ + "path" + ], "properties": { - "path": { "$ref": "#/definitions/nonEmptyString" } + "path": { + "$ref": "#/definitions/nonEmptyString" + } + } + }, + "host": { + "type": "object", + "additionalProperties": false, + "required": [ + "provider" + ], + "properties": { + "provider": { + "$ref": "#/definitions/nonEmptyString" + }, + "options": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/nonEmptyString" + } + } } }, "profile": { "type": "object", "additionalProperties": false, - "required": ["id", "name", "transport", "observation_path"], + "required": [ + "id", + "name", + "transport" + ], "properties": { - "id": { "$ref": "#/definitions/nonEmptyString" }, - "name": { "$ref": "#/definitions/nonEmptyString" }, - "description": { "$ref": "#/definitions/nonEmptyString" }, + "id": { + "$ref": "#/definitions/nonEmptyString" + }, + "name": { + "$ref": "#/definitions/nonEmptyString" + }, + "description": { + "$ref": "#/definitions/nonEmptyString" + }, "transport": { - "enum": ["github-actions", "local-folder"] + "enum": [ + "github-actions", + "local-folder", + "host" + ] + }, + "observation_path": { + "$ref": "#/definitions/nonEmptyString" }, - "observation_path": { "$ref": "#/definitions/nonEmptyString" }, "source_refs": { "type": "array", - "items": { "$ref": "#/definitions/sourceRef" } + "items": { + "$ref": "#/definitions/sourceRef" + } }, - "auth": { "$ref": "#/definitions/auth" }, - "github": { "$ref": "#/definitions/github" }, - "local_folder": { "$ref": "#/definitions/localFolder" } + "auth": { + "$ref": "#/definitions/auth" + }, + "github": { + "$ref": "#/definitions/github" + }, + "local_folder": { + "$ref": "#/definitions/localFolder" + }, + "host": { + "$ref": "#/definitions/host" + } }, "allOf": [ { "if": { "properties": { - "transport": { "const": "github-actions" } + "transport": { + "const": "github-actions" + } + } + }, + "then": { + "required": [ + "github", + "observation_path" + ] + } + }, + { + "if": { + "properties": { + "transport": { + "const": "local-folder" + } } }, "then": { - "required": ["github"] + "required": [ + "local_folder", + "observation_path" + ] } }, { "if": { "properties": { - "transport": { "const": "local-folder" } + "transport": { + "const": "host" + } } }, "then": { - "required": ["local_folder"] + "required": [ + "host" + ] } } ] diff --git a/packages/core/src/observation-sources/parse.ts b/packages/core/src/observation-sources/parse.ts index 6d24209..6e02bed 100644 --- a/packages/core/src/observation-sources/parse.ts +++ b/packages/core/src/observation-sources/parse.ts @@ -1,9 +1,12 @@ import { readFileSync } from "node:fs"; import { parseDocument } from "yaml"; +import { OBSERVATION_SOURCE_TRANSPORTS } from "./types"; import type { GitHubActionsObservationSourceConfig, + HostObservationSourceConfig, LocalFolderObservationSourceConfig, ObservationSourceProfile, + ObservationSourceTransport, ObservationSourceProfileDiagnostic, ObservationSourceProfileParseBatch, ObservationSourceProfileSource, @@ -21,9 +24,12 @@ const PROFILE_KEYS = new Set([ "source_refs", "auth", "github", - "local_folder" + "local_folder", + "host" ]); +const TRANSPORTS = new Set(OBSERVATION_SOURCE_TRANSPORTS); + function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } @@ -107,6 +113,41 @@ function localFolderConfig(value: unknown): LocalFolderObservationSourceConfig | return folderPath === undefined ? undefined : { path: folderPath }; } +// `options` is passed to the host handler untouched. Values are restricted to +// strings so config stays a flat, reviewable block in the repo rather than a +// place to smuggle arbitrary structure past a reader. +// +// A non-string value is REPORTED, not quietly dropped. YAML turns an unquoted +// `commit: 1234567` into a number, and dropping it silently leaves a source +// that reads fine and produces observations a commit-scoped evaluation then +// ignores — a wrong answer with nothing on screen to explain it. +function hostConfig( + value: unknown, + onInvalidOption: (key: string) => void +): HostObservationSourceConfig | undefined { + if (!isRecord(value)) { + return undefined; + } + + const provider = stringValue(value.provider); + if (provider === undefined) { + return undefined; + } + + const rawOptions = isRecord(value.options) ? value.options : {}; + const options: Record = {}; + for (const [key, optionValue] of Object.entries(rawOptions)) { + const normalized = stringValue(optionValue); + if (normalized === undefined) { + onInvalidOption(key); + continue; + } + options[key] = normalized; + } + + return { provider, options }; +} + function validateProfile( profile: ObservationSourceProfile, source: ObservationSourceProfileSource, @@ -134,6 +175,30 @@ function validateProfile( }) ); } + + if (profile.transport === "host" && profile.host === undefined) { + diagnostics.push( + diagnostic(source, { + severity: "error", + code: "INVALID_OBSERVATION_SOURCE_PROFILE", + message: `Profile ${profile.id} requires a host block with a provider for transport host.`, + yamlPath: `${yamlPath}.host` + }) + ); + } + + // Only the file-based transports address a file. Requiring observation_path + // of a host transport would force config to name a path that is never read. + if (profile.transport !== "host" && profile.observationPath === undefined) { + diagnostics.push( + diagnostic(source, { + severity: "error", + code: "INVALID_OBSERVATION_SOURCE_PROFILE", + message: `Profile ${profile.id} must define observation_path for its canonical quality-observations JSON file.`, + yamlPath: `${yamlPath}.observation_path` + }) + ); + } } function profileFrom( @@ -171,7 +236,7 @@ function profileFrom( ); } - if (id === undefined || name === undefined || (transport !== "github-actions" && transport !== "local-folder")) { + if (id === undefined || name === undefined || !TRANSPORTS.has(transport as ObservationSourceTransport)) { diagnostics.push( diagnostic(source, { severity: "error", @@ -182,28 +247,27 @@ function profileFrom( ); return undefined; } - if (observationPath === undefined) { - diagnostics.push( - diagnostic(source, { - severity: "error", - code: "INVALID_OBSERVATION_SOURCE_PROFILE", - message: `Profile ${id} must define observation_path for its canonical quality-observations JSON file.`, - yamlPath: `$.profiles[${index}].observation_path` - }) - ); - return undefined; - } const profile: ObservationSourceProfile = { id, name, description: stringValue(value.description), - transport, + transport: transport as ObservationSourceTransport, observationPath, requiredEnv: stringArray(isRecord(value.auth) ? value.auth.required_env : undefined), sourceRefs: sourceRefs(value.source_refs), github: githubConfig(value.github), - localFolder: localFolderConfig(value.local_folder) + localFolder: localFolderConfig(value.local_folder), + host: hostConfig(value.host, (key) => { + diagnostics.push( + diagnostic(source, { + severity: "error", + code: "INVALID_OBSERVATION_SOURCE_PROFILE", + message: `Profile entry ${index} host.options.${key} must be a non-empty string. Quote a value YAML would otherwise read as a number or boolean.`, + yamlPath: `$.profiles[${index}].host.options.${key}` + }) + ); + }) }; validateProfile(profile, source, `$.profiles[${index}]`, diagnostics); diff --git a/packages/core/src/observation-sources/serialize.ts b/packages/core/src/observation-sources/serialize.ts index f292c2e..dbd082c 100644 --- a/packages/core/src/observation-sources/serialize.ts +++ b/packages/core/src/observation-sources/serialize.ts @@ -20,7 +20,7 @@ export function serializeObservationSources(profiles: readonly ObservationSource ? {} : { description: profile.description }), transport: profile.transport, - observation_path: profile.observationPath, + ...(profile.observationPath === undefined ? {} : { observation_path: profile.observationPath }), ...(requiredEnv.length > 0 ? { auth: { required_env: requiredEnv } } : {}), ...(sourceRefs.length > 0 ? { source_refs: sourceRefs } : {}), ...(profile.github === undefined @@ -33,7 +33,17 @@ export function serializeObservationSources(profiles: readonly ObservationSource ...(profile.github.branch === undefined ? {} : { branch: profile.github.branch }) } }), - ...(profile.localFolder === undefined ? {} : { local_folder: { path: profile.localFolder.path } }) + ...(profile.localFolder === undefined ? {} : { local_folder: { path: profile.localFolder.path } }), + ...(profile.host === undefined + ? {} + : { + host: { + provider: profile.host.provider, + ...(Object.keys(profile.host.options).length === 0 + ? {} + : { options: { ...profile.host.options } }) + } + }) }; }) }); diff --git a/packages/core/src/observation-sources/types.ts b/packages/core/src/observation-sources/types.ts index 8040545..3d7f9ee 100644 --- a/packages/core/src/observation-sources/types.ts +++ b/packages/core/src/observation-sources/types.ts @@ -33,18 +33,64 @@ export interface LocalFolderObservationSourceConfig { readonly path: string; } +export interface HostObservationSourceConfig { + readonly provider: string; + readonly options: Readonly>; +} + +// The single list every consumer reads: the parser validates against it and the +// published JSON Schema enumerates it, so a new transport cannot be accepted by +// one and rejected by the other. +export const OBSERVATION_SOURCE_TRANSPORTS = ["github-actions", "local-folder", "host"] as const; + +export type ObservationSourceTransport = (typeof OBSERVATION_SOURCE_TRANSPORTS)[number]; + export interface ObservationSourceProfile { readonly id: string; readonly name: string; readonly description?: string; - readonly transport: "github-actions" | "local-folder"; - readonly observationPath: string; + readonly transport: ObservationSourceTransport; + /** + * Path of the canonical quality-observations JSON inside the fetched artifact + * or folder. Absent for `host` transports, which have no file to address — + * validation requires it for the two file-based transports. + */ + readonly observationPath?: string; readonly requiredEnv: readonly string[]; readonly sourceRefs: readonly ObservationSourceReference[]; readonly github?: GitHubActionsObservationSourceConfig; readonly localFolder?: LocalFolderObservationSourceConfig; + readonly host?: HostObservationSourceConfig; } +/** + * A transport the embedding application supplies, addressed by name from + * `host.provider` in the profile. + * + * This is the seam that lets an integration read results from somewhere the + * engine has no business knowing about — a platform database, a vendor API — + * without that knowledge entering this package. The handler's only job is to + * FETCH and SHAPE: it returns records in the canonical input form and the + * engine keeps everything that follows (normalization, identity, resolution + * against the quality map, every diagnostic). A handler that resolved + * observations onto checks itself would be deciding what proves what outside + * the engine, which is exactly what the independence rule forbids. + */ +export type HostObservationTransport = (input: { + readonly profile: ObservationSourceProfile; + readonly selection?: ObservationSourceExecutionSelection; + readonly projectRoot?: string; + readonly env?: NodeJS.ProcessEnv; +}) => Promise; + +export interface HostObservationTransportResult { + readonly batches: readonly import("../observations/types").ObservationBatchInput[]; + readonly diagnostics?: readonly import("../diagnostics/diagnostic").ScanDiagnostic[]; + readonly selectedRun?: ExecutedObservationSourceRun; +} + +export type HostObservationTransportRegistry = Readonly>; + export interface ParsedObservationSourceProfilesDocument { readonly profiles: readonly ObservationSourceProfile[]; } @@ -99,7 +145,7 @@ export interface ExecutedObservationSourceRun { export interface ObservationSourceExecutionResult { readonly profileId: string; readonly profileName: string; - readonly transport: ObservationSourceProfile["transport"]; + readonly transport: ObservationSourceTransport; readonly status: import("../observations/types").ObservationIngestionStatus; readonly envStatus: ObservationSourceProfileEnvStatus; readonly observations: readonly import("../observations/types").NormalizedObservationRecord[]; diff --git a/packages/core/src/observations/evaluate.ts b/packages/core/src/observations/evaluate.ts index 1912a13..4b96ead 100644 --- a/packages/core/src/observations/evaluate.ts +++ b/packages/core/src/observations/evaluate.ts @@ -218,7 +218,10 @@ export function buildTargetEvaluation( observationId: selected?.observationId, observedAt: selected?.observedAt, commit: selected?.revision.commit, - runUrl: selected?.source.runUrl + runUrl: selected?.source.runUrl, + // Only the selected observation's refs. An unobserved check has none, + // and refs from a run that lost selection describe a different commit. + evidenceRefs: selected?.evidenceRefs ?? [] } satisfies EvaluatedEvidenceObservation; }); diff --git a/packages/core/src/observations/github-actions.ts b/packages/core/src/observations/github-actions.ts index 97832e2..920a025 100644 --- a/packages/core/src/observations/github-actions.ts +++ b/packages/core/src/observations/github-actions.ts @@ -1,6 +1,7 @@ import { createDiagnostic } from "../diagnostics/diagnostic"; import { INTERNAL_OBSERVATION_CONTEXT } from "./types"; import type { ObservationIngestionResult, ObservationRecordInput, ObservationRecordStatus } from "./types"; +import { countProblems } from "./ingest-helpers"; import { normalizeObservationBatches } from "./normalize"; export interface GitHubActionsStepInput { @@ -282,7 +283,7 @@ export function ingestGitHubActionsRun( const mergedDiagnostics = [...diagnostics, ...normalized.diagnostics]; return { - status: statusFor(normalized.observations.length, mergedDiagnostics.length), + status: statusFor(normalized.observations.length, countProblems(mergedDiagnostics)), observations: normalized.observations, diagnostics: mergedDiagnostics }; diff --git a/packages/core/src/observations/index.ts b/packages/core/src/observations/index.ts index b3bc19d..fff56bd 100644 --- a/packages/core/src/observations/index.ts +++ b/packages/core/src/observations/index.ts @@ -8,4 +8,5 @@ export * from "./recommend"; export * from "./github-actions"; export * from "./junit"; export * from "./playwright-json"; +export * from "./shiplight-report"; export * from "./manifest"; diff --git a/packages/core/src/observations/ingest-helpers.ts b/packages/core/src/observations/ingest-helpers.ts index fd469eb..f1cba2c 100644 --- a/packages/core/src/observations/ingest-helpers.ts +++ b/packages/core/src/observations/ingest-helpers.ts @@ -1,3 +1,4 @@ +import type { ScanDiagnostic } from "../diagnostics/diagnostic"; import type { ObservationArtifactInput, ObservationIngestionResult } from "./types"; // Shared helpers for the observation ingest adapters (junit, playwright-json, @@ -22,6 +23,14 @@ export function isoTimestamp(value: unknown): string | undefined { return new Date(parsed).toISOString(); } +// Counts only diagnostics that report something going wrong. An `info` note is +// context for the reader, not a degraded read, and the source-execution and +// resolution paths already exclude it — an adapter that counted it would make +// the status depend on which ingestion path ran rather than on the inputs. +export function countProblems(diagnostics: readonly ScanDiagnostic[]): number { + return diagnostics.filter((entry) => entry.severity !== "info").length; +} + export function statusFor( observationCount: number, diagnosticsCount: number diff --git a/packages/core/src/observations/junit.ts b/packages/core/src/observations/junit.ts index 609ba37..933510e 100644 --- a/packages/core/src/observations/junit.ts +++ b/packages/core/src/observations/junit.ts @@ -8,7 +8,7 @@ import type { ObservationRecordInput, ObservationRecordStatus } from "./types"; -import { isoTimestamp, normalizeArtifact, normalizePath, statusFor, stringValue } from "./ingest-helpers"; +import { countProblems, isoTimestamp, normalizeArtifact, normalizePath, statusFor, stringValue } from "./ingest-helpers"; import { normalizeObservationBatches } from "./normalize"; interface ParsedJunitCase { @@ -246,7 +246,7 @@ export function ingestJunitXmlReport( const mergedDiagnostics = [...normalized.diagnostics, ...diagnostics]; return { - status: statusFor(normalized.observations.length, mergedDiagnostics.length), + status: statusFor(normalized.observations.length, countProblems(mergedDiagnostics)), observations: normalized.observations, diagnostics: mergedDiagnostics }; diff --git a/packages/core/src/observations/manifest.ts b/packages/core/src/observations/manifest.ts index 227a2b8..14e2814 100644 --- a/packages/core/src/observations/manifest.ts +++ b/packages/core/src/observations/manifest.ts @@ -7,12 +7,13 @@ import type { ObservationRecordInput, ObservationRecordStatus, QualityObservationManifest, + QualityObservationManifestArtifact, QualityObservationManifestParseResult, QualityObservationManifestRecord, QualityObservationManifestRevision, QualityObservationManifestRun } from "./types"; -import { isoTimestamp, normalizeArtifact, normalizePath, statusFor, stringValue } from "./ingest-helpers"; +import { countProblems, isoTimestamp, normalizeArtifact, normalizePath, statusFor, stringValue } from "./ingest-helpers"; import { normalizeObservationBatches } from "./normalize"; export const QUALITY_OBSERVATION_SCHEMA_VERSION = 1 as const; @@ -70,7 +71,22 @@ export function buildQualityObservationManifestJsonSchema(): Record(["pass", "fail", "error", "skipped"]); function isRecord(value: unknown): value is Record { @@ -203,6 +220,64 @@ function worstStatus( type ObservationEntryMode = "strict" | "tolerant"; +// A malformed evidence pointer must never cost us the result it points at. The +// pass/fail fact is what scores; the ref is only how a reviewer looks at it. So +// a bad entry is dropped and reported, and the observation survives without it. +// `validate` still fails the document, because entryDiagnostic raises these to +// errors in strict mode — a producer fixing its output wants to hear about it. +function parseObservationArtifacts( + value: unknown, + index: number, + entryDiagnostic: (message: string) => ScanDiagnostic, + diagnostics: ScanDiagnostic[] +): readonly QualityObservationManifestArtifact[] | undefined { + if (value === undefined) { + return undefined; + } + + if (!Array.isArray(value)) { + diagnostics.push(entryDiagnostic(`Quality observations entry ${index} artifacts must be an array.`)); + return undefined; + } + + const artifacts: QualityObservationManifestArtifact[] = []; + value.forEach((entry, artifactIndex) => { + const position = `entry ${index} artifact ${artifactIndex}`; + if (!isRecord(entry)) { + diagnostics.push(entryDiagnostic(`Quality observations ${position} must be an object and was skipped.`)); + return; + } + + const extras = unknownKeys(entry, OBSERVATION_ARTIFACT_KEYS); + if (extras.length > 0) { + diagnostics.push( + entryDiagnostic( + `Quality observations ${position} contains unknown fields and was skipped: ${extras.join(", ")}.` + ) + ); + return; + } + + const ref = stringValue(entry.ref); + if (ref === undefined) { + diagnostics.push(entryDiagnostic(`Quality observations ${position} requires a non-empty ref and was skipped.`)); + return; + } + + const label = stringValue(entry.label); + if (entry.label !== undefined && label === undefined) { + diagnostics.push( + entryDiagnostic(`Quality observations ${position} label must be a non-empty string when provided.`) + ); + return; + } + + artifacts.push({ ref, ...(label === undefined ? {} : { label }) }); + }); + + return artifacts.length === 0 ? undefined : artifacts; +} + function parseObservations( value: unknown, fallbackObservedAt: string | undefined, @@ -271,12 +346,15 @@ function parseObservations( return; } + const artifacts = parseObservationArtifacts(entry.artifacts, index, entryDiagnostic, diagnostics); + const record: QualityObservationManifestRecord = { path, ...(testCase === undefined ? {} : { test_case: testCase }), status, ...(observedAt === undefined ? {} : { observed_at: observedAt }), - ...(note === undefined ? {} : { note }) + ...(note === undefined ? {} : { note }), + ...(artifacts === undefined ? {} : { artifacts }) }; const key = qualityObservationIdentity(record); const seenIndex = seenKeys.get(key); @@ -454,7 +532,14 @@ export function ingestObservationManifest(input: IngestObservationManifestInput) observed_at: entry.observed_at ?? manifest.observed_at, revision: input.revision ?? { ...manifest.revision }, note: entry.note, - artifacts: artifact === undefined ? [] : [artifact] + artifacts: artifact === undefined ? [] : [artifact], + // The manifest's `artifacts` are evidence pointers; the `artifacts` above + // are the manifest's own provenance. Mapped field by field rather than + // spread, so the two never merge by accident. + evidence_refs: (entry.artifacts ?? []).map((entryArtifact) => ({ + ref: entryArtifact.ref, + label: entryArtifact.label + })) })); const normalized = normalizeObservationBatches([ @@ -467,7 +552,7 @@ export function ingestObservationManifest(input: IngestObservationManifestInput) const diagnostics = [...normalized.diagnostics, ...parsed.diagnostics]; return { - status: statusFor(normalized.observations.length, diagnostics.length), + status: statusFor(normalized.observations.length, countProblems(diagnostics)), observations: normalized.observations, diagnostics }; diff --git a/packages/core/src/observations/normalize.ts b/packages/core/src/observations/normalize.ts index 1694700..bdc1a27 100644 --- a/packages/core/src/observations/normalize.ts +++ b/packages/core/src/observations/normalize.ts @@ -1,12 +1,14 @@ import { createDiagnostic } from "../diagnostics/diagnostic"; import type { ScanDiagnostic } from "../diagnostics/diagnostic"; import type { + NormalizedEvidenceRef, NormalizedObservationArtifact, NormalizedObservationRecord, NormalizedObservationRevision, NormalizedObservationSource, ObservationArtifactInput, ObservationBatchInput, + ObservationEvidenceRefInput, ObservationIngestionResult, ObservationRecordInput, ObservationRecordStatus, @@ -76,6 +78,28 @@ function normalizeArtifacts(value: readonly ObservationArtifactInput[] | undefin })); } +// An entry with no usable ref points at nothing, so it is dropped rather than +// carried as an empty link the UI would have to special-case. The ref itself is +// only trimmed — never rewritten, resolved, or validated for shape, because +// only the integration that produced it knows what a valid one looks like. +function normalizeEvidenceRefs( + value: readonly ObservationEvidenceRefInput[] | undefined +): readonly NormalizedEvidenceRef[] { + if (!Array.isArray(value)) { + return []; + } + + return value.flatMap((entry) => { + const ref = stringValue(entry.ref); + if (ref === undefined) { + return []; + } + + const label = stringValue(entry.label); + return [{ ref, ...(label === undefined ? {} : { label }) }]; + }); +} + function normalizeStatus(value: unknown): ObservationRecordStatus | undefined { const normalized = stringValue(value)?.toLowerCase() as ObservationRecordStatus | undefined; return normalized !== undefined && validStatuses.has(normalized) ? normalized : undefined; @@ -212,7 +236,8 @@ export function normalizeObservationBatches( revision, source, note: stringValue(record.note), - artifacts: normalizeArtifacts(record.artifacts) + artifacts: normalizeArtifacts(record.artifacts), + evidenceRefs: normalizeEvidenceRefs(record.evidence_refs) }); }); }); diff --git a/packages/core/src/observations/playwright-json.ts b/packages/core/src/observations/playwright-json.ts index 12779f9..d27df66 100644 --- a/packages/core/src/observations/playwright-json.ts +++ b/packages/core/src/observations/playwright-json.ts @@ -3,11 +3,12 @@ import type { ScanDiagnostic } from "../diagnostics/diagnostic"; import { INTERNAL_OBSERVATION_CONTEXT } from "./types"; import type { IngestPlaywrightJsonReportInput, + ObservationBatchInput, ObservationIngestionResult, ObservationRecordInput, ObservationRecordStatus } from "./types"; -import { isoTimestamp, normalizeArtifact, normalizePath, statusFor, stringValue } from "./ingest-helpers"; +import { countProblems, isoTimestamp, normalizeArtifact, normalizePath, statusFor, stringValue } from "./ingest-helpers"; import { normalizeObservationBatches } from "./normalize"; interface ParsedPlaywrightCase { @@ -126,9 +127,14 @@ function observationIdFor( ].join(":"); } -export function ingestPlaywrightJsonReport( - input: IngestPlaywrightJsonReportInput -): ObservationIngestionResult { +// Parses the report into a canonical batch WITHOUT normalizing it. Split out so +// a host transport can read a Playwright report and still hand the engine the +// same un-normalized input a file-based transport would — there is no second, +// softer path into a score. +export function buildPlaywrightObservationBatch(input: IngestPlaywrightJsonReportInput): { + readonly batch?: ObservationBatchInput; + readonly diagnostics: readonly ScanDiagnostic[]; +} { const diagnostics: ScanDiagnostic[] = []; let parsed: PlaywrightJsonReport; @@ -143,11 +149,7 @@ export function ingestPlaywrightJsonReport( }) ); - return { - status: "invalid", - observations: [], - diagnostics - }; + return { diagnostics }; } if (!Array.isArray(parsed.suites)) { @@ -159,11 +161,7 @@ export function ingestPlaywrightJsonReport( }) ); - return { - status: "invalid", - observations: [], - diagnostics - }; + return { diagnostics }; } const reportFallbackObservedAt = @@ -192,11 +190,7 @@ export function ingestPlaywrightJsonReport( }) ); - return { - status: "invalid", - observations: [], - diagnostics - }; + return { diagnostics }; } const artifact = normalizeArtifact(input.artifact, "playwright-json"); @@ -215,17 +209,29 @@ export function ingestPlaywrightJsonReport( }); }); - const normalized = normalizeObservationBatches([ - { + return { + batch: { source: input.source, context: INTERNAL_OBSERVATION_CONTEXT, observations - } - ]); - const mergedDiagnostics = [...normalized.diagnostics, ...diagnostics]; + }, + diagnostics + }; +} + +export function ingestPlaywrightJsonReport( + input: IngestPlaywrightJsonReportInput +): ObservationIngestionResult { + const built = buildPlaywrightObservationBatch(input); + if (built.batch === undefined) { + return { status: "invalid", observations: [], diagnostics: built.diagnostics }; + } + + const normalized = normalizeObservationBatches([built.batch]); + const mergedDiagnostics = [...normalized.diagnostics, ...built.diagnostics]; return { - status: statusFor(normalized.observations.length, mergedDiagnostics.length), + status: statusFor(normalized.observations.length, countProblems(mergedDiagnostics)), observations: normalized.observations, diagnostics: mergedDiagnostics }; diff --git a/packages/core/src/observations/quality-observations.schema.json b/packages/core/src/observations/quality-observations.schema.json index 0629fa4..d67951d 100644 --- a/packages/core/src/observations/quality-observations.schema.json +++ b/packages/core/src/observations/quality-observations.schema.json @@ -94,6 +94,27 @@ "note": { "type": "string", "minLength": 1 + }, + "artifacts": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "additionalProperties": false, + "required": [ + "ref" + ], + "properties": { + "ref": { + "type": "string", + "minLength": 1 + }, + "label": { + "type": "string", + "minLength": 1 + } + } + } } } } diff --git a/packages/core/src/observations/resolve.ts b/packages/core/src/observations/resolve.ts index e9898d3..6c96ded 100644 --- a/packages/core/src/observations/resolve.ts +++ b/packages/core/src/observations/resolve.ts @@ -4,6 +4,7 @@ import type { NormalizedQualityGraph } from "@shiplightai/quality-map"; import { createDiagnostic } from "../diagnostics/diagnostic"; +import { countProblems } from "./ingest-helpers"; import type { ScanResult } from "../discovery/types"; import { OBSERVATION_SUITE_SEPARATOR } from "./types"; import type { @@ -219,7 +220,8 @@ function auditRowBase(record: NormalizedObservationRecord): Omit< sourceKind: record.source.kind, sourceLabel: record.source.label, runId: record.source.runId, - runUrl: record.source.runUrl + runUrl: record.source.runUrl, + evidenceRefs: record.evidenceRefs }; } @@ -383,7 +385,7 @@ export function resolveObservations( }); return { - status: statusFor(resolved, diagnostics.length), + status: statusFor(resolved, countProblems(diagnostics)), observations: resolved, auditRows, diagnostics diff --git a/packages/core/src/observations/shiplight-report.ts b/packages/core/src/observations/shiplight-report.ts new file mode 100644 index 0000000..d394739 --- /dev/null +++ b/packages/core/src/observations/shiplight-report.ts @@ -0,0 +1,160 @@ +import { createDiagnostic } from "../diagnostics/diagnostic"; +import type { ScanDiagnostic } from "../diagnostics/diagnostic"; +import { INTERNAL_OBSERVATION_CONTEXT } from "./types"; +import type { + ObservationBatchInput, + ObservationEvidenceRefInput, + ObservationRecordInput, + ObservationRecordStatus, + ObservationRevisionInput, + ObservationSourceInput +} from "./types"; +import { isoTimestamp, normalizePath, stringValue } from "./ingest-helpers"; + +/** + * Reads the report a Shiplight YAML run writes next to the suite + * (`shiplight-report/report-data.json`). + * + * It exists because a YAML suite's results are not otherwise readable without + * asking the producer to add a second reporter: the run transpiles to Playwright + * and then writes THIS file, and a repo that has it usually has no Playwright + * JSON at all. + */ + +interface ShiplightReportTest { + readonly file?: string; + readonly title?: string; + readonly baseTitle?: string; + readonly status?: string; + readonly startTime?: string; + readonly endTime?: string; +} + +interface ShiplightReport { + readonly tests?: readonly ShiplightReportTest[]; + readonly timestamp?: string; +} + +// The same vocabulary Playwright uses, because the run is a Playwright run. +function mapStatus(value: string | undefined): ObservationRecordStatus | undefined { + switch (value) { + case "passed": + return "pass"; + case "failed": + return "fail"; + case "timedOut": + case "interrupted": + return "error"; + case "skipped": + return "skipped"; + default: + return undefined; + } +} + +/** + * Maps a reported spec path back to the YAML it was generated from. + * + * The runner reports the TRANSPILED file, but that file is a build artifact — + * gitignored, absent from a fresh checkout, and regenerated on every run. The + * source is the `.test.yaml`, and that is what a quality map can honestly pin, + * so it is what the observation must be keyed on. Pinning the generated spec + * instead would point a check at a file no reviewer has. + * + * This inverts the transpiler's own rule, which names the output by replacing + * the source suffix: `yamlPath.replace(/\.test\.yaml$/, '.yaml.spec.ts')`. + * + * Deliberately confined to this adapter rather than applied to Playwright + * reports generally: a plain Playwright project may contain a hand-written file + * genuinely called `foo.yaml.spec.ts`, and rewriting that would corrupt an + * honest identity. Here the convention is the format's own. + */ +export function shiplightSourcePath(reported: string): string { + return reported.endsWith(".yaml.spec.ts") + ? `${reported.slice(0, -".yaml.spec.ts".length)}.test.yaml` + : reported; +} + +export interface BuildShiplightObservationBatchInput { + readonly report_json: string; + readonly source?: ObservationSourceInput; + readonly revision?: ObservationRevisionInput; + readonly evidence_refs?: readonly ObservationEvidenceRefInput[]; +} + +export function buildShiplightObservationBatch(input: BuildShiplightObservationBatchInput): { + readonly batch?: ObservationBatchInput; + readonly diagnostics: readonly ScanDiagnostic[]; +} { + const diagnostics: ScanDiagnostic[] = []; + const invalid = (message: string): ScanDiagnostic => + createDiagnostic({ severity: "error", code: "INVALID_OBSERVATION_ARTIFACT", message }); + + let parsed: ShiplightReport; + try { + parsed = JSON.parse(input.report_json) as ShiplightReport; + } catch (error) { + return { + diagnostics: [ + invalid( + `Shiplight report could not be parsed: ${error instanceof Error ? error.message : String(error)}` + ) + ] + }; + } + + if (!Array.isArray(parsed.tests)) { + return { diagnostics: [invalid("Shiplight report is missing a tests array.")] }; + } + + const reportObservedAt = isoTimestamp(parsed.timestamp); + const observations: ObservationRecordInput[] = []; + + parsed.tests.forEach((test, index) => { + const reported = normalizePath(stringValue(test.file)); + const status = mapStatus(stringValue(test.status)); + if (reported === undefined || status === undefined) { + diagnostics.push( + createDiagnostic({ + severity: "warning", + code: "INVALID_OBSERVATION_RECORD", + message: `Shiplight report test ${index} has no usable file or status and was skipped.` + }) + ); + return; + } + + // `baseTitle` is the YAML test's own name; `title` is that name with the + // run's tags prefixed. A quality map is authored against the YAML, so the + // bare name is the identity a human would pin — and the tagged form would + // not match it, since tags are prefixed rather than joined by the suite + // separator the resolver folds on. + const testCase = stringValue(test.baseTitle) ?? stringValue(test.title); + const observedAt = isoTimestamp(test.endTime) ?? isoTimestamp(test.startTime) ?? reportObservedAt; + + observations.push({ + observation_id: ["shiplight-report", reported, testCase ?? "test", index].join(":"), + test_file: shiplightSourcePath(reported), + ...(testCase === undefined ? {} : { test_case: testCase }), + status, + ...(observedAt === undefined ? {} : { observed_at: observedAt }), + revision: input.revision, + evidence_refs: input.evidence_refs ?? [] + }); + }); + + if (observations.length === 0) { + return { + diagnostics: [...diagnostics, invalid("Shiplight report contained no usable test results.")] + }; + } + + return { + batch: { + source: input.source, + context: INTERNAL_OBSERVATION_CONTEXT, + observations + }, + diagnostics + }; +} diff --git a/packages/core/src/observations/types.ts b/packages/core/src/observations/types.ts index 121f7e1..6420d15 100644 --- a/packages/core/src/observations/types.ts +++ b/packages/core/src/observations/types.ts @@ -25,6 +25,26 @@ export interface ObservationArtifactInput { readonly [key: string]: unknown; } +// A pointer to the run evidence a producer kept for one observation: the video, +// the screenshot gallery, the trace, the HTML report — whatever lets a reviewer +// see what the test actually did. +// +// `ref` is OPAQUE. Quality records it and hands it back; it never parses it for +// meaning, and no engine behaviour may branch on its contents. Interpretation +// belongs to whoever configured the producer: a Shiplight run URL, a local +// report path, and a CI artifact address are all the same thing here. The one +// exception lives in the presentation layer, which links an absolute http(s) +// ref directly rather than asking a host to resolve it. +// +// Deliberately distinct from ObservationArtifactInput above, which records +// where the observation MANIFEST came from. Conflating the two would show the +// manifest file itself as run evidence. +export interface ObservationEvidenceRefInput { + readonly ref?: string; + readonly label?: string; + readonly [key: string]: unknown; +} + export interface ObservationSourceInput { readonly id?: string; readonly kind?: string; @@ -53,6 +73,7 @@ export interface ObservationRecordInput { readonly revision?: ObservationRevisionInput; readonly note?: string; readonly artifacts?: readonly ObservationArtifactInput[]; + readonly evidence_refs?: readonly ObservationEvidenceRefInput[]; readonly [key: string]: unknown; } @@ -99,12 +120,20 @@ export interface QualityObservationManifestRun { readonly url?: string; } +// The public spelling of ObservationEvidenceRefInput inside the canonical +// manifest. `ref` is required here: an entry with no pointer is not evidence. +export interface QualityObservationManifestArtifact { + readonly ref: string; + readonly label?: string; +} + export interface QualityObservationManifestRecord { readonly path: string; readonly test_case?: string; readonly status: ObservationRecordStatus; readonly observed_at?: string; readonly note?: string; + readonly artifacts?: readonly QualityObservationManifestArtifact[]; } export interface QualityObservationManifest { @@ -128,6 +157,11 @@ export interface NormalizedObservationArtifact { readonly label?: string; } +export interface NormalizedEvidenceRef { + readonly ref: string; + readonly label?: string; +} + export interface NormalizedObservationSource { readonly id?: string; readonly kind?: string; @@ -155,6 +189,7 @@ export interface NormalizedObservationRecord { readonly source: NormalizedObservationSource; readonly note?: string; readonly artifacts: readonly NormalizedObservationArtifact[]; + readonly evidenceRefs: readonly NormalizedEvidenceRef[]; } export interface ObservationIngestionResult { @@ -195,6 +230,7 @@ export interface ObservationResolutionAuditRow { readonly evidenceId?: string; readonly evidenceLocalId?: string; readonly evidencePath?: string; + readonly evidenceRefs: readonly NormalizedEvidenceRef[]; } export interface ObservationResolutionResult { @@ -217,6 +253,7 @@ export interface EvaluatedEvidenceObservation { readonly observedAt?: string; readonly commit?: string; readonly runUrl?: string; + readonly evidenceRefs: readonly NormalizedEvidenceRef[]; } export interface EvaluatedExpectationSnapshot { diff --git a/packages/core/src/operations/index.ts b/packages/core/src/operations/index.ts index 716da2a..6f30155 100644 --- a/packages/core/src/operations/index.ts +++ b/packages/core/src/operations/index.ts @@ -30,6 +30,7 @@ import { serializeHumanSources, serializeObservationSets, serializeObservationSources, + type HostObservationTransportRegistry, type HumanSource, type ObservationContextQualityRollup, type ObservationResolutionAuditRow, @@ -220,6 +221,13 @@ export interface QcExecuteObservationSetInput { * injects the org's GitHub App installation token as GITHUB_TOKEN here; a client can't set it. */ readonly env?: NodeJS.ProcessEnv; + /** + * Transports the embedding application supplies for `transport: host` + * profiles, keyed by the profile's `host.provider`. Absent in the OSS CLI and + * Explorer, which register none — a repo declaring a host provider they do + * not serve gets an explicit diagnostic rather than a silent empty result. + */ + readonly hostTransports?: HostObservationTransportRegistry; } export interface QcExecuteObservationSourceInput { readonly projectPath: string; @@ -227,6 +235,8 @@ export interface QcExecuteObservationSourceInput { readonly selection?: ObservationSetExecutionSelection; /** See QcExecuteObservationSetInput.env — the github-actions token env, injected by the box. */ readonly env?: NodeJS.ProcessEnv; + /** See QcExecuteObservationSetInput.hostTransports. */ + readonly hostTransports?: HostObservationTransportRegistry; } interface QcExecutionResolution { readonly status: ObservationResolutionResult["status"]; @@ -1078,6 +1088,7 @@ export async function executeObservationSetOp( projectRoot: scan.target.resolvedPath, selection: input.selection, env: input.env, + hostTransports: input.hostTransports, }); const resolution = resolveObservations(scan, execution); const scopedScan = applySavedQcView(scan, input.viewId) ?? scan; @@ -1134,6 +1145,7 @@ export async function executeObservationSourceOp( projectRoot: scan.target.resolvedPath, selection: input.selection, env: input.env, + hostTransports: input.hostTransports, }); const resolution = resolveObservations(scan, execution); const usable = hasUsableProof({ diff --git a/packages/core/src/recommendation-export/index.ts b/packages/core/src/recommendation-export/index.ts index d6f4bb0..1186ef0 100644 --- a/packages/core/src/recommendation-export/index.ts +++ b/packages/core/src/recommendation-export/index.ts @@ -12,6 +12,7 @@ import { type ObservationSetExecutionSelection } from "../observation-sets"; import { findSavedQcView } from "../views/filter"; +import type { HostObservationTransportRegistry } from "../observation-sources"; import { resolveObservations } from "../observations/resolve"; import { scanProject } from "../discovery/scan-project"; import type { ScanDiagnostic } from "../diagnostics/diagnostic"; @@ -169,6 +170,8 @@ export interface BuildRecommendationExportInput { readonly limit?: number; readonly selection?: ObservationSetExecutionSelection; readonly env?: NodeJS.ProcessEnv; + /** Host transports for `transport: host` profiles, keyed by `host.provider`. */ + readonly hostTransports?: HostObservationTransportRegistry; readonly fixPromptRecords?: readonly RecommendationFixPromptRecord[]; readonly generatedAt?: Date; } @@ -186,6 +189,11 @@ function hasUsableRuntimeProofStatus(input: { return input.executionStatus !== "invalid" && input.resolutionStatus !== "invalid" && input.observationCount > 0; } +// KNOWN, PRE-EXISTING, tracked as issue #16: this is not injective. `my view` and `my-view` both +// reduce to `my-view`, so two saved views with those ids write the same export +// and one silently overwrites the other. Making the segment unique renames the +// file for every id that needs sanitizing, which breaks anything addressing an +// existing export by name — a trade that belongs in its own change, not here. function sanitizeFileSegment(value: string): string { return value.replace(/[^a-zA-Z0-9._-]+/g, "-"); } @@ -552,7 +560,8 @@ export async function buildRecommendationExport( observationSourceProfiles: scan.observationSourceProfiles.primary?.document?.profiles ?? [], projectRoot: scan.target.resolvedPath, env: input.env, - selection: input.selection + selection: input.selection, + hostTransports: input.hostTransports }); const resolution = execution === undefined ? undefined : resolveObservations(scan, execution); const effectiveResult = applySavedQcView(scan, input.viewId) ?? scan; diff --git a/packages/core/src/views/index.ts b/packages/core/src/views/index.ts index 3759e72..0d97275 100644 --- a/packages/core/src/views/index.ts +++ b/packages/core/src/views/index.ts @@ -1,2 +1,3 @@ export * from "./filter"; export * from "./types"; +export * from "./json-schema"; diff --git a/packages/core/src/views/json-schema.ts b/packages/core/src/views/json-schema.ts new file mode 100644 index 0000000..a37986d --- /dev/null +++ b/packages/core/src/views/json-schema.ts @@ -0,0 +1,61 @@ +// Emitted from this package's own contract constants, like the other config +// schemas, so the published schema and the parser that enforces it cannot +// disagree. Before this existed the only copy lived in the agent skill, with +// nothing tying it to the parser — and it had already drifted ahead of it. +import { WHOLE_PROJECT_VIEW_ID } from "./types"; + +const nonEmptyString = { $ref: "#/definitions/nonEmptyString" } as const; + +export function buildSavedViewsJsonSchema(): Record { + return { + $schema: "http://json-schema.org/draft-07/schema#", + $id: "https://shiplight.dev/schemas/quality/views.schema.json", + title: "Quality Saved Assessment Scopes", + type: "object", + additionalProperties: false, + required: ["views"], + properties: { + views: { + type: "array", + minItems: 1, + items: { $ref: "#/definitions/view" } + } + }, + definitions: { + nonEmptyString: { type: "string", minLength: 1 }, + viewId: { + type: "string", + minLength: 1, + // The scope shown when no view is selected. A saved view claiming it + // would shadow the default scope everywhere it is used as an id. + // + // No shape rule beyond that: ids only reach filenames through the + // recommendation export's own sanitizer, so a stricter pattern would + // reject working configuration for no gain. That sanitizer can collide + // on ids differing only in a separator — issue #16 — which a pattern + // here would narrow but not fix, and would break existing configs to do. + not: { const: WHOLE_PROJECT_VIEW_ID } + }, + view: { + type: "object", + additionalProperties: false, + required: ["id", "name", "feature_ids"], + properties: { + id: { $ref: "#/definitions/viewId" }, + name: nonEmptyString, + description: nonEmptyString, + feature_ids: { + type: "array", + minItems: 1, + uniqueItems: true, + items: nonEmptyString + } + } + } + } + }; +} + +export function serializeSavedViewsJsonSchema(): string { + return `${JSON.stringify(buildSavedViewsJsonSchema(), null, 2)}\n`; +} diff --git a/packages/core/src/views/parse.ts b/packages/core/src/views/parse.ts index 6e37a43..7d08d99 100644 --- a/packages/core/src/views/parse.ts +++ b/packages/core/src/views/parse.ts @@ -1,4 +1,5 @@ import { readFileSync } from "node:fs"; +import { WHOLE_PROJECT_VIEW_ID } from "./types"; import { parseDocument } from "yaml"; import type { ParsedSavedQcViews, @@ -93,6 +94,22 @@ function savedViewFrom( const name = stringValue(value.name); const nextFeatureIds = featureIds(value.feature_ids, source, `$.views[${index}].feature_ids`, diagnostics); + // Enforced here, not only in the published schema: the schema is what an + // author validates against, but this parser is what actually runs during a + // scan, and a rule only one of them applies is a rule the product does not + // really have. + if (id !== undefined && id === WHOLE_PROJECT_VIEW_ID) { + diagnostics.push( + diagnostic(source, { + severity: "error", + code: "INVALID_SAVED_VIEW", + message: `Saved view id ${WHOLE_PROJECT_VIEW_ID} is reserved for the unscoped assessment.`, + yamlPath: `$.views[${index}].id` + }) + ); + return undefined; + } + if (id === undefined || name === undefined) { diagnostics.push( diagnostic(source, { diff --git a/packages/core/src/views/types.ts b/packages/core/src/views/types.ts index a781ac4..37a4cd6 100644 --- a/packages/core/src/views/types.ts +++ b/packages/core/src/views/types.ts @@ -1,3 +1,8 @@ +// The scope the product shows when no saved view is selected. It is used as an +// id in its own right (recommendation export paths, the view picker), so a +// saved view may not claim it. +export const WHOLE_PROJECT_VIEW_ID = "whole-project"; + export type SavedQcViewDiagnosticSeverity = "error" | "warning" | "info"; export type SavedQcViewParseStatus = "parsed" | "invalid"; diff --git a/agent-skills/quality/references/improve/assets/views.schema.json b/packages/core/src/views/views.schema.json similarity index 61% rename from agent-skills/quality/references/improve/assets/views.schema.json rename to packages/core/src/views/views.schema.json index 6ecad50..e4d54f6 100644 --- a/agent-skills/quality/references/improve/assets/views.schema.json +++ b/packages/core/src/views/views.schema.json @@ -4,7 +4,9 @@ "title": "Quality Saved Assessment Scopes", "type": "object", "additionalProperties": false, - "required": ["views"], + "required": [ + "views" + ], "properties": { "views": { "type": "array", @@ -22,22 +24,35 @@ "viewId": { "type": "string", "minLength": 1, - "pattern": "^[a-z0-9]+(?:-[a-z0-9]+)*$", - "not": { "const": "whole-project" } + "not": { + "const": "whole-project" + } }, "view": { "type": "object", "additionalProperties": false, - "required": ["id", "name", "feature_ids"], + "required": [ + "id", + "name", + "feature_ids" + ], "properties": { - "id": { "$ref": "#/definitions/viewId" }, - "name": { "$ref": "#/definitions/nonEmptyString" }, - "description": { "$ref": "#/definitions/nonEmptyString" }, + "id": { + "$ref": "#/definitions/viewId" + }, + "name": { + "$ref": "#/definitions/nonEmptyString" + }, + "description": { + "$ref": "#/definitions/nonEmptyString" + }, "feature_ids": { "type": "array", "minItems": 1, "uniqueItems": true, - "items": { "$ref": "#/definitions/nonEmptyString" } + "items": { + "$ref": "#/definitions/nonEmptyString" + } } } } diff --git a/packages/core/tsup.config.ts b/packages/core/tsup.config.ts index 3adc5fe..ac49df9 100644 --- a/packages/core/tsup.config.ts +++ b/packages/core/tsup.config.ts @@ -31,5 +31,5 @@ export default defineConfig({ splitting: false, sourcemap: false, onSuccess: - "cp src/observation-sets/observation-sets.schema.json dist/observation-sets.schema.json && cp src/observation-sources/observation-sources.schema.json dist/observation-sources.schema.json && cp src/observations/quality-observations.schema.json dist/quality-observations.schema.json" + "cp src/observation-sets/observation-sets.schema.json dist/observation-sets.schema.json && cp src/observation-sources/observation-sources.schema.json dist/observation-sources.schema.json && cp src/observations/quality-observations.schema.json dist/quality-observations.schema.json && cp src/views/views.schema.json dist/views.schema.json" }); diff --git a/packages/quality-tools/package-size.json b/packages/quality-tools/package-size.json index 5e0ca99..06cc600 100644 --- a/packages/quality-tools/package-size.json +++ b/packages/quality-tools/package-size.json @@ -1,4 +1,10 @@ { "maxIncreasePercent": 1, - "approvedIncrease": null + "approvedIncrease": { + "version": "0.3.2", + "packedBytes": 46290, + "unpackedBytes": 168548, + "approvedBy": "Feng Qian", + "reason": "new features" + } } diff --git a/packages/quality-tools/src/cli.ts b/packages/quality-tools/src/cli.ts index 67fa63e..d65e730 100644 --- a/packages/quality-tools/src/cli.ts +++ b/packages/quality-tools/src/cli.ts @@ -4,6 +4,7 @@ import { runAnalyzeCommand } from "./commands/analyze"; import { runFixPromptsCommand } from "./commands/fix-prompts"; import { runObservationsCommand } from "./commands/observations"; import { runSchemaCommand } from "./commands/schema"; +import { runSetsCommand, runSourcesCommand, runViewsCommand } from "./commands/sources"; import { runValidateCommand } from "./commands/validate"; function printHelp(): void { @@ -17,7 +18,10 @@ Commands: fix-prompts Generate structural quality-evidence fix prompts. observations Produce and validate canonical workflow observations. schema Print the canonical quality-map JSON Schema. + sets Print the canonical observation-set JSON Schema. + sources Print the canonical observation-source profile JSON Schema. validate Validate a quality-map YAML file against the engine. + views Print the canonical saved-view JSON Schema. Run "quality-tools --help" for command options. `); @@ -43,6 +47,15 @@ async function main(argv: readonly string[]): Promise { if (command === "schema") { return runSchemaCommand(argv.slice(1)); } + if (command === "sets") { + return runSetsCommand(argv.slice(1)); + } + if (command === "sources") { + return runSourcesCommand(argv.slice(1)); + } + if (command === "views") { + return runViewsCommand(argv.slice(1)); + } if (command === "validate") { return runValidateCommand(argv.slice(1)); } diff --git a/packages/quality-tools/src/commands/analyze.ts b/packages/quality-tools/src/commands/analyze.ts index 90e06a1..7e1d5a2 100644 --- a/packages/quality-tools/src/commands/analyze.ts +++ b/packages/quality-tools/src/commands/analyze.ts @@ -2,7 +2,9 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { buildRecommendationExport, + createLocalReportsTransport, generateFixPrompts, + LOCAL_REPORTS_PROVIDER, type BuildRecommendationExportInput, type RecommendationFixPromptRecord } from "@shiplightai/quality-core"; @@ -230,6 +232,9 @@ export async function runAnalyzeCommand(argv: readonly string[]): Promise { expect(await runObservationsCommand(["validate", source])).toEqual({ exitCode: 0 }); }); + it("records a gate with a pointer to its run evidence", async () => { + const destination = output("quality-observations.json"); + + const result = await runObservationsCommand([ + "record", + "--path", + ".github/workflows/publish.yml", + "--status", + "pass", + "--commit", + "abc123", + "--observed-at", + "2026-07-26T18:00:00Z", + "--artifact-ref", + "https://app.shiplight.ai/runs/8412?test=99231", + "--artifact-label", + "Shiplight run 8412", + "--output", + destination + ]); + + expect(result).toEqual({ exitCode: 0 }); + expect(JSON.parse(readFileSync(destination, "utf8")).observations).toEqual([ + { + path: ".github/workflows/publish.yml", + status: "pass", + artifacts: [ + { ref: "https://app.shiplight.ai/runs/8412?test=99231", label: "Shiplight run 8412" } + ] + } + ]); + }); + + it("keeps run evidence when merging manifests from the same run", async () => { + // Merge is how a sharded CI job assembles one manifest. Losing the refs here + // would silently strip evidence from every sharded suite. + const first = output("first.json"); + const second = output("second.json"); + const merged = output("merged.json"); + const envelope = { + schema_version: 1, + revision: { commit: "abc123" }, + observed_at: "2026-07-26T18:00:00Z" + }; + writeFileSync( + first, + JSON.stringify({ + ...envelope, + observations: [ + { + path: "tests/checkout.yaml", + status: "pass", + artifacts: [{ ref: "https://app.shiplight.ai/runs/8412?test=1" }] + } + ] + }) + ); + writeFileSync( + second, + JSON.stringify({ + ...envelope, + observations: [{ path: "tests/login.yaml", status: "pass" }] + }) + ); + + expect(await runObservationsCommand(["merge", first, second, "--output", merged])).toEqual({ + exitCode: 0 + }); + expect(JSON.parse(readFileSync(merged, "utf8")).observations).toEqual([ + { + path: "tests/checkout.yaml", + status: "pass", + artifacts: [{ ref: "https://app.shiplight.ai/runs/8412?test=1" }] + }, + { path: "tests/login.yaml", status: "pass" } + ]); + }); + + it("prints the checked-in observation-source schema", async () => { + // The engine's JSON file and the emitted schema must stay identical, so the + // skill can fetch the schema instead of vendoring a copy that silently rots. + const write = vi.spyOn(process.stdout, "write").mockReturnValue(true); + const expected = readFileSync( + join(process.cwd(), "packages/core/src/observation-sources/observation-sources.schema.json"), + "utf8" + ); + + expect(runSourcesCommand(["schema"])).toEqual({ exitCode: 0 }); + expect(String(write.mock.calls[0]?.[0])).toEqual(expected); + }); + + it("prints the checked-in observation-set schema", async () => { + const write = vi.spyOn(process.stdout, "write").mockReturnValue(true); + const expected = readFileSync( + join(process.cwd(), "packages/core/src/observation-sets/observation-sets.schema.json"), + "utf8" + ); + + expect(runSetsCommand(["schema"])).toEqual({ exitCode: 0 }); + expect(String(write.mock.calls[0]?.[0])).toEqual(expected); + }); + + it("prints the checked-in saved-view schema", async () => { + const write = vi.spyOn(process.stdout, "write").mockReturnValue(true); + const expected = readFileSync( + join(process.cwd(), "packages/core/src/views/views.schema.json"), + "utf8" + ); + + expect(runViewsCommand(["schema"])).toEqual({ exitCode: 0 }); + expect(String(write.mock.calls[0]?.[0])).toEqual(expected); + }); + + it("rejects an unknown sources subcommand instead of printing something else", async () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + + expect(runSourcesCommand(["nonsense"])).toEqual({ exitCode: 1 }); + }); + it("prints the checked-in canonical schema", async () => { const write = vi.spyOn(process.stdout, "write").mockReturnValue(true); const expected = readFileSync( diff --git a/packages/quality-tools/src/commands/observations.ts b/packages/quality-tools/src/commands/observations.ts index b21b60b..f53b1b6 100644 --- a/packages/quality-tools/src/commands/observations.ts +++ b/packages/quality-tools/src/commands/observations.ts @@ -28,6 +28,8 @@ interface ProducerOptions { readonly testCase?: string; readonly status?: string; readonly note?: string; + readonly artifactRef?: string; + readonly artifactLabel?: string; readonly help: boolean; } @@ -50,6 +52,12 @@ Producer metadata: --run-id Optional run identity. Defaults to GITHUB_RUN_ID. --run-url Optional run URL. Derived from GitHub environment when available. +Run evidence (record only): + --artifact-ref Opaque pointer to this result's run evidence — a report + URL, a run page, a local path. Recorded verbatim and + never interpreted. + --artifact-label Optional human-readable label for that pointer. + Canonical statuses: pass, fail, error, skipped. `); } @@ -67,6 +75,8 @@ function parseOptions(argv: readonly string[]): ProducerOptions { testCase?: string; status?: string; note?: string; + artifactRef?: string; + artifactLabel?: string; dirty: boolean; help: boolean; } = { @@ -95,7 +105,9 @@ function parseOptions(argv: readonly string[]): ProducerOptions { "--path": "path", "--test-case": "testCase", "--status": "status", - "--note": "note" + "--note": "note", + "--artifact-ref": "artifactRef", + "--artifact-label": "artifactLabel" }; const key = keys[arg]; if (key !== undefined) { @@ -201,7 +213,11 @@ function canonicalRecord( ...(testCase === undefined ? {} : { test_case: testCase }), status: observation.status, ...(observation.observedAt === envelopeObservedAt ? {} : { observed_at: observation.observedAt }), - ...(observation.note === undefined ? {} : { note: observation.note }) + ...(observation.note === undefined ? {} : { note: observation.note }), + // Adapters that carry run evidence must not lose it at serialization. The + // bundled junit/playwright adapters emit none today, so this is a no-op for + // them — it is here so adding one later needs no change in this file. + ...(observation.evidenceRefs.length === 0 ? {} : { artifacts: observation.evidenceRefs }) }; } @@ -252,6 +268,9 @@ function convertNativeReport(kind: "junit" | "playwright", sourcePath: string, o } function recordObservation(options: ProducerOptions): void { + if (options.artifactRef === undefined && options.artifactLabel !== undefined) { + throw new Error("--artifact-label requires --artifact-ref."); + } const status = options.status as ObservationRecordStatus | undefined; if (status === undefined || !new Set(["pass", "fail", "error", "skipped"]).has(status)) { throw new Error("--status must be one of: pass, fail, error, skipped."); @@ -264,7 +283,21 @@ function recordObservation(options: ProducerOptions): void { path: nonEmpty(options.path, "--path"), ...(options.testCase === undefined ? {} : { test_case: nonEmpty(options.testCase, "--test-case") }), status, - ...(options.note === undefined ? {} : { note: nonEmpty(options.note, "--note") }) + ...(options.note === undefined ? {} : { note: nonEmpty(options.note, "--note") }), + // A label with no ref points at nothing. Silently dropping it would let a + // CI step believe it published run evidence when it published none. + ...(options.artifactRef === undefined + ? {} + : { + artifacts: [ + { + ref: nonEmpty(options.artifactRef, "--artifact-ref"), + ...(options.artifactLabel === undefined + ? {} + : { label: nonEmpty(options.artifactLabel, "--artifact-label") }) + } + ] + }) } ] }); diff --git a/packages/quality-tools/src/commands/sources.ts b/packages/quality-tools/src/commands/sources.ts new file mode 100644 index 0000000..135104b --- /dev/null +++ b/packages/quality-tools/src/commands/sources.ts @@ -0,0 +1,85 @@ +import { + serializeObservationSetsJsonSchema, + serializeObservationSourceProfilesJsonSchema, + serializeSavedViewsJsonSchema +} from "@shiplightai/quality-core"; +import { printCommandError, type CommandResult } from "./result"; + +function printSourcesHelp(): void { + console.log(`Print the canonical observation-source profile JSON Schema to stdout. + +Usage: + quality-tools sources schema + +The schema is emitted from this quality-tools version's own contract constants, so it always +matches the parser that reads .quality/config/observation-sources.yaml — there is no second +copy to keep in sync. Fetch it rather than vendoring it: + quality-tools sources schema > observation-sources.schema.json +`); +} + +export function runSourcesCommand(argv: readonly string[]): CommandResult { + return runSchemaSubcommand( + argv, + printSourcesHelp, + "sources", + serializeObservationSourceProfilesJsonSchema + ); +} + +function printSetsHelp(): void { + console.log(`Print the canonical observation-set JSON Schema to stdout. + +Usage: + quality-tools sets schema + +Emitted from this quality-tools version's own contract constants, so it always +matches the parser that reads .quality/config/observation-sets.yaml. +`); +} + +export function runSetsCommand(argv: readonly string[]): CommandResult { + return runSchemaSubcommand(argv, printSetsHelp, "sets", serializeObservationSetsJsonSchema); +} + +function printViewsHelp(): void { + console.log(`Print the canonical saved-view JSON Schema to stdout. + +Usage: + quality-tools views schema + +Emitted from this quality-tools version's own contract constants, so it always +matches the parser that reads .quality/config/views.yaml. +`); +} + +export function runViewsCommand(argv: readonly string[]): CommandResult { + return runSchemaSubcommand(argv, printViewsHelp, "views", serializeSavedViewsJsonSchema); +} + +// The three config schemas differ only in which serializer they print, so the +// argument handling — help, unknown subcommand, error shape — lives once. +function runSchemaSubcommand( + argv: readonly string[], + printHelp: () => void, + command: string, + serialize: () => string +): CommandResult { + const subcommand = argv[0]; + + if (argv.includes("--help") || argv.includes("-h") || subcommand === undefined) { + printHelp(); + return { exitCode: 0 }; + } + + try { + if (subcommand !== "schema") { + throw new Error(`Unknown ${command} subcommand: ${subcommand}`); + } + + process.stdout.write(serialize()); + return { exitCode: 0 }; + } catch (error) { + return printCommandError(error); + } +} diff --git a/packages/ui/package-size.json b/packages/ui/package-size.json index c9bdc43..43539b4 100644 --- a/packages/ui/package-size.json +++ b/packages/ui/package-size.json @@ -1,6 +1,6 @@ { "baselineVersion": "0.1.0", - "baselinePackedBytes": 37350, - "baselineUnpackedBytes": 161150, + "baselinePackedBytes": 39089, + "baselineUnpackedBytes": 166479, "maxIncreasePercent": 1 } diff --git a/packages/ui/src/components/FeaturePage.tsx b/packages/ui/src/components/FeaturePage.tsx index 6daa7e9..828a572 100644 --- a/packages/ui/src/components/FeaturePage.tsx +++ b/packages/ui/src/components/FeaturePage.tsx @@ -1,14 +1,16 @@ "use client"; -import { useQcApi, useQcRoute } from "../host"; +import { useQcApi, useQcHost, useQcRoute } from "../host"; +import { useQcScanCache } from "./scan-cache"; +import { evidenceRefHref } from "../lib/evidence-ref"; import { Breadcrumb } from "./Breadcrumb"; import { MarkdownOverlay } from "./MarkdownOverlay"; import Link from "next/link"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { CheckCircle2, ChevronRight, Copy } from "lucide-react"; +import { CheckCircle2, ChevronRight, Copy, ExternalLink } from "lucide-react"; import { Alert, Anchor, Badge, Button, Collapse, Group, Paper, Select, Stack, Text, TextInput, Title, Tooltip, UnstyledButton } from "@mantine/core"; -import type { ScanResult } from "@shiplightai/quality-core"; +import type { EvaluatedEvidenceObservation, NormalizedEvidenceRef, ScanResult } from "@shiplightai/quality-core"; import { buildProjectIndex } from "@shiplightai/quality-core/project-index"; import { buildGapTriage, type GapRecord } from "@shiplightai/quality-core/gap-triage"; import { canonicalFixPromptForGap } from "../lib/fix-prompt"; @@ -128,6 +130,21 @@ function checkEvidence(graph: QualityGraph, expectation: QualityCheck): QualityG return graph.evidence.filter((entry) => expectation.linkedEvidenceIds.includes(entry.normalizedId)); } +function observedStateColor(state: string): string { + switch (state) { + case "pass": + return "green"; + case "fail": + return "red"; + case "error": + return "orange"; + case "skipped": + return "gray"; + default: + return "gray"; + } +} + // Only Markdown artifacts can be previewed inline (the artifact/markdown endpoint reads text). function canPreviewMarkdownPath(path: string): boolean { const normalized = path.toLowerCase(); @@ -154,6 +171,8 @@ export function FeaturePage({ }): React.ReactElement { const qcApi = useQcApi(); const qcRoute = useQcRoute(); + const { servesEvidenceFiles = false } = useQcHost(); + const scanCache = useQcScanCache(); const [result, setResult] = useState(); const [isLoading, setIsLoading] = useState(false); // Set true in the effect body (not just useRef init): StrictMode/remount runs cleanup→setup, else a cleanup-only ref stays false and the loader discards its result. @@ -234,6 +253,43 @@ export function FeaturePage({ ); const expectations = graph?.expectations ?? []; + // Runtime proof for the observation set the viewer last ran on the scanner + // page. Inherited rather than re-run: this page has no picker, and running a + // set can mean a network fetch. + const runtime = projectKey === null ? undefined : scanCache?.getRuntime(projectKey); + const observed = useMemo(() => { + const byEvidenceId = new Map(); + const targetId = graph?.target.normalizedId; + if (runtime === undefined || targetId === undefined) { + return { byEvidenceId, coversFeature: false, evaluatedAt: undefined as string | undefined }; + } + + // Whether the run evaluated THIS target at all. A view-scoped run evaluates + // only the targets inside its view, so an empty map here can mean the run + // never looked rather than that it looked and found nothing — two states + // the page must not present the same way. + let coversFeature = false; + let evaluatedAt: string | undefined; + + for (const group of runtime.evaluations) { + for (const target of group.targets) { + if (target.targetId !== targetId) { + continue; + } + coversFeature = true; + evaluatedAt = target.evaluatedAt; + for (const expectation of target.expectations) { + for (const entry of expectation.evidence) { + byEvidenceId.set(entry.evidenceId, entry); + } + } + } + } + + return { byEvidenceId, coversFeature, evaluatedAt }; + }, [graph?.target.normalizedId, runtime]); + const observedEvidence = observed.byEvidenceId; + // Gap records per check (spec 045): the classified gaps — category label ("Weak evidence"), the // residual-risk text, the recommended next proof, and the fix-prompt lookup — the same model the old // Explorer used. Built from the feature's target, keyed by the check's expectation localId. @@ -419,6 +475,23 @@ export function FeaturePage({ ) : qualityMapPath === undefined || graph === undefined ? ( This feature has no quality checks yet. ) : ( + <> + {/* Runtime state on this page is only as good as its attribution: a + check reading "pass" means nothing unless a reviewer can see which + run said so. With nothing loaded the page stays structural rather + than painting every check `unobserved`, which reads like a failure + when it is only a question nobody has asked yet. */} + + {runtime === undefined + ? "No test results loaded. Run an observation set on the dashboard to see runtime proof here." + : observed.coversFeature + ? `Runtime proof from observation set: ${runtime.observationSetName}${ + observed.evaluatedAt === undefined ? "" : ` · evaluated ${observed.evaluatedAt}` + }` + : `Observation set ${runtime.observationSetName} did not cover this feature${ + runtime.viewId === undefined ? "" : ` — it ran scoped to the saved view ${runtime.viewId}` + }.`} + {expectations.length === 0 ? ( No quality checks yet. Copy the add-check instruction below to have your agent add one. @@ -483,12 +556,36 @@ export function FeaturePage({ ) : ( {evidence.length > 0 ? ( - - {evidence.map((entry) => ( - - {evidenceLabel(entry)} - - ))} + + {evidence.map((entry) => { + const observed = observedEvidence.get(entry.normalizedId); + return ( + + + + {evidenceLabel(entry)} + + {observed === undefined ? null : ( + + {observed.state} + + )} + + {observed?.evidenceRefs.map((ref: NormalizedEvidenceRef, refIndex: number) => { + const href = evidenceRefHref(ref.ref, qcApi, servesEvidenceFiles); + return href === undefined ? ( + + {ref.label ?? "Run evidence"}: {ref.ref} + + ) : ( + + {ref.label ?? "Run evidence"} + + ); + })} + + ); + })} ) : null} {checkGaps.map((gap) => { @@ -607,6 +704,7 @@ export function FeaturePage({ + )} diff --git a/packages/ui/src/components/ObservationAuditPanel.test.tsx b/packages/ui/src/components/ObservationAuditPanel.test.tsx new file mode 100644 index 0000000..384a9b3 --- /dev/null +++ b/packages/ui/src/components/ObservationAuditPanel.test.tsx @@ -0,0 +1,151 @@ +// @vitest-environment jsdom + +import "@testing-library/jest-dom/vitest"; +import { MantineProvider } from "@mantine/core"; +import { cleanup, screen } from "@testing-library/react"; +import { render } from "../testing"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import type { ObservationResolutionAuditRow } from "@shiplightai/quality-core"; +import { QcUiHostProvider, type QcUiHost } from "../host"; +import { ObservationAuditPanel } from "./ObservationAuditPanel"; + +beforeAll(() => { + Object.defineProperty(window, "matchMedia", { + writable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn() + })) + }); +}); + +afterEach(() => cleanup()); + +function auditRow(overrides: Partial = {}): ObservationResolutionAuditRow { + return { + observationId: "obs-1", + matchStatus: "matched", + testFile: "tests/checkout.yaml", + testCase: "guest can pay", + context: "runtime-review", + status: "pass", + observedAt: "2026-08-27T10:00:00.000Z", + evidenceRefs: [], + ...overrides + }; +} + +function hostWith(servesEvidenceFiles: boolean): QcUiHost { + return { + routeBase: "/quality-explorer", + apiBase: "/api/quality-explorer", + setProject: async () => ({ ok: true }), + servesEvidenceFiles + }; +} + +function renderPanel( + rows: readonly ObservationResolutionAuditRow[], + servesEvidenceFiles = true +): void { + render( + + + {}} /> + + + ); +} + +describe("ObservationAuditPanel run evidence", () => { + it("links an http ref and shows the host it points at", () => { + // Refs are written by evidence producers, so the viewer is told where the + // link goes before clicking rather than trusting a producer-chosen label. + renderPanel([ + auditRow({ + evidenceRefs: [ + { ref: "https://app.shiplight.ai/runs/8412?test=99231", label: "Shiplight run 8412" } + ] + }) + ]); + + const link = screen.getByRole("link", { name: /Shiplight run 8412/ }); + expect(link).toHaveAttribute("href", "https://app.shiplight.ai/runs/8412?test=99231"); + expect(link).toHaveAttribute("rel", expect.stringContaining("noopener")); + expect(link).toHaveAttribute("target", "_blank"); + expect(screen.getByText("app.shiplight.ai")).toBeInTheDocument(); + }); + + it("falls back to a generic label when the producer supplied none", () => { + renderPanel([auditRow({ evidenceRefs: [{ ref: "https://app.shiplight.ai/runs/8412" }] })]); + + expect(screen.getByRole("link", { name: /Run evidence/ })).toBeInTheDocument(); + }); + + it("serves a project-relative ref through the host, as path segments", () => { + // Path segments, not a query parameter: the reports these refs point at + // fetch their own video and trace with relative urls, which only resolve if + // the served page sits at the same shape of address as its folder. + renderPanel([ + auditRow({ evidenceRefs: [{ ref: "playwright-report/index.html", label: "Test report" }] }) + ]); + + const link = screen.getByRole("link", { name: /Test report/ }); + expect(link).toHaveAttribute( + "href", + "/api/quality-explorer/evidence-file/playwright-report/index.html" + ); + expect(screen.getByText("playwright-report/index.html")).toBeInTheDocument(); + }); + + it("encodes each path segment without collapsing the path", () => { + renderPanel([auditRow({ evidenceRefs: [{ ref: "reports/my run/index.html" }] })]); + + expect(screen.getByRole("link", { name: /Run evidence/ })).toHaveAttribute( + "href", + "/api/quality-explorer/evidence-file/reports/my%20run/index.html" + ); + }); + + it("shows a project-relative ref as text when the host cannot serve files", () => { + // A hosted reader with no local checkout has nothing to serve. Linking + // anyway would render a link that 404s on click. + renderPanel( + [auditRow({ evidenceRefs: [{ ref: "playwright-report/index.html", label: "Test report" }] })], + false + ); + + expect(screen.queryByRole("link", { name: /Test report/ })).not.toBeInTheDocument(); + expect(screen.getByText(/playwright-report\/index\.html/)).toBeInTheDocument(); + }); + + it("renders no run evidence section when the observation carried no refs", () => { + renderPanel([auditRow()]); + + expect(screen.queryByLabelText("Run evidence")).not.toBeInTheDocument(); + }); + + it("keeps run evidence separate from the workflow run link", () => { + renderPanel([ + auditRow({ + runUrl: "https://github.com/ShiplightAI/shipyard/actions/runs/42", + evidenceRefs: [{ ref: "https://app.shiplight.ai/runs/8412", label: "Shiplight run 8412" }] + }) + ]); + + expect(screen.getByRole("link", { name: /Open workflow result/ })).toHaveAttribute( + "href", + "https://github.com/ShiplightAI/shipyard/actions/runs/42" + ); + expect(screen.getByRole("link", { name: /Shiplight run 8412/ })).toHaveAttribute( + "href", + "https://app.shiplight.ai/runs/8412" + ); + }); +}); diff --git a/packages/ui/src/components/ObservationAuditPanel.tsx b/packages/ui/src/components/ObservationAuditPanel.tsx index 5ee7b31..6e45918 100644 --- a/packages/ui/src/components/ObservationAuditPanel.tsx +++ b/packages/ui/src/components/ObservationAuditPanel.tsx @@ -3,7 +3,9 @@ import { ExternalLink, X } from "lucide-react"; import { ActionIcon, Anchor, Badge, Code, Group, Paper, Stack, Text, Title } from "@mantine/core"; import { useMemo, useState } from "react"; -import type { ObservationResolutionAuditRow } from "@shiplightai/quality-core"; +import { useQcApi, useQcHost } from "../host"; +import { evidenceRefDestination, evidenceRefHref } from "../lib/evidence-ref"; +import type { NormalizedEvidenceRef, ObservationResolutionAuditRow } from "@shiplightai/quality-core"; type MatchFilter = "all" | ObservationResolutionAuditRow["matchStatus"]; @@ -12,6 +14,10 @@ interface ObservationAuditPanelProps { onClose(): void; } +function evidenceLabel(entry: NormalizedEvidenceRef): string { + return entry.label ?? "Run evidence"; +} + function sourceLabel(row: ObservationResolutionAuditRow): string { return row.testFile ?? row.testClass ?? row.observationId; } @@ -53,6 +59,8 @@ export function ObservationAuditPanel({ rows, onClose }: ObservationAuditPanelProps): React.ReactElement { + const qcApi = useQcApi(); + const { servesEvidenceFiles = false } = useQcHost(); const [filter, setFilter] = useState("all"); const filteredRows = useMemo( () => rows.filter((row) => filter === "all" || row.matchStatus === filter), @@ -128,6 +136,35 @@ export function ObservationAuditPanel({ Open workflow result ) : null} + + {row.evidenceRefs.length > 0 ? ( + + Run evidence + {row.evidenceRefs.map((entry, entryIndex) => { + const href = evidenceRefHref(entry.ref, qcApi, servesEvidenceFiles); + const destination = evidenceRefDestination(entry.ref); + return href === undefined ? ( + + {evidenceLabel(entry)}: {entry.ref} + + ) : ( + + + {evidenceLabel(entry)} + + + {destination} + + + ); + })} + + ) : null} ))} diff --git a/packages/ui/src/components/ObservationSourcesView.tsx b/packages/ui/src/components/ObservationSourcesView.tsx index d04be0d..499ef51 100644 --- a/packages/ui/src/components/ObservationSourcesView.tsx +++ b/packages/ui/src/components/ObservationSourcesView.tsx @@ -9,8 +9,9 @@ export interface ObservationSourceRow { readonly id: string; readonly name: string; readonly description?: string; - readonly transport: "github-actions" | "local-folder"; - readonly observationPath: string; + readonly transport: "github-actions" | "local-folder" | "host"; + // Absent for a host source, which addresses no file. + readonly observationPath?: string; readonly github?: { readonly repo: string; readonly workflow: string; @@ -18,6 +19,7 @@ export interface ObservationSourceRow { readonly branch?: string; }; readonly localFolder?: { readonly path: string }; + readonly host?: { readonly provider: string; readonly options: Readonly> }; } // Read-only view of the repo's observation sources (spec 045). Authoring moved to the repo — every @@ -97,11 +99,20 @@ export function ObservationSourcesView({ folder: {profile.localFolder.path || "—"} + ) : profile.host !== undefined ? ( + + provider: {profile.host.provider} + {Object.entries(profile.host.options) + .map(([key, value]) => ` · ${key}=${value}`) + .join("")} + ) : null} - - observations: {profile.observationPath} - + {profile.observationPath === undefined ? null : ( + + observations: {profile.observationPath} + + )} ); diff --git a/packages/ui/src/components/ProjectScanner.tsx b/packages/ui/src/components/ProjectScanner.tsx index 871767c..385c074 100644 --- a/packages/ui/src/components/ProjectScanner.tsx +++ b/packages/ui/src/components/ProjectScanner.tsx @@ -541,11 +541,16 @@ export function ProjectScanner({ setCurrentResult(scanResponse.result); setObservationSourceEnv(scanResponse.observationSourceEnv); // Cache this scan under the current project so navigating away and back reuses it. + // Drop the cached runtime with it: an evaluation is only meaningful against the + // structure it was resolved onto, and this scan may have replaced that structure. + // Keeping it would let a feature page paint pass/fail from a run that never saw + // the checks now on screen — a claim about the wrong thing, attributed to a real set. if (projectKey !== null) { scanCache?.set(projectKey, { result: scanResponse.result, observationSourceEnv: scanResponse.observationSourceEnv, }); + scanCache?.clearRuntime(projectKey); } setIsObservationAuditOpen(false); setLastAttemptDiagnostics( @@ -635,6 +640,27 @@ export function ProjectScanner({ const executionResponse = payload as ObservationSetExecutionResponse; setObservationExecution(executionResponse.result); + // Hand the evaluated result to the shared cache so a feature page shows + // proof for the set the viewer just ran, rather than running its own. + // + // Only when the run actually produced evaluations. A run with no usable + // proof — a local report that has not been generated yet, say — still + // answers 200 and returns an empty `evaluations`, and caching that would + // put "Runtime proof from " above a feature whose checks carry no + // badges at all: a claim of proof that is really an absence of it. + if (projectKey !== null && executionResponse.result.evaluations.length > 0) { + const observationSet = observationSets.find( + (candidate) => candidate.id === selectedObservationSetId + ); + scanCache?.setRuntime(projectKey, { + observationSetId: selectedObservationSetId, + observationSetName: observationSet?.name ?? selectedObservationSetId, + ...(selectedView?.id === undefined ? {} : { viewId: selectedView.id }), + evaluations: executionResponse.result.evaluations + }); + } else if (projectKey !== null) { + scanCache?.clearRuntime(projectKey); + } } catch { setObservationExecutionDiagnostics([ fallbackDiagnostic("The observation set could not be executed.") diff --git a/packages/ui/src/components/Settings.tsx b/packages/ui/src/components/Settings.tsx index cf6b7b8..8b46345 100644 --- a/packages/ui/src/components/Settings.tsx +++ b/packages/ui/src/components/Settings.tsx @@ -71,7 +71,8 @@ export function Settings({ artifactNames: p.github.artifactNames, branch: p.github.branch }, - localFolder: p.localFolder === undefined ? undefined : { path: p.localFolder.path } + localFolder: p.localFolder === undefined ? undefined : { path: p.localFolder.path }, + host: p.host === undefined ? undefined : { provider: p.host.provider, options: p.host.options } })), [profiles] ); diff --git a/packages/ui/src/components/scan-cache.tsx b/packages/ui/src/components/scan-cache.tsx index 19c879d..78e6dc1 100644 --- a/packages/ui/src/components/scan-cache.tsx +++ b/packages/ui/src/components/scan-cache.tsx @@ -1,7 +1,11 @@ "use client"; import { createContext, useContext, useRef, type ReactNode } from "react"; -import type { ObservationSourceProfileEnvStatus, ScanResult } from "@shiplightai/quality-core"; +import type { + ObservationSourceProfileEnvStatus, + ScanResult, + TargetEvaluationSnapshot, +} from "@shiplightai/quality-core"; // Per-project scan cache (spec 045). Mounted in the QC layout, which persists across its child // routes (overview / reviews / explorer), so a scan of a given project is reused when the user @@ -13,11 +17,43 @@ export interface CachedScan { readonly observationSourceEnv: readonly ObservationSourceProfileEnvStatus[]; } +/** + * The last observation set a viewer ran for a project, so a feature page shows + * runtime proof for that same set instead of asking again or re-running it. + * + * Cached rather than re-fetched because running a set is not free: a + * github-actions source downloads artifacts over the network, and re-running it + * on every feature page view would turn a navigation into a fetch. It is also + * the only way the selection can be *inherited* — the run happens on the + * scanner page, and the feature page has no picker of its own. + * + * The set id and name ride along because a feature page showing runtime state + * must be able to say which set produced it; "observed" with no attribution is + * a claim a reviewer cannot check. `viewId` rides along for the same reason: + * a view-scoped run evaluates only the targets inside that view, so a feature + * outside it has no snapshot for a reason the page must be able to state + * instead of rendering an empty proof column that reads like a failure. + */ +export interface CachedRuntime { + readonly observationSetId: string; + readonly observationSetName: string; + /** The saved view the run was scoped to, when it was scoped to one. */ + readonly viewId?: string; + readonly evaluations: readonly { readonly targets: readonly TargetEvaluationSnapshot[] }[]; +} + interface QcScanCache { get(projectKey: string): CachedScan | undefined; set(projectKey: string, value: CachedScan): void; + getRuntime(projectKey: string): CachedRuntime | undefined; + setRuntime(projectKey: string, value: CachedRuntime): void; + // Drop only the runtime, keeping the scan — what a re-scan needs, since the fresh + // structure is worth caching but the evaluation resolved onto the old one is not. + clearRuntime(projectKey: string): void; // Drop a project's cached scan (or all, when no key) after a draft mutation (save/publish/discard) // so a subsequent ProjectScanner page re-scans instead of showing the pre-mutation snapshot. + // Runtime goes with it: an evaluation is only meaningful against the structure it was resolved + // onto, so keeping it across a re-scan could show proof against checks that have since changed. invalidate(projectKey?: string): void; } @@ -25,14 +61,27 @@ const QcScanCacheContext = createContext(null); export function QcScanCacheProvider({ children }: { readonly children: ReactNode }): React.ReactElement { const store = useRef>(new Map()); + const runtimeStore = useRef>(new Map()); const cacheRef = useRef({ get: (key) => store.current.get(key), set: (key, value) => { store.current.set(key, value); }, + getRuntime: (key) => runtimeStore.current.get(key), + setRuntime: (key, value) => { + runtimeStore.current.set(key, value); + }, + clearRuntime: (key) => { + runtimeStore.current.delete(key); + }, invalidate: (key) => { - if (key === undefined) store.current.clear(); - else store.current.delete(key); + if (key === undefined) { + store.current.clear(); + runtimeStore.current.clear(); + } else { + store.current.delete(key); + runtimeStore.current.delete(key); + } }, }); return {children}; diff --git a/packages/ui/src/host.tsx b/packages/ui/src/host.tsx index 97cfa4a..518c6f0 100644 --- a/packages/ui/src/host.tsx +++ b/packages/ui/src/host.tsx @@ -41,6 +41,16 @@ export interface QcUiHost { readonly setProject: ( project: QcProjectSelection, ) => Promise<{ readonly ok: true } | { readonly error: string }>; + /** + * Whether this host serves run-evidence files out of the opened project at + * `{apiBase}/evidence-file/`. + * + * A run-evidence ref that is not already a URL is a path into the project, and + * only a host reading that project can turn it into something openable. + * Quality Explorer can; a hosted reader with no local checkout cannot, and + * without this flag it would render a link that 404s. Absent means no. + */ + readonly servesEvidenceFiles?: boolean; } const QcUiHostContext = createContext(null); @@ -56,7 +66,7 @@ export function QcUiHostProvider({ // invalidate every consumer. `setProject` is a stable Server Action reference in both hosts. const value = useMemo( () => host, - [host.routeBase, host.apiBase, host.setProject], + [host.routeBase, host.apiBase, host.setProject, host.servesEvidenceFiles], ); return {children}; } diff --git a/packages/ui/src/lib/evidence-ref.ts b/packages/ui/src/lib/evidence-ref.ts new file mode 100644 index 0000000..f309e26 --- /dev/null +++ b/packages/ui/src/lib/evidence-ref.ts @@ -0,0 +1,63 @@ +// How a run-evidence ref becomes something a viewer can open. One place, +// because two surfaces render refs — the feature page's proof column and the +// join audit panel — and a rule that lives in both diverges: they would start +// disagreeing about which refs are links, which is the sort of difference +// nobody notices until a reviewer reports a dead one. +// +// The engine never interprets a ref. This is the single point where the UI +// looks at one at all, and it looks only at whether the producer already gave +// us something a browser can open, or whether the host can turn a project path +// into something it can. + +export function isAbsoluteEvidenceUrl(ref: string): boolean { + return /^https?:\/\//i.test(ref); +} + +/** + * The URL to open for a ref, or `undefined` when it must render as text. + * + * A project path is passed through as PATH SEGMENTS rather than a query + * parameter: the reports these refs point at fetch their own video and trace + * with relative urls, so the served page has to sit at the same shape of + * address as the folder it came from, or those resolve to nothing. + * + * A host that cannot serve project files gets `undefined` rather than a link, + * because a hosted reader with no local checkout has nothing behind it. + */ +export function evidenceRefHref( + ref: string, + qcApi: (path: string) => string, + servesEvidenceFiles: boolean +): string | undefined { + if (isAbsoluteEvidenceUrl(ref)) { + return ref; + } + + if (!servesEvidenceFiles) { + return undefined; + } + + const segments = ref + .split("/") + .filter((segment) => segment.length > 0) + .map((segment) => encodeURIComponent(segment)); + return segments.length === 0 ? undefined : qcApi(`/evidence-file/${segments.join("/")}`); +} + +/** + * What to show beside the link. Refs are written by evidence producers, so the + * destination is shown rather than hidden behind a label the producer also + * chose. A project-relative ref shows its path instead: the host it resolves to + * is this application, which tells the reader nothing. + */ +export function evidenceRefDestination(ref: string): string { + if (!isAbsoluteEvidenceUrl(ref)) { + return ref; + } + + try { + return new URL(ref).host; + } catch { + return ref; + } +} diff --git a/scripts/check-quality-skill.sh b/scripts/check-quality-skill.sh index 5a31118..bbdb833 100755 --- a/scripts/check-quality-skill.sh +++ b/scripts/check-quality-skill.sh @@ -102,6 +102,17 @@ if [[ -e "${skill_root}/references/improve/assets/quality-observations.schema.js fail "use the published observations schema instead of a bundled static copy" fi +# Same rule for every config schema. A vendored copy cannot be checked against +# the engine's, so it drifts the moment the contract moves -- which is exactly +# what happened when the `host` transport landed, and what left the views copy +# enforcing rules the parser did not. Each is emitted from the engine's own +# constants by `quality-tools schema`; fetch that instead. +for vendored in observation-sources observation-sets views; do + if [[ -e "${skill_root}/references/improve/assets/${vendored}.schema.json" ]]; then + fail "use a quality-tools schema command instead of bundling ${vendored}.schema.json" + fi +done + forbid_regex \ 'machine-readable result|acquisition/parser problems|acquired and parsed' \ "${skill_root}" \ diff --git a/tests/contract/observation-run-evidence.contract.test.ts b/tests/contract/observation-run-evidence.contract.test.ts new file mode 100644 index 0000000..8b7bc76 --- /dev/null +++ b/tests/contract/observation-run-evidence.contract.test.ts @@ -0,0 +1,536 @@ +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + buildMarkdownFallbackBatch, + buildShiplightObservationBatch, + buildTargetEvaluation, + executeObservationSourceProfile, + ingestObservationManifest, + parseObservationSourceProfiles, + parseQualityObservationManifest, + resolveObservations, + type HostObservationTransportRegistry, + type ObservationSourceProfile +} from "@shiplightai/quality-core"; +import { parseQualityMaps, type QualityMapSource } from "@shiplightai/quality-map"; +import { createFixtureProject } from "../fixtures/quality-projects/build-fixtures"; +import { projectIndexScanResult } from "../fixtures/project-index/build-fixtures"; + +const checksFixtureRoot = path.resolve("tests/fixtures/observations/checks"); + +function checksScanResult() { + const source: QualityMapSource = { + projectRelativePath: "checks/quality-map.yaml", + resolvedLocalPath: path.join(checksFixtureRoot, "quality-map.yaml"), + targetCandidateId: "ci-runner", + sourcePattern: "tests/fixtures/observations/checks/**/quality-map.yaml" + }; + const qualityMaps = parseQualityMaps([source]); + return projectIndexScanResult({ + qualityMaps, + markdownFallback: buildMarkdownFallbackBatch({ sources: [], qualityMaps }) + }); +} + +// Run evidence is the pointer a producer keeps for one result — the video, the +// report, the run page. Quality records it and shows it; it never interprets it. +// These tests pin that boundary: what the contract accepts, that an opaque ref +// survives ingestion unchanged, and that a bad pointer never costs us the +// pass/fail fact it was attached to. + +function manifest(observations: readonly Record[]): string { + return JSON.stringify({ + schema_version: 1, + revision: { commit: "abc123", branch: "main" }, + run: { id: "run-42", url: "https://ci.example.test/runs/42" }, + observed_at: "2026-08-27T10:00:00Z", + observations + }); +} + +const SHIPLIGHT_REF = "https://app.shiplight.ai/runs/8412?test=99231"; + +describe("observation run evidence contract", () => { + it("carries an opaque artifact ref from the manifest onto the ingested observation", () => { + const result = ingestObservationManifest({ + report_json: manifest([ + { + path: "tests/checkout.yaml", + test_case: "guest can pay", + status: "pass", + artifacts: [{ ref: SHIPLIGHT_REF, label: "Shiplight run 8412" }] + } + ]), + source: { id: "shiplight", kind: "host", label: "Shiplight" } + }); + + expect(result.status).toBe("valid"); + expect(result.diagnostics).toEqual([]); + expect(result.observations[0]?.evidenceRefs).toEqual([ + { ref: SHIPLIGHT_REF, label: "Shiplight run 8412" } + ]); + }); + + it("records a ref that is not a URL without rewriting or resolving it", () => { + // A local report path is as valid a ref as a URL. The engine must not try to + // turn it into one — only the integration that wrote it knows what it means. + const localRef = "test-results/checkout/index.html"; + const result = ingestObservationManifest({ + report_json: manifest([ + { path: "tests/checkout.yaml", status: "pass", artifacts: [{ ref: localRef }] } + ]) + }); + + expect(result.observations[0]?.evidenceRefs).toEqual([{ ref: localRef }]); + }); + + it("leaves an observation with no artifacts carrying an empty ref list", () => { + const result = ingestObservationManifest({ + report_json: manifest([{ path: "tests/checkout.yaml", status: "pass" }]) + }); + + expect(result.observations[0]?.evidenceRefs).toEqual([]); + }); + + it("keeps the observed result when an evidence pointer is malformed", () => { + // The status is the measurement; the ref is only how a reviewer looks at it. + // Dropping a real pass because its video pointer was malformed would trade a + // measurement for a convenience. + const result = ingestObservationManifest({ + report_json: manifest([ + { path: "tests/checkout.yaml", test_case: "guest can pay", status: "fail", artifacts: [{ ref: "" }] } + ]) + }); + + expect(result.observations).toHaveLength(1); + expect(result.observations[0]?.status).toBe("fail"); + expect(result.observations[0]?.evidenceRefs).toEqual([]); + expect(result.diagnostics.map((entry) => entry.severity)).toEqual(["warning"]); + expect(result.diagnostics[0]?.message).toContain("requires a non-empty ref"); + }); + + it("rejects an artifact entry carrying unknown fields rather than ignoring them", () => { + // additionalProperties: false all the way down. A producer inventing a field + // must hear about it, not have it silently dropped. + const parsed = parseQualityObservationManifest( + manifest([ + { + path: "tests/checkout.yaml", + status: "pass", + artifacts: [{ ref: SHIPLIGHT_REF, kind: "video" }] + } + ]) + ); + + expect(parsed.status).toBe("invalid"); + expect(parsed.diagnostics[0]?.message).toContain("unknown fields"); + expect(parsed.diagnostics[0]?.message).toContain("kind"); + }); + + it("fails validation when an artifact entry has no ref", () => { + const parsed = parseQualityObservationManifest( + manifest([{ path: "tests/checkout.yaml", status: "pass", artifacts: [{ label: "video" }] }]) + ); + + expect(parsed.status).toBe("invalid"); + expect(parsed.diagnostics[0]?.severity).toBe("error"); + }); + + it("round-trips artifacts through a parsed manifest document", () => { + const parsed = parseQualityObservationManifest( + manifest([ + { + path: "tests/checkout.yaml", + status: "pass", + artifacts: [{ ref: SHIPLIGHT_REF, label: "Shiplight run 8412" }] + } + ]) + ); + + expect(parsed.status).toBe("valid"); + expect(parsed.document?.observations[0]?.artifacts).toEqual([ + { ref: SHIPLIGHT_REF, label: "Shiplight run 8412" } + ]); + }); +}); + +function hostProfile(overrides: Partial = {}): ObservationSourceProfile { + return { + id: "shiplight-e2e", + name: "Shiplight e2e runs", + transport: "host", + requiredEnv: [], + sourceRefs: [], + host: { provider: "shiplight", options: { repo: "ShiplightAI/shipyard" } }, + ...overrides + }; +} + +describe("host observation transport contract", () => { + it("runs a registered host provider and normalizes what it returns", async () => { + const hostTransports: HostObservationTransportRegistry = { + shiplight: async ({ profile, selection }) => ({ + batches: [ + { + source: { id: profile.id, kind: "host", label: profile.name }, + revision: { commit: selection?.commit ?? "abc123" }, + observed_at: "2026-08-27T10:00:00Z", + observations: [ + { + test_file: "tests/checkout.yaml", + test_case: "guest can pay", + status: "pass", + observed_at: "2026-08-27T10:00:00Z", + evidence_refs: [{ ref: SHIPLIGHT_REF, label: "Shiplight run 8412" }] + } + ] + } + ], + selectedRun: { runId: 8412, runUrl: "https://app.shiplight.ai/runs/8412", commit: "abc123" } + }) + }; + + const result = await executeObservationSourceProfile({ + profile: hostProfile(), + selection: { commit: "abc123" }, + hostTransports + }); + + expect(result.status).toBe("valid"); + expect(result.diagnostics).toEqual([]); + expect(result.transport).toBe("host"); + expect(result.selectedRun?.runId).toBe(8412); + expect(result.observations).toHaveLength(1); + expect(result.observations[0]?.evidenceRefs).toEqual([ + { ref: SHIPLIGHT_REF, label: "Shiplight run 8412" } + ]); + }); + + it("applies the same record validation to host records as to a parsed manifest", async () => { + // The seam is fetch-and-shape only. A host cannot get a record past a check + // that a file-based transport must pass, or it would become a second, softer + // way into a score. + const hostTransports: HostObservationTransportRegistry = { + shiplight: async () => ({ + batches: [ + { + source: { id: "shiplight", kind: "host" }, + revision: { commit: "abc123" }, + observed_at: "2026-08-27T10:00:00Z", + observations: [ + { test_file: "tests/checkout.yaml", status: "passed", observed_at: "2026-08-27T10:00:00Z" } + ] + } + ] + }) + }; + + const result = await executeObservationSourceProfile({ + profile: hostProfile(), + hostTransports + }); + + expect(result.observations).toEqual([]); + expect(result.diagnostics.some((entry) => entry.message.includes("status"))).toBe(true); + }); + + it("names the registered providers when the declared one is not available here", async () => { + // The OSS CLI reading a config written for the hosted app hits this. It has + // to be able to tell "this reader cannot serve that provider" from "typo". + const result = await executeObservationSourceProfile({ + profile: hostProfile(), + hostTransports: { "some-other-host": async () => ({ batches: [] }) } + }); + + expect(result.status).toBe("invalid"); + expect(result.observations).toEqual([]); + expect(result.diagnostics[0]?.message).toContain("shiplight"); + expect(result.diagnostics[0]?.message).toContain("some-other-host"); + }); + + it("reports a provider with no registry at all rather than returning an empty pass", async () => { + const result = await executeObservationSourceProfile({ profile: hostProfile() }); + + expect(result.status).toBe("invalid"); + expect(result.diagnostics[0]?.severity).toBe("error"); + expect(result.diagnostics[0]?.message).toContain("(none)"); + }); + + it("turns a throwing host provider into a diagnostic instead of an unhandled rejection", async () => { + const result = await executeObservationSourceProfile({ + profile: hostProfile(), + hostTransports: { + shiplight: async () => { + throw new Error("shiplight API returned 503"); + } + } + }); + + expect(result.status).toBe("invalid"); + expect(result.diagnostics[0]?.message).toContain("shiplight API returned 503"); + }); + + it("parses a host profile from config without requiring an observation path", async () => { + const fixture = await createFixtureProject("observation-source-host-transport", [ + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "shiplight-e2e" + name: "Shiplight e2e runs" + description: "Read YAML e2e results already reported to the platform." + transport: "host" + host: + provider: "shiplight" + options: + repo: "ShiplightAI/shipyard" +` + } + ]); + + try { + const batch = parseObservationSourceProfiles([ + { + projectRelativePath: ".quality/config/observation-sources.yaml", + resolvedLocalPath: path.join(fixture.root, ".quality/config/observation-sources.yaml"), + sourcePattern: ".quality/config/observation-sources.yaml" + } + ]); + + expect(batch.primary?.status).toBe("parsed"); + expect(batch.primary?.document?.profiles[0]).toEqual( + expect.objectContaining({ + id: "shiplight-e2e", + transport: "host", + observationPath: undefined, + host: { provider: "shiplight", options: { repo: "ShiplightAI/shipyard" } } + }) + ); + } finally { + await fixture.cleanup(); + } + }); + + it("rejects a host profile with no provider", async () => { + const fixture = await createFixtureProject("observation-source-host-no-provider", [ + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "shiplight-e2e" + name: "Shiplight e2e runs" + transport: "host" + host: + options: + repo: "ShiplightAI/shipyard" +` + } + ]); + + try { + const batch = parseObservationSourceProfiles([ + { + projectRelativePath: ".quality/config/observation-sources.yaml", + resolvedLocalPath: path.join(fixture.root, ".quality/config/observation-sources.yaml"), + sourcePattern: ".quality/config/observation-sources.yaml" + } + ]); + + expect(batch.primary?.document?.profiles ?? []).toEqual([]); + expect(batch.primary?.diagnostics[0]?.message).toContain("host block with a provider"); + } finally { + await fixture.cleanup(); + } + }); + + it("still requires an observation path for the file-based transports", async () => { + const fixture = await createFixtureProject("observation-source-missing-path", [ + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-review" + name: "Local review" + transport: "local-folder" + local_folder: + path: "artifacts/quality" +` + } + ]); + + try { + const batch = parseObservationSourceProfiles([ + { + projectRelativePath: ".quality/config/observation-sources.yaml", + resolvedLocalPath: path.join(fixture.root, ".quality/config/observation-sources.yaml"), + sourcePattern: ".quality/config/observation-sources.yaml" + } + ]); + + expect(batch.primary?.document?.profiles ?? []).toEqual([]); + expect(batch.primary?.diagnostics[0]?.message).toContain("observation_path"); + } finally { + await fixture.cleanup(); + } + }); +}); + +describe("run evidence through resolution and evaluation", () => { + const agentPayloadRef = "https://app.shiplight.ai/runs/8412?test=99231"; + + function resolvedFixture() { + const ingested = ingestObservationManifest({ + report_json: JSON.stringify({ + schema_version: 1, + revision: { commit: "abc123" }, + observed_at: "2026-08-27T10:00:00Z", + observations: [ + { + path: ".github/workflows/release-ci-runner.yml", + test_case: "agent_payload", + status: "pass", + artifacts: [{ ref: agentPayloadRef, label: "Shiplight run 8412" }] + }, + { + path: ".github/workflows/release-ci-runner.yml", + test_case: "image_inspect", + status: "pass" + } + ] + }) + }); + + return { scan: checksScanResult(), resolution: resolveObservations(checksScanResult(), ingested) }; + } + + it("carries refs onto the audit row for the observation that declared them", () => { + const { resolution } = resolvedFixture(); + const matched = resolution.auditRows.filter((row) => row.matchStatus === "matched"); + + expect(matched.length).toBeGreaterThan(0); + const withRefs = matched.filter((row) => row.evidenceRefs.length > 0); + expect(withRefs.map((row) => row.testCase)).toEqual(["agent_payload"]); + expect(withRefs[0]?.evidenceRefs).toEqual([{ ref: agentPayloadRef, label: "Shiplight run 8412" }]); + }); + + it("leaves the audit row of an observation with no refs carrying an empty list", () => { + const { resolution } = resolvedFixture(); + const imageInspect = resolution.auditRows.find((row) => row.testCase === "image_inspect"); + + expect(imageInspect?.evidenceRefs).toEqual([]); + }); + + it("exposes the selected observation's refs on the evaluated check", () => { + const { scan, resolution } = resolvedFixture(); + const evaluation = buildTargetEvaluation({ + result: scan, + targetId: "checks/quality-map.yaml#target:ci-runner", + observations: resolution, + selection: { commit: "abc123" } + }); + + const evidence = evaluation.expectations.flatMap((expectation) => expectation.evidence); + const agentPayload = evidence.find((entry) => entry.evidenceLocalId === "ci-runner-agent-payload"); + const imageInspect = evidence.find((entry) => entry.evidenceLocalId === "ci-runner-image-inspect"); + + expect(agentPayload?.evidenceRefs).toEqual([{ ref: agentPayloadRef, label: "Shiplight run 8412" }]); + expect(imageInspect?.evidenceRefs).toEqual([]); + }); + + it("reports no refs for a check nothing observed", () => { + // An unobserved check must not borrow evidence from a check that did run. + const { scan, resolution } = resolvedFixture(); + const evaluation = buildTargetEvaluation({ + result: scan, + targetId: "checks/quality-map.yaml#target:ci-runner", + observations: resolution, + selection: { commit: "abc123" } + }); + + const unobserved = evaluation.expectations + .flatMap((expectation) => expectation.evidence) + .filter((entry) => entry.state === "unobserved"); + + expect(unobserved.length).toBeGreaterThan(0); + expect(unobserved.every((entry) => entry.evidenceRefs.length === 0)).toBe(true); + }); +}); + +// A Shiplight YAML run transpiles to Playwright and reports the GENERATED spec. +// That file is gitignored and absent from a fresh checkout, so a quality map +// cannot honestly pin it — the adapter keys observations on the `.test.yaml` +// source instead, inverting the transpiler's own naming rule. +describe("shiplight report adapter", () => { + function report(tests: readonly Record[]): string { + return JSON.stringify({ timestamp: "2026-08-28T10:00:00.000Z", tests }); + } + + it("keys an observation on the YAML source, not the transpiled spec", () => { + const built = buildShiplightObservationBatch({ + report_json: report([ + { + file: "tests/authed/home-analytics-hover-nav.yaml.spec.ts", + baseTitle: "Home cards and charts expose analytics hover navigation", + title: "@e2e @home Home cards and charts expose analytics hover navigation", + status: "passed", + endTime: "2026-08-28T09:59:00.000Z" + } + ]) + }); + + expect(built.diagnostics).toEqual([]); + expect(built.batch?.observations?.[0]).toEqual( + expect.objectContaining({ + test_file: "tests/authed/home-analytics-hover-nav.test.yaml", + // The bare YAML test name, not the tag-prefixed title: a map is authored + // against the YAML, and tags are prefixed rather than joined by the + // suite separator the resolver folds on, so the tagged form would miss. + test_case: "Home cards and charts expose analytics hover navigation", + status: "pass" + }) + ); + }); + + it("leaves a path that is not a transpiled spec untouched", () => { + const built = buildShiplightObservationBatch({ + report_json: report([ + { file: "tests/authed/auth.setup.ts", title: "signup new authed user", status: "passed" } + ]) + }); + + expect(built.batch?.observations?.[0]?.test_file).toBe("tests/authed/auth.setup.ts"); + }); + + it("carries the run evidence ref onto every observation", () => { + const built = buildShiplightObservationBatch({ + report_json: report([ + { file: "a.yaml.spec.ts", baseTitle: "a", status: "passed" }, + { file: "b.yaml.spec.ts", baseTitle: "b", status: "failed" } + ]), + evidence_refs: [{ ref: "https://app.shiplight.ai/runs/8412", label: "Shiplight run 8412" }] + }); + + expect(built.batch?.observations?.map((o) => o.status)).toEqual(["pass", "fail"]); + for (const observation of built.batch?.observations ?? []) { + expect(observation.evidence_refs).toEqual([ + { ref: "https://app.shiplight.ai/runs/8412", label: "Shiplight run 8412" } + ]); + } + }); + + it("skips a test with no usable file or status without losing the rest", () => { + const built = buildShiplightObservationBatch({ + report_json: report([ + { file: "a.yaml.spec.ts", baseTitle: "a", status: "nonsense" }, + { file: "b.yaml.spec.ts", baseTitle: "b", status: "passed" } + ]) + }); + + expect(built.batch?.observations).toHaveLength(1); + expect(built.diagnostics.map((entry) => entry.severity)).toEqual(["warning"]); + }); + + it("rejects a report with no tests array", () => { + const built = buildShiplightObservationBatch({ report_json: JSON.stringify({ timestamp: "x" }) }); + + expect(built.batch).toBeUndefined(); + expect(built.diagnostics[0]?.severity).toBe("error"); + }); +}); diff --git a/tests/integration/local-reports-run-evidence.test.ts b/tests/integration/local-reports-run-evidence.test.ts new file mode 100644 index 0000000..d689be9 --- /dev/null +++ b/tests/integration/local-reports-run-evidence.test.ts @@ -0,0 +1,517 @@ +import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + createLocalReportsTransport, + executeObservationSourceProfile, + LOCAL_REPORTS_PROVIDER, + parseObservationSourceProfiles, + resolveObservations, + scanProject, + type ObservationSourceProfile +} from "@shiplightai/quality-core"; +import { createFixtureProject } from "../fixtures/quality-projects/build-fixtures"; + +// End-to-end validation of the host transport seam using the bundled +// local-reports provider: a real Playwright report on disk, read through a +// registered host transport, resolved against a quality map, arriving as run +// evidence on the matched check. Nothing here is platform-specific — this is +// the whole run-evidence path working with no service involved. +// +// The evidence is the report a reviewer opens, not a catalogue of the videos +// and screenshots inside it. Quality indexes checks to evidence; the runner's +// report is already the viewer. + +const hostTransports = { [LOCAL_REPORTS_PROVIDER]: createLocalReportsTransport() }; + +const QUALITY_MAP = `target: + id: "checkout" + name: "Checkout" + scope: "feature" +expectations: + - id: "guest-checkout" + title: "A guest can complete a purchase" + source_type: "SOURCE" + category: "functional" + priority: "P0" + evidence: + - id: "guest-pays" + type: "e2e" + path: "tests/checkout.spec.ts" + test_case: "guest can pay" + command: "npx playwright test" + contexts: + - "ci" +`; + +const OBSERVATION_SOURCES = `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "playwright-report/report.json" + report: "playwright-report/index.html" +`; + +function playwrightReport(): string { + return JSON.stringify({ + config: { version: "1.60.0" }, + suites: [ + { + title: "checkout.spec.ts", + file: "tests/checkout.spec.ts", + specs: [ + { + title: "guest can pay", + file: "tests/checkout.spec.ts", + tests: [ + { + projectName: "chromium", + results: [ + { + status: "passed", + duration: 4200, + startTime: "2026-08-27T10:00:00.000Z" + } + ] + } + ] + } + ] + } + ], + stats: { startTime: "2026-08-27T10:00:00.000Z", duration: 4200 } + }); +} + +async function fixture(reportJson: string | undefined) { + return createFixtureProject("local-reports-run-evidence", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { relativePath: ".quality/config/observation-sources.yaml", contents: OBSERVATION_SOURCES }, + // The HTML report must EXIST. Without it the ref path never resolves, and a + // ref that is wrong for an existing file looks correct against a missing + // one — which is how a broken `../../` ref passed this suite unnoticed. + { relativePath: "playwright-report/index.html", contents: "

report

" }, + ...(reportJson === undefined + ? [] + : [{ relativePath: "playwright-report/report.json", contents: reportJson }]) + ]); +} + +function profileFrom(root: string): ObservationSourceProfile { + const batch = parseObservationSourceProfiles([ + { + projectRelativePath: ".quality/config/observation-sources.yaml", + resolvedLocalPath: path.join(root, ".quality/config/observation-sources.yaml"), + sourcePattern: ".quality/config/observation-sources.yaml" + } + ]); + const profile = batch.primary?.document?.profiles[0]; + if (profile === undefined) { + throw new Error(`fixture profile did not parse: ${JSON.stringify(batch.primary?.diagnostics)}`); + } + return profile; +} + +describe("local-reports host transport", () => { + it("turns a Playwright report on disk into run evidence on the matched check", async () => { + const project = await fixture(playwrightReport()); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations).toHaveLength(1); + expect(execution.observations[0]?.status).toBe("pass"); + expect(execution.observations[0]?.evidenceRefs).toEqual([ + { ref: "playwright-report/index.html", label: "Test report" } + ]); + + const scan = await scanProject({ projectPath: project.root, mode: "scan" }); + const resolution = resolveObservations(scan, execution); + const matched = resolution.auditRows.filter((row) => row.matchStatus === "matched"); + + expect(matched).toHaveLength(1); + expect(matched[0]?.evidenceLocalId).toBe("guest-pays"); + expect(matched[0]?.evidenceRefs.map((entry) => entry.ref)).toEqual([ + "playwright-report/index.html" + ]); + } finally { + await project.cleanup(); + } + }); + + it("keeps the ref project-relative when the project root traverses a symlink", async () => { + // A temp dir on macOS is reached through /var -> /private/var, and any + // symlinked checkout behaves the same. Relativizing the realpath'd target + // against the un-realpath'd root produced `../../../private/var/...`, which + // the browser normalises away before the request is even sent. + const project = await fixture(playwrightReport()); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + const ref = execution.observations[0]?.evidenceRefs[0]?.ref ?? ""; + expect(ref).toBe("playwright-report/index.html"); + expect(ref.startsWith("..")).toBe(false); + expect(path.isAbsolute(ref)).toBe(false); + } finally { + await project.cleanup(); + } + }); + + it("produces observations with no refs when no report is configured", async () => { + // The results still count. Only the link to look at them is missing. + const project = await createFixtureProject("local-reports-no-report", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "playwright-report/report.json" +` + }, + { relativePath: "playwright-report/report.json", contents: playwrightReport() } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations).toHaveLength(1); + expect(execution.observations[0]?.status).toBe("pass"); + expect(execution.observations[0]?.evidenceRefs).toEqual([]); + } finally { + await project.cleanup(); + } + }); + + it("records an absolute report URL as given", async () => { + const project = await createFixtureProject("local-reports-url", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "playwright-report/report.json" + report: "https://ci.example.test/runs/42/index.html" +` + }, + { relativePath: "playwright-report/report.json", contents: playwrightReport() } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations[0]?.evidenceRefs).toEqual([ + { ref: "https://ci.example.test/runs/42/index.html", label: "Test report" } + ]); + } finally { + await project.cleanup(); + } + }); + + it("refuses a report pointer that escapes the project root", async () => { + const project = await createFixtureProject("local-reports-report-escape", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "playwright-report/report.json" + report: "../../etc/passwd" +` + }, + { relativePath: "playwright-report/report.json", contents: playwrightReport() } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations[0]?.evidenceRefs).toEqual([]); + expect( + execution.diagnostics.some( + (entry) => entry.severity === "error" && entry.message.includes("outside the project root") + ) + ).toBe(true); + } finally { + await project.cleanup(); + } + }); + + it("says the report records no commit rather than stamping one onto it", async () => { + // Stamping the working tree's HEAD onto a report that may predate it would + // invent provenance the producer never claimed. + const project = await fixture(playwrightReport()); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations[0]?.revision.commit).toBeUndefined(); + const unpinned = execution.diagnostics.find((entry) => entry.message.includes("records no commit")); + expect(unpinned?.severity).toBe("info"); + } finally { + await project.cleanup(); + } + }); + + it("pins the commit when config supplies one", async () => { + const project = await createFixtureProject("local-reports-pinned", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `${OBSERVATION_SOURCES} commit: "abc123"\n` + }, + { relativePath: "playwright-report/report.json", contents: playwrightReport() } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations[0]?.revision.commit).toBe("abc123"); + expect(execution.diagnostics).toEqual([]); + } finally { + await project.cleanup(); + } + }); + + it("warns rather than errors when the suite has not been run yet", async () => { + // No report on disk is an ordinary state locally. Every check it would have + // proven already reads unobserved; a red error adds nothing. + const project = await fixture(undefined); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations).toEqual([]); + expect(execution.diagnostics[0]?.severity).toBe("warning"); + expect(execution.diagnostics[0]?.code).toBe("MISSING_OBSERVATION_ARTIFACT_MATCH"); + } finally { + await project.cleanup(); + } + }); + + it("refuses a report path that escapes the project root", async () => { + const project = await createFixtureProject("local-reports-escape", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "../../etc/passwd" +` + } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations).toEqual([]); + expect(execution.diagnostics[0]?.severity).toBe("error"); + expect(execution.diagnostics[0]?.message).toContain("outside the project root"); + } finally { + await project.cleanup(); + } + }); + + it("refuses an absolute report path outside the project root", async () => { + // Containment has to cover the absolute form too. Letting `/etc/hosts` + // through while refusing `../../etc/hosts` would enforce nothing: a profile + // is repo config, and a PR can write either spelling. + const project = await createFixtureProject("local-reports-absolute-escape", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "/etc/hosts" +` + } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations).toEqual([]); + expect(execution.diagnostics[0]?.severity).toBe("error"); + expect(execution.diagnostics[0]?.message).toContain("outside the project root"); + } finally { + await project.cleanup(); + } + }); + + it("refuses an absolute report pointer outside the project root", async () => { + const project = await createFixtureProject("local-reports-absolute-report", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "playwright-report/report.json" + report: "/etc/hosts" +` + }, + { relativePath: "playwright-report/report.json", contents: playwrightReport() } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations[0]?.evidenceRefs).toEqual([]); + expect( + execution.diagnostics.some( + (entry) => entry.severity === "error" && entry.message.includes("outside the project root") + ) + ).toBe(true); + } finally { + await project.cleanup(); + } + }); + + it("refuses a symlink inside the project that resolves outside it", async () => { + // The lexical check structurally cannot see this: `path.resolve` never + // follows links, so a committed symlink is textually contained and would be + // read straight through. Same containment model as the evidence-file route. + const outside = mkdtempSync(path.join(tmpdir(), "local-reports-outside-")); + writeFileSync(path.join(outside, "report.json"), playwrightReport(), "utf8"); + + const project = await createFixtureProject("local-reports-symlink-escape", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "escape/report.json" +` + } + ]); + symlinkSync(outside, path.join(project.root, "escape")); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.observations).toEqual([]); + expect(execution.diagnostics[0]?.severity).toBe("error"); + expect(execution.diagnostics[0]?.message).toContain("outside the project root"); + } finally { + await project.cleanup(); + rmSync(outside, { force: true, recursive: true }); + } + }); + + it("rejects an unsupported report format by name", async () => { + const project = await createFixtureProject("local-reports-format", [ + { relativePath: ".quality/evidence/checkout/quality-map.yaml", contents: QUALITY_MAP }, + { + relativePath: ".quality/config/observation-sources.yaml", + contents: `profiles: + - id: "local-playwright" + name: "Local Playwright run" + transport: "host" + host: + provider: "local-reports" + options: + path: "reports/junit.xml" + format: "junit-xml" +` + } + ]); + + try { + const execution = await executeObservationSourceProfile({ + profile: profileFrom(project.root), + projectRoot: project.root, + hostTransports + }); + + expect(execution.diagnostics[0]?.message).toContain("junit-xml"); + expect(execution.diagnostics[0]?.message).toContain("playwright-json"); + } finally { + await project.cleanup(); + } + }); +});