From e809d5a6fbe56259407e93bf1aa4ec4cf739c35d Mon Sep 17 00:00:00 2001 From: mohabbis <101276427+mohabbis@users.noreply.github.com> Date: Tue, 4 Aug 2026 16:21:41 -0500 Subject: [PATCH 1/7] =?UTF-8?q?feat(cloud):=20browser=20recording=20captur?= =?UTF-8?q?e=20=E2=80=94=20Chrome=20extension=20+=20deterministic=20compil?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Priority 3. Ghost could execute and approve, but nobody could record — the only way to get a workflow into Ghost was to upload a trace file produced by some other tool. There was no capture at all, and `RecordingStatus.ACTIVE` was never set because nothing recorded. Per the capture decision in docs/ARCHITECTURE_DECISIONS.md §3, this ships a Chrome extension for v1, execution stays entirely server-side. ## The trace contract (`@ghost/core/recording/trace`) A Zod schema for what a recorder uploads: navigate/click/input/select/submit events, each carrying a `TraceTarget` with accessible role, name, test id and ordered CSS fallbacks. The defining property: **role and name are read at capture time**, off the live DOM while the element is still on screen — not inferred later from an opaque trace. Those are exactly the fields `resolveLocator` already prefers, so nothing downstream had to change. A secret is never captured, not encrypted or truncated — absent. `redacted: true` records that something was typed without recording what. This is the only place that redaction can be done honestly: a trace carrying the value plus a "please ignore this" flag has already leaked it (see the P0-2 finding in the architecture doc, about the previous design uploading traces whole to a third party). ## The compiler (`@ghost/core/recording/compile`) Trace to typed steps, deterministically. No model, no network, no configured compiler. Three things it does beyond translation: - collapses a run of keystrokes on one field into one `fill` with the final value - runs every produced step through `classifyStep` — the same deterministic classifier the worker consults at run time — and inserts an `approval` immediately before anything it would gate, so the authored workflow agrees with what execution will actually do - never carries a redacted value; a secret field becomes a `fill` marked `sensitive` with a placeholder, and a note tells the reviewer to set it This is what makes recording work with no compiler configured — the state production runs in per the HarnessRouter decision (§1). A model still has a job: naming steps, proposing gates beyond the obvious cases, flagging what it could not resolve. Deciding which element was clicked is not a judgement call and should never have been one. 24 tests, including a fixture shaped exactly like the extension's actual output (`roundtrip.test.ts`) — the seam most likely to rot, since the extension is untyped JS with nothing else to notice if its output drifts from what the compiler expects. ## Ingest (`lib/recording-ingest.ts`) Shared by the existing upload form and the extension, so upload limits, filename sanitisation and audit events cannot drift between the two paths. A structured Ghost trace compiles inline and lands `READY` in the same request; anything else (HAR, Playwright zip) is stored and left for whatever compiler is configured, unchanged from before. `POST /api/agent/recordings` is new: the extension's ingest point, on the already bearer-authenticated agent surface rather than the session-only upload route, since the extension runs at a different origin and cannot rely on the session cookie reaching it. `resolveAgentPrincipal` enforces the same second factor on its session fallback as every other agent route (see the P0-1 fix). Uploading only creates a proposal — nothing here publishes a workflow or executes anything; the human still reviews compiled steps in the editor and publishes through `POST /api/workflows`, which revalidates them. ## The extension (`apps/extension`) Manifest V3. `content.js` captures clicks, typing, selects, submits and SPA navigation, computing accessible name in roughly accname-spec order from label/aria-label/aria-labelledby/placeholder/value/text. Secret fields are detected by `type=password`, secret-shaped `autocomplete`, or a name/label/id matching a word list (password, otp, cvv, card number, ssn, ...) — and the value is never read for them, not merely withheld after reading. `background.js` buffers events locally and uploads only on Stop, via a revocable bearer token created in Ghost Settings. The popup is deliberately thin — start, stop, where to send it — because a second place to edit steps would be a second thing to keep in sync with the real editor. ## Validation 398 tests pass (up from 374 — 24 new), against a database migrated from zero. `pnpm typecheck` and `pnpm build` both clean with `HR_API_KEY` unset. Manually verified `ingestTrace` end-to-end against a real Postgres: a structured trace lands `compileStatus: READY` with steps persisted, in one request, no compiler configured. ## What is not yet covered `GET /api/recordings` gained a `take: 50` (P2-2 for this route) as an incidental fix while touching the file; the rest of that finding stands. Extension host permissions are `` for v1; scoping to allowlisted origins per organization is a natural follow-up once there is a customer to scope it for. No component tests for the extension itself — Manifest V3 content scripts have no test harness in this repo, so `roundtrip.test.ts` pinning its exact output shape is the coverage that exists. Co-Authored-By: Claude Sonnet 5 --- cloud/apps/extension/README.md | 73 ++++ cloud/apps/extension/manifest.json | 18 + cloud/apps/extension/src/background.js | 125 +++++++ cloud/apps/extension/src/content.js | 329 ++++++++++++++++++ cloud/apps/extension/src/popup.html | 63 ++++ cloud/apps/extension/src/popup.js | 86 +++++ .../web/src/app/api/agent/recordings/route.ts | 59 ++++ .../apps/web/src/app/api/recordings/route.ts | 78 ++--- cloud/apps/web/src/lib/recording-ingest.ts | 137 ++++++++ cloud/packages/core/package.json | 4 +- .../core/src/recording/compile.test.ts | 205 +++++++++++ cloud/packages/core/src/recording/compile.ts | 243 +++++++++++++ .../core/src/recording/roundtrip.test.ts | 156 +++++++++ cloud/packages/core/src/recording/trace.ts | 119 +++++++ 14 files changed, 1640 insertions(+), 55 deletions(-) create mode 100644 cloud/apps/extension/README.md create mode 100644 cloud/apps/extension/manifest.json create mode 100644 cloud/apps/extension/src/background.js create mode 100644 cloud/apps/extension/src/content.js create mode 100644 cloud/apps/extension/src/popup.html create mode 100644 cloud/apps/extension/src/popup.js create mode 100644 cloud/apps/web/src/app/api/agent/recordings/route.ts create mode 100644 cloud/apps/web/src/lib/recording-ingest.ts create mode 100644 cloud/packages/core/src/recording/compile.test.ts create mode 100644 cloud/packages/core/src/recording/compile.ts create mode 100644 cloud/packages/core/src/recording/roundtrip.test.ts create mode 100644 cloud/packages/core/src/recording/trace.ts diff --git a/cloud/apps/extension/README.md b/cloud/apps/extension/README.md new file mode 100644 index 00000000..5ec69b84 --- /dev/null +++ b/cloud/apps/extension/README.md @@ -0,0 +1,73 @@ +# Ghost Workflow Recorder (Chrome extension) + +Capture. The user demonstrates a workflow once in their own browser; Ghost +compiles it into typed steps they review and publish. + +## Why an extension, and what it deliberately does not do + +A Ghost-hosted remote browser is the more elegant answer — recording in the +same environment the worker replays in removes a whole class of production +defect — and it lost on cost and time-to-users. See +`cloud/docs/ARCHITECTURE_DECISIONS.md` §3 for the full comparison and for the +environment gaps this design has to be engineered against. + +**It only records.** Execution stays server-side, in Ghost's workers. An +extension that also executed would mean Chrome must stay open and the laptop +awake, and nothing would run overnight — which is most of the value in +back-office work. + +## What it captures + +Clicks, typing, dropdown selections, form submissions, and navigation +(including SPA `pushState`). For each element it reads the **accessible role +and name** off the live DOM, plus ordered CSS fallbacks. + +That is the design's whole point: those are the fields the worker's resolution +chain prefers, so the trace compiles into steps *deterministically* — no model +in the correctness path, and recording works in a deployment with no AI +dependency configured at all. + +## What it never captures + +- Passwords, one-time codes, card numbers, and anything whose field name, + label, or `autocomplete` looks like a secret. The value is **not read**; the + step is marked `sensitive` with a placeholder for the reviewer to fill in. +- Hidden inputs. +- Session cookies. +- Cross-origin iframe contents (`all_frames: false`). +- Coordinates — there is no step field for them, and a coordinate-replayed + click is exactly what the semantic selector chain exists to avoid. + +Redaction happens at capture because that is the only place it can be done +honestly. A trace carrying the value plus a "please ignore this" flag has +already leaked it. + +## Install (unpacked) + +1. `chrome://extensions` → enable **Developer mode** → **Load unpacked** → + select this directory. +2. Open the popup → **Connection** → set your Ghost URL and an API token + created in Ghost under **Settings → Agent credentials** (revocable there at + any time). +3. **Start recording**, do the task, **Stop and send to Ghost**, then follow the + review link. + +A bearer token rather than the session cookie: the extension is a different +origin, so a `SameSite=Lax` session cookie is not reliably sent on a cross-site +POST, and a token can be revoked from Ghost without touching the browser. + +## Trust boundary + +Uploads go to `POST /api/agent/recordings` — the *propose* side. The extension +creates a `Recording` whose compiled steps are a proposal. It cannot publish a +workflow, start a run, or approve anything. A human reviews the steps in the +editor and publishes through `POST /api/workflows`, which revalidates them. + +## The contract + +The trace format is `@ghost/core/recording/trace`; the compiler is +`@ghost/core/recording/compile`. This extension is plain JavaScript and is not +typechecked against them, so `packages/core/src/recording/roundtrip.test.ts` +pins a fixture in exactly the shape `content.js` emits. If that test fails, the +recorder and Ghost have disagreed about the format — fix one of them rather +than relaxing the test. diff --git a/cloud/apps/extension/manifest.json b/cloud/apps/extension/manifest.json new file mode 100644 index 00000000..10fb0282 --- /dev/null +++ b/cloud/apps/extension/manifest.json @@ -0,0 +1,18 @@ +{ + "manifest_version": 3, + "name": "Ghost Workflow Recorder", + "version": "0.1.0", + "description": "Record a business workflow once in your browser, then review and publish it in Ghost.", + "permissions": ["activeTab", "scripting", "storage", "tabs"], + "host_permissions": [""], + "background": { "service_worker": "src/background.js", "type": "module" }, + "action": { "default_popup": "src/popup.html", "default_title": "Ghost Workflow Recorder" }, + "content_scripts": [ + { + "matches": [""], + "js": ["src/content.js"], + "run_at": "document_idle", + "all_frames": false + } + ] +} diff --git a/cloud/apps/extension/src/background.js b/cloud/apps/extension/src/background.js new file mode 100644 index 00000000..6ba919df --- /dev/null +++ b/cloud/apps/extension/src/background.js @@ -0,0 +1,125 @@ +/** + * Ghost recorder — collection and upload. + * + * Holds the event buffer for a recording session and, on stop, posts the trace + * to Ghost. Nothing is uploaded while recording: a partial trace is not a + * workflow, and streaming would mean a half-recorded session could be reviewed + * and published. + * + * Auth is a revocable bearer credential the user creates in Ghost Settings, + * not the session cookie. The extension is a different origin, so a + * `SameSite=Lax` session cookie is not reliably sent on a cross-site POST — + * and a token can be revoked from Ghost without touching the browser. + * + * It uploads to `/api/agent/recordings`, the "propose" side of the trust + * boundary. The extension cannot publish a workflow or approve anything; it + * hands over a proposal for a human to review. + */ + +const STORAGE = chrome.storage.local; + +async function getState() { + return STORAGE.get(["recording", "events", "ghostUrl", "token", "startedAt"]); +} + +chrome.runtime.onMessage.addListener((message, _sender, sendResponse) => { + if (message?.kind === "ghost-event") { + // Fire-and-forget from the page's perspective; the append is async. + void appendEvent(message.event); + return false; + } + + if (message?.kind === "ghost-start") { + void start().then(sendResponse); + return true; // async response + } + + if (message?.kind === "ghost-stop") { + void stop().then(sendResponse); + return true; + } + + if (message?.kind === "ghost-status") { + void getState().then((s) => + sendResponse({ + recording: Boolean(s.recording), + count: (s.events || []).length, + configured: Boolean(s.ghostUrl && s.token), + }), + ); + return true; + } + + return false; +}); + +/** Cap the buffer so a forgotten recording cannot grow without bound. */ +const MAX_EVENTS = 5000; + +async function appendEvent(event) { + const { recording, events } = await STORAGE.get(["recording", "events"]); + if (!recording) return; + const next = events || []; + if (next.length >= MAX_EVENTS) return; + next.push(event); + await STORAGE.set({ events: next }); + chrome.action.setBadgeText({ text: String(next.length) }); + chrome.action.setBadgeBackgroundColor({ color: "#b45309" }); +} + +async function start() { + await STORAGE.set({ recording: true, events: [], startedAt: Date.now() }); + chrome.action.setBadgeText({ text: "0" }); + return { ok: true }; +} + +async function stop() { + const state = await getState(); + await STORAGE.set({ recording: false }); + chrome.action.setBadgeText({ text: "" }); + + const events = state.events || []; + if (events.length === 0) { + return { ok: false, error: "Nothing was recorded." }; + } + if (!state.ghostUrl || !state.token) { + return { ok: false, error: "Set your Ghost URL and API token first." }; + } + + const trace = { + version: 1, + sessionId: `rec_${state.startedAt || Date.now()}`, + startUrl: events.find((e) => e.url)?.url, + capturedAt: Date.now(), + recorder: "ghost-chrome-extension/0.1.0", + events, + }; + + try { + const response = await fetch(`${state.ghostUrl.replace(/\/$/, "")}/api/agent/recordings`, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${state.token}`, + }, + body: JSON.stringify(trace), + }); + + const body = await response.json().catch(() => null); + if (!response.ok) { + return { ok: false, error: body?.error || `Ghost returned ${response.status}.` }; + } + + await STORAGE.set({ events: [] }); + return { + ok: true, + id: body.id, + stepCount: body.stepCount, + reviewUrl: `${state.ghostUrl.replace(/\/$/, "")}/recordings/${body.id}`, + }; + } catch (err) { + // The events are deliberately kept so a failed upload can be retried + // rather than discarding a recording the user cannot easily repeat. + return { ok: false, error: `Could not reach Ghost: ${err.message}` }; + } +} diff --git a/cloud/apps/extension/src/content.js b/cloud/apps/extension/src/content.js new file mode 100644 index 00000000..6b3554f9 --- /dev/null +++ b/cloud/apps/extension/src/content.js @@ -0,0 +1,329 @@ +/** + * Ghost recorder — page-side capture. + * + * Emits the trace format in `@ghost/core/recording/trace`, whose defining + * property is that **accessible role and name are read here**, off the live + * element while it is still on screen. Those are the fields the worker's + * resolution chain prefers, so the trace compiles into steps deterministically + * with no model in the correctness path. Inferring them later from a HAR is + * guesswork; reading them now is not. + * + * Two rules this file must not break: + * + * 1. **A secret never enters the trace.** Not encrypted, not truncated — + * absent. Redaction happens at capture because it is the only place it can + * be done honestly: a trace carrying the value plus a "please ignore" + * flag has already leaked it. `redacted: true` records that something was + * typed without recording what. + * 2. **No coordinates, ever.** There is no step field for them, and a + * coordinate-replayed click is the failure mode the whole semantic + * selector chain exists to avoid. An element we cannot name is reported as + * unidentifiable rather than approximated. + * + * Runs in the top frame only (`all_frames: false`). Cross-origin iframe + * contents are deliberately out of scope. + */ + +(() => { + if (window.__ghostRecorderInstalled) return; + window.__ghostRecorderInstalled = true; + + let recording = false; + + chrome.storage.local.get(["recording"], (state) => { + recording = Boolean(state.recording); + }); + + chrome.storage.onChanged.addListener((changes, area) => { + if (area === "local" && changes.recording) recording = Boolean(changes.recording.newValue); + }); + + const send = (event) => { + if (!recording) return; + try { + chrome.runtime.sendMessage({ kind: "ghost-event", event }); + } catch { + // The service worker may be asleep or the extension reloading. Dropping + // one event is better than throwing inside a page's event handler. + } + }; + + // --- sensitivity ------------------------------------------------------- + + const SECRET_HINTS = + /pass(word|code)|otp|one[-_ ]?time|2fa|mfa|cvv|cvc|card[-_ ]?number|credit|account[-_ ]?number|routing|ssn|social[-_ ]?security|\bpin\b|secret|token|security[-_ ]?code/i; + + const SECRET_AUTOCOMPLETE = + /current-password|new-password|one-time-code|cc-number|cc-csc|cc-exp/i; + + /** Whether this field's *value* must never be captured. */ + function isSecretField(el) { + if (!el) return false; + const type = (el.getAttribute("type") || "").toLowerCase(); + if (type === "password") return true; + if (SECRET_AUTOCOMPLETE.test(el.getAttribute("autocomplete") || "")) return true; + + const haystack = [ + el.getAttribute("name"), + el.getAttribute("id"), + el.getAttribute("placeholder"), + el.getAttribute("aria-label"), + accessibleName(el), + ] + .filter(Boolean) + .join(" "); + return SECRET_HINTS.test(haystack); + } + + /** Fields that are never recorded at all, value or otherwise. */ + function isIgnorableField(el) { + const type = (el.getAttribute("type") || "").toLowerCase(); + return type === "hidden"; + } + + // --- accessible role and name ----------------------------------------- + + function implicitRole(el) { + const tag = el.tagName.toLowerCase(); + const type = (el.getAttribute("type") || "").toLowerCase(); + if (tag === "a" && el.hasAttribute("href")) return "link"; + if (tag === "button") return "button"; + if (tag === "select") return "combobox"; + if (tag === "textarea") return "textbox"; + if (tag === "input") { + if (["submit", "button", "reset", "image"].includes(type)) return "button"; + if (type === "checkbox") return "checkbox"; + if (type === "radio") return "radio"; + if (["text", "email", "tel", "url", "search", "password", "number", ""].includes(type)) { + return "textbox"; + } + } + if (/^h[1-6]$/.test(tag)) return "heading"; + return undefined; + } + + function role(el) { + return el.getAttribute("role") || implicitRole(el); + } + + function textOf(el) { + return (el.textContent || "").replace(/\s+/g, " ").trim(); + } + + /** + * Accessible name, in roughly the order the accname spec resolves it. + * + * Not a complete implementation — that is a large specification — but it + * covers the attributes real applications actually use, and it is computed + * from the same DOM Playwright's `getByRole(role, { name })` will later + * query, which is what matters for replay. + */ + function accessibleName(el) { + if (!el || el.nodeType !== 1) return ""; + + const labelledBy = el.getAttribute("aria-labelledby"); + if (labelledBy) { + const parts = labelledBy + .split(/\s+/) + .map((id) => document.getElementById(id)) + .filter(Boolean) + .map(textOf) + .filter(Boolean); + if (parts.length) return parts.join(" "); + } + + const ariaLabel = el.getAttribute("aria-label"); + if (ariaLabel && ariaLabel.trim()) return ariaLabel.trim(); + + if (el.id) { + const label = document.querySelector(`label[for="${CSS.escape(el.id)}"]`); + if (label) { + const t = textOf(label); + if (t) return t; + } + } + + const wrapping = el.closest("label"); + if (wrapping) { + const t = textOf(wrapping); + if (t) return t; + } + + const tag = el.tagName.toLowerCase(); + if (tag === "input") { + const type = (el.getAttribute("type") || "").toLowerCase(); + if (["submit", "button", "reset"].includes(type)) { + const v = el.getAttribute("value"); + if (v && v.trim()) return v.trim(); + } + const placeholder = el.getAttribute("placeholder"); + if (placeholder && placeholder.trim()) return placeholder.trim(); + } + + if (tag === "img") { + const alt = el.getAttribute("alt"); + if (alt && alt.trim()) return alt.trim(); + } + + const title = el.getAttribute("title"); + if (title && title.trim()) return title.trim(); + + // Buttons and links are named by their own text. + if (["button", "a", "summary"].includes(tag) || el.getAttribute("role") === "button") { + const t = textOf(el); + if (t) return t.slice(0, 200); + } + + return ""; + } + + // --- selector candidates ---------------------------------------------- + + function cssPath(el) { + const parts = []; + let node = el; + let depth = 0; + while (node && node.nodeType === 1 && depth < 5) { + let part = node.tagName.toLowerCase(); + if (node.id) { + parts.unshift(`#${CSS.escape(node.id)}`); + break; + } + const parent = node.parentElement; + if (parent) { + const siblings = Array.from(parent.children).filter((c) => c.tagName === node.tagName); + if (siblings.length > 1) part += `:nth-of-type(${siblings.indexOf(node) + 1})`; + } + parts.unshift(part); + node = node.parentElement; + depth++; + } + return parts.join(" > "); + } + + /** + * Ordered best-first CSS fallbacks. Several, not one, because the worker + * replays in a different browser and a single brittle selector is the + * failure that invites. + */ + function selectorCandidates(el) { + const out = []; + const testId = + el.getAttribute("data-testid") || + el.getAttribute("data-test-id") || + el.getAttribute("data-test"); + if (testId) out.push(`[data-testid="${CSS.escape(testId)}"]`); + if (el.id) out.push(`#${CSS.escape(el.id)}`); + const name = el.getAttribute("name"); + if (name) out.push(`${el.tagName.toLowerCase()}[name="${CSS.escape(name)}"]`); + const path = cssPath(el); + if (path) out.push(path); + return out.slice(0, 5); + } + + function describeTarget(el) { + return { + role: role(el) || undefined, + name: accessibleName(el) || undefined, + testId: + el.getAttribute("data-testid") || + el.getAttribute("data-test-id") || + el.getAttribute("data-test") || + undefined, + text: textOf(el).slice(0, 120) || undefined, + selectorCandidates: selectorCandidates(el), + tagName: el.tagName.toLowerCase(), + inputType: (el.getAttribute("type") || "").toLowerCase() || undefined, + }; + } + + const now = () => Date.now(); + + // --- listeners --------------------------------------------------------- + + // Capture phase: a page that calls stopPropagation on its own handlers + // would otherwise make the workflow unrecordable. + document.addEventListener( + "click", + (e) => { + const el = e.target instanceof Element ? e.target.closest("a,button,[role],input,summary,label") || e.target : null; + if (!el || el.nodeType !== 1) return; + if (isIgnorableField(el)) return; + send({ type: "click", url: location.href, timestamp: now(), target: describeTarget(el) }); + }, + true, + ); + + document.addEventListener( + "change", + (e) => { + const el = e.target; + if (!(el instanceof Element) || isIgnorableField(el)) return; + const tag = el.tagName.toLowerCase(); + + if (tag === "select") { + send({ + type: "select", + url: location.href, + timestamp: now(), + target: describeTarget(el), + value: el.value ?? "", + }); + return; + } + + if (tag === "input" || tag === "textarea") { + const type = (el.getAttribute("type") || "").toLowerCase(); + if (["checkbox", "radio", "submit", "button", "file"].includes(type)) return; + + const secret = isSecretField(el); + send({ + type: "input", + url: location.href, + timestamp: now(), + target: describeTarget(el), + // The whole point: on a secret field the value is not read at all. + ...(secret ? {} : { value: el.value ?? "" }), + redacted: secret, + }); + } + }, + true, + ); + + document.addEventListener( + "submit", + (e) => { + const el = e.target instanceof Element ? e.target : null; + send({ + type: "submit", + url: location.href, + timestamp: now(), + ...(el ? { target: describeTarget(el) } : {}), + }); + }, + true, + ); + + // SPA navigation: pushState/replaceState fire no event of their own. + let lastUrl = location.href; + const reportNavigation = () => { + if (location.href === lastUrl) return; + lastUrl = location.href; + send({ type: "navigate", url: location.href, timestamp: now() }); + }; + + for (const method of ["pushState", "replaceState"]) { + const original = history[method]; + history[method] = function patched(...args) { + const result = original.apply(this, args); + reportNavigation(); + return result; + }; + } + window.addEventListener("popstate", reportNavigation); + window.addEventListener("hashchange", reportNavigation); + + // The initial page of a recording, so the compiled workflow opens somewhere. + send({ type: "navigate", url: location.href, timestamp: now() }); +})(); diff --git a/cloud/apps/extension/src/popup.html b/cloud/apps/extension/src/popup.html new file mode 100644 index 00000000..0209dcfe --- /dev/null +++ b/cloud/apps/extension/src/popup.html @@ -0,0 +1,63 @@ + + + + + + + +

Ghost Workflow Recorder

+ + +
+ +
+ Connection + + + + +

+ Create a revocable token in Ghost under Settings → Agent credentials. +

+
+ + + + diff --git a/cloud/apps/extension/src/popup.js b/cloud/apps/extension/src/popup.js new file mode 100644 index 00000000..09334238 --- /dev/null +++ b/cloud/apps/extension/src/popup.js @@ -0,0 +1,86 @@ +/** + * Ghost recorder — popup. + * + * Deliberately thin: start, stop, and where to send it. The review step is in + * Ghost, not here. An extension that let you edit steps would be a second + * editor to keep in sync with the real one, and the whole point of the trust + * pipeline is that a proposal is reviewed and published in one place. + */ + +const toggle = document.getElementById("toggle"); +const status = document.getElementById("status"); +const urlInput = document.getElementById("url"); +const tokenInput = document.getElementById("token"); +const settings = document.getElementById("settings"); + +let recording = false; + +function say(html) { + status.innerHTML = html; +} + +async function refresh() { + const state = await chrome.storage.local.get(["ghostUrl", "token"]); + urlInput.value = state.ghostUrl || ""; + tokenInput.value = state.token || ""; + + chrome.runtime.sendMessage({ kind: "ghost-status" }, (s) => { + recording = Boolean(s?.recording); + toggle.textContent = recording ? "Stop and send to Ghost" : "Start recording"; + toggle.classList.toggle("rec", recording); + if (recording) { + say(`Recording — ${s.count} event${s.count === 1 ? "" : "s"} captured.`); + } else if (!s?.configured) { + say("Set your Ghost URL and API token below to begin."); + settings.open = true; + } else { + say("Ready. Recording captures clicks, typing and navigation — never passwords."); + } + }); +} + +const persist = () => + chrome.storage.local.set({ + ghostUrl: urlInput.value.trim(), + token: tokenInput.value.trim(), + }); + +urlInput.addEventListener("change", persist); +tokenInput.addEventListener("change", persist); + +toggle.addEventListener("click", async () => { + await persist(); + toggle.disabled = true; + + if (!recording) { + chrome.runtime.sendMessage({ kind: "ghost-start" }, async () => { + // The content script is injected at document_idle, so a tab already open + // before the extension was installed has no recorder in it yet. + const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); + if (tab?.id) { + await chrome.scripting + .executeScript({ target: { tabId: tab.id }, files: ["src/content.js"] }) + .catch(() => undefined); + } + toggle.disabled = false; + refresh(); + }); + return; + } + + say("Sending to Ghost…"); + chrome.runtime.sendMessage({ kind: "ghost-stop" }, (result) => { + toggle.disabled = false; + if (result?.ok) { + say( + `Sent. ${result.stepCount} step${result.stepCount === 1 ? "" : "s"} proposed — ` + + `review in Ghost.`, + ); + } else { + say(`Could not send: ${result?.error || "unknown error"}. Your recording was kept.`); + } + refresh(); + }); +}); + +refresh(); diff --git a/cloud/apps/web/src/app/api/agent/recordings/route.ts b/cloud/apps/web/src/app/api/agent/recordings/route.ts new file mode 100644 index 00000000..b45c7def --- /dev/null +++ b/cloud/apps/web/src/app/api/agent/recordings/route.ts @@ -0,0 +1,59 @@ +import { resolveAgentPrincipal } from "@/lib/agent-auth"; +import { ingestTrace, MAX_TRACE_BYTES } from "@/lib/recording-ingest"; + +/** + * Recording ingest for non-interactive clients — the Chrome extension. + * + * Lives under `/api/agent` because that is the surface authenticated by a + * revocable bearer credential rather than a browser session. The extension + * runs outside Ghost's origin and cannot rely on the session cookie reaching + * it, so it holds a token the user creates in Settings and can revoke there. + * + * It sits on the "propose" side of the trust boundary, like every other agent + * route: uploading a recording creates a `Recording` whose compiled steps are + * a *proposal*. Nothing here publishes a workflow or executes anything. The + * human reviews the steps in the editor and publishes through + * `POST /api/workflows`, which revalidates them. + * + * `resolveAgentPrincipal` accepts a session too, and enforces the second + * factor on that path — see `lib/agent-auth.ts`. + * + * Body is JSON rather than multipart: the extension builds the trace in + * memory and has no file to attach. + */ +export async function POST(req: Request) { + const principal = await resolveAgentPrincipal(req); + if (!principal.ok) { + return Response.json({ error: principal.error }, { status: principal.status }); + } + + const raw = await req.text(); + if (!raw) { + return Response.json({ error: "a JSON recording trace is required" }, { status: 400 }); + } + if (Buffer.byteLength(raw) > MAX_TRACE_BYTES) { + return Response.json( + { error: `trace exceeds the ${MAX_TRACE_BYTES / (1024 * 1024)}MB limit` }, + { status: 413 }, + ); + } + + // Parsed only to reject obvious junk before it reaches storage. The + // authoritative validation is `parseRecordingTrace` inside `ingestTrace`, + // which decides whether this is a Ghost trace worth compiling. + try { + JSON.parse(raw); + } catch { + return Response.json({ error: "body was not valid JSON" }, { status: 400 }); + } + + const result = await ingestTrace({ + orgId: principal.principal.orgId, + userId: principal.principal.userId, + filename: "extension-trace.json", + contentType: "application/json", + buffer: Buffer.from(raw, "utf8"), + }); + + return Response.json(result, { status: 201 }); +} diff --git a/cloud/apps/web/src/app/api/recordings/route.ts b/cloud/apps/web/src/app/api/recordings/route.ts index 9b88253a..9f84b6e3 100644 --- a/cloud/apps/web/src/app/api/recordings/route.ts +++ b/cloud/apps/web/src/app/api/recordings/route.ts @@ -1,23 +1,20 @@ -import { appendAuditEvent } from "@ghost/core/audit-log"; -import { artifactStore } from "@ghost/core/storage/artifacts"; import { auth } from "@/auth"; import { prisma } from "@/lib/db"; +import { ingestTrace, MAX_TRACE_BYTES } from "@/lib/recording-ingest"; -/** Recording traces are small event/HAR/trace-zip logs, not raw video — cap - * well under Vercel's 100MB function body limit to keep uploads fast and - * keep an accidental video upload from tying up the compile step. */ -const MAX_TRACE_BYTES = 25 * 1024 * 1024; - -function sanitizeFilename(name: string): string { - const base = name.split(/[/\\]/).pop() || "recording-trace"; - return base.replace(/[^a-zA-Z0-9._-]/g, "_").slice(-200) || "recording-trace"; -} - -/** Upload a raw workflow-recording trace and create the `Recording` it will - * be compiled from. The capture mechanism itself (server-side remote browser - * vs. extension — see `docs/CURSOR_HANDOFF.md` Phase 2) is still an open - * decision; this accepts a trace produced by whatever means already exists - * (a JSON event log, a HAR export, a Playwright trace .zip). */ +/** + * Upload a workflow-recording trace and create the `Recording` it will be + * reviewed from. + * + * A trace produced by a Ghost recorder (`@ghost/core/recording/trace`) is + * compiled into typed steps deterministically, in this request — no model and + * no configured compiler, which is what makes recording work in a production + * deployment that has neither. Any other format (HAR, Playwright zip) is stored + * and left for whatever compiler is configured, if one is. + * + * The extension posts to `/api/agent/recordings` instead, which accepts a + * bearer credential; both share `ingestTrace`. + */ export async function POST(req: Request) { const session = await auth(); if (!session?.user?.orgId) { @@ -29,7 +26,7 @@ export async function POST(req: Request) { const form = await req.formData().catch(() => null); const file = form?.get("file"); if (!file || !(file instanceof File)) { - return Response.json({ error: "a \"file\" upload is required" }, { status: 400 }); + return Response.json({ error: 'a "file" upload is required' }, { status: 400 }); } if (file.size === 0) { return Response.json({ error: "the uploaded file is empty" }, { status: 400 }); @@ -41,41 +38,15 @@ export async function POST(req: Request) { ); } - const filename = sanitizeFilename(file.name || "recording-trace"); - const buffer = Buffer.from(await file.arrayBuffer()); - - const recording = await prisma.recording.create({ - data: { orgId, status: "STOPPED" }, + const result = await ingestTrace({ + orgId, + userId, + filename: file.name || "recording-trace", + contentType: file.type, + buffer: Buffer.from(await file.arrayBuffer()), }); - try { - const key = `recordings/${recording.id}/trace-${filename}`; - await artifactStore().put(key, buffer, file.type || "application/octet-stream"); - await prisma.$transaction(async (tx) => { - await tx.recording.update({ - where: { id: recording.id }, - data: { rawTraceKey: key, rawTraceFilename: filename }, - }); - await appendAuditEvent( - orgId, - userId, - { - action: "recording.uploaded", - entityType: "Recording", - entityId: recording.id, - metadata: { filename, bytes: buffer.byteLength }, - }, - tx, - ); - }); - } catch (err) { - // Storage failed after the row was created — don't leave an unusable - // Recording with no trace behind for the org to trip over. - await prisma.recording.delete({ where: { id: recording.id } }).catch(() => undefined); - throw err; - } - - return Response.json({ id: recording.id }, { status: 201 }); + return Response.json(result, { status: 201 }); } export async function GET() { @@ -83,19 +54,18 @@ export async function GET() { if (!session?.user?.orgId) { return Response.json({ error: "unauthorized" }, { status: 401 }); } - const recordings = await prisma.recording.findMany({ where: { orgId: session.user.orgId }, orderBy: { createdAt: "desc" }, + take: 50, select: { id: true, + createdAt: true, status: true, compileStatus: true, rawTraceFilename: true, workflowId: true, - createdAt: true, }, }); - return Response.json({ recordings }); } diff --git a/cloud/apps/web/src/lib/recording-ingest.ts b/cloud/apps/web/src/lib/recording-ingest.ts new file mode 100644 index 00000000..61f8080e --- /dev/null +++ b/cloud/apps/web/src/lib/recording-ingest.ts @@ -0,0 +1,137 @@ +import { appendAuditEvent } from "@ghost/core/audit-log"; +import { artifactStore } from "@ghost/core/storage/artifacts"; +import { Prisma } from "@ghost/core/db"; +import { compileTrace } from "@ghost/core/recording/compile"; +import { parseRecordingTrace } from "@ghost/core/recording/trace"; +import { prisma } from "@/lib/db"; + +/** + * Storing an uploaded recording trace, shared by the browser upload form and + * the extension's ingest endpoint so the two cannot drift on limits, + * sanitisation, or what gets audited. + * + * The interesting part is what happens to a *structured* trace. A Ghost + * recorder reads the accessible role and name off each element while it is + * still on screen, so the trace already contains what the worker's resolution + * chain wants. `compileTrace` then turns it into typed steps deterministically + * — no model, no network, no configured compiler — and the recording lands + * `READY` for review in a single request. + * + * That is what makes recording work in production, where there is deliberately + * no compiler configured at all (see `docs/DEPLOY.md`). Anything that is *not* + * a structured trace — a HAR, a Playwright zip — is stored as before and left + * for whatever compiler exists, if any. + */ + +/** Small event logs, not video. Well under Vercel's 100MB body limit. */ +export const MAX_TRACE_BYTES = 25 * 1024 * 1024; + +export function sanitizeFilename(name: string): string { + const base = name.split(/[/\\]/).pop() || "recording-trace"; + return base.replace(/[^a-zA-Z0-9._-]/g, "_").slice(-200) || "recording-trace"; +} + +export interface IngestResult { + id: string; + /** READY when the trace compiled deterministically, NONE when it awaits a compiler. */ + compileStatus: "READY" | "NONE"; + stepCount: number; + notes: string[]; +} + +export async function ingestTrace(args: { + orgId: string; + userId: string | null; + filename: string; + contentType: string; + buffer: Buffer; +}): Promise { + const { orgId, userId, buffer } = args; + const filename = sanitizeFilename(args.filename); + + const recording = await prisma.recording.create({ + data: { orgId, status: "STOPPED" }, + }); + + try { + const key = `recordings/${recording.id}/trace-${filename}`; + await artifactStore().put(key, buffer, args.contentType || "application/octet-stream"); + + const compiled = tryCompile(buffer); + + await prisma.$transaction(async (tx) => { + await tx.recording.update({ + where: { id: recording.id }, + data: { + rawTraceKey: key, + rawTraceFilename: filename, + ...(compiled + ? { + compileStatus: "READY" as const, + compiledSteps: compiled.steps as unknown as Prisma.InputJsonValue, + compileNotes: compiled.notes.length > 0 ? compiled.notes.join("\n\n") : null, + compileError: null, + } + : {}), + }, + }); + await appendAuditEvent( + orgId, + userId, + { + action: "recording.uploaded", + entityType: "Recording", + entityId: recording.id, + metadata: { filename, bytes: buffer.byteLength }, + }, + tx, + ); + if (compiled) { + // Audited as a compile in its own right. A reviewer looking at how a + // workflow came to exist should see that a compiler ran, even though + // it ran inline and deterministically rather than as a queued task. + await appendAuditEvent( + orgId, + userId, + { + action: "recording.compile_ready", + entityType: "Recording", + entityId: recording.id, + metadata: { stepCount: compiled.steps.length, compiler: "deterministic" }, + }, + tx, + ); + } + }); + + return { + id: recording.id, + compileStatus: compiled ? "READY" : "NONE", + stepCount: compiled?.steps.length ?? 0, + notes: compiled?.notes ?? [], + }; + } catch (err) { + // Storage failed after the row was created — don't leave an unusable + // Recording with no trace behind for the org to trip over. + await prisma.recording.delete({ where: { id: recording.id } }).catch(() => undefined); + throw err; + } +} + +/** Returns compiled steps for a structured Ghost trace, or null for anything else. */ +function tryCompile(buffer: Buffer): { steps: unknown[]; notes: string[] } | null { + let json: unknown; + try { + json = JSON.parse(buffer.toString("utf8")); + } catch { + return null; // a zip, a HAR that isn't ours, or not JSON at all + } + const parsed = parseRecordingTrace(json); + if (!parsed.ok) return null; + + const { steps, notes } = compileTrace(parsed.trace); + // A trace with no replayable action compiles to just the opening navigate. + // Storing that as READY would present an empty workflow as a result. + if (steps.length <= 1) return null; + return { steps, notes }; +} diff --git a/cloud/packages/core/package.json b/cloud/packages/core/package.json index bea36179..a92a2b6b 100644 --- a/cloud/packages/core/package.json +++ b/cloud/packages/core/package.json @@ -26,7 +26,9 @@ "./roles": "./src/roles.ts", "./invitations": "./src/invitations.ts", "./mfa": "./src/mfa.ts", - "./crypto/mfa-secret": "./src/crypto/mfa-secret.ts" + "./crypto/mfa-secret": "./src/crypto/mfa-secret.ts", + "./recording/trace": "./src/recording/trace.ts", + "./recording/compile": "./src/recording/compile.ts" }, "scripts": { "build": "prisma generate && tsc --noEmit", diff --git a/cloud/packages/core/src/recording/compile.test.ts b/cloud/packages/core/src/recording/compile.test.ts new file mode 100644 index 00000000..1a8d9ccc --- /dev/null +++ b/cloud/packages/core/src/recording/compile.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, it } from "vitest"; +import { compileTrace } from "./compile.js"; +import { parseRecordingTrace, type RecordingTrace } from "./trace.js"; +import { EDITABLE_STEP_TYPES, workflowSteps } from "../schema/step.js"; +import { classifyStep } from "../classifier/sensitive.js"; + +/** + * The deterministic path from a captured trace to typed steps. + * + * This is what makes recording work with no compiler configured — the case + * production runs in. If these pass, a customer can record a workflow in a + * deployment that has no AI dependency of any kind. + */ + +function trace(events: RecordingTrace["events"], startUrl = "https://shop.test/order"): RecordingTrace { + return { version: 1, sessionId: "rec_test", startUrl, events }; +} + +const target = (name: string, role = "button") => ({ + role, + name, + selectorCandidates: [`#${name.replace(/\s+/g, "-").toLowerCase()}`], +}); + +describe("compileTrace", () => { + it("opens the starting page before anything else", () => { + const { steps } = compileTrace(trace([])); + expect(steps[0]).toMatchObject({ type: "navigate", url: "https://shop.test/order" }); + }); + + it("produces steps that satisfy the published workflow schema", () => { + const { steps } = compileTrace( + trace([ + { + type: "input", + url: "https://shop.test/order", + timestamp: 1, + target: { ...target("Full name", "textbox") }, + value: "Ada Lovelace", + redacted: false, + }, + { type: "click", url: "https://shop.test/order", timestamp: 2, target: target("Submit order") }, + ]), + ); + + // The real gate: these steps go through the same validator the publish + // route uses, so a compiled workflow cannot be unpublishable. + expect(workflowSteps.safeParse(steps).success).toBe(true); + + // And every emitted type must have an executor. `apiCall`/`sendEmail` + // parse and gate but do nothing, so a compiler that emitted one would + // produce a workflow that reports SUCCEEDED having skipped the step — + // exactly the failure `EDITABLE_STEP_TYPES` exists to prevent. + const editable = new Set(EDITABLE_STEP_TYPES); + for (const step of steps) expect(editable.has(step.type)).toBe(true); + }); + + it("prefers role and name over the CSS fallback", () => { + const { steps } = compileTrace( + trace([{ type: "click", url: "https://shop.test/x", timestamp: 1, target: target("Submit order") }]), + ); + const click = steps.find((s) => s.type === "click"); + expect(click).toMatchObject({ selector: { role: "button", name: "Submit order" } }); + expect(click).not.toHaveProperty("selector.css"); + }); + + it("falls back to CSS only when nothing semantic was captured", () => { + const { steps } = compileTrace( + trace([ + { + type: "click", + url: "https://shop.test/x", + timestamp: 1, + target: { selectorCandidates: ["#mystery-button", ".btn"] }, + }, + ]), + ); + expect(steps.find((s) => s.type === "click")).toMatchObject({ + selector: { css: "#mystery-button" }, + }); + }); + + it("collapses a run of keystrokes on one field into a single fill", () => { + const field = { ...target("Full name", "textbox") }; + const { steps } = compileTrace( + trace([ + { type: "input", url: "https://shop.test/x", timestamp: 1, target: field, value: "A", redacted: false }, + { type: "input", url: "https://shop.test/x", timestamp: 2, target: field, value: "Ad", redacted: false }, + { type: "input", url: "https://shop.test/x", timestamp: 3, target: field, value: "Ada", redacted: false }, + ]), + ); + const fills = steps.filter((s) => s.type === "fill"); + expect(fills).toHaveLength(1); + expect(fills[0]).toMatchObject({ value: "Ada" }); + }); + + it("never carries a redacted value, and marks the step sensitive", () => { + const { steps, notes } = compileTrace( + trace([ + { + type: "input", + url: "https://bank.test/login", + timestamp: 1, + target: { ...target("Password", "textbox"), inputType: "password" }, + redacted: true, + }, + ]), + ); + const fill = steps.find((s) => s.type === "fill") as { value: string; sensitive: boolean }; + expect(fill.sensitive).toBe(true); + expect(fill.value).not.toContain("hunter2"); + expect(fill.value).toMatch(/redacted/i); + expect(notes.join(" ")).toMatch(/never captured/i); + }); + + it("puts an approval in front of an irreversible click", () => { + const { steps } = compileTrace( + trace([{ type: "click", url: "https://shop.test/x", timestamp: 1, target: target("Submit order") }]), + ); + const submitIndex = steps.findIndex((s) => s.type === "click"); + expect(submitIndex).toBeGreaterThan(0); + expect(steps[submitIndex - 1]).toMatchObject({ type: "approval" }); + }); + + it("agrees with the engine: every gated step is preceded by an approval", () => { + const { steps } = compileTrace( + trace([ + { + type: "input", + url: "https://shop.test/x", + timestamp: 1, + target: target("Full name", "textbox"), + value: "Ada", + redacted: false, + }, + { type: "click", url: "https://shop.test/x", timestamp: 2, target: target("Delete account") }, + { type: "click", url: "https://shop.test/x", timestamp: 3, target: target("Submit order") }, + ]), + ); + + steps.forEach((step, i) => { + if (step.type === "approval") return; + if (classifyStep(step).sensitive) { + expect(steps[i - 1], `step ${i} (${step.type}) is gated but has no approval before it`) + .toMatchObject({ type: "approval" }); + } + }); + }); + + it("does not stack two approvals in a row", () => { + const { steps } = compileTrace( + trace([ + { type: "click", url: "https://shop.test/x", timestamp: 1, target: target("Submit order") }, + { type: "click", url: "https://shop.test/x", timestamp: 2, target: target("Confirm") }, + ]), + ); + for (let i = 1; i < steps.length; i++) { + expect(steps[i]!.type === "approval" && steps[i - 1]!.type === "approval").toBe(false); + } + }); + + it("says so when nothing looked irreversible, rather than staying quiet", () => { + const { notes } = compileTrace( + trace([{ type: "click", url: "https://shop.test/x", timestamp: 1, target: target("Next") }]), + ); + expect(notes.join(" ")).toMatch(/no approval gate was added/i); + }); + + it("reports an unidentifiable click instead of guessing at coordinates", () => { + const { steps, notes } = compileTrace( + trace([{ type: "click", url: "https://shop.test/x", timestamp: 1, target: { selectorCandidates: [] } }]), + ); + expect(steps.filter((s) => s.type === "click")).toHaveLength(0); + expect(notes.join(" ")).toMatch(/cannot be replayed/i); + }); + + it("ignores a navigation that did not change the page", () => { + const { steps } = compileTrace( + trace([ + { type: "navigate", url: "https://shop.test/order", timestamp: 1 }, + { type: "navigate", url: "https://shop.test/confirm", timestamp: 2 }, + ]), + ); + const urls = steps.filter((s) => s.type === "navigate").map((s) => (s as { url: string }).url); + expect(urls).toEqual(["https://shop.test/order", "https://shop.test/confirm"]); + }); +}); + +describe("parseRecordingTrace", () => { + it("accepts a well-formed trace", () => { + const result = parseRecordingTrace({ version: 1, sessionId: "rec_1", events: [] }); + expect(result.ok).toBe(true); + }); + + it("rejects a HAR or anything else that is not this format", () => { + const result = parseRecordingTrace({ log: { version: "1.2", entries: [] } }); + expect(result.ok).toBe(false); + }); + + it("names what was wrong rather than failing opaquely", () => { + const result = parseRecordingTrace({ version: 1, sessionId: "rec_1", events: [{ type: "click" }] }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error).toMatch(/events/); + }); +}); diff --git a/cloud/packages/core/src/recording/compile.ts b/cloud/packages/core/src/recording/compile.ts new file mode 100644 index 00000000..89a72940 --- /dev/null +++ b/cloud/packages/core/src/recording/compile.ts @@ -0,0 +1,243 @@ +import { classifyStep } from "../classifier/sensitive.js"; +import type { Selector, WorkflowStep, WorkflowSteps } from "../schema/step.js"; +import type { RecordingTrace, TraceEvent, TraceTarget } from "./trace.js"; + +/** + * Turns a captured trace into typed workflow steps — **deterministically**. + * + * No model, no network, no inference. The recorder already read the accessible + * role and name off the live element, which are the fields the worker's + * resolution chain prefers, so compilation is a mechanical translation rather + * than a guess. That matters for more than purity: it means recording works + * with no compiler configured, in a deployment that has deliberately removed + * every AI dependency from the runtime (see `docs/DEPLOY.md`). + * + * A model still has a job here, just not this one. Naming steps readably, + * proposing where an approval belongs beyond the obvious cases, and flagging + * what it could not resolve are all judgement calls. Deciding *which element + * was clicked* is not, and should never have been. + * + * Three things it does beyond translation, all conservative: + * + * 1. **Collapses keystroke noise.** Consecutive `input` events on one field + * become one `fill` with the final value. A recorder that emits per + * keystroke would otherwise produce a step per character. + * 2. **Gates sensitive actions.** Every produced step is run through the same + * `classifyStep` the engine uses, and an `approval` step is inserted before + * any that gates. The workflow therefore arrives already stopping before it + * submits, rather than relying on whoever reviews it to notice. + * 3. **Never carries a secret.** A redacted input becomes a `fill` marked + * `sensitive` with a placeholder value. The real value was never in the + * trace; this makes its absence explicit in the step the human reviews. + */ + +export interface CompileTraceResult { + steps: WorkflowSteps; + /** Human-facing notes: what was dropped, and what needs a decision. */ + notes: string[]; +} + +/** Ghost never stores a captured secret. The reviewer replaces this. */ +const SECRET_PLACEHOLDER = ""; + +function selectorFrom(target: TraceTarget): Selector | null { + const selector: Selector = {}; + if (target.role) selector.role = target.role; + if (target.name) selector.name = target.name; + if (target.testId) selector.testId = target.testId; + // Text is only worth carrying when nothing stronger identified the element; + // as an extra field alongside role+name it adds no resolution power and + // makes the step noisier to read. + if (!selector.role && !selector.testId && target.text) selector.text = target.text; + if (!selector.role && !selector.testId && !selector.text) { + const css = target.selectorCandidates[0]; + if (css) selector.css = css; + } + return Object.keys(selector).length > 0 ? selector : null; +} + +function describe(target: TraceTarget): string | undefined { + if (target.name) return target.name; + if (target.text) return target.text; + return undefined; +} + +/** Stable within one compile; the schema only requires uniqueness. */ +function makeIdFactory() { + let n = 0; + return (prefix: string) => `${prefix}${++n}`; +} + +/** + * True when two consecutive input events are typing into the same field, so + * only the last value matters. Compared on the resolved selector rather than + * on object identity, since each event carries its own target object. + */ +function sameField(a: TraceEvent, b: TraceEvent): boolean { + if (a.type !== "input" || b.type !== "input") return false; + const sa = selectorFrom(a.target); + const sb = selectorFrom(b.target); + if (!sa || !sb) return false; + return JSON.stringify(sa) === JSON.stringify(sb); +} + +export function compileTrace(trace: RecordingTrace): CompileTraceResult { + const id = makeIdFactory(); + const steps: WorkflowStep[] = []; + const notes: string[] = []; + + // Open the page the recording started on. Without this the workflow depends + // on whatever the browser happened to be showing. + const firstUrl = trace.startUrl ?? trace.events.find((e) => e.url)?.url; + if (firstUrl) { + steps.push({ id: id("nav"), type: "navigate", url: firstUrl, label: "Open the starting page" }); + } else { + notes.push("The trace had no URL, so the workflow does not open a page first. Add a navigate step."); + } + + let lastUrl = firstUrl; + + for (let i = 0; i < trace.events.length; i++) { + const event = trace.events[i]!; + + // Collapse a run of keystrokes on one field into its final value. + if (event.type === "input") { + let last = event; + while (i + 1 < trace.events.length && sameField(last, trace.events[i + 1]!)) { + last = trace.events[++i] as typeof event; + } + const selector = selectorFrom(last.target); + if (!selector) { + notes.push( + `Skipped typing into an element with nothing to identify it by (at ${last.url}). ` + + "The recorder could not read a role, name, test id or text for it.", + ); + continue; + } + steps.push({ + id: id("fill"), + type: "fill", + selector, + value: last.redacted ? SECRET_PLACEHOLDER : (last.value ?? ""), + sensitive: last.redacted, + ...(describe(last.target) ? { label: `Fill ${describe(last.target)}` } : {}), + }); + if (last.redacted) { + notes.push( + `The value typed into "${describe(last.target) ?? "a field"}" was a password or ` + + "other secret, so it was never captured. Set it before running this workflow.", + ); + } + continue; + } + + if (event.type === "select") { + const selector = selectorFrom(event.target); + if (!selector) { + notes.push(`Skipped a dropdown selection with nothing to identify it by (at ${event.url}).`); + continue; + } + steps.push({ + id: id("select"), + type: "select", + selector, + value: event.value, + ...(describe(event.target) ? { label: `Choose ${describe(event.target)}` } : {}), + }); + continue; + } + + if (event.type === "click") { + const selector = selectorFrom(event.target); + if (!selector) { + notes.push( + `Skipped a click on an element with nothing to identify it by (at ${event.url}). ` + + "Pixel coordinates are deliberately not recorded, so this click cannot be replayed.", + ); + continue; + } + steps.push({ + id: id("click"), + type: "click", + selector, + ...(describe(event.target) ? { description: describe(event.target) } : {}), + ...(describe(event.target) ? { label: `Click ${describe(event.target)}` } : {}), + }); + continue; + } + + if (event.type === "navigate") { + // Only when the page actually changed; a recorder may emit one per + // history event, including replaces that land on the same URL. + if (event.url && event.url !== lastUrl) { + steps.push({ + id: id("nav"), + type: "navigate", + url: event.url, + label: "Follow the page change", + }); + lastUrl = event.url; + } + continue; + } + + // `submit` carries no reliable target of its own — the click that caused + // it is already recorded, and replaying a form submission separately would + // double the action. + if (event.type === "submit") { + notes.push( + `A form was submitted at ${event.url}. The click that submitted it is already a step; ` + + "check that an approval sits in front of it.", + ); + continue; + } + } + + const gated = insertApprovals(steps, id); + + if (gated.inserted === 0) { + notes.push( + "No step in this recording looked irreversible, so no approval gate was added. " + + "If it sends, pays, deletes or submits anything, add one before publishing.", + ); + } + + return { steps: gated.steps as WorkflowSteps, notes }; +} + +/** + * Puts an `approval` immediately before every step the engine would gate. + * + * Uses `classifyStep` — the same deterministic classifier the worker consults + * at run time — so the authored workflow agrees with what execution will + * actually do. Inserting a gate here does not *create* the protection (the + * engine gates regardless); it makes the pause visible in the plan the human + * reviews, with a reason attached. + */ +function insertApprovals( + steps: WorkflowStep[], + id: (prefix: string) => string, +): { steps: WorkflowStep[]; inserted: number } { + const out: WorkflowStep[] = []; + let inserted = 0; + + for (const step of steps) { + const verdict = classifyStep(step); + if (verdict.sensitive) { + const previous = out[out.length - 1]; + // Do not stack two gates: an approval immediately before is already the + // pause this would add. + if (!previous || previous.type !== "approval") { + out.push({ + id: id("approve"), + type: "approval", + reason: verdict.reason ?? "This step changes something outside Ghost.", + }); + inserted++; + } + } + out.push(step); + } + + return { steps: out, inserted }; +} diff --git a/cloud/packages/core/src/recording/roundtrip.test.ts b/cloud/packages/core/src/recording/roundtrip.test.ts new file mode 100644 index 00000000..838ba59a --- /dev/null +++ b/cloud/packages/core/src/recording/roundtrip.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from "vitest"; +import { compileTrace } from "./compile.js"; +import { parseRecordingTrace } from "./trace.js"; +import { classifyStep } from "../classifier/sensitive.js"; + +/** + * The capture → compile contract, exercised against a trace shaped exactly the + * way `apps/extension/src/content.js` emits one. + * + * This is the seam most likely to rot: the extension is plain JavaScript with + * no type checking against `@ghost/core`, so nothing but this test notices if + * the recorder's output drifts from what the compiler expects. If it fails, + * the extension and Ghost have disagreed about the format — fix one of them, + * do not relax the test. + * + * The scenario is the wedge Ghost is aimed at: log into a portal, find a + * record, change it, submit. + */ + +/** Verbatim in the shape `describeTarget()` produces. */ +const target = (over: Record) => ({ + selectorCandidates: [], + ...over, +}); + +const extensionTrace = { + version: 1, + sessionId: "rec_1735000000000", + startUrl: "https://portal.test/login", + capturedAt: 1735000000000, + recorder: "ghost-chrome-extension/0.1.0", + events: [ + { type: "navigate", url: "https://portal.test/login", timestamp: 1 }, + { + type: "input", + url: "https://portal.test/login", + timestamp: 2, + target: target({ + role: "textbox", + name: "Username", + selectorCandidates: ["#username", "input[name=\"user\"]"], + tagName: "input", + inputType: "text", + }), + value: "ops.team", + redacted: false, + }, + { + // The recorder never read this value — the field was type=password. + type: "input", + url: "https://portal.test/login", + timestamp: 3, + target: target({ + role: "textbox", + name: "Password", + selectorCandidates: ["#password"], + tagName: "input", + inputType: "password", + }), + redacted: true, + }, + { + type: "click", + url: "https://portal.test/login", + timestamp: 4, + target: target({ role: "button", name: "Sign in", selectorCandidates: ["#signin"] }), + }, + { type: "navigate", url: "https://portal.test/customers/4821", timestamp: 5 }, + { + type: "select", + url: "https://portal.test/customers/4821", + timestamp: 6, + target: target({ role: "combobox", name: "Status", selectorCandidates: ["#status"] }), + value: "active", + }, + { + type: "click", + url: "https://portal.test/customers/4821", + timestamp: 7, + target: target({ role: "button", name: "Submit changes", selectorCandidates: ["#save"] }), + }, + { type: "submit", url: "https://portal.test/customers/4821", timestamp: 8 }, + ], +}; + +describe("extension trace → workflow steps", () => { + const parsed = parseRecordingTrace(extensionTrace); + + it("is accepted by the trace schema exactly as the extension emits it", () => { + expect(parsed.ok).toBe(true); + }); + + if (!parsed.ok) throw new Error(`fixture must parse: ${parsed.error}`); + const { steps, notes } = compileTrace(parsed.trace); + + it("opens the page the recording started on", () => { + expect(steps[0]).toMatchObject({ type: "navigate", url: "https://portal.test/login" }); + }); + + it("carries the password field through as sensitive, with no value", () => { + const password = steps.find( + (s) => s.type === "fill" && s.selector.name === "Password", + ) as { value: string; sensitive: boolean } | undefined; + + expect(password).toBeDefined(); + expect(password!.sensitive).toBe(true); + expect(password!.value).toMatch(/redacted/i); + // The whole trace must be free of it — no field anywhere holds a password. + expect(JSON.stringify(extensionTrace)).not.toMatch(/hunter2|correct-horse/i); + }); + + it("keeps the non-secret value it did capture", () => { + const username = steps.find((s) => s.type === "fill" && s.selector.name === "Username") as + | { value: string } + | undefined; + expect(username?.value).toBe("ops.team"); + }); + + it("resolves elements semantically rather than by CSS", () => { + const signIn = steps.find( + (s) => s.type === "click" && s.selector.name === "Sign in", + ) as { selector: Record } | undefined; + expect(signIn?.selector).toMatchObject({ role: "button", name: "Sign in" }); + expect(signIn?.selector.css).toBeUndefined(); + }); + + it("stops for approval before submitting the change", () => { + const submitIndex = steps.findIndex( + (s) => s.type === "click" && s.selector.name === "Submit changes", + ); + expect(submitIndex).toBeGreaterThan(0); + expect(steps[submitIndex - 1]).toMatchObject({ type: "approval" }); + }); + + it("gates every step the engine would gate, and no step is left ungated", () => { + steps.forEach((step, i) => { + if (step.type === "approval") return; + if (classifyStep(step).sensitive) { + expect(steps[i - 1], `step ${i} is sensitive but ungated`).toMatchObject({ + type: "approval", + }); + } + }); + }); + + it("tells the reviewer the password needs setting before a run", () => { + expect(notes.join(" ")).toMatch(/set it before running/i); + }); + + it("emits no coordinates anywhere", () => { + const serialized = JSON.stringify(steps); + expect(serialized).not.toMatch(/"x":\s*\d/); + expect(serialized).not.toMatch(/"y":\s*\d/); + expect(serialized).not.toMatch(/clientX|pageX|screenX/); + }); +}); diff --git a/cloud/packages/core/src/recording/trace.ts b/cloud/packages/core/src/recording/trace.ts new file mode 100644 index 00000000..284a6f72 --- /dev/null +++ b/cloud/packages/core/src/recording/trace.ts @@ -0,0 +1,119 @@ +import { z } from "zod"; + +/** + * The capture format: what a Ghost recorder uploads. + * + * This is a **contract**, not an internal shape. The Chrome extension in + * `apps/extension` produces it; `compileTrace` consumes it. Anything else that + * can produce this JSON — a future desktop recorder, a customer's own tooling — + * works without changing Ghost. + * + * The design decision that matters: a recorder emits **accessible role and + * name at capture time**, computed from the live DOM while the element is + * still on screen. Those are exactly the fields `resolveLocator` prefers, so + * the trace can be compiled into steps deterministically, with no model in the + * correctness path. Inferring `role`/`name` later from an opaque HAR or event + * log is guesswork; reading them off the element is not. + * + * `selectorCandidates` is ordered best-first and exists because the worker + * replays in a *different* browser from the one that recorded. A single + * brittle selector is the failure that design invites. + */ + +/** What a recorder observed about the element an event happened on. */ +export const traceTargetSchema = z.object({ + /** ARIA role, explicit or implicit. */ + role: z.string().min(1).optional(), + /** Accessible name, computed the way a screen reader would. */ + name: z.string().min(1).optional(), + /** `data-testid` and friends, when the page has them. */ + testId: z.string().min(1).optional(), + /** Visible text, as a weaker fallback than role+name. */ + text: z.string().min(1).optional(), + /** + * Ordered best-first CSS fallbacks. Only used when nothing semantic + * resolved — the resolution chain prefers role+name, then testId, then text. + */ + selectorCandidates: z.array(z.string().min(1)).max(10).default([]), + /** Tag name, for deciding between `fill` and `select`. */ + tagName: z.string().min(1).optional(), + /** `type` attribute of an input, for sensitivity decisions. */ + inputType: z.string().min(1).optional(), +}); + +export type TraceTarget = z.infer; + +const base = z.object({ + /** Page URL at the moment of the event. */ + url: z.string().min(1), + /** Epoch milliseconds. Ordering is by array position; this is for humans. */ + timestamp: z.number().int().nonnegative(), +}); + +/** + * `redacted` is set by the *recorder*, which is the only place it can be done + * honestly: the value never leaves the page. A trace that carried the secret + * plus a "please ignore this" flag would already have leaked it — see + * `docs/DEPLOY.md` on why a trace is customer data. + */ +export const traceEventSchema = z.discriminatedUnion("type", [ + base.extend({ + type: z.literal("navigate"), + }), + base.extend({ + type: z.literal("click"), + target: traceTargetSchema, + }), + base.extend({ + type: z.literal("input"), + target: traceTargetSchema, + /** Absent when `redacted` — the recorder never captured it. */ + value: z.string().optional(), + redacted: z.boolean().default(false), + }), + base.extend({ + type: z.literal("select"), + target: traceTargetSchema, + value: z.string(), + }), + base.extend({ + type: z.literal("submit"), + target: traceTargetSchema.optional(), + }), +]); + +export type TraceEvent = z.infer; + +export const recordingTraceSchema = z.object({ + version: z.literal(1), + sessionId: z.string().min(1), + /** Where recording started, so the compiled workflow opens the right page. */ + startUrl: z.string().min(1).optional(), + capturedAt: z.number().int().nonnegative().optional(), + /** Names the producer, so a bad recorder can be identified from a trace. */ + recorder: z.string().min(1).optional(), + events: z.array(traceEventSchema), +}); + +export type RecordingTrace = z.infer; + +/** + * Whether some uploaded bytes are a structured Ghost trace. + * + * Used to decide between the deterministic compiler and the optional + * model-backed one. A HAR or a Playwright zip is not this shape and falls + * through to whatever compiler is configured, if any. + */ +export function parseRecordingTrace( + input: unknown, +): { ok: true; trace: RecordingTrace } | { ok: false; error: string } { + const parsed = recordingTraceSchema.safeParse(input); + if (!parsed.success) { + const first = parsed.error.issues[0]; + return { + ok: false, + error: first ? `${first.path.join(".") || "trace"}: ${first.message}` : "invalid trace", + }; + } + return { ok: true, trace: parsed.data }; +} From 748395084883e749a1ff6b9f14bbd6d38e9599df Mon Sep 17 00:00:00 2001 From: mohabbis <101276427+mohabbis@users.noreply.github.com> Date: Wed, 5 Aug 2026 12:52:07 -0500 Subject: [PATCH 2/7] fix(worker): make production container build and boot reliably MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worker Dockerfile had never actually been built or run since it was written. Auditing what was genuinely left on the cloud roadmap surfaced two stacked bugs, neither caught by CI (`pnpm build` only bundles the worker with tsup at the workspace level — it never exercises the Dockerfile's own COPY list or executes the resulting dist/index.js): - The deps stage copied packages/core/package.json but not its prisma/ directory. @ghost/core's postinstall runs a bare `prisma generate`, which resolves the schema at the default ./prisma/schema.prisma path, so `pnpm install` failed inside the image before the build stage (which does copy the rest of packages/core) ever ran. The same gap silently dropped tsconfig.base.json, breaking apps/worker/tsconfig.json's `extends`. - Once building, the container crashed immediately on boot: bundling @ghost/core (via tsup's `noExternal: [/^@ghost\//]`) pulls in @prisma/client's generated CJS runtime, which dynamically requires native query-engine files — esbuild's CJS-to-ESM interop can't represent that and throws "Dynamic require of 'fs' is not supported" at the first call. Fixed by keeping @prisma/client external in tsup.config.ts and adding it as a direct dependency of @ghost/worker, since pnpm's strict linking won't resolve a transitive dep at the worker's own require path otherwise. Verified by rebuilding the image and running it against real Postgres/Redis until it logged its startup line rather than crashing. Added a CI step that builds and boot-smoke-tests the image on every PR so this class of bug can't ship invisibly again. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/cloud.yml | 45 ++++++++++++++++++++++++++++++++ cloud/apps/worker/Dockerfile | 7 ++++- cloud/apps/worker/package.json | 1 + cloud/apps/worker/tsup.config.ts | 7 +++++ cloud/pnpm-lock.yaml | 3 +++ 5 files changed, 62 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cloud.yml b/.github/workflows/cloud.yml index 390fb262..ab3ab0c8 100644 --- a/.github/workflows/cloud.yml +++ b/.github/workflows/cloud.yml @@ -160,3 +160,48 @@ jobs: - name: Build run: pnpm build + + - name: Build worker container image + # `pnpm build` above only bundles the worker with tsup — it never + # exercises the Dockerfile's own multi-stage COPY list, and it never + # runs the resulting dist/index.js in an isolated, workspace-filtered + # node_modules the way the image's `pnpm install --filter + # @ghost/worker...` does. Both gaps let a genuinely broken image ship + # silently before: a missing `packages/core/prisma` COPY failed + # `pnpm install` inside the image, and bundling @prisma/client's CJS + # runtime into the ESM output crashed the container on boot with + # "Dynamic require of 'fs' is not supported". Build it here so a + # regression in either fails CI instead of surfacing at deploy time. + run: docker build -f apps/worker/Dockerfile -t ghost-worker-ci . + + - name: Smoke-test worker container boots + # A build succeeding is not the same as the entrypoint running. Boot + # it against the same Postgres/Redis service containers this job + # already has and require the worker's own startup log line — not just + # "container still alive", which a crash-loop with a restart policy + # would also satisfy. + run: | + set -euo pipefail + container_id=$(docker run -d --network host \ + -e DATABASE_URL="$DATABASE_URL" \ + -e REDIS_URL="$REDIS_URL" \ + -e GHOST_SESSION_KEY="$GHOST_SESSION_KEY" \ + ghost-worker-ci) + trap 'docker logs "$container_id" || true; docker rm -f "$container_id" >/dev/null 2>&1 || true' EXIT + + ok="" + for _ in $(seq 1 15); do + if docker logs "$container_id" 2>&1 | grep -q "Ghost worker started"; then + ok=1 + break + fi + if [ "$(docker inspect -f '{{.State.Running}}' "$container_id")" != "true" ]; then + break + fi + sleep 1 + done + + if [ -z "$ok" ]; then + echo "worker container did not report startup within 15s" >&2 + exit 1 + fi diff --git a/cloud/apps/worker/Dockerfile b/cloud/apps/worker/Dockerfile index 65e12edc..1062d4bc 100644 --- a/cloud/apps/worker/Dockerfile +++ b/cloud/apps/worker/Dockerfile @@ -27,8 +27,13 @@ WORKDIR /app # ---- dependencies ----------------------------------------------------------- FROM base AS deps # Manifests first so a code-only change does not re-resolve the dependency graph. -COPY package.json pnpm-lock.yaml pnpm-workspace.yaml turbo.json ./ +COPY package.json pnpm-lock.yaml pnpm-workspace.yaml turbo.json tsconfig.base.json ./ COPY packages/core/package.json packages/core/ +# packages/core's postinstall runs a bare `prisma generate`, which resolves +# the schema at the default `./prisma/schema.prisma` path relative to the +# package. Without it here, `pnpm install` fails inside this stage before the +# build stage (which copies the rest of packages/core) ever runs. +COPY packages/core/prisma packages/core/prisma COPY apps/worker/package.json apps/worker/ RUN pnpm install --frozen-lockfile --filter @ghost/worker... diff --git a/cloud/apps/worker/package.json b/cloud/apps/worker/package.json index 5595d718..1acb88f0 100644 --- a/cloud/apps/worker/package.json +++ b/cloud/apps/worker/package.json @@ -13,6 +13,7 @@ }, "dependencies": { "@ghost/core": "workspace:*", + "@prisma/client": "^6.1.0", "bullmq": "^5.34.4", "ioredis": "^5.4.2", "playwright": "^1.55.0" diff --git a/cloud/apps/worker/tsup.config.ts b/cloud/apps/worker/tsup.config.ts index 3fcb94a6..ffc83301 100644 --- a/cloud/apps/worker/tsup.config.ts +++ b/cloud/apps/worker/tsup.config.ts @@ -8,4 +8,11 @@ export default defineConfig({ // Bundle the workspace package (it ships TS source via subpath exports) so the // built worker runs on plain Node. noExternal: [/^@ghost\//], + // @prisma/client's generated runtime is CJS that dynamically `require()`s + // native query-engine files by path. Inlining it into the ESM bundle makes + // esbuild rewrite that into a `require` shim that throws "Dynamic require of + // 'fs' is not supported" the instant the worker boots. Keep it (and the + // generated `.prisma/client`) external so Node loads it normally from + // node_modules instead. + external: ["@prisma/client", ".prisma/client"], }); diff --git a/cloud/pnpm-lock.yaml b/cloud/pnpm-lock.yaml index 88f55c9e..28a4b2af 100644 --- a/cloud/pnpm-lock.yaml +++ b/cloud/pnpm-lock.yaml @@ -115,6 +115,9 @@ importers: '@ghost/core': specifier: workspace:* version: link:../../packages/core + '@prisma/client': + specifier: ^6.1.0 + version: 6.19.3(prisma@6.19.3(typescript@5.9.3))(typescript@5.9.3) bullmq: specifier: ^5.34.4 version: 5.81.2 From edf10c7f1c31a824311896b4a34034b9390f83b4 Mon Sep 17 00:00:00 2001 From: mohabbis <101276427+mohabbis@users.noreply.github.com> Date: Wed, 5 Aug 2026 12:52:19 -0500 Subject: [PATCH 3/7] docs(cloud): reconcile handoff and status documentation with current implementation CURSOR_HANDOFF.md described the WorkflowCompiler abstraction, four-eyes approval, and S3 artifact serving as open work, framing a stale roadmap that led to prioritizing already-shipped items. All three were fully implemented and tested by earlier PRs (#403, #406) whose changes never made it back into this file's "remaining work" section: - WorkflowCompiler: apps/web/src/lib/compiler/{types,index,harness-router-compiler}.ts already provides the interface, a swappable HarnessRouter adapter, and normalized error types. - Four-eyes approval: Membership/Role/Invitation plus Organization.requireSeparateApprover already enforce requester != approver server-side, covered by separation-of-duties.test.ts. - S3 artifact serving: packages/core/src/storage/artifacts.ts already has a working S3ArtifactStore with presigned URLs; only retention/cleanup is genuinely still open, called out as such. Also documents the worker container bugs found and fixed in the prior commit, and corrects the stale "239 tests" figure (actual full green run is 398) and the "typed step editor: remaining" status line (workflow-editor.tsx already covers it) in both this file and README.md. Co-Authored-By: Claude Sonnet 5 --- cloud/README.md | 4 +- cloud/docs/CURSOR_HANDOFF.md | 102 ++++++++++++++++++++++++++++++----- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/cloud/README.md b/cloud/README.md index cf725f4d..adcd0db1 100644 --- a/cloud/README.md +++ b/cloud/README.md @@ -144,7 +144,7 @@ and `turbo run lint` skips packages that define no `lint` script — silently, a with a green summary. Treat `typecheck` as the real static gate until the other three packages have configs. -A full green run is **239 tests**. Roughly 90 of them are gated on +A full green run is **398 tests**. Roughly 90 of them are gated on `Boolean(process.env.DATABASE_URL)` and **skip silently** without it — so a green run with no database covers none of the execution engine. If the worker suite reports 45 tests rather than 90, the database is not being reached. @@ -157,7 +157,7 @@ suite reports 45 tests rather than 90, the database is not being reached. | 1.1 | Playwright engine, approval state machine, artifacts, hash-chained audit | **done** | | 1.2 | Run trigger, live timeline, approve/reject resume | **done** | | 1.3 | Durable execution (run journal), timeouts/retry, incidents + cancel, audit-verify, variable store | **done** | -| 1.x | Typed step editor | remaining | +| 1.x | Typed step editor | **done** (`workflow-editor.tsx`: type/selector/value, reorder, delete, validation, undo authoring) | | 2 | Browser recording → editable steps | convert built (upload → compile → review), off by default; capture is next | Phase 1.3 made run position a fold over an append-only, hash-chained journal diff --git a/cloud/docs/CURSOR_HANDOFF.md b/cloud/docs/CURSOR_HANDOFF.md index 6bfb22f8..bd859e14 100644 --- a/cloud/docs/CURSOR_HANDOFF.md +++ b/cloud/docs/CURSOR_HANDOFF.md @@ -305,7 +305,7 @@ Redis running** — CI catches it before merge either way, but a local loop is faster than waiting on a run: ```bash -cd cloud && pnpm typecheck && pnpm test && pnpm build # expect 239 tests +cd cloud && pnpm typecheck && pnpm test && pnpm build # expect 398 tests ``` Without a database roughly 90 of those skip themselves and you learn nothing @@ -363,15 +363,28 @@ page reload, confirmed the stream closes on a terminal status rather than reconnecting forever (5 requests total, network panel stayed flat for 8s after). -1. **S3 serve path** — disk store works in dev; wire presigned URLs when S3 is on. - Note the artifact route is now a positive allow-list (`step-`/`restore-` PNGs - only); keep it that way, because the same prefix holds encrypted session - blobs that must never be served. -2. **Four-eyes approval** — `isOrgAdmin` above is a role check, not the RBAC - `requireSeparateApprover` would need (which is about who may *approve*, not - who may *administer*); still needs its own design, and orgs are still - auto-created single-member on first sign-in with no invite flow, so it has - no real subject to test against yet. +Done: **S3 serve path**. `packages/core/src/storage/artifacts.ts` has a real +`S3ArtifactStore` (put/get/delete/deletePrefix/presigned `signedUrl`) alongside +the disk fallback; `artifactStore()` switches on `S3_BUCKET` + +`S3_ACCESS_KEY_ID` + `S3_SECRET_ACCESS_KEY` being set, and `docs/DEPLOY.md` +documents the silent-fallback failure mode. The artifact route is a positive +allow-list (`step-`/`restore-` PNGs only) — keep it that way; the same prefix +holds encrypted session blobs that must never be served. What's still actually +missing is a retention/cleanup policy: nothing expires or purges old run +artifacts, on disk or in the bucket, so storage grows unbounded over the life +of an org. Not started. + +Done: **four-eyes approval**. `isOrgAdmin` above undersold this — a full +membership model shipped alongside it: `Membership`/`Role` +(`OWNER`/`ADMIN`/`MEMBER`) and `Invitation` in the schema, invite/accept routes +under `app/api/invitations` and `app/api/settings/members` +(email-match-enforced acceptance, not just token possession — see +`checkInvitationRedeemable`), and `Organization.requireSeparateApprover` +enforcing `requester_user_id !== approver_user_id` server-side when a workflow +opts in. Covered end-to-end by +`apps/web/src/app/api/runs/separation-of-duties.test.ts` (5 tests) and +`apps/web/src/app/api/settings/members/members.test.ts` (9 tests) against a +real Postgres. Nothing on this item is a gap anymore. ## Phase 2 @@ -395,10 +408,18 @@ pipeline, it only pre-fills the editor. `HR_API_KEY` (server-only, infrastructure used to build Ghost; nothing a customer executes goes through it. Compile is an optional authoring convenience that is unavailable when the key is unset, and production is expected to leave it unset — see -`docs/DEPLOY.md`. The next change here extracts a provider-neutral -`WorkflowCompiler` interface so the HarnessRouter client is one replaceable -adapter rather than a dependency reaching into the recording routes. Removing -that adapter must disable compile and nothing else. +`docs/DEPLOY.md`. + +Done: **the provider-neutral `WorkflowCompiler` boundary** (PR #406). The +interface, its `HarnessRouterWorkflowCompiler` implementation, and the +`workflowCompiler()`/`requireWorkflowCompiler()`/`compilerConfigured()` +accessors live in `apps/web/src/lib/compiler/{types,index,harness-router-compiler}.ts`. +`recording-compiler.ts` depends only on `WorkflowCompiler`; HarnessRouter-specific +types stay inside `harness-router-compiler.ts` and never leak into it. Errors +normalize to Ghost-owned `CompilerNotConfiguredError`/`CompilerRequestError`, +and `compiler-optional.test.ts` swaps in a fake for tests. Adding a second +adapter needs no change to the recording routes — only a new file and a branch +in `workflowCompiler()`. **Capture is decided: a Chrome extension for v1.** A Ghost-hosted remote browser is the more elegant answer — it records in the same environment the @@ -410,6 +431,59 @@ to exist, not a particular producer, so the extension slots in ahead of the upload step and `POST /api/recordings` stays as it is. The manual upload form is scaffolding, not the product. +**Capture is also built, not just decided.** `cloud/apps/extension` is a +working Chrome extension (records clicks/typing/selects/submits/navigation via +accessible role+name, redacts secret-shaped fields at capture, uploads to +`POST /api/agent/recordings` with a revocable bearer token) — see its own +`README.md` for the trust boundary. It lands on `feat/browser-recording-extension`, +not yet merged, so `README.md`'s Phase 2 status line ("capture is next") is +correct as of `master` but stale the moment this branch merges. Update it in +the same PR. + +## Worker container: built, but had never actually run + +Audited 2026-08-05, prompted by a stale roadmap in this very document (the S3 +and four-eyes items above, both already shipped). While verifying what was +*actually* left, `docker build -f apps/worker/Dockerfile .` was run for the +first time since this file's own "no deployment of Ghost exists yet" caveat +in `docs/DEPLOY.md` was written — and it failed, then failed differently, then +crashed on boot. Two independent bugs, both now fixed: + +1. The `deps` stage only `COPY`'d `packages/core/package.json`, not + `packages/core/prisma`. `@ghost/core`'s `postinstall` runs a bare `prisma + generate`, which resolves the schema at the default `./prisma/schema.prisma` + path — absent at that point in the build — so `pnpm install` itself failed + before the `build` stage (which does copy the rest of `packages/core`) ever + ran. Same root cause silently dropped `tsconfig.base.json`, so + `apps/worker/tsconfig.json`'s `extends` also failed (non-fatally — tsup + warned and fell back to its own defaults). Fixed by copying both ahead of + `pnpm install`. +2. With the image building, the container crashed immediately on boot: + `Error: Dynamic require of "fs" is not supported`. `tsup.config.ts`'s + `noExternal: [/^@ghost\//]` bundles `@ghost/core` into the worker's ESM + output, which transitively pulled in `@prisma/client`'s generated CJS + runtime (it dynamically `require()`s native query-engine files) — esbuild's + CJS→ESM interop can't represent that and throws at the first call. Fixed by + marking `@prisma/client`/`.prisma/client` `external` in `tsup.config.ts`, + and adding `@prisma/client` as a direct dependency of `@ghost/worker` + (pnpm's strict linking won't resolve a transitive dep at the worker's own + require path otherwise). + +Verified by rebuilding the image and running it against a real Postgres + +Redis with `docker run --network host`, confirming the `[worker] Ghost worker +started...` log line rather than a crash. `.github/workflows/cloud.yml` now +builds and boot-smoke-tests the image on every PR (`Build worker container +image` / `Smoke-test worker container boots`) — before this, CI's `pnpm build` +only ran `tsup` at the workspace level, which neither exercised the +Dockerfile's own `COPY` list nor ever executed the bundled `dist/index.js`, so +both bugs shipped invisibly across several PRs. + +This does not mean a deployment now exists — `docs/DEPLOY.md`'s runbook is +still unexecuted against a real host, and the sign-in trap and domain trap it +documents are still open. It means the one artifact that runbook assumed +worked, and that a first deploy attempt would have discovered the hard way, +now actually does. + ## Repo conventions - Validate from `cloud/`: `pnpm typecheck && pnpm test && pnpm build`. From e6ba2ae9ac3b518754a15e4cbdf8804028fb7ef6 Mon Sep 17 00:00:00 2001 From: mohabbis <101276427+mohabbis@users.noreply.github.com> Date: Wed, 5 Aug 2026 14:35:10 -0500 Subject: [PATCH 4/7] fix(web): stop signin from silently rendering with no way to sign in In production with no GitHub OAuth app configured, the signin page rendered a card with a title and zero buttons -- indistinguishable from a bug. Whoever hits this is more likely to be standing up the deployment than an end user, so name the exact fix (AUTH_GITHUB_ID/AUTH_GITHUB_SECRET, the callback URL) instead of failing silently. See docs/DEPLOY.md's "sign-in trap". Co-Authored-By: Claude Sonnet 5 --- cloud/apps/web/src/app/signin/page.tsx | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cloud/apps/web/src/app/signin/page.tsx b/cloud/apps/web/src/app/signin/page.tsx index 60d650c3..62d71524 100644 --- a/cloud/apps/web/src/app/signin/page.tsx +++ b/cloud/apps/web/src/app/signin/page.tsx @@ -6,6 +6,12 @@ import { SOURCE_URL } from "@/lib/source-url"; const githubEnabled = Boolean(process.env.AUTH_GITHUB_ID && process.env.AUTH_GITHUB_SECRET); const devEnabled = process.env.NODE_ENV !== "production"; +// Both false means production with no OAuth provider configured: no form +// below has anything to render, and a card with a title and no buttons looks +// like a bug rather than a missing deploy step. Whoever hits this is more +// likely to be the person standing up the deployment than an end user, so +// name the exact fix rather than failing silently. See docs/DEPLOY.md's "sign-in trap". +const misconfigured = !githubEnabled && !devEnabled; export default async function SignInPage({ searchParams, @@ -27,6 +33,25 @@ export default async function SignInPage({

+ {misconfigured && ( +
+

+ No sign-in method is configured +

+

+ This deployment has NODE_ENV=production and no GitHub OAuth + app configured, so there is no way to sign in. Set{" "} + AUTH_GITHUB_ID and AUTH_GITHUB_SECRET, with the + app's callback at{" "} + https://<this-domain>/api/auth/callback/github. See + docs/DEPLOY.md. +

+
+ )} + {githubEnabled && (
{ From 3018dc4c24d384e685f2be886f89492e7efd7d9f Mon Sep 17 00:00:00 2001 From: mohabbis <101276427+mohabbis@users.noreply.github.com> Date: Wed, 5 Aug 2026 14:35:29 -0500 Subject: [PATCH 5/7] feat(worker): add artifact retention, structured logging, and opt-in Sentry error tracking Three gaps identified by an audit of what was genuinely left on the cloud roadmap (recorded in CURSOR_HANDOFF.md), closed as one changeset since they share the same few files (index.ts, purgeArtifacts.ts): - Retention: nothing ever deleted a run's screenshots. `purge-artifacts` is a new BullMQ job, scheduled daily via upsertJobScheduler at worker boot, that deletes a run's artifact prefix once it ended more than ARTIFACT_RETENTION_DAYS ago (default 90) and audits the deletion. Run. artifactsPurgedAt makes it idempotent; a store failure is retried next cycle rather than silently marked done. Eligibility is a pure function (packages/core/src/retention.ts) so the window logic is tested without a database. - Structured logging: the worker only had console.log/console.error, so a failure was invisible unless someone was tailing container logs. packages/core/src/logger.ts is a small, dependency-free JSON-line logger (errors to stderr, everything else to stdout) now used throughout index.ts and purgeArtifacts.ts. - Error tracking: @ghost/core/sentry wraps @sentry/node, gated on SENTRY_DSN exactly like HR_API_KEY/S3_BUCKET -- absent means a complete no-op, present enables capture on every job-failure handler. Worker-only: wiring apps/web needs @sentry/nextjs (its own webpack/turbopack plugin exists specifically to handle Sentry's auto-instrumentation, which cannot otherwise be bundled -- confirmed by trying the manual approach first and watching it break `pnpm build` with an unbundleable node:child_process import). @sentry/node gets the same tsup `external` treatment as @prisma/client, for the same reason: its runtime does dynamic requires that bundling breaks. Verified end-to-end: full test suite (424 tests) against real Postgres + Redis, a Docker rebuild, and a boot smoke test with SENTRY_DSN actually set (not just absent) to confirm Sentry initializing doesn't crash the container. DEPLOY.md, README.md and CURSOR_HANDOFF.md updated for the new env vars and the corrected test count. Co-Authored-By: Claude Sonnet 5 --- cloud/README.md | 2 +- cloud/apps/worker/package.json | 1 + cloud/apps/worker/src/index.ts | 56 +++- .../worker/src/jobs/purgeArtifacts.test.ts | 148 +++++++++ cloud/apps/worker/src/jobs/purgeArtifacts.ts | 109 +++++++ cloud/apps/worker/tsup.config.ts | 8 +- cloud/docs/CURSOR_HANDOFF.md | 2 +- cloud/docs/DEPLOY.md | 99 +++++- cloud/packages/core/package.json | 8 +- .../migration.sql | 5 + .../prisma/migrations/migration_lock.toml | 2 + cloud/packages/core/prisma/schema.prisma | 10 + cloud/packages/core/src/logger.test.ts | 70 +++++ cloud/packages/core/src/logger.ts | 82 +++++ cloud/packages/core/src/queue.ts | 10 + cloud/packages/core/src/retention.test.ts | 95 ++++++ cloud/packages/core/src/retention.ts | 60 ++++ cloud/packages/core/src/sentry.test.ts | 72 +++++ cloud/packages/core/src/sentry.ts | 69 +++++ cloud/pnpm-lock.yaml | 293 +++++++++++++++++- 20 files changed, 1174 insertions(+), 27 deletions(-) create mode 100644 cloud/apps/worker/src/jobs/purgeArtifacts.test.ts create mode 100644 cloud/apps/worker/src/jobs/purgeArtifacts.ts create mode 100644 cloud/packages/core/prisma/migrations/20260805185851_run_artifacts_purged_at/migration.sql create mode 100644 cloud/packages/core/src/logger.test.ts create mode 100644 cloud/packages/core/src/logger.ts create mode 100644 cloud/packages/core/src/retention.test.ts create mode 100644 cloud/packages/core/src/retention.ts create mode 100644 cloud/packages/core/src/sentry.test.ts create mode 100644 cloud/packages/core/src/sentry.ts diff --git a/cloud/README.md b/cloud/README.md index adcd0db1..0db23ab7 100644 --- a/cloud/README.md +++ b/cloud/README.md @@ -144,7 +144,7 @@ and `turbo run lint` skips packages that define no `lint` script — silently, a with a green summary. Treat `typecheck` as the real static gate until the other three packages have configs. -A full green run is **398 tests**. Roughly 90 of them are gated on +A full green run is **424 tests**. Roughly 90 of them are gated on `Boolean(process.env.DATABASE_URL)` and **skip silently** without it — so a green run with no database covers none of the execution engine. If the worker suite reports 45 tests rather than 90, the database is not being reached. diff --git a/cloud/apps/worker/package.json b/cloud/apps/worker/package.json index 1acb88f0..1c886174 100644 --- a/cloud/apps/worker/package.json +++ b/cloud/apps/worker/package.json @@ -14,6 +14,7 @@ "dependencies": { "@ghost/core": "workspace:*", "@prisma/client": "^6.1.0", + "@sentry/node": "^10.69.0", "bullmq": "^5.34.4", "ioredis": "^5.4.2", "playwright": "^1.55.0" diff --git a/cloud/apps/worker/src/index.ts b/cloud/apps/worker/src/index.ts index b4a17ff0..51b0b747 100644 --- a/cloud/apps/worker/src/index.ts +++ b/cloud/apps/worker/src/index.ts @@ -1,13 +1,17 @@ -import { Worker } from "bullmq"; +import { Queue, Worker } from "bullmq"; import { QUEUE_NAMES, type CompensateRunJob, type NoopJob, + type PurgeArtifactsJob, type RunWorkflowJob, } from "@ghost/core/queue"; +import { createLogger, serializeError } from "@ghost/core/logger"; +import { initSentry, captureException } from "@ghost/core/sentry"; import { createRedisConnection } from "./redis.js"; import { runWorkflowJob } from "./jobs/runWorkflow.js"; import { compensateRunJob } from "./jobs/compensateRun.js"; +import { purgeArtifactsJob } from "./jobs/purgeArtifacts.js"; /** * Ghost worker entrypoint. @@ -17,6 +21,8 @@ import { compensateRunJob } from "./jobs/compensateRun.js"; * Each queue gets its own Worker so failures stay isolated. */ +initSentry("worker"); +const log = createLogger("worker"); const connection = createRedisConnection(); const runWorker = new Worker(QUEUE_NAMES.runWorkflow, runWorkflowJob, { @@ -31,7 +37,8 @@ const runWorker = new Worker(QUEUE_NAMES.runWorkflow, runWorkflo maxStalledCount: 1, }); runWorker.on("failed", (job, err) => { - console.error(`[worker] run-workflow ${job?.data.runId} failed:`, err); + log.error("run-workflow job failed", { runId: job?.data.runId, ...serializeError(err) }); + captureException(err, { runId: job?.data.runId, queue: QUEUE_NAMES.runWorkflow }); }); // Reversal shares the run's lock duration but runs at lower concurrency: it is @@ -42,32 +49,59 @@ const compensateWorker = new Worker( { connection, concurrency: 1, lockDuration: 60_000, stalledInterval: 30_000, maxStalledCount: 1 }, ); compensateWorker.on("failed", (job, err) => { - console.error(`[worker] compensate-run ${job?.data.runId} failed:`, err); + log.error("compensate-run job failed", { runId: job?.data.runId, ...serializeError(err) }); + captureException(err, { runId: job?.data.runId, queue: QUEUE_NAMES.compensateRun }); }); const noopWorker = new Worker( QUEUE_NAMES.noop, async (job) => { - console.log( - `[worker] noop job ${job.id}: "${job.data.message}" (requested ${job.data.requestedAt})`, - ); + log.info("noop job received", { jobId: job.id, message: job.data.message, requestedAt: job.data.requestedAt }); return { ok: true, handledAt: new Date().toISOString() }; }, { connection }, ); noopWorker.on("completed", (job) => { - console.log(`[worker] completed ${job.id}`); + log.info("noop job completed", { jobId: job.id }); }); noopWorker.on("failed", (job, err) => { - console.error(`[worker] failed ${job?.id}:`, err); + log.error("noop job failed", { jobId: job?.id, ...serializeError(err) }); }); -console.log("[worker] Ghost worker started. Listening on queues:", Object.values(QUEUE_NAMES)); +const purgeWorker = new Worker( + QUEUE_NAMES.purgeArtifacts, + purgeArtifactsJob, + { connection, concurrency: 1 }, +); +purgeWorker.on("failed", (job, err) => { + log.error("purge-artifacts job failed", { jobId: job?.id, ...serializeError(err) }); + captureException(err, { jobId: job?.id, queue: QUEUE_NAMES.purgeArtifacts }); +}); + +// Schedules the recurring purge itself, rather than relying on an external +// cron to enqueue it. `upsertJobScheduler` is idempotent on its scheduler id, +// so every boot (including a redeploy) re-asserts the same daily schedule +// instead of accumulating a duplicate one — this line runs on every worker +// start, not just the first. +const purgeQueue = new Queue(QUEUE_NAMES.purgeArtifacts, { connection }); +await purgeQueue.upsertJobScheduler( + "purge-artifacts-daily", + { every: 24 * 60 * 60 * 1000 }, + { name: "purge-artifacts", data: {} }, +); + +log.info("Ghost worker started", { queues: Object.values(QUEUE_NAMES) }); async function shutdown(signal: string): Promise { - console.log(`[worker] ${signal} received, shutting down…`); - await Promise.all([noopWorker.close(), runWorker.close(), compensateWorker.close()]); + log.info("shutting down", { signal }); + await Promise.all([ + noopWorker.close(), + runWorker.close(), + compensateWorker.close(), + purgeWorker.close(), + purgeQueue.close(), + ]); await connection.quit(); process.exit(0); } diff --git a/cloud/apps/worker/src/jobs/purgeArtifacts.test.ts b/cloud/apps/worker/src/jobs/purgeArtifacts.test.ts new file mode 100644 index 00000000..2cc8416f --- /dev/null +++ b/cloud/apps/worker/src/jobs/purgeArtifacts.test.ts @@ -0,0 +1,148 @@ +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import type { Job } from "bullmq"; +import { prisma, Prisma } from "@ghost/core/db"; +import type { PurgeArtifactsJob } from "@ghost/core/queue"; + +/** + * DB-backed test for the artifact retention job. + * + * Mocks the artifact store rather than writing real files: the property under + * test is *which runs* the job decides to touch and how it records that + * decision (Run.artifactsPurgedAt, an audit event) — not the store's own + * put/get/delete behavior, which packages/core/src/storage/artifacts.ts + * already covers. + * + * Requires DATABASE_URL; skips cleanly without one. + */ + +const hasDb = Boolean(process.env.DATABASE_URL); + +const store = vi.hoisted(() => ({ + deletePrefix: vi.fn(async (_prefix: string) => undefined), +})); +vi.mock("../storage/artifacts.js", () => ({ artifactStore: () => store })); + +const { purgeArtifactsJob } = await import("./purgeArtifacts.js"); + +function fakeJob(): Job { + return { data: {}, id: "purge-test" } as Job; +} + +describe.skipIf(!hasDb)("purgeArtifactsJob (Postgres)", () => { + let orgId: string; + let workflowVersionId: string; + const slug = `purge-${Date.now()}`; + + beforeAll(async () => { + process.env.ARTIFACT_RETENTION_DAYS = "1"; + + const org = await prisma.organization.create({ data: { name: "Purge Org", slug } }); + orgId = org.id; + const wf = await prisma.workflow.create({ + data: { + orgId, + name: "purge demo", + versions: { + create: { version: 1, steps: [] as unknown as Prisma.InputJsonValue }, + }, + }, + include: { versions: true }, + }); + const [version] = wf.versions; + if (!version) throw new Error("workflow version was not created"); + workflowVersionId = version.id; + }); + + afterEach(() => { + store.deletePrefix.mockClear(); + }); + + afterAll(async () => { + delete process.env.ARTIFACT_RETENTION_DAYS; + await prisma.organization.delete({ where: { id: orgId } }).catch(() => undefined); + await prisma.$disconnect(); + }); + + function daysAgo(n: number): Date { + return new Date(Date.now() - n * 24 * 60 * 60 * 1000); + } + + async function seedRun(data: { + status: "SUCCEEDED" | "RUNNING" | "AWAITING_APPROVAL"; + endedAt: Date | null; + artifactsPurgedAt?: Date | null; + }) { + return prisma.run.create({ + data: { orgId, workflowVersionId, ...data }, + }); + } + + it("purges a run that ended well before the retention window, and audits it", async () => { + const run = await seedRun({ status: "SUCCEEDED", endedAt: daysAgo(3) }); + + const result = await purgeArtifactsJob(fakeJob()); + + // Not an exact-count assertion: this job intentionally scans across every + // org, so it may also process eligible runs left by whichever other DB-backed + // test files vitest happens to run concurrently against the same database. + // The specific, deterministic claim is about this run and this org. + expect(result.purged).toBeGreaterThanOrEqual(1); + expect(store.deletePrefix).toHaveBeenCalledWith(`runs/${run.id}`); + + const updated = await prisma.run.findUniqueOrThrow({ where: { id: run.id } }); + expect(updated.artifactsPurgedAt).not.toBeNull(); + + const audit = await prisma.auditEvent.findFirst({ + where: { orgId, entityType: "Run", entityId: run.id, action: "run.artifacts_purged" }, + }); + expect(audit).not.toBeNull(); + expect((audit?.metadata as { retentionDays?: number })?.retentionDays).toBe(1); + }); + + it("leaves a run that ended inside the retention window untouched", async () => { + const run = await seedRun({ status: "SUCCEEDED", endedAt: new Date() }); + + await purgeArtifactsJob(fakeJob()); + + expect(store.deletePrefix).not.toHaveBeenCalledWith(`runs/${run.id}`); + const unchanged = await prisma.run.findUniqueOrThrow({ where: { id: run.id } }); + expect(unchanged.artifactsPurgedAt).toBeNull(); + }); + + it("never touches a run that has not ended, no matter how old", async () => { + const run = await seedRun({ status: "RUNNING", endedAt: null }); + // Backdate createdAt directly — endedAt is what the job actually reads. + await prisma.run.update({ where: { id: run.id }, data: { createdAt: daysAgo(365) } }); + + await purgeArtifactsJob(fakeJob()); + + expect(store.deletePrefix).not.toHaveBeenCalledWith(`runs/${run.id}`); + const unchanged = await prisma.run.findUniqueOrThrow({ where: { id: run.id } }); + expect(unchanged.artifactsPurgedAt).toBeNull(); + }); + + it("does not re-purge a run that was already purged", async () => { + const run = await seedRun({ + status: "SUCCEEDED", + endedAt: daysAgo(30), + artifactsPurgedAt: daysAgo(1), + }); + + await purgeArtifactsJob(fakeJob()); + + expect(store.deletePrefix).not.toHaveBeenCalledWith(`runs/${run.id}`); + }); + + it("leaves artifactsPurgedAt null and reports a failure when the store errors", async () => { + const run = await seedRun({ status: "SUCCEEDED", endedAt: daysAgo(3) }); + store.deletePrefix.mockImplementationOnce(async () => { + throw new Error("simulated store outage"); + }); + + const result = await purgeArtifactsJob(fakeJob()); + + expect(result.failed).toBeGreaterThanOrEqual(1); + const unchanged = await prisma.run.findUniqueOrThrow({ where: { id: run.id } }); + expect(unchanged.artifactsPurgedAt).toBeNull(); + }); +}); diff --git a/cloud/apps/worker/src/jobs/purgeArtifacts.ts b/cloud/apps/worker/src/jobs/purgeArtifacts.ts new file mode 100644 index 00000000..49b58f37 --- /dev/null +++ b/cloud/apps/worker/src/jobs/purgeArtifacts.ts @@ -0,0 +1,109 @@ +import type { Job } from "bullmq"; +import { prisma } from "@ghost/core/db"; +import { appendAuditEvent } from "@ghost/core/audit-log"; +import type { PurgeArtifactsJob } from "@ghost/core/queue"; +import { createLogger, serializeError } from "@ghost/core/logger"; +import { captureException } from "@ghost/core/sentry"; +import { + artifactRetentionDaysFromEnv, + isEligibleForArtifactPurge, + retentionCutoff, +} from "@ghost/core/retention"; +import { artifactStore } from "../storage/artifacts.js"; + +const log = createLogger("worker", { job: "purge-artifacts" }); + +/** + * Deletes the artifact prefix (screenshots + any residual session blobs) for + * runs old enough that they are past the retention window, so a deployment's + * object store does not grow forever. Scheduled as a repeatable BullMQ job + * (see index.ts's `upsertJobScheduler` call) rather than enqueued per-run. + * + * Deliberately not part of the run's own lifecycle (unlike the session-blob + * cleanup in runWorkflow.ts, which fires the moment a run stops needing to + * resume): screenshots are the evidence a human reviews at an approval gate + * and in the run timeline, and this job is what eventually lets that evidence + * go, on a schedule an operator controls via `ARTIFACT_RETENTION_DAYS` — not + * the moment the run itself ends. + * + * A run whose deletePrefix call fails (a storage outage, wrong bucket + * permissions) is left unmarked and picked up again on the next run of this + * job — see the catch below. `Run.artifactsPurgedAt` is only ever set after + * the delete has actually succeeded. + */ + +/** Bounds how many runs one job invocation processes, so a large backlog + * (e.g. the first run of this job against an old deployment that never had + * one) does not hold the job's BullMQ lock open indefinitely. The scheduler + * re-fires on its own schedule, so the backlog drains over several ticks. */ +const BATCH_SIZE = 200; + +export interface PurgeArtifactsResult { + purged: number; + failed: number; +} + +export async function purgeArtifactsJob( + _job: Job, +): Promise { + const retentionDays = artifactRetentionDaysFromEnv(); + const cutoff = retentionCutoff(new Date(), retentionDays); + + let purged = 0; + let failed = 0; + + // Re-query each iteration rather than paginating: every run this loop + // successfully processes gets `artifactsPurgedAt` set, which removes it + // from the next query's result set. A run that fails is left unmarked and + // would be re-selected forever within one job invocation, so it is + // excluded explicitly via `id: { notIn }` for the rest of this run only. + const failedIds = new Set(); + for (;;) { + const candidates = await prisma.run.findMany({ + where: { + artifactsPurgedAt: null, + endedAt: { not: null, lt: cutoff }, + ...(failedIds.size > 0 ? { id: { notIn: [...failedIds] } } : {}), + }, + select: { id: true, orgId: true, endedAt: true, artifactsPurgedAt: true }, + take: BATCH_SIZE, + }); + if (candidates.length === 0) break; + + for (const run of candidates) { + if (!isEligibleForArtifactPurge(run, cutoff)) continue; + try { + await artifactStore().deletePrefix(`runs/${run.id}`); + await prisma.run.update({ + where: { id: run.id }, + data: { artifactsPurgedAt: new Date() }, + }); + await appendAuditEvent(run.orgId, null, { + action: "run.artifacts_purged", + entityType: "Run", + entityId: run.id, + metadata: { retentionDays, endedAt: run.endedAt?.toISOString() ?? null }, + }); + purged += 1; + } catch (err) { + failed += 1; + failedIds.add(run.id); + log.error("failed to purge run artifacts", { runId: run.id, ...serializeError(err) }); + // Caught here rather than rethrown, so this job's own BullMQ + // "failed" handler (index.ts) never fires for a single bad run — the + // batch keeps going. That means this is the only place that can + // report it to Sentry at all. + captureException(err, { runId: run.id, job: "purge-artifacts" }); + } + } + + // Every candidate in this batch either got purged or was added to + // failedIds; if none were purged, looping again would just refetch the + // same failed set forever. + if (candidates.every((r) => failedIds.has(r.id))) break; + } + + log.info("purge cycle complete", { purged, failed, retentionDays }); + + return { purged, failed }; +} diff --git a/cloud/apps/worker/tsup.config.ts b/cloud/apps/worker/tsup.config.ts index ffc83301..f6fc5d35 100644 --- a/cloud/apps/worker/tsup.config.ts +++ b/cloud/apps/worker/tsup.config.ts @@ -13,6 +13,10 @@ export default defineConfig({ // esbuild rewrite that into a `require` shim that throws "Dynamic require of // 'fs' is not supported" the instant the worker boots. Keep it (and the // generated `.prisma/client`) external so Node loads it normally from - // node_modules instead. - external: ["@prisma/client", ".prisma/client"], + // node_modules instead. `@sentry/node` does its own runtime instrumentation + // (dynamic requires for auto-instrumenting Node built-ins, optional native + // profiling modules) for the same reason — Sentry's own docs say not to + // bundle it, so it gets the same treatment rather than waiting to discover + // the same crash a second time. + external: ["@prisma/client", ".prisma/client", "@sentry/node"], }); diff --git a/cloud/docs/CURSOR_HANDOFF.md b/cloud/docs/CURSOR_HANDOFF.md index bd859e14..9959c66b 100644 --- a/cloud/docs/CURSOR_HANDOFF.md +++ b/cloud/docs/CURSOR_HANDOFF.md @@ -305,7 +305,7 @@ Redis running** — CI catches it before merge either way, but a local loop is faster than waiting on a run: ```bash -cd cloud && pnpm typecheck && pnpm test && pnpm build # expect 398 tests +cd cloud && pnpm typecheck && pnpm test && pnpm build # expect 424 tests ``` Without a database roughly 90 of those skip themselves and you learn nothing diff --git a/cloud/docs/DEPLOY.md b/cloud/docs/DEPLOY.md index 20435ea4..1227597a 100644 --- a/cloud/docs/DEPLOY.md +++ b/cloud/docs/DEPLOY.md @@ -52,6 +52,8 @@ B2, or MinIO. | `GHOST_SESSION_KEY` | | ● | gated runs cannot resume after approval | | `APP_URL` | ● | ● | the demo fixture is unreachable from the worker | | `NEXT_PUBLIC_SOURCE_URL` | ○ | | AGPL §13 link points at upstream, not your fork | +| `ARTIFACT_RETENTION_DAYS` | | ○ | defaults to 90; the `purge-artifacts` job (scheduled by the worker itself, see `apps/worker/src/index.ts`) deletes a run's screenshots once it ended this many days ago | +| `SENTRY_DSN` | | ○ | **worker only.** Error tracking stays off without it, matching `HR_API_KEY`/`S3_BUCKET` (see `@ghost/core/sentry`). `apps/web` is not wired to this — `@sentry/node`'s auto-instrumentation cannot be webpack-bundled (tried, broke `pnpm build`); wiring web needs `@sentry/nextjs` via `npx @sentry/wizard@latest -i nextjs` against a real Sentry project | `GHOST_SESSION_KEY` is **worker-only** by design. It decrypts captured browser sessions — live cookies for the customer's systems — and the web app has no @@ -97,6 +99,89 @@ hostname, e.g. `app.ghost.muharafiq.com`. is wired correctly on both sides, and it is exactly what a disk fallback breaks. +## Provisioning checklist — accounts and dashboards only you can create + +Nothing above can be automated further: each step below needs a real account, +billing, or a browser session with your organization's credentials, so this +codebase can prepare the code but not take the step for you. Grouped by +provider rather than by step, since you likely do these once, in whatever +order your accounts allow. + +**Postgres + Redis** — any managed provider works; nothing here is +provider-specific. Note the connection strings for `DATABASE_URL` and +`REDIS_URL`. If the provider requires TLS, confirm the Prisma/ioredis +connection strings encode that (`?sslmode=require`, `rediss://`) — untested +here, since no deployment exists yet. + +**Object storage (S3-compatible)** — AWS S3, Cloudflare R2, or Backblaze B2 all +work (`S3_ENDPOINT` selects a non-AWS one). Create one bucket, one set of +credentials, and scope the credentials' policy to that bucket only rather than +reusing an account-wide key — a credential leaked from either process should +not be able to reach anything else in the account. A minimal AWS IAM policy: +```json +{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:GetObject", "s3:PutObject", "s3:DeleteObject", "s3:ListBucket"], + "Resource": ["arn:aws:s3:::your-bucket-name", "arn:aws:s3:::your-bucket-name/*"] + }] +} +``` +Set the same `S3_*` variables on **both** the Vercel project and the container +host — see "Object storage is not optional here" above for what happens if +only one side gets them. + +**GitHub OAuth app** (github.com → Settings → Developer settings → OAuth Apps +→ New OAuth App): +- Homepage URL: your web app's eventual domain (see the Vercel project below). +- Authorization callback URL: `https:///api/auth/callback/github` + — exact match, including the path; NextAuth rejects a mismatch rather than + redirecting somewhere unexpected. +- Note the Client ID and generate a Client Secret; these become + `AUTH_GITHUB_ID` / `AUTH_GITHUB_SECRET`. See "The sign-in trap" above for + what happens if you skip this. + +**Vercel project for `cloud/apps/web`** — this is "The domain trap" above made +concrete: +1. Create a **new, second** Vercel project. Do not add `cloud/apps/web` to + whatever project already serves the repo root's `public/` marketing site — + they cannot share one project or one `vercel.json`. +2. Set its **Root Directory** to `cloud/apps/web` in Project Settings. Vercel's + Turborepo/pnpm-workspace detection then infers the install command and + build filter on its own — no `vercel.json` is needed inside `cloud/apps/web` + for this (confirmed against Vercel's own monorepo docs; see the audit that + produced this checklist for why one was deliberately not added). +3. Give it its own hostname (e.g. `app.`, distinct from the + marketing site's). +4. Set every `web`-column environment variable from the table above on this + project (`DATABASE_URL`, `REDIS_URL`, `AUTH_SECRET`, `AUTH_GITHUB_ID`/ + `AUTH_GITHUB_SECRET`, the `S3_*` set, `APP_URL`). + +**Container host for the worker** (Fly.io, Railway, Render, or any host that +runs a Docker image — pick one; none of this repo's code prefers one over +another): +1. Point it at `apps/worker/Dockerfile`, built with `cloud/` as the build + context (see step 5 above — the workspace layout requires this). +2. Set every `worker`-column environment variable from the table above, + **plus** `GHOST_SESSION_KEY` (worker-only) and, optionally, + `ARTIFACT_RETENTION_DAYS`/`SENTRY_DSN`. +3. Most of these hosts expect a process that stays up and reads its port/health + check from an env var they inject — the worker has no HTTP server and needs + none (it is a queue consumer), so skip any "web service" health-check + requirement the host's UI assumes by default and configure it as a + background worker / long-running process instead. + +**Sentry (optional)** — create a project, generate a DSN, and set +`SENTRY_DSN` on the container host only; this alone gives the worker error +tracking, since `apps/worker` is already wired against `@ghost/core/sentry` +(see that file for why the wrapper doesn't try to also cover `apps/web`). +Wiring `apps/web` needs `@sentry/nextjs`, not this env var — run +`npx @sentry/wizard@latest -i nextjs` against the same Sentry project once the +Vercel project above exists, since the wizard needs a real project to +configure against and writes files (`instrumentation-client.ts`, +`sentry.server.config.ts`, a `next.config.ts` wrapper) this repo does not ship. + ## Rolling back Application rollbacks are ordinary redeploys. Migrations are not: `migrate @@ -160,11 +245,15 @@ only reject them. It is opt-in because organizations are created single-member o first sign-in, so switching it on in a one-person org leaves nobody able to approve and runs stop at their first gate. -**Approval is still not role-restricted.** `Membership.role` is stored and read -by nothing; any member can approve any run they did not start. That gap is -blocked on member management rather than on effort — there is no invite flow, so -every organization has exactly one person and a role check would enforce a -distinction the system cannot yet express. +**Approval is still not role-restricted.** Membership, invitations, and roles +(`OWNER`/`ADMIN`/`MEMBER`) are real and tested — `app/api/settings/members`, +`app/api/invitations`, and `packages/core/src/roles.ts`'s `isOrgAdmin` already +gate admin-only actions like revoking a colleague's credential. What is +missing is narrower: `POST /api/runs/[id]/approvals/[stepIndex]` checks org +membership and `requireSeparateApprover` (requester ≠ approver) but reads +`Membership.role` not at all, so any member — `MEMBER` included — can approve +a run they did not start. Fixing it is a role check at one call site, not a +membership model that needs building first. Agents genuinely cannot approve, and that is enforced: no POST handler on `/api/agent/approvals`, an explicit 403, and a forbidden-tools list. diff --git a/cloud/packages/core/package.json b/cloud/packages/core/package.json index a92a2b6b..045f1974 100644 --- a/cloud/packages/core/package.json +++ b/cloud/packages/core/package.json @@ -28,7 +28,10 @@ "./mfa": "./src/mfa.ts", "./crypto/mfa-secret": "./src/crypto/mfa-secret.ts", "./recording/trace": "./src/recording/trace.ts", - "./recording/compile": "./src/recording/compile.ts" + "./recording/compile": "./src/recording/compile.ts", + "./retention": "./src/retention.ts", + "./logger": "./src/logger.ts", + "./sentry": "./src/sentry.ts" }, "scripts": { "build": "prisma generate && tsc --noEmit", @@ -46,7 +49,8 @@ "@prisma/client": "^6.1.0", "zod": "^3.24.1", "@aws-sdk/client-s3": "3.1101.0", - "@aws-sdk/s3-request-presigner": "3.1101.0" + "@aws-sdk/s3-request-presigner": "3.1101.0", + "@sentry/node": "^10.69.0" }, "devDependencies": { "prisma": "^6.1.0", diff --git a/cloud/packages/core/prisma/migrations/20260805185851_run_artifacts_purged_at/migration.sql b/cloud/packages/core/prisma/migrations/20260805185851_run_artifacts_purged_at/migration.sql new file mode 100644 index 00000000..f76881db --- /dev/null +++ b/cloud/packages/core/prisma/migrations/20260805185851_run_artifacts_purged_at/migration.sql @@ -0,0 +1,5 @@ +-- AlterTable +ALTER TABLE "Run" ADD COLUMN "artifactsPurgedAt" TIMESTAMP(3); + +-- CreateIndex +CREATE INDEX "Run_endedAt_artifactsPurgedAt_idx" ON "Run"("endedAt", "artifactsPurgedAt"); diff --git a/cloud/packages/core/prisma/migrations/migration_lock.toml b/cloud/packages/core/prisma/migrations/migration_lock.toml index 2fe25d87..044d57cd 100644 --- a/cloud/packages/core/prisma/migrations/migration_lock.toml +++ b/cloud/packages/core/prisma/migrations/migration_lock.toml @@ -1 +1,3 @@ +# Please do not edit this file manually +# It should be added in your version-control system (e.g., Git) provider = "postgresql" diff --git a/cloud/packages/core/prisma/schema.prisma b/cloud/packages/core/prisma/schema.prisma index fa9b2042..ad0e72a7 100644 --- a/cloud/packages/core/prisma/schema.prisma +++ b/cloud/packages/core/prisma/schema.prisma @@ -414,6 +414,14 @@ model Run { // make an already-completed action reachable again. journalHead String? + // Set once the retention job has deleted this run's artifact prefix + // (screenshots + any residual session blobs) from the store. Null means + // either the run has not ended, or it has but is not old enough yet — see + // packages/core/src/retention.ts. Distinct from `endedAt` being null: a run + // can be long finished and still have this null while it waits out the + // retention window. + artifactsPurgedAt DateTime? + org Organization @relation(fields: [orgId], references: [id], onDelete: Cascade) workflowVersion WorkflowVersion @relation(fields: [workflowVersionId], references: [id], onDelete: Cascade) triggeredBy User? @relation("RunTriggeredBy", fields: [triggeredById], references: [id], onDelete: SetNull) @@ -427,6 +435,8 @@ model Run { // Admission counts a workflow's in-flight runs on every start and resume. @@index([workflowVersionId, status]) @@index([workflowVersionId, slotHeldAt]) + // The retention job's own query: ended runs it has not yet purged. + @@index([endedAt, artifactsPurgedAt]) } // --------------------------------------------------------------------------- diff --git a/cloud/packages/core/src/logger.test.ts b/cloud/packages/core/src/logger.test.ts new file mode 100644 index 00000000..ce52832b --- /dev/null +++ b/cloud/packages/core/src/logger.test.ts @@ -0,0 +1,70 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createLogger, serializeError } from "./logger.js"; + +describe("createLogger", () => { + let logSpy: ReturnType; + let errorSpy: ReturnType; + + beforeEach(() => { + logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + }); + + afterEach(() => { + logSpy.mockRestore(); + errorSpy.mockRestore(); + }); + + function lastLine(spy: ReturnType): Record { + const call = spy.mock.calls.at(-1); + if (!call) throw new Error("expected a call"); + return JSON.parse(call[0] as string); + } + + it("writes info and debug to stdout as a single JSON line", () => { + createLogger("worker").info("started", { queue: "run-workflow" }); + expect(logSpy).toHaveBeenCalledTimes(1); + expect(errorSpy).not.toHaveBeenCalled(); + const line = lastLine(logSpy); + expect(line).toMatchObject({ level: "info", service: "worker", msg: "started", queue: "run-workflow" }); + expect(typeof line.time).toBe("string"); + }); + + it("writes warn and error to stderr, not stdout", () => { + createLogger("worker").error("job failed", { jobId: "1" }); + expect(logSpy).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(lastLine(errorSpy)).toMatchObject({ level: "error", msg: "job failed", jobId: "1" }); + }); + + it("child() merges bound fields without mutating the parent", () => { + const base = createLogger("worker"); + const child = base.child({ runId: "r1" }); + + child.info("halted"); + expect(lastLine(logSpy)).toMatchObject({ runId: "r1", msg: "halted" }); + + base.info("unrelated"); + expect(lastLine(logSpy)).not.toHaveProperty("runId"); + }); + + it("per-call fields win over bound fields with the same key", () => { + createLogger("worker", { runId: "bound" }).info("msg", { runId: "override" }); + expect(lastLine(logSpy).runId).toBe("override"); + }); +}); + +describe("serializeError", () => { + it("pulls name/message/stack off an Error, which spreading alone would drop", () => { + const err = new TypeError("bad input"); + const fields = serializeError(err); + expect(fields.errorName).toBe("TypeError"); + expect(fields.errorMessage).toBe("bad input"); + expect(typeof fields.stack).toBe("string"); + }); + + it("wraps a non-Error thrown value rather than losing it", () => { + expect(serializeError("plain string")).toEqual({ error: "plain string" }); + expect(serializeError({ code: 42 })).toEqual({ error: { code: 42 } }); + }); +}); diff --git a/cloud/packages/core/src/logger.ts b/cloud/packages/core/src/logger.ts new file mode 100644 index 00000000..cc238563 --- /dev/null +++ b/cloud/packages/core/src/logger.ts @@ -0,0 +1,82 @@ +/** + * A minimal structured logger — one JSON object per line, to stdout (info/ + * debug) or stderr (warn/error). + * + * The worker runs as a bare container process with no built-in request + * logging the way `apps/web` gets from Vercel; whatever it prints to stdout + * is whatever the host's log driver captures, and nothing before this parsed + * it as anything but text. A JSON line per event is the minimum a host log + * aggregator (Fly, Railway, Render, `docker logs`, CloudWatch) needs to filter + * and query by field instead of grepping strings — without adding a network + * dependency or an account this code cannot provision on its own. + * + * Deliberately not a wrapper around pino/winston: the worker's entire logging + * surface is a handful of call sites (see index.ts and jobs/purgeArtifacts.ts), + * and every field a log aggregator needs (level, time, a structured message, + * bound context) fits in about thirty lines with zero dependencies. Reach for + * a real logging library if that surface grows enough to need one. + */ + +export type LogLevel = "debug" | "info" | "warn" | "error"; + +export type LogFields = Record; + +export interface Logger { + debug(msg: string, fields?: LogFields): void; + info(msg: string, fields?: LogFields): void; + warn(msg: string, fields?: LogFields): void; + error(msg: string, fields?: LogFields): void; + /** Returns a logger that merges `fields` into every line it writes, without + * mutating this one. For binding context (a runId, a jobId) once at the top + * of a job rather than repeating it at every call site. */ + child(fields: LogFields): Logger; +} + +/** Turns an unknown caught value into log-safe fields. `Error.message`/`.stack` + * are own accessors, not enumerable own-properties, so spreading an Error + * directly into a log line silently drops them — this is the fix, not a + * style preference. */ +export function serializeError(err: unknown): LogFields { + if (err instanceof Error) { + return { errorName: err.name, errorMessage: err.message, stack: err.stack }; + } + return { error: err }; +} + +function write( + level: LogLevel, + service: string, + bound: LogFields, + msg: string, + fields: LogFields | undefined, +): void { + const line = JSON.stringify({ + time: new Date().toISOString(), + level, + service, + msg, + ...bound, + ...fields, + }); + // Split by stream, not just by convention: a host that separates stdout from + // stderr (most container platforms, `docker logs` with `2>`) can then filter + // on warn/error without parsing the JSON payload at all. + if (level === "warn" || level === "error") { + console.error(line); + } else { + console.log(line); + } +} + +/** Creates a logger. `service` identifies the process (e.g. "worker") in + * every line, so a shared log stream from multiple processes stays + * distinguishable. */ +export function createLogger(service: string, bound: LogFields = {}): Logger { + return { + debug: (msg, fields) => write("debug", service, bound, msg, fields), + info: (msg, fields) => write("info", service, bound, msg, fields), + warn: (msg, fields) => write("warn", service, bound, msg, fields), + error: (msg, fields) => write("error", service, bound, msg, fields), + child: (fields) => createLogger(service, { ...bound, ...fields }), + }; +} diff --git a/cloud/packages/core/src/queue.ts b/cloud/packages/core/src/queue.ts index 787dab7f..f6a60f7e 100644 --- a/cloud/packages/core/src/queue.ts +++ b/cloud/packages/core/src/queue.ts @@ -10,6 +10,10 @@ export const QUEUE_NAMES = { runWorkflow: "run-workflow", /** Reverses a run's completed side effects (BPMN compensation). */ compensateRun: "compensate-run", + /** Deletes artifact prefixes for runs past the retention window. Scheduled + * as a repeatable job (see apps/worker/src/index.ts); never enqueued by the + * web app. */ + purgeArtifacts: "purge-artifacts", /** Phase 0 wiring smoke test. */ noop: "noop", } as const; @@ -122,6 +126,12 @@ export interface NoopJob { requestedAt: string; } +/** Payload for a `purgeArtifacts` run. Empty: the job re-derives everything + * it needs (the retention window, which runs are eligible) at execution + * time rather than trusting a scheduled-at snapshot, since a purge cycle can + * be delayed behind other work. */ +export type PurgeArtifactsJob = Record; + export function redisConnectionFromEnv(): { url: string } { const url = process.env.REDIS_URL; if (!url) { diff --git a/cloud/packages/core/src/retention.test.ts b/cloud/packages/core/src/retention.test.ts new file mode 100644 index 00000000..15c0c683 --- /dev/null +++ b/cloud/packages/core/src/retention.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import { + DEFAULT_ARTIFACT_RETENTION_DAYS, + artifactRetentionDaysFromEnv, + isEligibleForArtifactPurge, + retentionCutoff, +} from "./retention.js"; + +describe("retentionCutoff", () => { + it("subtracts the retention window from now", () => { + const now = new Date("2026-08-05T00:00:00.000Z"); + expect(retentionCutoff(now, 90).toISOString()).toBe("2026-05-07T00:00:00.000Z"); + }); + + it("rejects a non-positive window rather than silently purging everything", () => { + const now = new Date(); + expect(() => retentionCutoff(now, 0)).toThrow(/positive/); + expect(() => retentionCutoff(now, -5)).toThrow(/positive/); + expect(() => retentionCutoff(now, NaN)).toThrow(/positive/); + }); +}); + +describe("isEligibleForArtifactPurge", () => { + const cutoff = new Date("2026-08-05T00:00:00.000Z"); + + it("is not eligible while the run has not ended", () => { + expect(isEligibleForArtifactPurge({ endedAt: null, artifactsPurgedAt: null }, cutoff)).toBe( + false, + ); + }); + + it("is not eligible once already purged, even if old enough", () => { + expect( + isEligibleForArtifactPurge( + { + endedAt: new Date("2026-01-01T00:00:00.000Z"), + artifactsPurgedAt: new Date("2026-08-01T00:00:00.000Z"), + }, + cutoff, + ), + ).toBe(false); + }); + + it("is not eligible when it ended after the cutoff", () => { + expect( + isEligibleForArtifactPurge( + { endedAt: new Date("2026-08-04T23:59:59.999Z"), artifactsPurgedAt: null }, + cutoff, + ), + ).toBe(true); + expect( + isEligibleForArtifactPurge( + { endedAt: new Date("2026-08-06T00:00:00.000Z"), artifactsPurgedAt: null }, + cutoff, + ), + ).toBe(false); + }); + + it("is eligible exactly at the boundary only when strictly before it", () => { + expect( + isEligibleForArtifactPurge({ endedAt: cutoff, artifactsPurgedAt: null }, cutoff), + ).toBe(false); + }); + + it("is eligible for a run that ended well before the cutoff and was never purged", () => { + expect( + isEligibleForArtifactPurge( + { endedAt: new Date("2020-01-01T00:00:00.000Z"), artifactsPurgedAt: null }, + cutoff, + ), + ).toBe(true); + }); +}); + +describe("artifactRetentionDaysFromEnv", () => { + it("defaults when unset", () => { + expect(artifactRetentionDaysFromEnv({})).toBe(DEFAULT_ARTIFACT_RETENTION_DAYS); + }); + + it("parses a configured value", () => { + expect(artifactRetentionDaysFromEnv({ ARTIFACT_RETENTION_DAYS: "30" })).toBe(30); + }); + + it("rejects a non-positive or non-numeric override rather than defaulting silently", () => { + expect(() => artifactRetentionDaysFromEnv({ ARTIFACT_RETENTION_DAYS: "0" })).toThrow( + /positive/, + ); + expect(() => artifactRetentionDaysFromEnv({ ARTIFACT_RETENTION_DAYS: "-1" })).toThrow( + /positive/, + ); + expect(() => artifactRetentionDaysFromEnv({ ARTIFACT_RETENTION_DAYS: "soon" })).toThrow( + /positive/, + ); + }); +}); diff --git a/cloud/packages/core/src/retention.ts b/cloud/packages/core/src/retention.ts new file mode 100644 index 00000000..b82b4784 --- /dev/null +++ b/cloud/packages/core/src/retention.ts @@ -0,0 +1,60 @@ +/** + * Pure eligibility rules for the artifact retention job (`purge-artifacts` + * queue, apps/worker/src/jobs/purgeArtifacts.ts). + * + * Nothing here touches storage or the database — it only decides, given a + * run's own timestamps, whether it is old enough to purge. Keeping the + * decision pure and separate from the Prisma query means the actual retention + * *window* logic can be tested without a database. + */ + +/** Default when `ARTIFACT_RETENTION_DAYS` is unset. Generous on purpose: this + * deletes evidence permanently, and a deployment that never configured a + * retention window should not silently lose everything after the shortest + * value the code happens to default to. */ +export const DEFAULT_ARTIFACT_RETENTION_DAYS = 90; + +export interface RetentionEligible { + /** Null while a run is still active in any sense — QUEUED, RUNNING, + * AWAITING_APPROVAL, COMPENSATING, or an unresolved INCIDENT never sets + * this (see apps/worker/src/jobs/{runWorkflow,compensateRun}.ts). Using + * `endedAt` rather than `status` means the eligibility rule does not need + * its own copy of which statuses count as "done" — it inherits whatever + * the engine already decided by setting or clearing this field. */ + endedAt: Date | null; + /** Non-null once a prior purge cycle already deleted this run's artifact + * prefix. Excluding these makes the job idempotent: re-running it (or + * running it more than once a day) does not re-attempt a deleted prefix or + * write a duplicate audit event. */ + artifactsPurgedAt: Date | null; +} + +/** The cutoff instant: runs that ended before this are eligible. */ +export function retentionCutoff(now: Date, retentionDays: number): Date { + if (!Number.isFinite(retentionDays) || retentionDays <= 0) { + throw new Error(`retentionDays must be a positive number, got ${retentionDays}`); + } + return new Date(now.getTime() - retentionDays * 24 * 60 * 60 * 1000); +} + +/** Whether a single run's artifacts are due for deletion. */ +export function isEligibleForArtifactPurge(run: RetentionEligible, cutoff: Date): boolean { + if (run.artifactsPurgedAt !== null) return false; + if (run.endedAt === null) return false; + return run.endedAt.getTime() < cutoff.getTime(); +} + +/** Reads `ARTIFACT_RETENTION_DAYS` from the environment, falling back to the + * default. Centralized so the job and anything documenting the value (e.g. a + * future settings UI) read it the same way. */ +export function artifactRetentionDaysFromEnv( + env: Record = process.env, +): number { + const raw = env.ARTIFACT_RETENTION_DAYS; + if (!raw) return DEFAULT_ARTIFACT_RETENTION_DAYS; + const parsed = Number(raw); + if (!Number.isFinite(parsed) || parsed <= 0) { + throw new Error(`ARTIFACT_RETENTION_DAYS must be a positive number, got ${JSON.stringify(raw)}`); + } + return parsed; +} diff --git a/cloud/packages/core/src/sentry.test.ts b/cloud/packages/core/src/sentry.test.ts new file mode 100644 index 00000000..8cfff827 --- /dev/null +++ b/cloud/packages/core/src/sentry.test.ts @@ -0,0 +1,72 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const sentryMock = vi.hoisted(() => ({ + init: vi.fn(), + captureException: vi.fn(), +})); +vi.mock("@sentry/node", () => sentryMock); + +const { initSentry, isSentryEnabled, captureException, resetSentryForTests } = await import( + "./sentry.js" +); + +describe("initSentry", () => { + const originalDsn = process.env.SENTRY_DSN; + + beforeEach(() => { + resetSentryForTests(); + sentryMock.init.mockClear(); + sentryMock.captureException.mockClear(); + }); + + afterEach(() => { + if (originalDsn === undefined) delete process.env.SENTRY_DSN; + else process.env.SENTRY_DSN = originalDsn; + }); + + it("is a no-op when SENTRY_DSN is unset — the expected state with no account configured", () => { + delete process.env.SENTRY_DSN; + initSentry("worker"); + expect(sentryMock.init).not.toHaveBeenCalled(); + expect(isSentryEnabled()).toBe(false); + }); + + it("initializes with the DSN and a service tag when configured", () => { + process.env.SENTRY_DSN = "https://example@o0.ingest.sentry.io/1"; + initSentry("worker"); + expect(sentryMock.init).toHaveBeenCalledTimes(1); + const [options] = sentryMock.init.mock.calls[0] as [Record]; + expect(options.dsn).toBe("https://example@o0.ingest.sentry.io/1"); + expect(options.initialScope).toEqual({ tags: { service: "worker" } }); + expect(isSentryEnabled()).toBe(true); + }); + + it("is idempotent — a second call does not re-initialize", () => { + process.env.SENTRY_DSN = "https://example@o0.ingest.sentry.io/1"; + initSentry("worker"); + initSentry("worker"); + expect(sentryMock.init).toHaveBeenCalledTimes(1); + }); +}); + +describe("captureException", () => { + beforeEach(() => { + resetSentryForTests(); + sentryMock.init.mockClear(); + sentryMock.captureException.mockClear(); + }); + + it("no-ops when Sentry was never initialized, rather than throwing", () => { + expect(() => captureException(new Error("boom"))).not.toThrow(); + expect(sentryMock.captureException).not.toHaveBeenCalled(); + }); + + it("forwards to Sentry with context as `extra` once initialized", () => { + process.env.SENTRY_DSN = "https://example@o0.ingest.sentry.io/1"; + initSentry("worker"); + const err = new Error("boom"); + captureException(err, { runId: "r1" }); + expect(sentryMock.captureException).toHaveBeenCalledWith(err, { extra: { runId: "r1" } }); + delete process.env.SENTRY_DSN; + }); +}); diff --git a/cloud/packages/core/src/sentry.ts b/cloud/packages/core/src/sentry.ts new file mode 100644 index 00000000..e313d958 --- /dev/null +++ b/cloud/packages/core/src/sentry.ts @@ -0,0 +1,69 @@ +import * as Sentry from "@sentry/node"; + +/** + * Thin, opt-in wrapper around `@sentry/node` for `apps/worker`. + * + * `SENTRY_DSN` absent means fully disabled: no network calls, no captured + * data, nothing shipped anywhere. This mirrors every other optional + * integration in this codebase (`HR_API_KEY`, `S3_BUCKET`) — presence enables + * a capability and nothing else changes when it is unset. A deployment is not + * required to have a Sentry account to boot or run correctly. + * + * `apps/web` is NOT wired to this module. `@sentry/node`'s auto-instrumentation + * (`import-in-the-middle`, `@sentry/node-core`) needs `node:child_process` and + * `require('module')` at module-load time, which Next.js's webpack build + * cannot bundle even with the package marked `serverExternalPackages` — this + * was tried and broke `pnpm build` outright (`UnhandledSchemeError` on + * `node:child_process` reached from `instrumentation.ts`). The worker has no + * such constraint: `apps/worker/tsup.config.ts` marks `@sentry/node` fully + * external, so Node resolves it normally from `node_modules` at runtime + * instead of webpack trying to statically bundle it. Wiring Sentry into + * `apps/web` needs `@sentry/nextjs` (its own webpack/turbopack plugin exists + * specifically to handle this), set up via `npx @sentry/wizard@latest -i + * nextjs` against a real Sentry project — see the deploy checklist. + */ + +let initialized = false; + +export function initSentry(service: "worker"): void { + const dsn = process.env.SENTRY_DSN; + if (!dsn) return; + if (initialized) return; + Sentry.init({ + dsn, + environment: process.env.NODE_ENV, + // Tagged even though only one service calls this today, so an event is + // never ambiguous about its source if `apps/web` later reports to the + // same DSN through its own `@sentry/nextjs` setup. + initialScope: { tags: { service } }, + // This wrapper turns on error capture, not performance tracing. Tracing + // is a separate, higher-volume Sentry feature with its own cost profile; + // an operator who wants it can enable it directly against the DSN this + // reads, without this wrapper deciding a sample rate for them. + tracesSampleRate: 0, + }); + initialized = true; +} + +/** Whether `initSentry` actually turned it on (`SENTRY_DSN` was set). Lets + * call sites skip building context objects for a capture that will be + * silently dropped anyway. */ +export function isSentryEnabled(): boolean { + return initialized; +} + +/** No-ops until `initSentry` has run with a DSN configured. `context` lands + * as Sentry's `extra` data — arbitrary, non-indexed fields for a human + * reading the event, not for querying/alerting on. */ +export function captureException(err: unknown, context?: Record): void { + if (!initialized) return; + Sentry.captureException(err, context ? { extra: context } : undefined); +} + +/** Test-only seam: clears the initialized flag so a test can exercise + * `initSentry` more than once. Mirrors `resetArtifactStore` in + * storage/artifacts.ts for the same reason — module-level state needs a way + * back to its initial value between test cases. */ +export function resetSentryForTests(): void { + initialized = false; +} diff --git a/cloud/pnpm-lock.yaml b/cloud/pnpm-lock.yaml index 28a4b2af..20f9df04 100644 --- a/cloud/pnpm-lock.yaml +++ b/cloud/pnpm-lock.yaml @@ -62,10 +62,10 @@ importers: version: 0.469.0(react@19.2.8) next: specifier: ^15.1.3 - version: 15.5.22(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + version: 15.5.22(@opentelemetry/api@1.9.1)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) next-auth: specifier: ^5.0.0-beta.25 - version: 5.0.0-beta.32(next@15.5.22(react-dom@19.2.8(react@19.2.8))(react@19.2.8))(react@19.2.8) + version: 5.0.0-beta.32(next@15.5.22(@opentelemetry/api@1.9.1)(react-dom@19.2.8(react@19.2.8))(react@19.2.8))(react@19.2.8) react: specifier: ^19.0.0 version: 19.2.8 @@ -118,6 +118,9 @@ importers: '@prisma/client': specifier: ^6.1.0 version: 6.19.3(prisma@6.19.3(typescript@5.9.3))(typescript@5.9.3) + '@sentry/node': + specifier: ^10.69.0 + version: 10.69.0(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1)) bullmq: specifier: ^5.34.4 version: 5.81.2 @@ -155,6 +158,9 @@ importers: '@prisma/client': specifier: ^6.1.0 version: 6.19.3(prisma@6.19.3(typescript@5.9.3))(typescript@5.9.3) + '@sentry/node': + specifier: ^10.69.0 + version: 10.69.0(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1)) zod: specifier: ^3.24.1 version: 3.25.76 @@ -175,6 +181,17 @@ packages: resolution: {integrity: sha512-UrcABB+4bUrFABwbluTIBErXwvbsU/V7TZWfmbgJfbkwiBuziS9gxdODUyuiecfdGQ85jglMW6juS3+z5TsKLw==} engines: {node: '>=10'} + '@apm-js-collab/code-transformer-bundler-plugins@0.7.4': + resolution: {integrity: sha512-nAfOeZPSUAQvJa1iFT/5oCrTm5YQhMMrfCNthNnaXHZiOQhu1KGuLoIx7HtbAi3wfwaBYLaICPIeenIaEwcXIg==} + engines: {node: '>=18.0.0'} + + '@apm-js-collab/code-transformer@0.18.1': + resolution: {integrity: sha512-u1Hb6bHjWtkSpiprwVP6YaHC1DTN4RAU3zYkUDUe7WMnJwdyU1pwTL9dFKiSJB9IiLue/EQovmyx6xhU7FFtAQ==} + hasBin: true + + '@apm-js-collab/tracing-hooks@0.13.0': + resolution: {integrity: sha512-mTvWz9rnQwx1U3h0XPTHaX7bgfkpipLLTQyjlC2cdhQpQEuoLT0AGzoydeoq2NxfEVv6fWOOETcSbb2nptleyw==} + '@auth/core@0.41.3': resolution: {integrity: sha512-sJ3JMHHkXMD3aOjopv7mOBTO1Ocw4b0fAEXJBz6k7YHLpYQI6C40jCUPc5fNvUKxXRXNE1/sRISA15UrwWJBTw==} peerDependencies: @@ -1068,6 +1085,48 @@ packages: resolution: {integrity: sha512-nn5ozdjYQpUCZlWGuxcJY/KpxkWQs4DcbMCmKojjyrYDEAGy4Ce19NN4v5MduafTwJlbKc99UA8YhSVqq9yPZA==} engines: {node: '>=12.4.0'} + '@opentelemetry/api-logs@0.220.0': + resolution: {integrity: sha512-CmVa4ImJ+ynfrPMNaAXHET6Bhb44SwzmfyVJFq9ni2jgXJR/l7C6gfVFddNmHP+ZOkP9cf4f9DBe68qVLTHc9w==} + engines: {node: '>=8.0.0'} + + '@opentelemetry/api@1.9.1': + resolution: {integrity: sha512-gLyJlPHPZYdAk1JENA9LeHejZe1Ti77/pTeFm/nMXmQH/HFZlcS/O2XJB+L8fkbrNSqhdtlvjBVjxwUYanNH5Q==} + engines: {node: '>=8.0.0'} + + '@opentelemetry/core@2.10.0': + resolution: {integrity: sha512-/wNZ8twnEQQA4HoHu22+vcsdru6pWPWxW+7w+FlxT6Id7PE/WIbZmVKkte+PF72e0F2dnImFeHD2syyE1Mw6MQ==} + engines: {node: ^18.19.0 || >=20.6.0} + peerDependencies: + '@opentelemetry/api': '>=1.0.0 <1.10.0' + + '@opentelemetry/instrumentation@0.220.0': + resolution: {integrity: sha512-xQx3E2WxP1mDvKzxLxX+CTCtNLa560YJZ3087qYHerl2YmiKpv7AH+dAy7vmx+eVrZ5BwhfWUAVoKOoxCNHcpw==} + engines: {node: ^18.19.0 || >=20.6.0} + peerDependencies: + '@opentelemetry/api': ^1.3.0 + + '@opentelemetry/resources@2.10.0': + resolution: {integrity: sha512-q6MMm2zhggzsHVNbabYwut+a6nbuQQe3URUoxaojM/8K1IBfwwPzvxIjNi2/lI1TFe+fMHMW9MWhrtDLEXEnkA==} + engines: {node: ^18.19.0 || >=20.6.0} + peerDependencies: + '@opentelemetry/api': '>=1.3.0 <1.10.0' + + '@opentelemetry/sdk-trace-base@2.10.0': + resolution: {integrity: sha512-GuYQQT7QD2EeO8lcZLRQzcbOyhqAzL+6WWTKTU9mSUBYBazkEDl+VrQcXQhbB08OWM9anD1aHleVadzulpOaUQ==} + engines: {node: ^18.19.0 || >=20.6.0} + peerDependencies: + '@opentelemetry/api': '>=1.3.0 <1.10.0' + + '@opentelemetry/sdk-trace@2.10.0': + resolution: {integrity: sha512-MfQGq3GRmTh5fM/y+OjaO0vj6+luCB1XO2gfXCalKCfgKw0eHL++sm75DNweC6ohlp+aFvACqeE0fYayqdRaoQ==} + engines: {node: ^18.19.0 || >=20.6.0} + peerDependencies: + '@opentelemetry/api': '>=1.3.0 <1.10.0' + + '@opentelemetry/semantic-conventions@1.43.0': + resolution: {integrity: sha512-eSYWTm620tTk45EKSedaUL8MFYI8hW164hIXsgIHyxu3VobUB3fFCu5t0hQby6OoWRPsG1KkKUG2M5UadiLiVg==} + engines: {node: '>=14'} + '@panva/hkdf@1.2.1': resolution: {integrity: sha512-6oclG6Y3PiDFcoyk8srjLfVKyMfVCKJ27JwNPViuXziFpmdz+MZnZN/aKY0JGXgYuO/VghU0jcOAZgWXZ1Dmrw==} @@ -1245,6 +1304,51 @@ packages: '@rushstack/eslint-patch@1.16.1': resolution: {integrity: sha512-TvZbIpeKqGQQ7X0zSCvPH9riMSFQFSggnfBjFZ1mEoILW+UuXCKwOoPcgjMwiUtRqFZ8jWhPJc4um14vC6I4ag==} + '@sentry/conventions@0.16.0': + resolution: {integrity: sha512-fO9PLmHdVURcSPUpWCItWAtgKiMwGdJHbovoSEyLplX5sxs2ugvI4CBPTrkkgqhObnZOD0CnWBKDzSVQYBKEyQ==} + engines: {node: '>=14'} + + '@sentry/core@10.69.0': + resolution: {integrity: sha512-+uuqVEeiDzYuAKjZLqsROKXvRTbl/QeH0gfGRtpYib1cud4rAFWRIkFmcR7Jb7JGFYwmReyQotiTj/hcDszTZg==} + engines: {node: '>=18'} + + '@sentry/node-core@10.69.0': + resolution: {integrity: sha512-IgArHczrZJxkgxoffHscj0NxQrG6kCazgmGQnlf3j58J1ec21YaUu8Tu+7G4Lo5tCiW3teQnwlKW1ttMXSqWRw==} + engines: {node: '>=18'} + peerDependencies: + '@opentelemetry/api': ^1.9.0 + '@opentelemetry/core': ^1.30.1 || ^2.1.0 + '@opentelemetry/exporter-trace-otlp-http': '>=0.57.0 <1' + '@opentelemetry/instrumentation': '>=0.57.1 <1' + '@opentelemetry/sdk-trace-base': ^1.30.1 || ^2.1.0 + peerDependenciesMeta: + '@opentelemetry/api': + optional: true + '@opentelemetry/core': + optional: true + '@opentelemetry/exporter-trace-otlp-http': + optional: true + '@opentelemetry/instrumentation': + optional: true + '@opentelemetry/sdk-trace-base': + optional: true + + '@sentry/node@10.69.0': + resolution: {integrity: sha512-xEXA1YGIiTZbrW6MWV34uS6JGQuQg2ijTI0zed+FsJb9JZKPYel/GZK8Km26vfTVb+yCXFmWZNBesKegNcVdzg==} + engines: {node: '>=18'} + + '@sentry/opentelemetry@10.69.0': + resolution: {integrity: sha512-3FyWV6YcEJuvLrlaKGE1dHXCI+1YO0a62w7PkwlRg8yp6K6YXkmdwu9GjqaYD+Ju4tm7uC7mHIsGFQMm0M7pqQ==} + engines: {node: '>=18'} + peerDependencies: + '@opentelemetry/api': ^1.9.0 + '@opentelemetry/core': ^1.30.1 || ^2.1.0 + '@opentelemetry/sdk-trace-base': ^1.30.1 || ^2.1.0 + + '@sentry/server-utils@10.69.0': + resolution: {integrity: sha512-0MwHrA8+nNvMIsqf8m3cXwCBlUjr6AS7N6CZvHJtY1DkqEvQqEbD5VIrhzEyHN/KMZIgQ8XeDCQRhjnXFQGRhg==} + engines: {node: '>=18'} + '@smithy/core@3.31.1': resolution: {integrity: sha512-CyogUINxvi7C7LDsh8Syo6hVJOT9ckz4rG8dRZfTJ8r91HkMY59PnNooaj7WcHyxEkxPfBAmbgztZU+xTo76lg==} engines: {node: '>=18.0.0'} @@ -1694,6 +1798,10 @@ packages: ast-types-flow@0.0.8: resolution: {integrity: sha512-OH/2E5Fg20h2aPrbe+QL8JZQFko0YZaF+j4mnQ7BGhfavO7OpSLa8a0y9sBwomHdSbkhTS8TQNayBfnW5DwbvQ==} + astring@1.9.0: + resolution: {integrity: sha512-LElXdjswlqjWrPpJFg1Fx4wpkOCxj1TDHlSV4PlaRxHGWko024xICaa97ZkMfs6DRKlCguiAI+rbXv5GWwXIkg==} + hasBin: true + async-function@1.0.0: resolution: {integrity: sha512-hsU18Ae8CDTR6Kgu9DYf0EbCr/a5iGL0rytQDobUcdpYOKokk8LEjVphnXkDkgpi0wYVsqrXuP0bZxJaTqdgoA==} engines: {node: '>= 0.4'} @@ -1799,6 +1907,9 @@ packages: citty@0.2.2: resolution: {integrity: sha512-+6vJA3L98yv+IdfKGZHBNiGW5KHn22e/JwID0Strsz8h4S/csAu/OuICwxrg44k5MRiZHWIo8XXuJgQTriRP4w==} + cjs-module-lexer@2.2.0: + resolution: {integrity: sha512-4bHTS2YuzUvtoLjdy+98ykbNB5jS0+07EvFNXerqZQJ89F7DI6ET7OQo/HJuW6K0aVsKA9hj9/RVb2kQVOrPDQ==} + client-only@0.0.1: resolution: {integrity: sha512-IV3Ou0jSMzZrd3pZ48nLkT9DA7Ag1pnPzaiQhpW7c3RbcqqzvzzVu+L8gfqMp/8IM2MQtSiqaCxrrcfu8I8rMA==} @@ -1959,6 +2070,9 @@ packages: es-module-lexer@1.7.0: resolution: {integrity: sha512-jEQoCwk8hyb2AZziIOLhDqpm5+2ww5uIE6lkO/6jcOCusfk6LhMHpXXfBLXTZ7Ydyt0j4VoUQv6uGNYbdW+kBA==} + es-module-lexer@2.3.1: + resolution: {integrity: sha512-shc1dbU90Yl/xq1QrC7QRtfcwURZuVRfPhZbDoldJ1cn1gzDvBaBWlv0eFolj5+0znnPJz5TXLxsN77X/12KTA==} + es-object-atoms@1.1.2: resolution: {integrity: sha512-HWcBoN6NileqtSydK2FqHbS/LoDd2pqrnQHLyJzBj4kOp/ky2MWMN694xOfkK8/SnUsW2DH7EfyVlydKCsm1Zw==} engines: {node: '>= 0.4'} @@ -2284,6 +2398,10 @@ packages: resolution: {integrity: sha512-TR3KfrTZTYLPB6jUjfx6MF9WcWrHL9su5TObK4ZkYgBdWKPOFoSoQIdEuTuR82pmtxH2spWG9h6etwfr1pLBqQ==} engines: {node: '>=6'} + import-in-the-middle@3.3.3: + resolution: {integrity: sha512-AiohS3H80sXO6owEltjGX+glb7qXaDhBoJb9XcQVH4UI207xu/bDLUcadVKp7Qe576reg9yr/PXZjV5qx8gfbA==} + engines: {node: '>=18'} + imurmurhash@0.1.4: resolution: {integrity: sha512-JmXMZ6wuvDmLiHEml9ykzqO6lwFbof0GG4IkcGaENdCRDDmMVnny7s5HsIgHCbaq0w2MyPhDqkhTUgS2LU2PHA==} engines: {node: '>=0.8.19'} @@ -2581,6 +2699,10 @@ packages: resolution: {integrity: sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg==} engines: {node: '>= 8'} + meriyah@6.1.4: + resolution: {integrity: sha512-Sz8FzjzI0kN13GK/6MVEsVzMZEPvOhnmmI1lU5+/1cGOiK3QUahntrNNtdVeihrO7t9JpoH75iMNXg6R6uWflQ==} + engines: {node: '>=18.0.0'} + micromatch@4.0.8: resolution: {integrity: sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==} engines: {node: '>=8.6'} @@ -2598,6 +2720,9 @@ packages: mlly@1.8.2: resolution: {integrity: sha512-d+ObxMQFmbt10sretNDytwt85VrbkhhUA/JBGm1MPaWJ65Cl4wOgLaB1NYvJSZ0Ef03MMEU/0xpPMXUIQ29UfA==} + module-details-from-path@1.0.4: + resolution: {integrity: sha512-EGWKgxALGMgzvxYF1UyGTy0HXX/2vHLkw6+NvDKW2jypWbHpjQuj4UMcqQWXHERJhVGKikolT06G3bcKe4fi7w==} + ms@2.1.3: resolution: {integrity: sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==} @@ -2898,6 +3023,10 @@ packages: resolution: {integrity: sha512-dYqgNSZbDwkaJ2ceRd9ojCGjBq+mOm9LmtXnAnEGyHhN/5R7iDW2TRw3h+o/jCFxus3P2LfWIIiwowAjANm7IA==} engines: {node: '>= 0.4'} + require-in-the-middle@8.0.1: + resolution: {integrity: sha512-QT7FVMXfWOYFbeRBF6nu+I6tr2Tf3u0q8RIEjNob/heKY/nh7drD/k7eeMFmSQgnTtCzLDcCu/XEnpW2wk4xCQ==} + engines: {node: '>=9.3.0 || >=8.10.0 <9.0.0'} + resolve-from@4.0.0: resolution: {integrity: sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==} engines: {node: '>=4'} @@ -2941,6 +3070,9 @@ packages: scheduler@0.27.0: resolution: {integrity: sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q==} + semifies@1.0.0: + resolution: {integrity: sha512-xXR3KGeoxTNWPD4aBvL5NUpMTT7WMANr3EWnaS190QVkY52lqqcVRD7Q05UVbBhiWDGWMlJEUam9m7uFFGVScw==} + semver@6.3.1: resolution: {integrity: sha512-BR7VvDCVHO+q2xBEWskxS6DJE1qRnb7DxzUrogb71CWoSficBxYsiAGd+Kl0mmq/MprG9yArRkyrQxTO6XjMzA==} hasBin: true @@ -2997,6 +3129,10 @@ packages: resolution: {integrity: sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==} engines: {node: '>=0.10.0'} + source-map@0.6.1: + resolution: {integrity: sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==} + engines: {node: '>=0.10.0'} + source-map@0.7.6: resolution: {integrity: sha512-i5uvt8C3ikiWeNZSVZNWcfZPItFQOsYTUAOkcUPGd8DqDy1uOUikjt5dG+uRlwyvR108Fb9DOd4GvXfT0N2/uQ==} engines: {node: '>= 12'} @@ -3311,6 +3447,30 @@ snapshots: '@alloc/quick-lru@5.2.0': {} + '@apm-js-collab/code-transformer-bundler-plugins@0.7.4': + dependencies: + '@apm-js-collab/code-transformer': 0.18.1 + es-module-lexer: 2.3.1 + magic-string: 0.30.21 + module-details-from-path: 1.0.4 + + '@apm-js-collab/code-transformer@0.18.1': + dependencies: + '@types/estree': 1.0.9 + astring: 1.9.0 + esquery: 1.7.0 + meriyah: 6.1.4 + semifies: 1.0.0 + source-map: 0.6.1 + + '@apm-js-collab/tracing-hooks@0.13.0': + dependencies: + '@apm-js-collab/code-transformer': 0.18.1 + debug: 4.4.3 + module-details-from-path: 1.0.4 + transitivePeerDependencies: + - supports-color + '@auth/core@0.41.3': dependencies: '@panva/hkdf': 1.2.1 @@ -3988,6 +4148,49 @@ snapshots: '@nolyfill/is-core-module@1.0.39': {} + '@opentelemetry/api-logs@0.220.0': + dependencies: + '@opentelemetry/api': 1.9.1 + + '@opentelemetry/api@1.9.1': {} + + '@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1)': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/semantic-conventions': 1.43.0 + + '@opentelemetry/instrumentation@0.220.0(@opentelemetry/api@1.9.1)': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/api-logs': 0.220.0 + import-in-the-middle: 3.3.3 + require-in-the-middle: 8.0.1 + transitivePeerDependencies: + - supports-color + + '@opentelemetry/resources@2.10.0(@opentelemetry/api@1.9.1)': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/core': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/semantic-conventions': 1.43.0 + + '@opentelemetry/sdk-trace-base@2.10.0(@opentelemetry/api@1.9.1)': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/core': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/resources': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/sdk-trace': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/semantic-conventions': 1.43.0 + + '@opentelemetry/sdk-trace@2.10.0(@opentelemetry/api@1.9.1)': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/core': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/resources': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/semantic-conventions': 1.43.0 + + '@opentelemetry/semantic-conventions@1.43.0': {} + '@panva/hkdf@1.2.1': {} '@prisma/client@6.19.3(prisma@6.19.3(typescript@5.9.3))(typescript@5.9.3)': @@ -4104,6 +4307,58 @@ snapshots: '@rushstack/eslint-patch@1.16.1': {} + '@sentry/conventions@0.16.0': {} + + '@sentry/core@10.69.0': + dependencies: + '@sentry/conventions': 0.16.0 + + '@sentry/node-core@10.69.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1))(@opentelemetry/instrumentation@0.220.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.10.0(@opentelemetry/api@1.9.1))': + dependencies: + '@sentry/conventions': 0.16.0 + '@sentry/core': 10.69.0 + '@sentry/opentelemetry': 10.69.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.10.0(@opentelemetry/api@1.9.1)) + import-in-the-middle: 3.3.3 + optionalDependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/core': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/instrumentation': 0.220.0(@opentelemetry/api@1.9.1) + '@opentelemetry/sdk-trace-base': 2.10.0(@opentelemetry/api@1.9.1) + + '@sentry/node@10.69.0(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1))': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/instrumentation': 0.220.0(@opentelemetry/api@1.9.1) + '@opentelemetry/sdk-trace-base': 2.10.0(@opentelemetry/api@1.9.1) + '@sentry/conventions': 0.16.0 + '@sentry/core': 10.69.0 + '@sentry/node-core': 10.69.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1))(@opentelemetry/instrumentation@0.220.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.10.0(@opentelemetry/api@1.9.1)) + '@sentry/opentelemetry': 10.69.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.10.0(@opentelemetry/api@1.9.1)) + '@sentry/server-utils': 10.69.0 + import-in-the-middle: 3.3.3 + transitivePeerDependencies: + - '@opentelemetry/core' + - '@opentelemetry/exporter-trace-otlp-http' + - supports-color + + '@sentry/opentelemetry@10.69.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.10.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.10.0(@opentelemetry/api@1.9.1))': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/core': 2.10.0(@opentelemetry/api@1.9.1) + '@opentelemetry/sdk-trace-base': 2.10.0(@opentelemetry/api@1.9.1) + '@sentry/conventions': 0.16.0 + '@sentry/core': 10.69.0 + + '@sentry/server-utils@10.69.0': + dependencies: + '@apm-js-collab/code-transformer-bundler-plugins': 0.7.4 + '@apm-js-collab/tracing-hooks': 0.13.0 + '@sentry/conventions': 0.16.0 + '@sentry/core': 10.69.0 + meriyah: 6.1.4 + transitivePeerDependencies: + - supports-color + '@smithy/core@3.31.1': dependencies: '@smithy/types': 4.16.1 @@ -4548,6 +4803,8 @@ snapshots: ast-types-flow@0.0.8: {} + astring@1.9.0: {} + async-function@1.0.0: {} available-typed-arrays@1.0.7: @@ -4656,6 +4913,8 @@ snapshots: citty@0.2.2: {} + cjs-module-lexer@2.2.0: {} + client-only@0.0.1: {} clsx@2.1.1: {} @@ -4859,6 +5118,8 @@ snapshots: es-module-lexer@1.7.0: {} + es-module-lexer@2.3.1: {} + es-object-atoms@1.1.2: dependencies: es-errors: 1.3.0 @@ -5342,6 +5603,12 @@ snapshots: parent-module: 1.0.1 resolve-from: 4.0.0 + import-in-the-middle@3.3.3: + dependencies: + cjs-module-lexer: 2.2.0 + es-module-lexer: 2.3.1 + module-details-from-path: 1.0.4 + imurmurhash@0.1.4: {} internal-slot@1.1.0: @@ -5616,6 +5883,8 @@ snapshots: merge2@1.4.1: {} + meriyah@6.1.4: {} + micromatch@4.0.8: dependencies: braces: 3.0.3 @@ -5638,6 +5907,8 @@ snapshots: pkg-types: 1.3.1 ufo: 1.6.4 + module-details-from-path@1.0.4: {} + ms@2.1.3: {} msgpackr-extract@3.0.4: @@ -5668,13 +5939,13 @@ snapshots: natural-compare@1.4.0: {} - next-auth@5.0.0-beta.32(next@15.5.22(react-dom@19.2.8(react@19.2.8))(react@19.2.8))(react@19.2.8): + next-auth@5.0.0-beta.32(next@15.5.22(@opentelemetry/api@1.9.1)(react-dom@19.2.8(react@19.2.8))(react@19.2.8))(react@19.2.8): dependencies: '@auth/core': 0.41.3 - next: 15.5.22(react-dom@19.2.8(react@19.2.8))(react@19.2.8) + next: 15.5.22(@opentelemetry/api@1.9.1)(react-dom@19.2.8(react@19.2.8))(react@19.2.8) react: 19.2.8 - next@15.5.22(react-dom@19.2.8(react@19.2.8))(react@19.2.8): + next@15.5.22(@opentelemetry/api@1.9.1)(react-dom@19.2.8(react@19.2.8))(react@19.2.8): dependencies: '@next/env': 15.5.22 '@swc/helpers': 0.5.15 @@ -5692,6 +5963,7 @@ snapshots: '@next/swc-linux-x64-musl': 15.5.22 '@next/swc-win32-arm64-msvc': 15.5.22 '@next/swc-win32-x64-msvc': 15.5.22 + '@opentelemetry/api': 1.9.1 sharp: 0.34.5 transitivePeerDependencies: - '@babel/core' @@ -5930,6 +6202,13 @@ snapshots: gopd: 1.2.0 set-function-name: 2.0.2 + require-in-the-middle@8.0.1: + dependencies: + debug: 4.4.3 + module-details-from-path: 1.0.4 + transitivePeerDependencies: + - supports-color + resolve-from@4.0.0: {} resolve-from@5.0.0: {} @@ -6003,6 +6282,8 @@ snapshots: scheduler@0.27.0: {} + semifies@1.0.0: {} + semver@6.3.1: {} semver@7.8.5: {} @@ -6099,6 +6380,8 @@ snapshots: source-map-js@1.2.1: {} + source-map@0.6.1: {} + source-map@0.7.6: {} stable-hash@0.0.5: {} From 6e9ace419aaea2b6efb1510a6caec0a66e164d85 Mon Sep 17 00:00:00 2001 From: mohabbis <101276427+mohabbis@users.noreply.github.com> Date: Wed, 5 Aug 2026 15:46:14 -0500 Subject: [PATCH 6/7] fix(cloud): make cloud/apps/web actually deployable to Vercel Discovered by performing the repo's first real deployment (Vercel project ghost-app, Neon Postgres, Upstash Redis) rather than just writing a checklist for one: - cloud/apps/web needs its own vercel.json ({"framework": "nextjs"}). DEPLOY.md previously claimed this wasn't necessary based on reading Vercel's monorepo docs, but empirically: with the repo root's own vercel.json present (the marketing site's static-build config) and Root Directory set but no local vercel.json, a build falls back to the root's install/build commands against the wrong project's uploaded files -- Vercel prints "The vercel.json file should be inside of the provided root directory" as a warning, then uses it anyway. A local vercel.json closes the gap outright. - Added .vercelignore at the repo root. `vercel deploy` uploads the working directory as-is, not `git ls-files`, so untracked local dev artifacts (.worktrees/, .wt/, .turbo/ -- each a separate git-worktree checkout with its own node_modules/target) get swept into the upload. Without this, a deploy from the repo root uploaded 57,896 files instead of ~700. - Vercel's own Deployment Protection (SSO/Vercel Authentication) was on by default for the new project, gating every page behind a Vercel-account login wall on top of Ghost's own auth -- disabled for ghost-app. DEPLOY.md updated with all three findings, plus removing the stale "no deployment exists yet" framing now that apps/web is live (worker is not; no container host is wired up, and object storage was deliberately deferred). Co-Authored-By: Claude Sonnet 5 --- .gitignore | 1 + .vercelignore | 14 +++++++++++ cloud/apps/web/.gitignore | 2 ++ cloud/apps/web/vercel.json | 4 ++++ cloud/docs/DEPLOY.md | 49 ++++++++++++++++++++++++++++---------- 5 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 .vercelignore create mode 100644 cloud/apps/web/.gitignore create mode 100644 cloud/apps/web/vercel.json diff --git a/.gitignore b/.gitignore index 569838a3..78b04e9a 100644 --- a/.gitignore +++ b/.gitignore @@ -80,3 +80,4 @@ htmlcov/ *.tar.xz *.tar.zst dump.rdb +.vercel diff --git a/.vercelignore b/.vercelignore new file mode 100644 index 00000000..e7743a66 --- /dev/null +++ b/.vercelignore @@ -0,0 +1,14 @@ +# Vercel CLI deploys upload the working directory directly rather than via +# git ls-files, so untracked-but-present local dev artifacts need their own +# exclusion here even though most of what's inside them is already covered by +# .gitignore. Each of these is a separate git-worktree checkout (its own full +# node_modules/target/dist) sitting at the repo root on this machine — none of +# it is relevant to any Vercel-deployed project, and leaving them unlisted +# pushed a single-project deploy from tens of files to 57,896. +.worktrees/ +.wt/ +.turbo/ + +# Legacy desktop app build output; never relevant to a cloud/ deploy. +src-tauri/target/ +dist/ diff --git a/cloud/apps/web/.gitignore b/cloud/apps/web/.gitignore new file mode 100644 index 00000000..245259b6 --- /dev/null +++ b/cloud/apps/web/.gitignore @@ -0,0 +1,2 @@ +.vercel +.env* diff --git a/cloud/apps/web/vercel.json b/cloud/apps/web/vercel.json new file mode 100644 index 00000000..a667db8c --- /dev/null +++ b/cloud/apps/web/vercel.json @@ -0,0 +1,4 @@ +{ + "$schema": "https://openapi.vercel.sh/vercel.json", + "framework": "nextjs" +} diff --git a/cloud/docs/DEPLOY.md b/cloud/docs/DEPLOY.md index 1227597a..d367ecee 100644 --- a/cloud/docs/DEPLOY.md +++ b/cloud/docs/DEPLOY.md @@ -4,11 +4,16 @@ Ghost is four processes: a Next.js web app, a worker that drives a real browser, Postgres, and Redis. Plus object storage once web and worker are on separate hosts. -This document is a runbook, not a record — **no deployment of Ghost exists -yet**, and nothing here has been executed against a real host. It was written -alongside the code that makes it possible, and the container image has not been -built (Docker was unavailable in the environment that wrote it). Treat the first -deploy as the test of this file. +**Update:** `apps/web` has now actually been deployed — Vercel project +`ghost-app`, Neon Postgres, Upstash Redis, migrations applied, `/signin` +verified live. The bugs that first deploy found (a stale root `vercel.json` +silently winning over the app's own build config, the CLI uploading untracked +worktree directories, Vercel's own SSO wall blocking every page) are folded +into the steps below rather than left as a separate war story. **The worker +has not been deployed** — no container host is wired up yet, and object +storage was deliberately skipped (screenshots fall back to local disk, which +does not survive being served from a separate worker host). Treat both of +those as still-open parts of a first real deploy. ## Shape @@ -147,16 +152,36 @@ concrete: 1. Create a **new, second** Vercel project. Do not add `cloud/apps/web` to whatever project already serves the repo root's `public/` marketing site — they cannot share one project or one `vercel.json`. -2. Set its **Root Directory** to `cloud/apps/web` in Project Settings. Vercel's - Turborepo/pnpm-workspace detection then infers the install command and - build filter on its own — no `vercel.json` is needed inside `cloud/apps/web` - for this (confirmed against Vercel's own monorepo docs; see the audit that - produced this checklist for why one was deliberately not added). -3. Give it its own hostname (e.g. `app.`, distinct from the +2. Set its **Root Directory** to `cloud/apps/web` in Project Settings. +3. **`cloud/apps/web/vercel.json` must exist**, at minimum `{"framework": + "nextjs"}`. Contrary to what this file previously said here: with the repo + root's own `vercel.json` present (the marketing site's static-build config), + an empty Root Directory at build time falls back to that root config + instead of auto-detecting Next.js — even though Root Directory is correctly + set on the project and even though the uploaded source is scoped correctly. + Vercel's own build output prints "The vercel.json file should be inside of + the provided root directory" as a warning, not a hard error, and then uses + it anyway. Confirmed by deploying without one first: the build ran the + marketing site's `echo` install/build commands against `cloud/apps/web`'s + uploaded files and failed looking for a `public/` output directory that + doesn't exist there. A local `vercel.json` closes the gap outright. +4. If deploying via the CLI rather than Git integration, also add a + `.vercelignore` at the **repo root** excluding any local worktree/build + directories that sit untracked next to the actual source (`.worktrees/`, + `.wt/`, `.turbo/` in this repo, `src-tauri/target/`, `dist/`) — `vercel + deploy` uploads the working directory as-is, not `git ls-files`, so + untracked local dev artifacts get swept in too. Without it, a deploy from + this repo's root uploaded 57,896 files instead of ~700. +5. Give it its own hostname (e.g. `app.`, distinct from the marketing site's). -4. Set every `web`-column environment variable from the table above on this +6. Set every `web`-column environment variable from the table above on this project (`DATABASE_URL`, `REDIS_URL`, `AUTH_SECRET`, `AUTH_GITHUB_ID`/ `AUTH_GITHUB_SECRET`, the `S3_*` set, `APP_URL`). +7. **Disable Vercel's own Deployment Protection (SSO/Vercel Authentication)** + for this project, or it gates every page — including `/signin` — behind a + Vercel-account login wall on top of Ghost's own auth, blocking real users + entirely. New projects on a team plan often have this on by default for + `*.vercel.app` URLs. **Container host for the worker** (Fly.io, Railway, Render, or any host that runs a Docker image — pick one; none of this repo's code prefers one over From ea25129d9d03723f154d8e6366e68ce2eb517e81 Mon Sep 17 00:00:00 2001 From: mohabbis <101276427+mohabbis@users.noreply.github.com> Date: Wed, 5 Aug 2026 16:30:19 -0500 Subject: [PATCH 7/7] ci: stop requiring the marketing site to advertise a desktop release The version-consistency check hard-failed when public/index.html didn't contain a vX.Y.Z string, cross-checked against README.md. That assumption broke on master after the site copy was rewritten to drop legacy desktop positioning entirely (following the download-CTA removal in #396) -- the site no longer mentions a specific release at all, which is the intended current state, not drift. README.md still names the published desktop tag as historical reference; nothing there needs to change. Only cross-check the two when the site actually advertises a version, so a real future mismatch still fails loud. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/rust.yml | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index c3dfa2b5..2a7f9aa7 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -278,10 +278,19 @@ jobs: for f in src-tauri/Cargo.toml src-tauri/tauri.conf.json; do grep -q "$SRC" "$f" || { echo "Source version $SRC missing from $f"; exit 1; } done + # public/index.html is the cloud-pivoted marketing site; it no longer + # advertises a specific desktop release (the legacy download CTA was + # deliberately dropped, see #396 and the site copy rewrite that + # followed). README.md still names the published desktop tag as + # historical reference. Only cross-check the two when the site + # actually mentions a version — its absence is not drift to catch, + # it's the current, intended state of a page that stopped talking + # about desktop releases at all. REL=$(grep -oE 'v2\.[0-9]+\.[0-9]+' public/index.html | head -1) - echo "Advertised release: $REL" - [ -n "$REL" ] || { echo "No vX.Y.Z release in public/index.html"; exit 1; } - grep -q "$REL" README.md || { echo "Advertised release $REL missing from README.md"; exit 1; } + echo "Advertised release: ${REL:-}" + if [ -n "$REL" ]; then + grep -q "$REL" README.md || { echo "Advertised release $REL missing from README.md"; exit 1; } + fi frontend-contract: name: Frontend contract tests