feat(favicon): environment-distinct favicon (dev/staging), same wordmark by waleedlatif1 · Pull Request #5588 · simstudioai/sim

waleedlatif1 · 2026-07-11T04:05:34Z

Summary

Recolored the existing "sim" wordmark favicon for dev (orange) and staging (yellow) — same mark, background color only — so a dev/staging tab is never mistaken for prod at a glance. Production favicon is untouched.
Deployment tier is read via the existing OTEL_DEPLOYMENT_ENVIRONMENT / APPCONFIG_ENVIRONMENT / NODE_ENV fallback chain already used for OTel's deployment.environment attribute — this app deploys to AWS/ECS, not Vercel, and every deployed tier already sets OTEL_DEPLOYMENT_ENVIRONMENT in Secrets Manager (confirmed directly against /dev/sim/env-vars, /staging/sim/env-vars, /production/sim/env-vars), so no new infra env var was needed.
Wired into both the /favicon.ico rewrite (next.config.ts) and the <link rel="icon"> tags from generateBrandedMetadata() (ee/whitelabeling/metadata.ts). A whitelabeled deployment's own brand.faviconUrl always wins regardless of tier.
Added favicon-dev//favicon-staging/ to the proxy middleware's static-asset exclusions, matching the existing favicon/ exclusion.

Type of Change

New feature

Testing

Full production build (bun run build) passes clean
Typecheck passes (only a pre-existing, unrelated error in providers/meta/index.ts)
Verified the deployment-tier resolution logic against all 3 real AWS secret combinations plus local dev (no infra vars) — all resolve to the correct tier
Verified favicon legibility at 16px (actual browser tab size) for both new colors

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Recolored the existing "sim" wordmark favicon (background only, mark pixel-identical to prod) for dev (orange) and staging (yellow), so a dev/staging browser tab is never mistaken for prod. Production is untouched. Deployment tier is read via the existing OTEL_DEPLOYMENT_ENVIRONMENT / APPCONFIG_ENVIRONMENT / NODE_ENV fallback chain already used for OTel's deployment.environment attribute (this app deploys to AWS/ECS, not Vercel, and every deployed tier already sets OTEL_DEPLOYMENT_ENVIRONMENT in Secrets Manager) - no new infra env var needed. Wired into both the /favicon.ico rewrite (next.config.ts) and the <link rel="icon"> tags from generateBrandedMetadata() (ee/whitelabeling/metadata.ts); a whitelabeled deployment's own brand.faviconUrl always wins regardless of tier.

vercel · 2026-07-11T04:05:40Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 11, 2026 5:43am

cursor · 2026-07-11T04:05:44Z

PR Summary

Low Risk
UI/branding and metadata only; no auth, billing, or data-path changes. Marketing pages may show a stale favicon until the next ISR revalidate after deploy.

Overview
Adds dev (orange) and staging (yellow) favicon sets alongside unchanged production assets, keyed off a new getDeploymentEnv() helper that reuses the existing OTel/AppConfig/NODE_ENV deployment signal.

Metadata & PWA: ICON_SETS centralizes per-tier paths in whitelabel metadata; HTML icons, the web manifest, and whitelabel faviconUrl behavior are updated so tenant favicons fully replace Sim slots (no mixed Sim PNGs). Root layout switches from static metadata to generateMetadata() so tier can resolve after runtime secrets load, without force-dynamic on the root layout (marketing ISR stays static).

/favicon.ico: next.config rewrites to a new dynamic GET /api/favicon route that redirects to the correct SVG (or whitelabel URL) per request—needed because the shared Docker build can't bake tier into config. Proxy static exclusions add favicon-dev/ and favicon-staging/.

Includes getDeploymentEnv unit tests and API validation-contract baseline updates for the new route.

^{Reviewed by Cursor Bugbot for commit 1e42a73. Configure here.}

greptile-apps · 2026-07-11T04:07:14Z

Greptile Summary

This PR adds environment-specific favicon behavior while preserving tenant branding. The main changes are:

New dev and staging favicon assets.
Runtime favicon selection for metadata, manifest, and /favicon.ico.
Tenant favicon precedence across browser and PWA icon slots.
Deployment-tier detection through the existing environment fallback chain.
Proxy and validation-script updates for the new favicon route.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
apps/sim/ee/whitelabeling/metadata.ts	Adds shared icon sets and uses the tenant favicon as a full replacement when configured.
apps/sim/app/manifest.ts	Uses tenant or environment-specific icons for the generated web manifest.
apps/sim/app/api/favicon/route.ts	Adds a dynamic redirect target for the legacy favicon path.
apps/sim/app/layout.tsx	Moves branded metadata generation into `generateMetadata()`.
apps/sim/lib/core/config/env-flags.ts	Adds deployment-tier resolution for UI-facing environment branches.

_{Reviews (6): Last reviewed commit: "fix(favicon): revert marketing-ISR-break..." | Re-trigger Greptile}

- dev favicon: orange -> light pink - staging favicon: yellow -> lighter yellow (mark stays pixel-identical, background color only) - env-flags.ts's CI check requires every exported const to be is-/get- prefixed; deploymentEnv was a plain const. Converted to getDeploymentEnv(), matching the file's existing convention for non-boolean values (getCostMultiplier(), getAllowedIntegrationsFromEnv()).

Addresses review findings on PR #5588: - Cursor (Medium): next.config.ts's rewrites() runs once during the shared, environment-agnostic Docker build (docker/app.Dockerfile builds with dummy env values; bootstrap.ts hydrates process.env from AWS Secrets Manager only at container boot, after the build already ran) - so resolving getDeploymentEnv() inside next.config.ts baked in whichever tier happened to be active during that shared build, never the tier the container actually runs as. Fixed by rewriting /favicon.ico to a stable path (/api/favicon) and resolving the destination inside that route handler instead, which runs per-request at runtime like generateBrandedMetadata() already correctly does. Marked force-dynamic per Next's own docs: generated icon routes are statically cached by default unless they use a request-time API or dynamic config. - Greptile (P1): a whitelabeled deployment's favicon slots (16/32/192/512 PNGs, apple-touch-icon) fell back to the environment-tinted set instead of neutral branding when brand.faviconUrl was set. Now falls back to the production (neutral) asset set specifically for whitelabeled deployments, matching pre-PR behavior for those slots. - Greptile (P2, ico-serves-svg): verified via `git show origin/staging` that /favicon.ico -> /icon.svg is pre-existing, unchanged production behavior from before this PR - not addressed here, out of scope. Also: added app/api/favicon/route.ts to the check:api-validation INDIRECT_ZOD_ROUTES allowlist (no client-supplied input - only reads the server's own deployment-tier env var) and bumped the audit baseline.

waleedlatif1 · 2026-07-11T04:27:47Z

@greptile

waleedlatif1 · 2026-07-11T04:27:48Z

@cursor review

…l favicon Addresses the second round of review findings on PR #5588: - Cursor (High): app/layout.tsx exported a static `export const metadata = generateBrandedMetadata()`. Next.js treats a static metadata object as truly constant (eligible for build-time resolution on any statically-rendered route sharing the layout), so it doesn't know this one secretly depends on process.env via getDeploymentEnv() - same bake-in risk already fixed for next.config.ts. Converted to generateMetadata(), the documented way to force per-request resolution. - Greptile (P1): the earlier whitelabel fallback fixed the *tint* but not the underlying precedence gap - Sim's exact-size PNG/apple entries were still emitted alongside the tenant's generic sizes:'any' entry, and browsers can prefer the more specific size match. Now the tenant favicon fully replaces every icon slot instead of being appended alongside Sim's. - Cursor (Medium): app/api/favicon/route.ts never checked brand.faviconUrl, so a whitelabeled dev/staging deployment still served Sim's tinted icon at the legacy /favicon.ico path. Now checks brand.faviconUrl first, matching generateBrandedMetadata()'s precedence.

waleedlatif1 · 2026-07-11T04:41:12Z

@greptile

waleedlatif1 · 2026-07-11T04:41:13Z

@cursor review

…tEnv() test coverage Self-review pass, not a review-comment fix: - app/api/favicon/route.ts maintained its own env-to-svg-path map (FAVICON_DESTINATIONS) duplicating ICON_SETS in ee/whitelabeling/metadata.ts. The two could silently drift apart if an asset path ever changed in only one place. Exported ICON_SETS from metadata.ts (and the ee/whitelabeling barrel) and had the route reuse it directly instead. - Added lib/core/config/env-flags.test.ts covering getDeploymentEnv()'s resolution chain: each real AWS Secrets Manager value (dev/staging/prod), the APPCONFIG_ENVIRONMENT production->prod mapping, the NODE_ENV fallback, precedence ordering between the three signals, and an unrecognized-value case bucketing safely to development rather than throwing. This logic went through three rounds of review fixes: worth a regression net now that it's settled.

waleedlatif1 · 2026-07-11T04:51:00Z

@greptile

waleedlatif1 · 2026-07-11T04:51:01Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

2 issues from previous reviews remain unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 5412f70. Configure here.}

Addresses the third round of review findings on PR #5588: - Cursor (Medium): generateMetadata() alone does not force per-request evaluation - only the overall page being dynamic does, and a plain process.env read is not a Next.js "dynamic signal" Next can detect. Verified this doesn't matter for any route TODAY (0 of 1007 real page routes are static/ISR-eligible - only non-HTML RSS/sitemap/robots/OG files are), but it was a latent gap: any future static or ISR page added under the root layout would silently get build-time (production-tier) favicon metadata baked in, with no error. Added `export const dynamic = 'force-dynamic'` to app/layout.tsx, matching the same declaration already on app/manifest.ts and app/api/favicon/route.ts. Verified via full build that the 9 legitimately-static routes (RSS/sitemap/robots/OG-image files, which don't render through the root layout's page tree) are unaffected - same count, same routes, before and after. - Greptile (P1, reported as two separate threads): app/manifest.ts (PWA install/home-screen icons) never checked brand.faviconUrl at all, so a whitelabeled deployment would still show Sim's icon when installed even after the HTML favicon was fixed. Now uses brand.faviconUrl when set, same precedence as generateBrandedMetadata(). - Cursor (Low): the `brand.faviconUrl ? 'production' : getDeploymentEnv()` ternary computed a value never actually read when brand.faviconUrl is set (every consumer already short-circuits on brand.faviconUrl first). Simplified to always resolve the environment tier.

waleedlatif1 · 2026-07-11T05:08:36Z

@greptile

waleedlatif1 · 2026-07-11T05:08:37Z

@cursor review

…st env-tint Addresses the fourth round of review findings on PR #5588: - Cursor (High, confirmed via testing, not just code review): the previous commit's `export const dynamic = 'force-dynamic'` on app/layout.tsx would have forced the entire marketing site ((landing) route group - blog/library posts, changelog, solutions pages, all with their own `revalidate` exports) into per-request rendering, breaking their Lighthouse/LCP budget. My earlier "verified via build, no regression" claim was itself wrong: my local build masked this because NEXT_PUBLIC_APP_URL=localhost makes isHosted() false, which makes app/layout.tsx render next-runtime-env's RuntimePublicEnvScript - which unconditionally calls unstable_noStore() - already forcing marketing pages dynamic before my change even ran, for a completely unrelated pre-existing reason. Rebuilt with NEXT_PUBLIC_APP_URL=https://www.sim.ai (matching real hosted deployments, isHosted() true, the safe script path) to get an unmasked answer: confirmed force-dynamic was the actual regression (marketing pages went from ● to ƒ), and confirmed the revert restores them to ○ with their normal 1h revalidate window. Documented the accepted tradeoff instead: marketing-page favicon metadata may lag by up to one revalidate window after a deploy (bounded, self-healing at the next ISR regeneration, which runs in the live server process after bootstrap.ts has loaded secrets) - the actual workspace app (this feature's real audience) is already fully dynamic today (session/auth), so it needs no explicit marker. - Cursor (Medium): app/manifest.ts's non-whitelabeled branch still hardcoded production favicon paths instead of ICON_SETS[getDeploymentEnv()], so PWA install/home-screen icons stayed production-tinted while the HTML favicon was correctly env-tinted. Now uses the same per-tier set.

waleedlatif1 · 2026-07-11T05:44:24Z

@greptile

waleedlatif1 · 2026-07-11T05:44:25Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 1e42a73. Configure here.}

greptile-apps Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/ee/whitelabeling/metadata.ts Outdated

Comment thread apps/sim/next.config.ts Outdated

cursor Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/next.config.ts Outdated

vercel Bot temporarily deployed to Preview July 11, 2026 04:13 Inactive

greptile-apps Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/ee/whitelabeling/metadata.ts Outdated

cursor Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/ee/whitelabeling/metadata.ts Outdated

Comment thread apps/sim/app/api/favicon/route.ts

vercel Bot deployed to Preview July 11, 2026 04:31 View deployment

vercel Bot temporarily deployed to Preview July 11, 2026 04:40 Inactive

cursor Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/app/layout.tsx

Comment thread apps/sim/ee/whitelabeling/metadata.ts Outdated

greptile-apps Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/ee/whitelabeling/metadata.ts

vercel Bot temporarily deployed to Preview July 11, 2026 04:50 Inactive

greptile-apps Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/ee/whitelabeling/metadata.ts

cursor Bot reviewed Jul 11, 2026

View reviewed changes

vercel Bot temporarily deployed to Preview July 11, 2026 05:07 Inactive

cursor Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread apps/sim/app/layout.tsx Outdated

Comment thread apps/sim/app/manifest.ts

vercel Bot temporarily deployed to Preview July 11, 2026 05:43 Inactive

cursor Bot reviewed Jul 11, 2026

View reviewed changes

Uh oh!

Conversation

waleedlatif1 commented Jul 11, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

greptile-apps Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

waleedlatif1 commented Jul 11, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Jul 11, 2026 •

edited

Loading

cursor Bot commented Jul 11, 2026 •

edited

Loading

greptile-apps Bot commented Jul 11, 2026 •

edited

Loading