site: drop the legacy desktop download CTA and unused platform-detect code - #396
Merged
Conversation
cloud.yml was activated in PR #393, but the handoff doc still told the next reader "cloud/ has no CI" and to validate everything locally. Update both docs to reflect that CI is live, path-scoped, and runs against real Postgres/Redis, and correct the stale "expect 220 tests" count to the current 239. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… code
The marketing site is cloud-first now, but still ran a full "Desktop
preview downloads" section with dl-mac/dl-win buttons for the
superseded v2.0.3 Tauri app — a prominent CTA for a product that
isn't what's being built or sold. Remove it; fold the one honest fact
(v2.0.3 is archived on GitHub releases) into the existing legacy
blurb as a link instead of a button row.
Also:
- drop platformDetection() in main.js and the CSS/markup it drove
(#download-sub, data-download-label, dl-mac/dl-win platform
dimming, data-platform attribute) — all dead once the section is
gone.
- drop the now-unused i-download/i-apple/i-windows/i-layers SVG
symbol defs and the .pill utility (its only consumer was the
removed download pill).
- fix a garbled sentence in the About section ("Direction will not:
approval-gated execution...") that was missing a verb; it now
reads as intended and states plainly that the name isn't changing
yet.
- drop deploy-website.yml's "Validate download links" step, which
hard-required Ghost.dmg/Ghost_Setup.exe links in the markup and
would otherwise fail every future deploy.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Removes "no fake logos" (hero note), "Not a fake customer page" (positioning row), the "Not built yet" proof-list item, and the About section's "Working name: Ghost — not changing it yet" note. These were meta-commentary about the site's own honesty rather than claims about the product; the remaining content (What's real / Known limits sections) already carries the substantive proof. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mohabbis
added a commit
that referenced
this pull request
Aug 5, 2026
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 <noreply@anthropic.com>
mohabbis
added a commit
that referenced
this pull request
Aug 5, 2026
* feat(cloud): browser recording capture — Chrome extension + deterministic compile
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 `<all_urls>` 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 <noreply@anthropic.com>
* fix(worker): make production container build and boot reliably
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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
platformDetection()inmain.js, the CSS/markup it drove (#download-sub,data-download-label, dl-mac/dl-win platform dimming,data-platformbody attribute), the unusedi-download/i-apple/i-windows/i-layersSVG symbol defs, and the.pillutility (its only consumer was the removed pill).deploy-website.yml's "Validate download links" step, which hard-required Ghost.dmg/Ghost_Setup.exe links in the markup and would otherwise fail every future deploy once those links are gone.Test plan
<section>/<li>tags balanced.node --check public/main.jspasses.🤖 Generated with Claude Code