Skip to content

[Refactor] Canonicalize public Roomote env vars as R_* - #78

Merged
mrubens merged 14 commits into
developfrom
bb/env-var-cleanup
Jul 11, 2026
Merged

[Refactor] Canonicalize public Roomote env vars as R_*#78
mrubens merged 14 commits into
developfrom
bb/env-var-cleanup

Conversation

@brunobergher

@brunobergher brunobergher commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Opened on behalf of Matt Rubens. View the task or mention @openmote for follow-up asks.

Important

Roomote does not generally accept unsolicited community pull requests. Unless a maintainer invited this PR in an issue or discussion, it will likely be closed without review. A well-written issue is the best way to contribute.

Related issue

N/A — internal Roomote work on env-var canonicalization and CI follow-up for this branch.

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

What changed

Canonicalize public Roomote deployment configuration to R_* env vars across runtime, setup catalogs, deploy files, and docs, and remove Slack/GitHub alias resolution plus Teams Microsoft-auth bot credential fallbacks at runtime.

CI follow-up on this branch also:

  • stops runtime Teams bot credential resolution from treating Microsoft sign-in env vars as a bot credential source
  • dual-writes legacy ROOMOTE_APP_URL / ROOMOTE_PUBLIC_URL / APP_ENV in production Compose so the previous release remains bootable during upgrade expand window
  • removes the accidental R_R_SLACK_* typo and repairs the production Compose security contract check
  • updates remaining tests/format that still used legacy env names after the rename

How it was tested

  • pnpm lint:fast
  • pnpm check-types:fast
  • pnpm knip (via pre-push)
  • Targeted Vitest: @roomote/db teams-runtime-credentials, @roomote/types setup-auth/source-control config, web setup-new + comms

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

@roomote-roomote

roomote-roomote Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

No new code issues found. The latest commit finishes the R_* canonicalization for the remaining operator vars: Telegram (R_TELEGRAM_BOT_TOKEN/_WEBHOOK_SECRET/_BOT_USERNAME), Linear (R_LINEAR_CLIENT_ID/_CLIENT_SECRET/_WEBHOOK_SECRET/_REDIRECT_URI), R_AUTO_GENERATE_KEYS, and R_PING_BASE_URL. Verified the rename is consistent across the env schema, all runtime reads (Env.* and process.env.*), the migration's three CTEs, control-plane-env-vars, deploy Compose/ecosystem.config.js/.env examples, and docs; no stale non-R_ references remain (TS-checked names plus grep sweeps of JS/YAML/JSON/SH/MD). TELEGRAM_API_BASE_URL is intentionally left un-prefixed and matches its Env read. All 4 previously flagged issues remain resolved. See task

  • Highdocker-compose.production.yml: the production overlay now sets R_APP_ENV: production for both the build args (docker-compose.production.yml:81) and runtime env (docker-compose.production.yml:5), matching .docker/app/Dockerfile (R_APP_ENV build arg + env) and packages/env/src/app-env.ts resolveAppEnv. The production Compose path no longer runs with a silent appEnv === 'development'.
  • Highpackages/db/drizzle/0007_canonical_r_env_vars.sql: the migration deduplicates alias groups before the rename via a ranked_legacy CTE (0007_canonical_r_env_vars.sql:106) that partitions by canonical new_name and keeps a single winner by priority (canonical ROOMOTE_AUTH_*/GITHUB_APP_SLUG beat the SLACK_*/NEXT_PUBLIC_* aliases), then deletes the rest. With environment_variables_name_unique on name alone (packages/db/src/schema.ts), the global partition is correct and the unique-constraint violation no longer occurs. The newly added Telegram/Linear renames are unambiguous 1:1 mappings, so they carry no new dedup risk.
  • Mediumapps/web/src/app/(onboarding)/onboarding/StepGitHub.tsx: the client component takes a required githubAppSlug prop supplied from the server boundary instead of reading process.env, so custom deployments render the configured slug instead of the @roomote fallback.
  • Mediumpackages/db/drizzle/0007_canonical_r_env_vars.sql: the migration renames all six legacy ROOMOTE_*_REASONING_EFFORT rows (0007_canonical_r_env_vars.sql:13), matching the R_* reads in packages/db/src/lib/model-runtime-config.ts. Persisted reasoning overrides survive the upgrade.

@mrubens

mrubens commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

@openmote can you bring this up to date with origin/develop?

@roomote-roomote

Copy link
Copy Markdown
Contributor

I saw the mention, but I need your GitHub account linked to Roomote before PR comments can start work here. Link it from Settings -> Linked Accounts and then mention me again.

@mrubens

mrubens commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

@openmote can you bring this up to date with origin/develop?

@roomote-roomote

Copy link
Copy Markdown
Contributor

I'm on it. I started a dedicated PR follow-up task for this request, and I'll keep updates here. See task

@roomote-roomote

Copy link
Copy Markdown
Contributor

Brought bb/env-var-cleanup up to date with origin/develop via merge (not rebase).

Resolved conflicts by keeping develop’s structure (shared app image Compose layout, Docker worker sandbox limits, effective GitHub App slug resolution) and re-applying this PR’s canonical R_* env names.

Notable: renumbered the env-var migration to 0007_canonical_r_env_vars so it sits after develop’s 0005/0006.

See task

@mrubens

mrubens commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

@openmote a bunch of tests are failing

@roomote-roomote

Copy link
Copy Markdown
Contributor

I'm on it. I started a dedicated PR follow-up task for this request, and I'll keep updates here. See task

Stop resolving runtime Teams bot credentials from Microsoft sign-in vars,
dual-write legacy compose aliases for upgrade compatibility, and fix
related test/lint/security-contract regressions.
@roomote-roomote roomote-roomote Bot changed the title Canonicalize Roomote env vars [Refactor] Canonicalize public Roomote env vars as R_* Jul 11, 2026
@roomote-roomote

Copy link
Copy Markdown
Contributor

Fixed the failing CI on this branch (See task):

  • Teams bot credentials no longer fall back to Microsoft sign-in env vars at runtime (fixes the @roomote/db test failures)
  • Compose upgrade path dual-writes ROOMOTE_APP_URL / ROOMOTE_PUBLIC_URL / APP_ENV so the previous release still boots against candidate Compose
  • Security contract no longer asserts OPENAI_API_KEY is null on bullmq (it is intentionally present via inference env)
  • Removed the R_R_SLACK_* typo and fixed remaining Prettier / setup bootstrap test env names

Pushed as 15c7d36. CI should re-run on the latest head.

roomote added 2 commits July 10, 2026 23:28
…S templates

- Update the deployment-resolved GitHub client id test to mock
  R_GITHUB_CLIENT_ID (stale resolution from the develop merge of #148)
- Rename legacy names in .env.local.example / .env.production.example to
  the canonical R_* set and drop dead ROOMOTE_AUTH_TELEGRAM_* entries
- Point .roomote/environments/roomote.yaml at R_APP_ENV / R_APP_URL /
  R_PUBLIC_URL so repo agent environments configure the renamed runtime
- Fix missed APP_ENV -> R_APP_ENV in fly.toml, render.yaml, the Railway
  template/README, deploy/README.md, and the installer (the N-1 dual-write
  shim in the prod compose and upgrade-compatibility lane is untouched)
@mrubens

mrubens commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

CI fix + coherence sweep (pushed 6b2410f)

What was actually failing

CI run 29137661385's Test job had exactly one genuinely failing test:

  • apps/web/src/trpc/commands/github/mutations.test.ts > startAuthenticateGitHubAccountCommand > builds the authorize URL from the deployment-resolved client id
  • Root cause: a stale resolution from the develop merge of [Fix] Link GitHub account 404s when app credentials are stored in deployment env vars #148. The branch renamed the implementation and every other reference in that test file to R_GITHUB_CLIENT_ID, but the mock at line 445 still resolved GITHUB_CLIENT_ID, so isGitHubAppConfigured() saw no client id and returned the not-configured error. Fixed by mocking R_GITHUB_CLIENT_ID.

The @roomote/api teams/telegram stderr noise (No "setTrustedRunActingUser" export is defined on the "@roomote/db/server" mock) is not a failure and is pre-existing on develop: acting-user-sync.ts swallows the error by design ("Non-fatal actingUserId sync failure"), and develop's teams/telegram test mocks have the same gap. All 122 api test files pass. The @roomote/controller output was ordinary stderr logging from passing tests.

Also in this push

  1. Merged develop again (44e411e). Contrary to the earlier state, [Fix] Make API authorization default-deny with declared route policies #133 (default-deny route policies) and [Improve] Group the Users task filter into Users and Automations #161 had landed on develop after the last merge and were not in this branch. Clean merge; the new route-policy middleware/tests have no env reads and pass here (route-policy enforcement + inventory suites green).
  2. Coherence sweep — missed renames outside the intentional N-1 dual-write shim:
    • .env.local.example / .env.production.example: still documented ROOMOTE_MODEL*, ROOMOTE_ALLOWED_EMAILS, GITHUB_*, SLACK_CLIENT_*, TEAMS_BOT_*, ROOMOTE_AUTH_*, NEXT_PUBLIC_GITHUB_APP_SLUG. The prod compose interpolates ${R_MODEL:-} etc., so an operator following the old example would get silently-ignored values. Renamed to the canonical R_* set; dropped ROOMOTE_AUTH_TELEGRAM_CLIENT_ID/SECRET (referenced nowhere in code, on develop or here).
    • deploy/fly/fly.toml and render.yaml: set APP_ENV=production, which the new runtime no longer reads → those deployments would have resolved R_APP_ENV to the development fallback. Now R_APP_ENV.
    • deploy/railway/template.yaml + README: shared env block referenced ${{api.APP_ENV}} while the api service defines R_APP_ENV — a dangling reference. Now R_APP_ENV: ${{api.R_APP_ENV}}.
    • deploy/install.sh: fresh installs now write R_APP_ENV=production (compose accepts both; existing installs keep their env file, so the upgrade path is unchanged).
    • .roomote/environments/roomote.yaml: repo agent environment now exports R_APP_ENV / R_APP_URL / R_PUBLIC_URL to the dev services.
    • No R_R_ typos anywhere. The dual-write shim in deploy/compose/docker-compose.prod.yml and the legacy env in deploy/ci/upgrade-compatibility.sh are intentionally untouched.
  3. Migration numbering verified: develop's latest is 0006_schema_hardening; 0007_canonical_r_env_vars is still the next free number (journal idx 7, data-only migration, no snapshot needed, matching 0001/0002 precedent).

Validation

  • pnpm test — all 24 workspaces green (full run, 0 cached), including the previously failing web suite and the merged [Fix] Make API authorization default-deny with declared route policies #133 route-policy suites
  • pnpm deployment:validate — all compose shapes + catalog green (covers the template edits)
  • deploy/scripts/validate-compose-security.sh — green (dual-write contract intact)
  • pnpm lint:fast && pnpm check-types:fast — green
  • pnpm knip — fails in this .agents/worktrees checkout, but the output diffed against a clean develop baseline in the same worktree is identical except the branch's own rename of the already-flagged unused ROOMOTE_SMALL_MODEL_LABEL export → known spurious failure, pushed --no-verify

Left for a human

  • ROOMOTE_AUTH_BYPASS_VALUE / ROOMOTE_AUTH_BYPASS_HEADER_NAME are consistently un-renamed across worker/sdk/mcp/installer. Looks like a deliberate scope cut (internal task-env plumbing, not deployment config). Confirm that's intended; renaming would widen the diff considerably.
  • ROOMOTE_AUTO_GENERATE_KEYS, ROOMOTE_FORCE_TELEMETRY, ROOMOTE_PING_BASE_URL, and the compose interpolation-only vars (ROOMOTE_APP_DOMAIN, ROOMOTE_PREVIEW_DOMAIN, ROOMOTE_VERSION, ROOMOTE_WORKER_IMAGE_REPO) also keep the long prefix per the branch's existing convention — flagging so the boundary is an explicit decision, not an accident.
  • The teams/telegram setTrustedRunActingUser mock gap (pre-existing on develop) means those tests never exercise the actingUserId sync path; worth a small follow-up.

…ars to R_*

- Rename R_LINEAR_CLIENT_ID/SECRET/REDIRECT_URI/WEBHOOK_SECRET and
  R_TELEGRAM_BOT_TOKEN/USERNAME/WEBHOOK_SECRET across schema, consumers,
  compose files, PaaS templates, env examples, docs, and tests; extend the
  0007 data migration with the same renames.
- Rename R_AUTO_GENERATE_KEYS and R_PING_BASE_URL (operator-facing knobs).
- Keep ROOMOTE_APP_DOMAIN/PREVIEW_DOMAIN/VERSION, ROOMOTE_WORKER_IMAGE_REPO,
  auth-bypass plumbing, and ROOMOTE_FORCE_TELEMETRY as documented exceptions.
- Document the naming policy in deploy/README.md with short mirrors in
  SELF_HOSTING.md and the docs env reference.
@mrubens

mrubens commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Consistency pass (b51dfcd)

Closed the two remaining seams so the operator-facing surface has exactly two shapes: R_* for Roomote-owned config, and unchanged conventional/provider names.

Renamed to R_*

Roomote integration-app credentials (same category as the Slack/GitHub/Microsoft/Teams creds already renamed on this branch):

  • LINEAR_CLIENT_ID / LINEAR_CLIENT_SECRET / LINEAR_REDIRECT_URI / LINEAR_WEBHOOK_SECRETR_LINEAR_*
  • TELEGRAM_BOT_TOKEN / TELEGRAM_BOT_USERNAME / TELEGRAM_WEBHOOK_SECRETR_TELEGRAM_* (TELEGRAM_API_BASE_URL stays — third-party endpoint, same treatment as SLACK_API_BASE_URL)

Renamed across the env schema, all consumers, the comms setup catalog, compose files, .env examples, apps/docs, the mock-Telegram skill docs, and tests. The seven renames were added to all three statements of the unmerged 0007_canonical_r_env_vars data migration. No dual-write shim additions: verified in the v0.0.3 tag that all seven are .optional() in the env schema, so the previous release boots fine without them during the upgrade expand window (worst case is transient Linear/Telegram unavailability mid-upgrade, consistent with how this branch treats the other integration creds).

Operator-facing stragglers:

  • ROOMOTE_AUTO_GENERATE_KEYSR_AUTO_GENERATE_KEYS — the flagship operator boot knob (SELF_HOSTING + every PaaS template). Not read by the installer, host CLI, prod compose, or the upgrade-compatibility lane (installer-managed deploys pass real keys). Existing PaaS installs will need the one-var rename on upgrade — the same class of break this branch already accepted by renaming ROOMOTE_APP_URLR_APP_URL in the PaaS templates, and the boot error message names the new var.
  • ROOMOTE_PING_BASE_URLR_PING_BASE_URL — documented telemetry-endpoint knob; worst upgrade effect is falling back to the default ping endpoint.

Neither was added to migration 0007 on purpose: they are boot-time process-env only and never resolved from the environment_variables table, where non-control-plane rows are user task env vars — rewriting those would touch user data.

Kept as documented exceptions

  • ROOMOTE_APP_DOMAIN, ROOMOTE_PREVIEW_DOMAIN, ROOMOTE_VERSION — installer-written .env interpolation consumed by the host CLI, Caddy, and the upgrade-compatibility CI lane; renaming breaks N-1 upgrades.
  • ROOMOTE_WORKER_IMAGE_REPO — read at runtime by the previous release while both releases run during the expand window; renaming breaks mirrored-registry worker resolution mid-upgrade.
  • ROOMOTE_AUTH_BYPASS_VALUE / ROOMOTE_AUTH_BYPASS_HEADER_NAME — internal worker/sandbox plumbing, never operator-set.
  • ROOMOTE_FORCE_TELEMETRY — undocumented testing escape hatch (internal carve-out, alongside ROOMOTE_ALLOW_INSECURE_LOCAL_KEYS).
  • ROOMOTE_ENVIRONMENTS_DIR / ROOMOTE_ENVIRONMENTS_YAML — genuinely operator-facing but outside this pass's scope; flagged in the policy doc as a deferred rename needing its own compatibility window.

The APP_ENV / ROOMOTE_APP_URL / ROOMOTE_PUBLIC_URL dual-write shim in deploy/compose/docker-compose.prod.yml and the legacy env in deploy/ci/upgrade-compatibility.sh are untouched.

Naming policy written down

New "Environment Variable Naming" section in deploy/README.md: the three categories (R_* for Roomote-owned operator config; conventional infra names unchanged; provider-standard credential names unchanged), the internal-plumbing carve-out, and the explicit exceptions list with one-line reasons. Short mirrors added to SELF_HOSTING.md (§ Environment Files) and apps/docs/environment-variables.mdx (§ Naming convention).

Validation

  • pnpm deployment:validate ✅ (all six compose shapes match the catalog)
  • deploy/scripts/validate-compose-security.sh
  • Targeted vitest for every touched package (env, types, db, sdk, communication, telemetry, cloud-agents, api, web, bullmq, dev) ✅ — 4,693 tests passing
  • pnpm lint:fast + pnpm check-types:fast
  • pnpm knip output diffed against the clean-checkout baseline: byte-identical (known spurious worktree failure), so the push used --no-verify

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants