Skip to content

Latest commit

 

History

History
168 lines (129 loc) · 24.1 KB

AGENTS.md

File metadata and controls

168 lines (129 loc) · 24.1 KB

Repository Guidelines

Project Structure & Module Organization

  • cmd/api/ holds the API server (hub-api): HTTP API, ingestion, record retrieval, tenant/auth, semantic search; enqueues jobs to River (insert-only). Build/run: go run ./cmd/api or make run.
  • cmd/worker/ holds the worker (hub-worker): runs River job workers — webhook delivery and the enrichment pipelines (embeddings, translation, sentiment, emotions). No HTTP. Build/run: go run ./cmd/worker or make run-worker.
  • cmd/backfill-*/ are one-off enqueue commands that (re)enrich an existing backlog: backfill-embeddings, backfill-translations, and backfill-classify -type sentiment|emotions. hub-worker processes the jobs they enqueue.
  • internal/ contains the application layers: api/handlers, api/middleware, service, repository, models, config, workers, observability (OTel metrics/tracing), the LLM seam (llm, openai, googleai), datatypes, and huberrors.
  • pkg/ provides shared utilities: database, cursor (keyset pagination), and embeddings.
  • sdks/typescript/ holds the generator config for the published @formbricks/hub npm package. The generated client is not committed — see TypeScript SDK below.
  • migrations/ stores SQL migration files (goose); use -- +goose up / -- +goose down annotations.
  • tests/ contains integration tests (they require a pgvector database — see Testing Guidelines).
  • docs/ holds the documentation site for hub.formbricks.com (Astro + Starlight) — the only non-Go part of the repo. See Documentation Site below.

Build, Test, and Development Commands

  • make dev-setup: start Postgres via Docker, install Go deps/tools, and initialize database schema.
  • make run: apply River migrations, then run both hub-api and hub-worker (config from .env if present, else environment variables; copy .env.example to .env or set env vars).
  • make run-api / make run-worker: run a single process (worker requires DATABASE_URL from .env or environment variables).
  • make run-backfill-embeddings / make run-backfill-translations / make run-backfill-classify TYPE=sentiment|emotions: enqueue a one-off (re)enrichment of the existing backlog (loads .env). make build-backfill-* build the corresponding binaries.
  • make build: build both bin/hub-api and bin/hub-worker. Use make build-api or make build-worker for a single binary.
  • make tests: run integration tests in tests/.
  • make tests-coverage: generate coverage.html.
  • make check-coverage: run all tests with coverage and fail if below COVERAGE_THRESHOLD (excludes cmd/api and cmd/worker main packages).
  • make init-db: run goose migrations up using DATABASE_URL. make migrate-status and make migrate-validate for status and validation. New migrations go in migrations/ with goose annotations (-- +goose up / -- +goose down). Name files with a sequential number and short description (e.g. 002_add_webhooks_table.sql); goose orders by the numeric prefix. For webhook delivery, run make river-migrate after init-db to apply River job queue migrations.
  • SDK commands run from sdks/typescript/ and use pnpm, not make — see TypeScript SDK below.
  • make fmt: format code (runs golangci-lint run --fix; uses gofumpt/gci from config).
  • make lint: run golangci-lint (includes format checks; requires make install-tools).
  • Docs commands run from docs/ and use pnpm, not make — see Documentation Site below.

Coding Style & Naming Conventions

  • Language: Go; format with make fmt (golangci-lint applies gofumpt/gci).
  • Prefer Go naming conventions (CamelCase for exported, lowerCamel for unexported).
  • Keep package names short and domain-focused (e.g., repository, service).

TypeScript SDK (sdks/typescript/)

The published @formbricks/hub client, generated from openapi.yaml by @hey-api/openapi-ts.

The SDK is a build artifact. The config is committed; the output is not. src/generated/ and dist/ are gitignored, produced in CI, and published to npm by .github/workflows/publish-sdk.yml on a stable Hub release. Do not commit generated files, and do not hand-edit them — they are overwritten on every run.

  • openapi-ts.config.ts — the generator config. Reads ../../openapi.yaml.
  • src/index.ts — the hand-written entry point, and the place for anything that cannot be generated (it exports createHubClient on top of the generated client).
  • src/hub-fetch.ts — the retry and per-attempt timeout policy every request goes through, hand-written because the generated client has none. createHubClient uses it, and src/client-config.ts gives it to the generated default client through the generator's runtimeConfigPath hook. The policy only re-sends a request when that cannot apply it twice — the Hub has no idempotency keys; the one known exception, a retried tenant purge DELETE starting a second run, is documented in the README — so do not widen it to retry POST/PATCH on 5xx, network errors or timeouts (a 408 or 503 only with Retry-After). It decides from status and headers, keeps the attempt's deadline running until the returned body is read, and holds the attempt's Requests until then: Node's fetch links a Request's signal only weakly, and a collected Request takes the timeout and the caller's abort with it.
  • src/schemas.ts — re-exports the generated zod validators behind the @formbricks/hub/schemas entry, so importing the client itself pulls in no dependencies and zod stays an optional peer.
  • scripts/ — the CI-only helpers, not in files, so not shipped: prepare-comparison.mjs packs this package and fetches the one on npm (shared by both workflows, so the release's summary and the preview measure the same things), compare-packed.mjs diffs the packed contents for the release's summary, surface.mjs diffs the public surface for the preview, and verify-publish-guards.mjs is the release's last check before the publish job (below).
  • tests/ — wire.test.mjs and retry.test.mjs import the built package by its own name, so they resolve through the exports map (wire.test.mjs loads the CJS build through require too), and retry.test.mjs pins each retry and timeout case against a real local server, including under forced garbage collection; compare-packed.test.mjs, surface.test.mjs and verify-publish-guards.test.mjs cover the CI helpers, since a check that quietly stops checking is the failure mode that matters for all three.

Commands, from sdks/typescript/: pnpm install, pnpm generate, pnpm build, pnpm check, pnpm test.

To change the API surface, change openapi.yaml. Never patch the SDK to paper over a spec problem — the next generation run erases it.

The SDK's version is the Hub release's, so @formbricks/hub@X is the client for Hub X. Never bump version in sdks/typescript/package.json: it is a 0.0.0-development placeholder, and publish-sdk.yml stamps the release tag into the manifest (scripts/release-version.mjs) before building and packing. Every stable Hub release publishes its SDK, unchanged or not, so each Hub version has one. A release above every published version goes out as latest; one below — a backport, such as a 0.14.1 security fix released after 0.15.0 — goes out under release-X.Y (release-0.14), so latest stays put, as it does for Hub's own images. The build fails, before anything is published, when the tag is not a plain X.Y.Z, when that version is already on npm (Stainless published 0.2.0–0.13.0, so Hub's first aligned release is 0.14.0), or when it is a backport older than a version already published on its line. The comparison against the package on npm stays, for the job summary: it shows what the release changes for SDK users. Removed exports show up in the surface diff the sdk-preview workflow writes on the pull request that removes them, and make the Hub release that ships them a breaking one.

A release publishes as soon as it is created; there is no approval step (ENG-3857). Releases run one at a time (cancel-in-progress: false), and GitHub keeps one pending run per group, so cutting a third release while one runs and a second is queued cancels the second's pending run. Re-run the cancelled one.

The generator version is pinned exactly. Bumping it is a deliberate change: regenerate, run pnpm test, and read the surface diff the sdk-preview workflow writes to the job summary.

Publishing is credential-free by design. Authentication is npm trusted publishing (OIDC), bound to this repository, the publish-sdk.yml filename and the npm-publish environment. There is no npm token, and none should be introduced. Consequences: renaming that workflow file breaks publishing until the npm configuration is updated, and the job that runs the generator deliberately holds no publishing permission — only the separate publish job does. That job validates the artifact's manifest, re-packs it and requires the same integrity hash the build job recorded, and runs npm publish --ignore-scripts — so what reaches npm is the package that was compared and tested, and no code the build job produced runs next to the credential. Four details there are deliberate and easy to undo by accident:

  • Both jobs pin the same npm, via one NPM_VERSION in publish-sdk.yml (and sdk-preview.yml pins the same, so pull requests exercise the release's npm). The integrity check compares a pack across the two jobs, and npm's --json output changes shape between majors — npm 12 made npm pack --json an object keyed by package name and wraps every npm view --json result in an array — so the scripts accept both, and bumping the pin should be tested rather than assumed.
  • No --tag on a latest publish. latest is the default anyway, and leaving it implicit keeps npm's own refusal to apply latest to a prerelease or to a version lower than the current one. --tag latest switches that off. Only a backport passes --tag, with its release-X.Y; the publish job re-derives that tag from the release tag itself and refuses any other the build job names.
  • No registry-url on setup-node in the publish job. It writes an .npmrc with a ${NODE_AUTH_TOKEN} placeholder; when the trusted-publisher login fails, npm then sends that placeholder and the registry answers the PUT with a bare 404 Not Found, which is how Hub 0.9.0's publish failed. Without it, a failed login is an honest ENEEDAUTH. npm logs why the login failed only at verbose level, so the publish writes its log to its own --logs-dir and, on failure, prints the oidc lines from it (npm does not log tokens).
  • No dependency cache in the release build. Its output is what gets published and its hash is computed there too, so a poisoned cache would go unnoticed; setup-node's guidance is to disable caching in publishing workflows.

Only a release tag publishes, and only through a protected npm-publish environment. npm's trusted-publisher binding covers the repository, the workflow file and the environment name — not the ref — so the environment's rules decide which runs can obtain the publishing identity. A job that references a missing environment creates it on the spot with no protection rules, so the release path refuses to run without one: the guards job in publish-sdk.yml runs scripts/verify-publish-guards.mjs, which fails the release unless the run is for a tag, the tagged commit is on main, and npm-publish exists with rules that restrict it and still admit a release. It warns — without failing — when the rules work but are looser than they need to be.

The configuration it expects (repo settings → Environments → npm-publish; needs admin):

  • Deployment branches and tags: "Selected branches and tags", with one tag rule, [0-9]*.[0-9]*.[0-9]*. Hub release tags are bare semver (0.8.7), so the usual v* would reject every release. No branch rule is needed, since only tags publish. Not "Protected branches only": that admits no tags at all.
  • No required reviewers. Whoever may create a Hub release may publish its SDK, with no second person approving (ENG-3857): the team is small, and releases are trust-based. The trade is explicit: anyone with write access can cut a release tag, and edit this workflow at that tag, so the guard catches mistakes and misconfiguration, not a malicious committer. Adding required reviewers (with Prevent self-review) would bring a second person back, and the guard accepts that. A ruleset limiting who may create release tags is a different, optional restriction: it narrows who can release, but that person still publishes alone, and the guard does not inspect repository rulesets.
  • "Allow administrators to bypass configured protection rules" unticked. This one is settings-UI-only; the REST API can read it but not set it.
  • No secrets or variables — the publish authenticates over OIDC.

To check a configuration without cutting a release, either run publish-sdk.yml manually from the latest release tag with dry_run ticked — that runs every check a real release would, with the CI token's real permissions, and publishes nothing — or run the guard locally, which needs only read access:

GITHUB_TOKEN="$(gh auth token)" GITHUB_REPOSITORY=formbricks/hub \
  GITHUB_REF=refs/tags/<latest release> GITHUB_SHA="$(gh api repos/formbricks/hub/commits/<latest release> --jq .sha)" \
  PUBLISH_ENVIRONMENT=npm-publish node sdks/typescript/scripts/verify-publish-guards.mjs

The npm side can be read back, not validated. npm does not check a trusted-publisher configuration when it is saved; a wrong one surfaces only as Unable to authenticate on the first publish, which fails safely — the build job holds no credentials and nothing is published. But a package maintainer can confirm what is saved with npm trust list @formbricks/hub (npm ≥ 11.15, logged in, 2FA on). The field to check hardest is environment = npm-publish: it is optional on npm's side, and if it is blank, npm accepts a publish from a workflow that never enters the environment at all, which sidesteps every protection above.

Keep example: values in openapi.yaml synthetic. They now ship twice — into the published SDK's docblocks and onto the docs site — so a real tenant id, key or customer name in an example is published, not merely committed.

Enrichment Framework

Sentiment, emotions, and translation are "classify" enrichments (text → structured result via an LLM) that share one scaffold; embedding is a separate pipeline that reuses only the light shared layer (client registry, metrics, content-hash). Do not copy an existing enrichment to add a new one — reuse the shared pieces:

  • internal/service/enrichment_client_factory.go — provider registry + validation and the shared EnrichmentClientConfig (per-type configs are aliases of it).
  • internal/service/enrichment_client.go — classifyStructured + structuredSpec[R] (prompt/schema/parse); the single place that calls the provider.
  • internal/service/enrichment_provider.go — EnrichmentProvider (eligibility, per-type triggers, per-tenant gate, dedupe, buildArgs).
  • internal/workers/enrichment_worker.go — the generic enrichmentWorker[A, R] Work body (load → clear-on-empty or classify → persist, with all outcome/error mapping).
  • internal/observability/enrichment_metrics.go — one metrics impl behind the per-type interfaces.

A new enrichment is then a thin per-type spec: an enum/result type in models, a *_client.go (prompt + parse + structuredSpec), a *_provider.go (config for EnrichmentProvider), a *_job_args.go, a one-line *_worker.go (a configured enrichmentWorker), a metrics adapter, a config struct, and wiring in cmd/*. Enrichment outputs are read-only, server-generated columns — NULL until enriched, cleared and re-enqueued when value_text changes (translation also on language). Writes are guarded against content supersession, so a job that read older text skips rather than overwriting a newer result; carry tenant_id (and, for correlation, the originating event_id) through job args and log lines.

Bound what you send. value_text is accepted up to 30,000 characters, so a classify hook takes its text through trimForProvider with that type's *_MAX_INPUT_BYTES cap (see service.TextCap for why the unit is bytes) and counts the cut in hub_enrichment_text_trimmed_total. Trim where a partial result is still a result — a label or a vector for the start of a record beats none — and turn the input away as a terminal TerminalReasonLength failure wrapping errInputOverCap where a partial result would be wrong stored as complete, which is translation's case. A pipeline that refuses must also add observability.WorkerReasonInputOverCap to its allowed*WorkerReasons in internal/observability/names.go: a worker-error reason that allowlist does not name is exported as other, and the counting doubles in the worker tests do not apply the allowlist, so assert through the real New*Metrics. Skipping a cross-cutting step here does not merely overspend: an over-long embedding input is a terminal provider failure, so the record stays out of the index and out of the reconciler's reach for good.

Multi-Tenancy & Data Isolation

  • Treat tenant_id as a security boundary, not a convenience filter. Tenant-owned data must never be read, enqueued, dispatched, cached, searched, embedded, or deleted across tenants.
  • Exception: GDPR/right-to-erasure flows may intentionally delete all records for a data subject across tenants when that is the documented API contract. Make that all-tenant behavior explicit in the API docs, service/repository names or comments, logs, and tests; do not reuse it for normal tenant-owned workflows.
  • When making a model, migration, API request, or repository change involving tenant-owned data, audit every downstream path that carries or derives from that data: handlers, services, repositories, message publishers, River job args, workers, webhook payloads, search, embeddings, bulk operations, logs, and metrics.
  • Tenant access rules must be consistent across every path that can observe, mutate, derive from, or act on the same resource. If one API endpoint, repository method, search path, webhook dispatch path, worker, backfill, bulk operation, or export path requires tenant scope, every alternate path for that resource must enforce the same tenant boundary.
  • Do not model tenant_id as an optional filter for tenant-owned resources. Prefer required tenant parameters in service/repository method signatures (tenantID string, not *string) unless the domain explicitly supports global resources and documents that behavior.
  • Prefer tenant-aware repository/service methods for tenant-owned workflows. Avoid adding broad helpers that return all enabled/all matching resources when the caller is dispatching, processing, deriving, exporting, or exposing tenant data.
  • Async jobs must carry the tenant boundary when the source data has one, and workers must re-check tenant scope before doing side effects. Do not rely only on enqueue-time filtering.
  • Global resources may intentionally have tenant_id = NULL only when the domain explicitly documents them as non-tenant-owned. Webhooks are tenant-owned and must require a non-empty tenant_id; a missing tenant on event data must not match any webhook.
  • When changing access rules for a tenant-owned model, search for all alternate access paths by resource name and by derived side effects: list, get, update, delete, bulk delete, webhook fan-out, River jobs, workers, embeddings, search, exports, cache invalidation, logs, and metrics.
  • Every tenant-owned mutation must run through the tenant write lock helper (internal/repository/tenant_write_lock.go): a transaction that first try-acquires the shared advisory lock on tenant_write|<len>:<tenant_id> (resolve the row's tenant inside the transaction for ID-based mutations, then mutate with a tenant-scoped WHERE). The tenant data purge holds the same key exclusively. Lock order is fixed: tenant shared lock → scope-specific advisory locks (e.g. taxonomy run scope) → row locks; never block on an advisory lock while holding row locks. A path that skips the helper silently reopens the purge/write race.
  • For any tenant-scoping change, include verification at the boundary where the leak could happen: database query behavior, service fan-out, worker execution, and API behavior when relevant. Include at least one alternate-path regression test proving that data allowed through the primary path cannot leak through async dispatch, bulk operations, derived indexes, exports, or background workers. Tests are the evidence; the invariant belongs in the architecture.

Documentation Site (docs/)

The docs site for hub.formbricks.com. Astro + Starlight with its own package.json and node_modules — the only Node tooling in the repo, so it uses pnpm, not make.

  • docs/src/content/docs/ — the pages (.md/.mdx); landing page is index.mdx, and the three prose sections are core-concepts/, guides/, reference/.
  • docs/astro.config.ts — sidebar topics, the OpenAPI reference wiring, head/theme.
  • docs/theme.css — brand tokens (--fb-*) layered over Starlight's own (--sl-*).
  • docs/public/ — served as-is (favicons, web manifest). docs/src/assets/ — images imported from content.

Commands, from docs/: pnpm install, pnpm dev (localhost:4321), pnpm build, pnpm preview, pnpm check (astro check — types and content-collection schemas), pnpm format.

The API reference is generated, never hand-written. It comes from openapi.yaml via starlight-openapi and is served under /api/. To change it, change the spec. Because the docs build parses the spec, an invalid spec fails the docs build as well as code-quality.yml's Spectral lint.

The build takes no secrets and makes no network calls. That is the point of ENG-2369: the previous site fetched its API reference from Stainless at build time with a STAINLESS_API_KEY. If the build ever needs a credential again, something has regressed.

Writing conventions:

  • kebab-case filenames (e.g. quickstart.mdx); keep frontmatter minimal and explicit (title, description).
  • Use Starlight components from @astrojs/starlight/components — <Aside>, <Card>, <LinkButton>, <Tabs>. Do not reintroduce a bespoke component library.
  • Adding a page means adding it to the sidebar in astro.config.ts. There is no autogenerated section, so an unlisted page is unreachable.
  • 2-space indentation; for the TypeScript config, ES modules, double quotes, semicolons.

Validating a docs change: pnpm check and pnpm build must both pass, then pnpm preview and review the changed pages. Anything touching theme.css or the landing page needs checking in light and dark mode.

Testing Guidelines

  • Unit tests live beside the code in internal/**, cmd/api and cmd/worker and need no database: make test-unit.
  • Integration tests live under tests/ and require a pgvector-enabled Postgres (DATABASE_URL pointing at a test_db with migrations applied): make tests. make test-all runs both.
  • Name test files *_test.go and test functions TestXxx.
  • Use make tests-coverage / make check-coverage when adding meaningful logic (the threshold excludes the cmd/* main packages).
  • Tenant-isolation changes must include an alternate-path regression test (see Multi-Tenancy & Data Isolation).

Commit & Pull Request Guidelines

  • Always create a pull request for changes and don't commit directly to main.
  • Commit messages follow a Conventional Commits style (e.g., chore: fix linting errors).
  • PRs should describe the change, include test evidence (command + result), and link related issues when available.
  • For API behavior changes, include request/response examples or screenshots of Swagger UI.

Security & Configuration Tips

  • Configure API_KEY and DATABASE_URL via .env or environment variables.
  • Enrichment providers and other runtime options are env-configured (EMBEDDING_*, TRANSLATION_*, SENTIMENT_*, EMOTIONS_*, TENANT_SETTINGS_CACHE_*, OTEL_*); .env.example is the full, documented set.
  • Do not commit .env or secrets; use .env.example as the base.