Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Description

  • fix runtime vars for docker deployments
  • client-side envvars need to be avaialble at build time but we weren't setting the variable in docker

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally and in CI (bun run test)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have updated version numbers as needed (if needed)
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Security Considerations:

  • My changes do not introduce any new security vulnerabilities
  • I have considered the security implications of my changes

@vercel
Copy link

vercel bot commented Jul 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 18, 2025 6:05pm
sim 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 18, 2025 6:05pm

@waleedlatif1 waleedlatif1 merged commit 11264ed into staging Jul 18, 2025
2 of 5 checks passed
@waleedlatif1 waleedlatif1 deleted the improvement/dock branch July 18, 2025 18:05
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR addresses a critical bug fix in the Docker deployment configuration related to client-side environment variables. The key changes are:

  • Added NEXT_PUBLIC_SOCKET_URL environment variable to both docker-compose.local.yml and docker-compose.prod.yml
  • This fixes a runtime issue where client-side code couldn't access the socket URL because it wasn't properly exposed at build time
  • The change aligns with Next.js conventions where client-side variables must be prefixed with NEXT_PUBLIC_

The change ensures proper WebSocket connectivity in production Docker deployments by making the socket URL available to client-side code during the build process.

Confidence score: 5/5

  1. This PR is very safe to merge as it follows Next.js best practices and fixes a clear environment configuration issue
  2. The changes are straightforward environment variable additions that don't affect runtime logic or security
  3. Files that need attention:
    • docker-compose.prod.yml - Verify the default URL is appropriate for production
    • docker-compose.local.yml - Ensure local development setup remains functional

2 files reviewed, no comments
Edit PR Review Bot Settings | Greptile

@delve-auditor
Copy link

delve-auditor bot commented Jul 18, 2025

No security or compliance issues detected. Reviewed everything up to 98add99.

Security Overview
  • 🔎 Scanned files: 2 changed file(s)
Detected Code Changes
Change Type Relevant files
Bug Fix ► docker

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

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.

2 participants