Customizable ports #962
Customizable ports #962
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces dynamic port prefixing via NEXT_PUBLIC_STACK_PORT_PREFIX across CI, Docker, apps, tests, and runtime bootstrap, replacing many fixed host ports (e.g., 8102/5432/2500/4318) with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ENV as ENV (NEXT_PUBLIC_STACK_PORT_PREFIX)
participant PREFIX as Port Helper / Polyfill
participant START as App/CI Startup
participant DOCKER as Docker Compose (host ports)
participant TESTS as E2E Tests / Snapshot Serializer
Note over ENV,PREFIX: configuration-time
ENV->>PREFIX: expose NEXT_PUBLIC_STACK_PORT_PREFIX
PREFIX->>START: expand placeholders in env files & process.env
START->>DOCKER: start services with computed host ports (${PREFIX}XX)
DOCKER-->>START: services reachable at localhost:${PREFIX}XX
START->>TESTS: tests use withPortPrefix()/localhostUrl()
TESTS-->>START: assertions/snapshots normalized (ports masked)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR changes the default ports for local development services from standard ports to custom ports in the 8128-8131 range. This allows developers to run Stack Auth's dependencies without conflicting with other commonly used services on their machines.
- PostgreSQL moved from 5432 to 8128
- Inbucket SMTP moved from 2500 to 8129
- Inbucket POP3 moved from 1100 to 8130
- OTLP collector moved from 4318 to 8131
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated pg_isready health check to use new PostgreSQL port 8128 |
| docs/templates/others/self-host.mdx | Updated self-hosting documentation with new PostgreSQL port mapping |
| docker/server/.env.example | Updated database connection strings to use port 8128 |
| docker/emulator/docker.compose.yaml | Configured custom ports for all services in the emulator compose file |
| docker/dependencies/docker.compose.yaml | Updated port mappings and internal references for all dependency services |
| claude/CLAUDE-KNOWLEDGE.md | Added new knowledge base entry documenting the custom port assignments |
| apps/e2e/tests/backend/workflows.test.ts | Updated test configuration to use new Inbucket SMTP port 8129 |
| apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts | Updated email configuration to use port 8129 |
| apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts | Updated test email configuration to use port 8129 |
| apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts | Updated expected port values in test assertions |
| apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts | Updated email configuration to use port 8129 |
| apps/e2e/.env.development | Updated database connection string to use port 8128 |
| apps/dev-launchpad/public/index.html | Updated UI documentation to display new port numbers |
| apps/backend/.env.development | Updated database and email port configurations |
| apps/backend/.env | Updated example configuration comments with new ports |
| .github/workflows/restart-dev-and-test.yaml | Updated CI database connection string |
| .github/workflows/e2e-source-of-truth-api-tests.yaml | Updated CI configuration for both databases and wait commands |
| .github/workflows/e2e-api-tests.yaml | Updated CI wait command for Inbucket |
| .github/workflows/docker-server-test.yaml | Updated Docker postgres port mapping |
| .github/workflows/check-prisma-migrations.yaml | Updated Prisma migration commands with new port |
| .devcontainer/devcontainer.json | Removed old port forwards as they're now in the 8100-8131 range |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| "Group": "Servers", | ||
| "Host": "db", | ||
| "Port": 5432, | ||
| "Port": 8129, |
There was a problem hiding this comment.
Port value should be 8128, not 8129. This pgAdmin configuration is connecting to the Postgres database, which is on port 8128 internally, not the Inbucket SMTP port 8129.
| "Port": 8129, | |
| "Port": 8128, |
docker/emulator/docker.compose.yaml
Outdated
| STACK_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe" | ||
| STACK_DIRECT_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe" |
There was a problem hiding this comment.
Database connection strings should use port 5432, not 8128. The 'db' hostname refers to the internal Docker network where the Postgres container listens on its default port 5432. Port 8128 is only the external host mapping.
| STACK_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe" | |
| STACK_DIRECT_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe" | |
| STACK_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:5432/stackframe" | |
| STACK_DIRECT_DATABASE_CONNECTION_STRING: "postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:5432/stackframe" |
| image: ankane/pghero:latest | ||
| environment: | ||
| DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:5432/stackframe | ||
| DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe |
There was a problem hiding this comment.
Database connection string should use port 5432, not 8128. The 'db' hostname refers to the internal Docker network where Postgres listens on its default port 5432. Port 8128 is only the external host mapping.
| DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe | |
| DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:5432/stackframe |
| WAIT_FOR: 'true' | ||
| SVIX_REDIS_DSN: redis://:PASSWORD-PLACEHOLDER--oVn8GSD6b9@svix-redis:6379 | ||
| SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:5432/svix | ||
| SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:8128/svix |
There was a problem hiding this comment.
Database connection string should use port 5432, not 8128. The 'svix-db' hostname refers to the internal Docker network where the svix Postgres container listens on its default port 5432. Port 8128 is only used for external host mappings.
| SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:8128/svix | |
| SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:5432/svix |
|
Greptile encountered an error while reviewing this PR. Please reach out to support@greptile.com for assistance. |
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 542d444..afae79b
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (21)
• .devcontainer/devcontainer.json
• .github/workflows/check-prisma-migrations.yaml
• .github/workflows/docker-server-test.yaml
• .github/workflows/e2e-api-tests.yaml
• .github/workflows/e2e-source-of-truth-api-tests.yaml
• .github/workflows/restart-dev-and-test.yaml
• apps/backend/.env
• apps/backend/.env.development
• apps/dev-launchpad/public/index.html
• apps/e2e/.env.development
• apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts
• apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts
• apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
• apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts
• apps/e2e/tests/backend/workflows.test.ts
• claude/CLAUDE-KNOWLEDGE.md
• docker/dependencies/docker.compose.yaml
• docker/emulator/docker.compose.yaml
• docker/server/.env.example
• docs/templates/others/self-host.mdx
• package.json
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.devcontainer/devcontainer.json(0 hunks).github/workflows/check-prisma-migrations.yaml(1 hunks).github/workflows/docker-server-test.yaml(1 hunks).github/workflows/e2e-api-tests.yaml(2 hunks).github/workflows/e2e-source-of-truth-api-tests.yaml(2 hunks).github/workflows/restart-dev-and-test.yaml(1 hunks)apps/backend/.env(2 hunks)apps/backend/.env.development(1 hunks)apps/dev-launchpad/public/index.html(1 hunks)apps/e2e/.env.development(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts(2 hunks)apps/e2e/tests/backend/workflows.test.ts(1 hunks)claude/CLAUDE-KNOWLEDGE.md(1 hunks)docker/dependencies/docker.compose.yaml(7 hunks)docker/emulator/docker.compose.yaml(5 hunks)docker/server/.env.example(1 hunks)docs/templates/others/self-host.mdx(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- .devcontainer/devcontainer.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
In tests, prefer .toMatchInlineSnapshot where possible; refer to snapshot-serializer.ts for snapshot formatting and handling of non-deterministic values
Files:
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/workflows.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/email.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.tsapps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/workflows.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/email.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.tsapps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
🪛 Checkov (3.2.334)
.github/workflows/e2e-api-tests.yaml
[medium] 20-21: Basic Auth Credentials
(CKV_SECRET_4)
.github/workflows/check-prisma-migrations.yaml
[medium] 65-66: Basic Auth Credentials
(CKV_SECRET_4)
docker/emulator/docker.compose.yaml
[medium] 24-25: Basic Auth Credentials
(CKV_SECRET_4)
[medium] 131-132: Basic Auth Credentials
(CKV_SECRET_4)
.github/workflows/e2e-source-of-truth-api-tests.yaml
[medium] 20-21: Basic Auth Credentials
(CKV_SECRET_4)
docker/dependencies/docker.compose.yaml
[medium] 24-25: Basic Auth Credentials
(CKV_SECRET_4)
[medium] 126-127: Basic Auth Credentials
(CKV_SECRET_4)
.github/workflows/restart-dev-and-test.yaml
[medium] 21-22: Basic Auth Credentials
(CKV_SECRET_4)
🪛 dotenv-linter (3.3.0)
apps/e2e/.env.development
[warning] 7-7: [UnorderedKey] The STACK_DIRECT_DATABASE_CONNECTION_STRING key should go before the STACK_INTERNAL_PROJECT_ADMIN_KEY key
(UnorderedKey)
apps/backend/.env
[warning] 39-39: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 40-40: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 41-41: [UnorderedKey] The STACK_EMAIL_PASSWORD key should go before the STACK_EMAIL_PORT key
(UnorderedKey)
[warning] 41-41: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 42-42: [UnorderedKey] The STACK_EMAIL_SENDER key should go before the STACK_EMAIL_USERNAME key
(UnorderedKey)
[warning] 42-42: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 43-43: [UnorderedKey] The STACK_EMAILABLE_API_KEY key should go before the STACK_EMAIL_HOST key
(UnorderedKey)
[warning] 43-43: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 47-47: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 79-79: [UnorderedKey] The OTEL_EXPORTER_OTLP_ENDPOINT key should go before the STACK_ACCESS_TOKEN_EXPIRATION_TIME key
(UnorderedKey)
[warning] 79-79: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build (22.x)
- GitHub Check: Vercel Agent Review
- GitHub Check: Cursor Bugbot
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (24)
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts (1)
12-12: LGTM! Port migration applied correctly.The email configuration port updates from 2500 to 8129 are consistent with the PR's systematic port remapping strategy for local development and testing environments.
Also applies to: 105-105
.github/workflows/restart-dev-and-test.yaml (1)
21-21: LGTM! Database port migration applied correctly.The PostgreSQL connection string port update from 5432 to 8128 aligns with the PR's port remapping strategy.
Note: The static analysis warning about credentials can be ignored—this is a placeholder password for the test environment.
apps/e2e/tests/backend/workflows.test.ts (1)
11-11: LGTM! Email server port migration applied correctly.The SMTP port update from 2500 to 8129 is consistent with the repository-wide port remapping.
apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts (1)
36-36: LGTM! Test snapshots updated correctly.The snapshot expectations for
sender_config.porthave been updated from 2500 to 8129, correctly reflecting the new SMTP port configuration.Also applies to: 51-51, 98-98
apps/dev-launchpad/public/index.html (1)
102-111: LGTM! Documentation updated with new port mappings.The background services port listings have been correctly updated to reflect the new development environment port scheme:
- PostgreSQL: 8128
- Inbucket SMTP: 8129
- Inbucket POP3: 8130
- OTel collector: 8131
docker/server/.env.example (1)
6-7: LGTM! Database connection strings updated correctly.Both database connection string environment variables have been updated from port 5432 to 8128, maintaining consistency with the new PostgreSQL port mapping.
apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts (1)
7-7: LGTM! Email configuration port updated correctly.The custom email configuration port has been updated from 2500 to 8129, aligning with the SMTP port remapping.
.github/workflows/e2e-api-tests.yaml (2)
20-20: LGTM! Database connection string port updated correctly.The PostgreSQL connection string port has been updated from 5432 to 8128, consistent with the repository-wide port remapping.
Note: The static analysis warning about credentials can be ignored—this is a placeholder password for the test environment.
97-97: LGTM! Inbucket wait target port updated correctly.The wait-on target for Inbucket has been updated from port 2500 to 8129, ensuring the CI pipeline waits for the correct SMTP service port.
claude/CLAUDE-KNOWLEDGE.md (1)
1-4: LGTM! Helpful documentation for the new port mappings.This knowledge base entry provides a clear reference for developers working with the local development environment.
apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts (2)
7-15: LGTM! SMTP port correctly updated to 8129.The test email configuration now references the new Inbucket SMTP port, consistent with the repository-wide port migration.
363-377: LGTM! Inline SMTP configuration updated consistently.The port change to 8129 matches the shared
testEmailConfigobject and maintains test consistency across the file..github/workflows/e2e-source-of-truth-api-tests.yaml (2)
20-22: LGTM! Database connection strings updated to port 8128.The PostgreSQL port migration is applied consistently to both the source-of-truth database and the direct database connection string. The static analysis warning about credentials is a false positive—these are test credentials used in the CI environment.
99-107: LGTM! Wait commands and database setup updated to new ports.The Inbucket wait command (port 8129) and psql commands (port 8128) are correctly aligned with the new port scheme.
apps/backend/.env.development (2)
27-28: LGTM! Database connection strings migrated to port 8128.Both the standard and direct database connection strings now reference the new PostgreSQL port.
30-35: LGTM! Email configuration updated to port 8129.The SMTP port for Inbucket is correctly updated to match the new local development port scheme.
apps/e2e/.env.development (1)
7-7: LGTM! Database connection string updated to port 8128.The E2E test environment now uses the new PostgreSQL port consistently with other configuration files. The static analysis warning about key ordering is cosmetic and doesn't affect functionality.
package.json (1)
29-30: LGTM! PostgreSQL readiness check updated to port 8128.The
pg_isreadycommand now checks the correct port for the local development PostgreSQL instance..github/workflows/docker-server-test.yaml (1)
20-24: LGTM! PostgreSQL Docker port mapping updated correctly.The host port is now 8128 while the container continues to use the standard PostgreSQL port 5432 internally. This mapping is consistent with the repository-wide port migration.
docs/templates/others/self-host.mdx (1)
33-36: LGTM! Self-hosting documentation updated with new port mapping.The PostgreSQL Docker example now uses the new port scheme (host port 8128 → container port 5432), ensuring consistency with the development environment and other documentation.
apps/backend/.env (1)
39-39: Port updates are correct and consistent.The changes properly remap the service ports across the environment file documentation:
- Line 39: Email port updated to 8129
- Lines 46–47: Database connection examples updated to port 8128
- Line 79: OpenTelemetry endpoint updated to 8131
All updates align with the repository-wide port remapping strategy.
Also applies to: 46-47, 79-79
.github/workflows/check-prisma-migrations.yaml (1)
65-65: Workflow database ports correctly updated.Both Prisma operations (schema migration metadata removal and migration diff) now use the remapped PostgreSQL port 8128:
- Line 65: Auto-migration metadata removal
- Line 70: Migration diff command with shadow database
Updates are consistent with the repository-wide port remapping.
Also applies to: 70-70
docker/dependencies/docker.compose.yaml (1)
13-13: Service port mappings are correct across the stack.Port updates are consistent and properly aligned:
- Line 13: PostgreSQL external port 8128
- Line 24: PgHero connects to db:8128 (database service)
- Line 76: PgMeta DB port 8128
- Lines 87, 89: Inbucket SMTP (8129) and POP3 (8130)
- Line 101: Jaeger OTLP collector (8131)
- Line 126: Svix connects to svix-db:8128
Also applies to: 24-24, 76-76, 87-87, 89-89, 101-101, 126-126
docker/emulator/docker.compose.yaml (1)
24-25: Emulator port configuration is correctly remapped.All port references in the emulator stack are consistently updated:
- Lines 24–25: Database connection strings use port 8128
- Line 39: Email service port 8129
- Line 84: PostgreSQL server configured to listen on port 8128
- Line 92: Healthcheck uses correct port 8128
- Lines 101–103: Inbucket SMTP (8129) and POP3 (8130) environment variables
- Line 131: Svix database connection uses port 8128
Configuration is consistent and internally coherent.
Also applies to: 39-39, 84-84, 92-92, 101-103, 131-131
| image: ankane/pghero:latest | ||
| environment: | ||
| DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:5432/stackframe | ||
| DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe |
There was a problem hiding this comment.
The pghero service is incorrectly configured to connect to PostgreSQL using port 8128, but it should use the internal Docker network port 5432.
View Details
📝 Patch Details
diff --git a/docker/dependencies/docker.compose.yaml b/docker/dependencies/docker.compose.yaml
index d230bdf7..0b21ecfe 100644
--- a/docker/dependencies/docker.compose.yaml
+++ b/docker/dependencies/docker.compose.yaml
@@ -21,7 +21,7 @@ services:
pghero:
image: ankane/pghero:latest
environment:
- DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe
+ DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:5432/stackframe
ports:
- 8116:8080
@@ -73,7 +73,7 @@ services:
environment:
PG_META_PORT: 8080
PG_META_DB_HOST: db
- PG_META_DB_PORT: 8128
+ PG_META_DB_PORT: 5432
PG_META_DB_NAME: stackframe
PG_META_DB_USER: postgres
PG_META_DB_PASSWORD: PASSWORD-PLACEHOLDER--uqfEC1hmmv
@@ -123,7 +123,7 @@ services:
environment:
WAIT_FOR: 'true'
SVIX_REDIS_DSN: redis://:PASSWORD-PLACEHOLDER--oVn8GSD6b9@svix-redis:6379
- SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:8128/svix
+ SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:5432/svix
SVIX_CACHE_TYPE: memory
SVIX_JWT_SECRET: secret
SVIX_LOG_LEVEL: trace
@@ -245,7 +245,7 @@ configs:
"Name": "Local Postgres DB",
"Group": "Servers",
"Host": "db",
- "Port": 8129,
+ "Port": 5432,
"Username": "postgres",
"PasswordExecCommand": "echo 'PASSWORD-PLACEHOLDER--uqfEC1hmmv'",
"MaintenanceDB": "stackframe"
Analysis
Docker Compose services use incorrect PostgreSQL ports for internal network communication
What fails: Multiple services (pghero, supabase-meta, svix-server, pgadmin) attempt to connect to PostgreSQL using external port mappings (8128, 8129) instead of internal container port 5432
How to reproduce:
cd docker/dependencies
docker compose up pghero svix-server supabase-meta
# Services fail to connect to PostgreSQL with connection errorsResult: Connection failures because PostgreSQL containers internally listen on port 5432, but services try to connect to non-existent ports 8128/8129
Expected: Services should connect using internal container port 5432 per Docker Compose networking documentation - internal service communication uses container ports, not host-mapped ports
Evidence: The svix-db service has no port mappings at all, yet svix-server was trying to connect to svix-db:8128, proving the configuration was incorrect
| "Group": "Servers", | ||
| "Host": "db", | ||
| "Port": 5432, | ||
| "Port": 8129, |
There was a problem hiding this comment.
The pgAdmin server configuration specifies the wrong port 8129 for PostgreSQL, but it should use 5432.
View Details
📝 Patch Details
diff --git a/docker/dependencies/docker.compose.yaml b/docker/dependencies/docker.compose.yaml
index d230bdf7..0b21ecfe 100644
--- a/docker/dependencies/docker.compose.yaml
+++ b/docker/dependencies/docker.compose.yaml
@@ -21,7 +21,7 @@ services:
pghero:
image: ankane/pghero:latest
environment:
- DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:8128/stackframe
+ DATABASE_URL: postgres://postgres:PASSWORD-PLACEHOLDER--uqfEC1hmmv@db:5432/stackframe
ports:
- 8116:8080
@@ -73,7 +73,7 @@ services:
environment:
PG_META_PORT: 8080
PG_META_DB_HOST: db
- PG_META_DB_PORT: 8128
+ PG_META_DB_PORT: 5432
PG_META_DB_NAME: stackframe
PG_META_DB_USER: postgres
PG_META_DB_PASSWORD: PASSWORD-PLACEHOLDER--uqfEC1hmmv
@@ -123,7 +123,7 @@ services:
environment:
WAIT_FOR: 'true'
SVIX_REDIS_DSN: redis://:PASSWORD-PLACEHOLDER--oVn8GSD6b9@svix-redis:6379
- SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:8128/svix
+ SVIX_DB_DSN: postgres://postgres:PASSWORD-PLACEHOLDER--KsoIMcchtp@svix-db:5432/svix
SVIX_CACHE_TYPE: memory
SVIX_JWT_SECRET: secret
SVIX_LOG_LEVEL: trace
@@ -245,7 +245,7 @@ configs:
"Name": "Local Postgres DB",
"Group": "Servers",
"Host": "db",
- "Port": 8129,
+ "Port": 5432,
"Username": "postgres",
"PasswordExecCommand": "echo 'PASSWORD-PLACEHOLDER--uqfEC1hmmv'",
"MaintenanceDB": "stackframe"
Analysis
Incorrect PostgreSQL port configuration in Docker Compose services
What fails: Multiple services in docker/dependencies/docker.compose.yaml use incorrect ports when connecting to PostgreSQL databases within the Docker network
How to reproduce:
- Start the Docker Compose stack:
docker-compose -f docker/dependencies/docker.compose.yaml up - Check service logs for connection errors to PostgreSQL databases
- PgAdmin will fail to connect to database when accessed via web interface
Result: Services fail to connect to PostgreSQL because they reference external host ports (8128/8129) instead of internal container port (5432)
Expected: Services should use port 5432 for inter-service communication per Docker Compose networking docs
Services affected:
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.ts (1)
141-141: Hardcoded port in issuer URL should use port prefix placeholder for consistency.The
issparameter at line 141 contains hardcoded port8102while the same parameter type at line 181 uses theSTACK_PORT_PREFIX02placeholder. This creates an inconsistency in snapshot expectations and should be updated tohttp%3A%2F%2Flocalhost%3ASTACK_PORT_PREFIX02%2Fapi%2Fv1%2Fintegrations%2Fneon%2Foauth%2Fidpto match the pattern used elsewhere in the file.
♻️ Duplicate comments (1)
docker/dependencies/docker.compose.yaml (1)
76-76: Critical: Internal Docker network connection uses wrong port.Line 76 configures
PG_META_DB_PORT: 8128, but thesupabase-metaservice connects to thedbservice via Docker's internal network where PostgreSQL listens on its default container port5432. Port 8128 is only the external host mapping and is not accessible within the Docker network.This misconfiguration will cause
supabase-metato fail when attempting to connect to the database.Apply this fix:
- PG_META_DB_PORT: 8128 + PG_META_DB_PORT: 5432
🧹 Nitpick comments (6)
examples/convex/package.json (1)
8-8: Consider documenting the STACK_PORT_PREFIX convention in README or inline.The scripts use
STACK_PORT_PREFIXto construct service ports via string concatenation (e.g.,${STACK_PORT_PREFIX:-81}27→ 8127). This pattern works but may not be immediately obvious to developers unfamiliar with the port scheme.Consider adding a brief comment or linking to documentation (e.g., CLAUDE-KNOWLEDGE.md mentioned in the PR) that explains valid prefix values and the port mapping for this example.
Also applies to: 11-11, 13-13
apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts (2)
281-286: Consider using thecreateClientDataJSONhelper for consistency.This inline
btoa(JSON.stringify(...))pattern duplicates logic that thecreateClientDataJSONhelper already provides. Using the helper would improve maintainability and ensure consistent clientDataJSON structure across all tests.Apply this diff:
- "clientDataJSON": btoa(JSON.stringify({ - type: "webauthn.create", - challenge: "TU9DSw", - origin: passkeyOrigin, // Doesn't match https://app.production.com - crossOrigin: false - })), + "clientDataJSON": createClientDataJSON(passkeyOrigin, "webauthn.create"), // Doesn't match https://app.production.com
359-364: Consider using thecreateClientDataJSONhelper for consistency.Similar to the previous occurrence, this inline
btoa(JSON.stringify(...))pattern should use thecreateClientDataJSONhelper to maintain consistency and reduce duplication.Apply this diff:
- "clientDataJSON": btoa(JSON.stringify({ - type: "webauthn.get", - challenge: "TU9DSw", - origin: passkeyOrigin, // Doesn't match *.example.com - crossOrigin: false - })), + "clientDataJSON": createClientDataJSON(passkeyOrigin), // Doesn't match *.example.com.github/workflows/e2e-custom-base-port-api-tests.yaml (2)
28-28: Consider updating to actions/checkout@v4 and actions/setup-node@v4.The workflow uses older v3 versions of GitHub Actions that have Node.js 16 runners, which are deprecated. Upgrading to v4 will use Node.js 20 runners and ensure future compatibility.
Apply this diff:
steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Setup Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }}Also applies to: 31-31
20-20: Note: Database connection string contains credentials.While this appears to be a placeholder password, consider using GitHub secrets for sensitive values even in test environments to follow security best practices.
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts (1)
4-6: Consider clarifying the constant name.The
oauthMockServerPortconstant is used in a test case that expects the OAuth flow to fail with a non-matching wildcard pattern. The name suggests it represents the actual OAuth mock server port, but it's actually used to construct an intentionally non-matching domain pattern. Consider renaming to something likenonMatchingPortor adding a comment to clarify its purpose.-const oauthMockServerPort = withPortPrefix("07"); +// Port used to construct a non-matching domain pattern in negative test cases +const oauthMockServerPort = withPortPrefix("07");Also applies to: 290-290
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
.claude/settings.json(1 hunks).github/workflows/e2e-custom-base-port-api-tests.yaml(1 hunks)AGENTS.md(1 hunks)apps/backend/package.json(2 hunks)apps/backend/src/lib/freestyle.tsx(1 hunks)apps/backend/src/lib/stripe.tsx(1 hunks)apps/dashboard/package.json(1 hunks)apps/dev-launchpad/.gitignore(1 hunks)apps/dev-launchpad/package.json(1 hunks)apps/dev-launchpad/public/index.html(14 hunks)apps/dev-launchpad/scripts/write-env-config.js(1 hunks)apps/e2e/tests/backend/backend-helpers.ts(4 hunks)apps/e2e/tests/backend/endpoints/api/migration-tests.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts(10 hunks)apps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.ts(6 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/oauth.test.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts(4 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/teams.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts(4 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(4 hunks)apps/e2e/tests/backend/workflows.test.ts(2 hunks)apps/e2e/tests/helpers/ports.ts(1 hunks)apps/e2e/tests/js/payments.test.ts(2 hunks)apps/e2e/tests/snapshot-serializer.ts(2 hunks)apps/mock-oauth-server/src/index.ts(2 hunks)claude/CLAUDE-KNOWLEDGE.md(1 hunks)docker/dependencies/docker.compose.yaml(13 hunks)docker/server/entrypoint.sh(1 hunks)docs/package.json(1 hunks)examples/cjs-test/package.json(1 hunks)examples/convex/package.json(1 hunks)examples/convex/stack/client.tsx(0 hunks)examples/demo/package.json(1 hunks)examples/docs-examples/package.json(1 hunks)examples/e-commerce/package.json(1 hunks)examples/js-example/package.json(1 hunks)examples/middleware/package.json(1 hunks)examples/partial-prerendering/package.json(1 hunks)examples/react-example/package.json(1 hunks)examples/supabase/package.json(1 hunks)package.json(1 hunks)packages/stack-shared/src/utils/strings.nicify.test.ts(1 hunks)packages/stack-shared/src/utils/strings.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- examples/convex/stack/client.tsx
✅ Files skipped from review due to trivial changes (4)
- apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/oauth.test.ts
- packages/stack-shared/src/utils/strings.tsx
- apps/dev-launchpad/.gitignore
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
- apps/e2e/tests/backend/workflows.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/internal/email.test.ts
- package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
In tests, prefer .toMatchInlineSnapshot where possible; refer to snapshot-serializer.ts for snapshot formatting and handling of non-deterministic values
Files:
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.tspackages/stack-shared/src/utils/strings.nicify.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.tsapps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.tsapps/e2e/tests/backend/endpoints/api/v1/users.test.tsapps/e2e/tests/backend/endpoints/api/v1/projects.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.tsapps/e2e/tests/backend/endpoints/api/v1/teams.test.tsapps/e2e/tests/backend/endpoints/api/migration-tests.test.tsapps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.tsapps/e2e/tests/js/payments.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.tsapps/mock-oauth-server/src/index.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.tsapps/e2e/tests/snapshot-serializer.tspackages/stack-shared/src/utils/strings.nicify.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.tsapps/backend/src/lib/stripe.tsxapps/dev-launchpad/scripts/write-env-config.jsapps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.tsapps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.tsapps/e2e/tests/backend/endpoints/api/v1/users.test.tsapps/e2e/tests/helpers/ports.tsapps/e2e/tests/backend/endpoints/api/v1/projects.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.tsapps/e2e/tests/backend/endpoints/api/v1/teams.test.tsapps/e2e/tests/backend/endpoints/api/migration-tests.test.tsapps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/backend-helpers.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.tsapps/backend/src/lib/freestyle.tsxapps/e2e/tests/js/payments.test.ts
🧬 Code graph analysis (15)
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
packages/stack-shared/src/utils/strings.nicify.test.ts (1)
packages/stack-shared/src/utils/strings.tsx (1)
nicify(463-637)
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
apps/backend/src/lib/stripe.tsx (1)
packages/stack-shared/src/utils/env.tsx (1)
getEnvVariable(16-58)
apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
apps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.ts (2)
apps/e2e/tests/helpers/ports.ts (1)
localhostUrl(11-12)apps/e2e/tests/helpers.ts (1)
niceFetch(168-192)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
localhostUrl(11-12)
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
localhostUrl(11-12)
apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts (2)
apps/e2e/tests/helpers/ports.ts (2)
withPortPrefix(6-6)localhostUrl(11-12)apps/e2e/tests/backend/backend-helpers.ts (1)
niceBackendFetch(108-172)
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts (1)
apps/e2e/tests/helpers/ports.ts (2)
withPortPrefix(6-6)localhostUrl(11-12)
apps/e2e/tests/backend/backend-helpers.ts (1)
apps/e2e/tests/helpers/ports.ts (2)
localhostUrl(11-12)withPortPrefix(6-6)
apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
apps/backend/src/lib/freestyle.tsx (2)
apps/dev-launchpad/scripts/write-env-config.js (1)
prefix(6-6)packages/stack-shared/src/utils/env.tsx (1)
getEnvVariable(16-58)
apps/e2e/tests/js/payments.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
🪛 actionlint (1.7.8)
.github/workflows/e2e-custom-base-port-api-tests.yaml
16-16: label "ubicloud-standard-8" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
28-28: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
31-31: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Biome (2.1.2)
apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts
[error] 330-330: unexpected token \
(parse)
[error] 330-330: unterminated string literal
The closing quote must be on the same line.
(parse)
🪛 Checkov (3.2.334)
.github/workflows/e2e-custom-base-port-api-tests.yaml
[medium] 20-21: Basic Auth Credentials
(CKV_SECRET_4)
🪛 LanguageTool
AGENTS.md
[grammar] ~37-~37: There might be a mistake here.
Context: ...STACK_PORT_PREFIX>02(defaults to 8102) - Main API routes in/apps/backend/src/ap...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...efaults to 8102) - Main API routes in /apps/backend/src/app/api/latest - Database models using Prisma - **dashboa...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...latest - Database models using Prisma - **dashboard** (/apps/dashboard`): Admin d...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...STACK_PORT_PREFIX>01` (defaults to 8101) - dev-launchpad: Development portal on p...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...STACK_PORT_PREFIX>00 (defaults to 8100) - **e2e**: End-to-end tests ### Packages (...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: Vercel Agent Review
- GitHub Check: Cursor Bugbot
- GitHub Check: setup-tests
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (41)
examples/cjs-test/package.json (1)
6-8: LGTM!Both script modifications correctly implement the dynamic port prefix pattern. The bash parameter expansion syntax is sound, defaults to the original port (8110) when
STACK_PORT_PREFIXis unset, and maintains consistency acrossdevandstartscripts. This aligns well with the repository-wide customizable ports strategy.examples/middleware/package.json (1)
6-6: Consistent implementation of STACK_PORT_PREFIX pattern.The port configuration correctly implements the dynamic prefix pattern using bash parameter expansion. The default fallback to
81produces port8112, maintaining backward compatibility while enabling runtime customization.Also applies to: 8-8
examples/e-commerce/package.json (1)
6-6: Dynamic port configuration follows project pattern well.The npm scripts correctly use the
STACK_PORT_PREFIXfallback pattern with suffix11, defaulting to port 8111. This aligns with the PR's port customization scheme and maintains consistency with other services in the stack.Please confirm that the hardcoded suffix
11is documented in the knowledge base (added in this PR) to ensure developers understand the port allocation scheme across different services (e.g., why 8111 for e-commerce, 8128 for PostgreSQL, etc.).Also applies to: 8-8
AGENTS.md (1)
37-41: Architecture documentation correctly updated for dynamic port configuration.The port references now use the stack-prefixed notation (
<$STACK_PORT_PREFIX>02,<$STACK_PORT_PREFIX>01,<$STACK_PORT_PREFIX>00) with appropriate defaults, aligning with the PR's port remapping scheme. The grammar warnings from static analysis appear to be false positives triggered by the template syntax; the markdown structure is correct.examples/convex/package.json (2)
8-8: Port remapping in dev/start scripts looks good.The use of
${STACK_PORT_PREFIX:-81}with suffix27correctly generates port 8127 for the Next.js frontend in both dev and start modes. The string interpolation pattern is consistent.Also applies to: 13-13
11-11: Verify port 8102 for the backend API is correct and documented.The script constructs
NEXT_PUBLIC_STACK_API_URL=http://localhost:${PREFIX}02which defaults to port 8102. However, the PR description lists port mappings for PostgreSQL (8128), Inbucket SMTP (8129), POP3 (8130), and OTLP (8131), but does not explicitly document port 8102 as the backend API port.Additionally, the script hardcodes port 3210 for the
wait-oncheck (Convex dev server), which is not part of the port remapping scheme—confirm this is intentional.Please verify:
- Is port 8102 the intended backend API port for this Convex example?
- Should the hardcoded
3210also be configurable viaSTACK_PORT_PREFIX, or is it intentionally fixed?- Are the port mappings for this example documented in the PR description or a related knowledge base document?
examples/supabase/package.json (1)
6-6: Approve: Port variables correctly updated for customizable prefix.The dynamic port formula
${STACK_PORT_PREFIX:-81}15in both dev and start scripts is syntactically correct and maintains backward compatibility by defaulting to port8115(the previous fixed port) while enabling customization via theSTACK_PORT_PREFIXenvironment variable. Both lines are internally consistent.However, verify that no other example applications or configuration files in the repository require similar port customization updates for consistency across the codebase.
apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts (3)
3-7: LGTM! Dynamic port configuration is properly implemented.The use of
withPortPrefixandlocalhostUrlhelpers correctly replaces hard-coded localhost:8103 references, making the tests resilient to port changes.
9-22: LGTM! Well-structured helper reduces duplication.The
createClientDataJSONhelper effectively centralizes the generation of base64-encoded clientDataJSON payloads and follows Node.js conventions withBuffer.from().toString("base64").
120-120: LGTM! Dynamic origin and port usage is correctly implemented across all test cases.All test scenarios properly reference
passkeyPortandpasskeyOriginin wildcard configurations, clientDataJSON payloads, and comments. The dynamic approach ensures tests remain valid when port mappings change.Also applies to: 148-148, 181-181, 184-184, 191-191, 219-219, 393-393, 403-403, 431-431
packages/stack-shared/src/utils/strings.nicify.test.ts (1)
154-161: Tests are correct and directly validatenicify's template literal escaping behavior.The tests verify the escaping that occurs at line 532 of
strings.tsxwherenicifyusesescapeTemplateLiteralwhen constructing template literals:
- Line 155-157: Correctly verifies lone
$is not escaped (only${triggers interpolation)- Line 159-161: Correctly verifies
${is escaped to\${to prevent unwanted template interpolationThese tests are properly placed and necessary to validate
nicify's string handling logic.examples/js-example/package.json (1)
8-8: LGTM! Port prefix implementation looks correct.The dynamic port computation using
${STACK_PORT_PREFIX:-81}19correctly defaults to port 8119 and aligns with the project-wide port customization strategy.examples/react-example/package.json (1)
7-7: LGTM! Consistent port prefix implementation.The dynamic port derivation matches the pattern used across other examples, correctly defaulting to port 8120.
apps/e2e/tests/backend/endpoints/api/migration-tests.test.ts (1)
25-25: LGTM! Snapshot correctly updated for dynamic ports.The snapshot now uses the
<$STACK_PORT_PREFIX>placeholder syntax, which aligns with the project's snapshot serialization strategy for handling dynamic port values in test expectations.apps/backend/src/lib/stripe.tsx (1)
12-16: LGTM! Dynamic Stripe mock port configuration is correctly implemented.The port computation using
Number(\${stackPortPrefix}23`)correctly produces port 8123 by default. Since this is only active in development/test environments (controlled byuseStripeMock`), the implementation is appropriate.examples/docs-examples/package.json (1)
9-11: LGTM! Dev and start scripts correctly use dynamic ports.Both scripts properly compute the port using
${STACK_PORT_PREFIX:-81}08, defaulting to 8108, consistent with the project-wide port prefix strategy.examples/partial-prerendering/package.json (1)
6-8: LGTM! Port prefix correctly applied to both dev and start scripts.The dynamic port computation using
${STACK_PORT_PREFIX:-81}09is consistent with the broader implementation pattern, correctly defaulting to port 8109.docs/package.json (1)
10-11: LGTM! Documentation server ports correctly configured.Both dev and start scripts properly use
${STACK_PORT_PREFIX:-81}04for port 8104, maintaining consistency with the port customization strategy.claude/CLAUDE-KNOWLEDGE.md (1)
1-4: LGTM! Clear documentation of the port prefix scheme.The Q&A format effectively documents how development ports are derived using
STACK_PORT_PREFIX. This provides valuable context for developers working with the customizable port configuration.apps/backend/package.json (1)
10-10: LGTM! Dynamic port configuration implemented correctly.The shell parameter expansion syntax
${STACK_PORT_PREFIX:-81}02is correct and maintains backward compatibility with the default prefix of81(resulting in port 8102). The pattern is applied consistently across dev, start, and prisma-studio scripts.Also applies to: 15-15, 24-24
apps/dashboard/package.json (1)
10-10: LGTM! Consistent dynamic port implementation.The dynamic port configuration follows the same pattern as the backend package, maintaining consistency across the monorepo.
Also applies to: 14-14
apps/e2e/tests/backend/endpoints/api/v1/teams.test.ts (1)
536-536: LGTM! Snapshot expectations updated for dynamic ports.The snapshot expectations now use the
<$STACK_PORT_PREFIX>21placeholder pattern, which aligns with the coding guideline to use snapshot serializers for handling non-deterministic values. This ensures tests remain stable across different port configurations.Also applies to: 556-556
.claude/settings.json (1)
24-24: LGTM! Dynamic port check with improved error messaging.The reachability check correctly computes the dynamic port and includes it in the error message, which improves debugging when the port prefix is customized.
apps/dev-launchpad/scripts/write-env-config.js (1)
1-11: LGTM! Clean build-time configuration script.The script correctly reads
STACK_PORT_PREFIXfrom the environment, properly escapes the value usingJSON.stringify, and writes it to a public JavaScript file for runtime consumption. The synchronouswriteFileSyncis appropriate for this build-time script.apps/dev-launchpad/package.json (1)
6-6: LGTM! Properly sequenced dev script with dynamic port.The script correctly generates the environment configuration before starting the server, and the
&&operator ensures the server won't start if the config generation fails. The dynamic port follows the established pattern.examples/demo/package.json (1)
9-9: LGTM! Consistent implementation across examples.The dynamic port configuration follows the established pattern and maintains backward compatibility with the default port 8103.
Also applies to: 11-11
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.ts (1)
20-20: LGTM! Snapshot correctly uses dynamic port token.The confirmation URL now uses the dynamic
STACK_PORT_PREFIX01token, which aligns with the snapshot serializer's port-masking strategy and the broader PR objective of customizable ports.apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts (1)
399-399: LGTM! Port field correctly stripped in snapshots.The port field in email configuration snapshots is now masked as
<stripped field 'port'>, consistent with the snapshot serializer's stripFields array. This makes the tests resilient to port configuration changes.Also applies to: 459-459, 611-611
apps/backend/src/lib/freestyle.tsx (1)
18-19: LGTM! Dynamic port construction maintains backward compatibility.The mock Freestyle baseUrl now uses a dynamic port prefix (default "81") concatenated with "22" to form port 8122, maintaining backward compatibility while supporting the customizable port scheme.
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts (1)
3-3: LGTM! Dynamic port helper correctly integrated.The test now uses
withPortPrefix("01")to construct the expected purchase URL pattern dynamically, replacing the hardcoded port. The RegExp construction is correct and aligns with the customizable ports strategy.Also applies to: 330-330
apps/e2e/tests/snapshot-serializer.ts (1)
68-68: LGTM! Port field added to stripFields.Adding "port" to the stripFields array ensures port values are consistently masked in snapshots as
<stripped field 'port'>, making tests resilient to port configuration changes.apps/e2e/tests/backend/endpoints/api/v1/connected-accounts.test.ts (1)
2-2: LGTM! Well-structured helper for dynamic OAuth URLs.The
mockOAuthUrlhelper cleanly centralizes OAuth mock server URL construction with dynamic ports. All hardcoded URLs are consistently replaced, improving maintainability while supporting the customizable ports strategy.Also applies to: 5-5, 27-27, 58-58, 66-66, 90-90, 108-108, 139-139, 147-147
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts (1)
13-13: LGTM! Email port configuration correctly uses dynamic port.The SMTP port now uses
Number(withPortPrefix("29"))which resolves to 8129 (with default prefix "81"), correctly aligning with the updated Inbucket SMTP port from the PR's port remapping strategy.Also applies to: 107-107
docker/server/entrypoint.sh (1)
24-30: LGTM! Clean port prefix configuration.The environment variable setup correctly derives port values from STACK_PORT_PREFIX with a sensible default of "81", making the service ports configurable while maintaining backward compatibility.
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
4-4: LGTM! Consistent adoption of port helpers.The changes correctly use the
localhostUrlhelper for constructing URLs in both test payloads and assertions, with corresponding snapshot updates reflecting the dynamic port prefix pattern.Also applies to: 702-723, 1938-1948
apps/e2e/tests/helpers/ports.ts (1)
1-12: LGTM! Clean and focused helper module.The port prefix utilities are well-designed and properly documented. The implementation correctly reads the environment variable with a sensible default and provides convenient functions for constructing prefixed ports and URLs.
apps/e2e/tests/backend/backend-helpers.ts (2)
649-657: Improved URL validation with proper parsing.The refactor from hardcoded string checks to URL parsing with dynamic origin validation is a cleaner approach that better handles the configurable port prefix while making the test intent clearer.
1488-1488: Dynamic port prefix in RegExp.Good use of
withPortPrefixto construct the expected URL pattern dynamically. The RegExp correctly validates the purchase URL format with the configurable port.apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts (1)
8-13: Better test assertions with URL parsing.Extracting the Location header, asserting its presence, and then parsing it as a URL to check origin and pathname separately is more robust than the previous approach. This makes the test more maintainable and the assertions clearer.
apps/mock-oauth-server/src/index.ts (2)
6-10: LGTM! Port prefix integration in mock OAuth server.The port computation logic correctly adopts the STACK_PORT_PREFIX scheme, deriving the mock OAuth server port (prefix + "14") and backend redirect port (prefix + "02"). The fallback defaults are sensible.
26-26: Consistent redirect URI construction.Using
backendPortForRedirectsfor the OAuth callback URI ensures the mock server redirects to the correct backend port based on the configured prefix.
| with: | ||
| run: pnpm run start:backend --log-order=stream & | ||
| wait-on: | | ||
| http://localhost:8102 |
There was a problem hiding this comment.
Critical: Port mismatch in wait-on URLs.
The workflow sets STACK_PORT_PREFIX: "67" (Line 21), but the wait-on URLs are hardcoded to http://localhost:8102. With a prefix of 67, the backend should be running on port 6702. This mismatch will cause the workflow to fail as it waits for services on the wrong port.
Apply this diff to fix the port references:
- name: Start stack-backend in background
uses: JarvusInnovations/background-action@v1.0.7
with:
run: pnpm run start:backend --log-order=stream &
wait-on: |
- http://localhost:8102
+ http://localhost:6702
tail: true
wait-for: 30s
log-output-if: true
- name: Start stack-dashboard in background
uses: JarvusInnovations/background-action@v1.0.7
with:
run: pnpm run start:dashboard --log-order=stream &
wait-on: |
- http://localhost:8102
+ http://localhost:6701
tail: true
wait-for: 30s
log-output-if: true
- name: Start mock-oauth-server in background
uses: JarvusInnovations/background-action@v1.0.7
with:
run: pnpm run start:mock-oauth-server --log-order=stream &
wait-on: |
- http://localhost:8102
+ http://localhost:6707
tail: true
wait-for: 30s
log-output-if: trueAlso applies to: 120-120, 129-129
🤖 Prompt for AI Agents
.github/workflows/e2e-custom-base-port-api-tests.yaml around line 111 (also
applies to lines 120 and 129): the wait-on URLs are hardcoded to
http://localhost:8102 while the workflow sets STACK_PORT_PREFIX: "67", so update
those URLs to use the correct port constructed from the prefix (e.g., 6702) or
interpolate the STACK_PORT_PREFIX env var into the port (STACK_PORT_PREFIX +
"02") so the wait-on targets match the actual backend port.
| }); | ||
| expect(response.status).toBe(200); | ||
| expect(response.body.url).toMatch(/^https?:\/\/localhost:8101\/purchase\/[a-z0-9-_]+$/); | ||
| expect(response.body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\/purchase\/[a-z0-9-_]+$`)); |
There was a problem hiding this comment.
Remove unnecessary escape characters in regex pattern.
Forward slashes don't need escaping in template literals passed to the RegExp constructor.
Apply this fix:
- expect(response.body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\\/purchase\\/[a-z0-9-_]+$`));
+ expect(response.body.url).toMatch(new RegExp(`^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$`));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(response.body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\/purchase\/[a-z0-9-_]+$`)); | |
| expect(response.body.url).toMatch(new RegExp(`^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$`)); |
🤖 Prompt for AI Agents
In
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts
around line 279, the RegExp string unnecessarily escapes forward slashes; remove
the backslashes before "/" in the template literal so the pattern becomes new
RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$`)
(keep the escaping for the backslashes in "https?:\\/\\/" and do not escape the
forward slash before "purchase").
| expect(response.status).toBe(200); | ||
| const body = response.body as { url: string }; | ||
| expect(body.url).toMatch(/^https?:\/\/localhost:8101\/purchase\/[a-z0-9-_]+$/); | ||
| expect(body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\/purchase\/[a-z0-9-_]+$`)); |
There was a problem hiding this comment.
Remove unnecessary escape characters in regex pattern.
Forward slashes don't need escaping in template literals passed to the RegExp constructor.
Apply this fix:
- expect(body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\\/purchase\\/[a-z0-9-_]+$`));
+ expect(body.url).toMatch(new RegExp(`^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$`));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts
around line 317, the regex string passed to RegExp unnecessarily escapes forward
slashes; update the template literal to remove the backslashes before '/' so the
pattern reads something like
^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$ (i.e. remove
all '\/' escapes), leaving other necessary escapes intact.
apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts
Show resolved
Hide resolved
apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts
Outdated
Show resolved
Hide resolved
apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts
Outdated
Show resolved
Hide resolved
| const expectedUnsubscribeLink = localhostUrl("02", "/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E"); | ||
| expect(sentEmail!.body?.html).toBe(expectedUnsubscribeLink); |
There was a problem hiding this comment.
Verify the email HTML assertion logic.
The test asserts that sentEmail!.body?.html exactly equals expectedUnsubscribeLink (a URL string), but later at line 58 it extracts a link from the HTML using a regex pattern href="([^"]+)". This suggests the HTML should contain HTML markup with an href attribute, not be a plain URL string. The exact equality check appears incorrect.
Please verify whether:
- The email body HTML is actually just a plain URL (seems unusual)
- The assertion should check if the HTML contains the link, not equals it
- The snapshot serializer is somehow transforming the HTML to just the URL
#!/bin/bash
# Check how email HTML is generated and what format it has
ast-grep --pattern 'body?.html' apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts
# Check if there are other email HTML assertions in tests
rg "sentEmail.*body.*html" apps/e2e/tests --type=ts -C3🤖 Prompt for AI Agents
In apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts around lines
54-55, the test currently asserts the entire HTML body equals a bare URL string
but later extracts an href from the HTML with a regex — change the assertion to
match the actual HTML structure: either assert that sentEmail!.body?.html
contains the expected URL (use a contains/substring assertion) or extract the
href with the same regex and assert that extractedHref ===
expectedUnsubscribeLink; also verify/remove any snapshot serializer
transformation that might be collapsing the anchor into a plain URL so the test
reflects the real generated HTML.
|
|
||
| const stringRegexReplacements = [ | ||
| [/(\/integrations\/(neon|custom)\/oauth\/idp\/(interaction|auth)\/)[a-zA-Z0-9_-]+/gi, "$1<stripped $3 UID>"], | ||
| [/localhost\:(\d+)(\d\d)/gi, "localhost:<$$STACK_PORT_PREFIX>$2"], |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the regex replacement pattern produces expected output.
The regex replacement "localhost:<$$STACK_PORT_PREFIX>$2" includes angle brackets and dollar signs, but the actual snapshot output shows "localhost:STACK_PORT_PREFIX01" without these characters.
Please verify that this replacement pattern produces the intended output format. The standard JavaScript replace behavior would yield "localhost:<$STACK_PORT_PREFIX>01", which doesn't match what appears in the snapshots.
🏁 Script executed:
#!/bin/bash
# Search for actual snapshot examples to verify the format
rg -A2 -B2 "localhost:STACK_PORT_PREFIX" --type=tsLength of output: 12891
🏁 Script executed:
#!/bin/bash
# Get the actual snapshot-serializer.ts file and examine the regex replacement at line 104
cat apps/e2e/tests/snapshot-serializer.ts | sed -n '100,110p'Length of output: 349
🏁 Script executed:
#!/bin/bash
# Check how stringRegexReplacements is used in the file
cat apps/e2e/tests/snapshot-serializer.ts | sed -n '125,145p'Length of output: 848
Fix the regex replacement pattern to match the intended output format.
The replacement string "localhost:<$$STACK_PORT_PREFIX>$2" will produce "localhost:<$STACK_PORT_PREFIX>01" through standard JavaScript replace behavior, but the snapshots show the intended output is "localhost:STACK_PORT_PREFIX01" (without angle brackets or dollar sign).
The replacement should be "localhost:STACK_PORT_PREFIX$2" to match the expected output format.
🤖 Prompt for AI Agents
In apps/e2e/tests/snapshot-serializer.ts around line 104, the replacement string
currently uses "localhost:<$$STACK_PORT_PREFIX>$2" which inserts unwanted angle
brackets and a dollar sign; change the replacement to
"localhost:STACK_PORT_PREFIX$2" so the output becomes
"localhost:STACK_PORT_PREFIX01" as expected in snapshots. Ensure you remove the
extra angle brackets and single dollar sign before STACK_PORT_PREFIX while
keeping the $2 backreference intact.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (2)
279-279: Remove unnecessary escape characters in regex pattern.Forward slashes don't need escaping in template literals passed to the RegExp constructor.
318-318: Remove unnecessary escape characters in regex pattern.Forward slashes don't need escaping in template literals passed to the RegExp constructor. The
\?escape is correct and should be kept.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts(10 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts(4 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
In tests, prefer .toMatchInlineSnapshot where possible; refer to snapshot-serializer.ts for snapshot formatting and handling of non-deterministic values
Files:
apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.tsapps/e2e/tests/backend/endpoints/api/v1/users.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.tsapps/e2e/tests/backend/endpoints/api/v1/users.test.ts
🧬 Code graph analysis (3)
apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts (2)
apps/e2e/tests/helpers/ports.ts (2)
withPortPrefix(6-6)localhostUrl(11-12)apps/e2e/tests/backend/backend-helpers.ts (1)
niceBackendFetch(108-172)
apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
localhostUrl(11-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: lint_and_build (latest)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: Vercel Agent Review
- GitHub Check: docker
- GitHub Check: Cursor Bugbot
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (13)
apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (1)
3-3: LGTM! Import correctly added for port helper.The import is properly structured and the helper function is used consistently throughout the file for dynamic port configuration.
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (4)
4-4: LGTM! Import aligns with dynamic port configuration.The import of
localhostUrlcorrectly supports the PR's objective to enable dynamic port configuration across tests.
696-712: LGTM! Correctly tests localhost URL rejection with client access.The test properly verifies that localhost URLs are rejected when using client access, which is the expected security behavior. The use of
localhostUrlensures compatibility with dynamic port configuration.
714-724: LGTM! Snapshot correctly handles dynamic ports.The snapshot properly masks the dynamic port with
STACK_PORT_PREFIX21, ensuring test determinism while supporting the dynamic port configuration. This follows the coding guideline to use snapshot serializers for non-deterministic values.
1938-1948: LGTM! Correctly tests localhost URL acceptance with server access.The test properly verifies that localhost URLs are accepted as-is when using server access (trusted context), contrasting with the client access rejection at line 702. The use of
localhostUrlwith.toEqualensures the URL is preserved exactly and works with dynamic port configuration.apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts (8)
1-8: LGTM! Dynamic port configuration correctly implemented.The imports and constant definitions properly leverage the new port helpers to enable dynamic port configuration for passkey tests.
10-23: LGTM! Well-structured helper function.The
createClientDataJSONhelper provides a clean abstraction for generating WebAuthn client data, reducing code duplication across tests.
121-121: LGTM! Dynamic port correctly used in wildcard pattern.The wildcard domain configuration properly incorporates the dynamic
passkeyPort.
149-149: LGTM! Proper use of helper function.The
createClientDataJSONhelper is correctly used to generate the client data with the dynamic origin.
192-192: LGTM! Dynamic port correctly used in double wildcard pattern.The double wildcard domain configuration properly incorporates the dynamic
passkeyPort.
220-220: LGTM! Consistent use of helper.The
createClientDataJSONhelper ensures consistent client data generation with the dynamic origin.
404-404: LGTM! Dynamic port in wildcard configuration.The wildcard pattern correctly uses the dynamic
passkeyPort.
432-432: LGTM! Proper helper usage.Consistent use of the
createClientDataJSONhelper for generating client data.
| "clientDataJSON": btoa(JSON.stringify({ | ||
| type: "webauthn.create", | ||
| challenge: "TU9DSw", | ||
| origin: "http://localhost:8103", // Doesn't match https://app.production.com | ||
| origin: passkeyOrigin, // Doesn't match https://app.production.com | ||
| crossOrigin: false | ||
| })), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use the createClientDataJSON helper for consistency.
The inline btoa(JSON.stringify(...)) duplicates logic already abstracted in the createClientDataJSON helper function (lines 10-23).
Apply this diff:
- "clientDataJSON": btoa(JSON.stringify({
- type: "webauthn.create",
- challenge: "TU9DSw",
- origin: passkeyOrigin, // Doesn't match https://app.production.com
- crossOrigin: false
- })),
+ "clientDataJSON": createClientDataJSON(passkeyOrigin, "webauthn.create"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "clientDataJSON": btoa(JSON.stringify({ | |
| type: "webauthn.create", | |
| challenge: "TU9DSw", | |
| origin: "http://localhost:8103", // Doesn't match https://app.production.com | |
| origin: passkeyOrigin, // Doesn't match https://app.production.com | |
| crossOrigin: false | |
| })), | |
| "clientDataJSON": createClientDataJSON(passkeyOrigin, "webauthn.create"), |
🤖 Prompt for AI Agents
In apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts
around lines 282-287, replace the inline btoa(JSON.stringify({ type:
"webauthn.create", challenge: "TU9DSw", origin: passkeyOrigin, crossOrigin:
false })) with a call to the existing createClientDataJSON helper (defined
earlier on lines 10-23) passing the same object; this removes duplication and
ensures consistent encoding/format used across tests.
| "clientDataJSON": btoa(JSON.stringify({ | ||
| type: "webauthn.get", | ||
| challenge: "TU9DSw", | ||
| origin: "http://localhost:8103", // Doesn't match *.example.com | ||
| origin: passkeyOrigin, // Doesn't match *.example.com | ||
| crossOrigin: false | ||
| })), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use the createClientDataJSON helper for consistency.
The inline btoa(JSON.stringify(...)) duplicates the helper logic and should be replaced for consistency with the rest of the test file.
Apply this diff:
- "clientDataJSON": btoa(JSON.stringify({
- type: "webauthn.get",
- challenge: "TU9DSw",
- origin: passkeyOrigin, // Doesn't match *.example.com
- crossOrigin: false
- })),
+ "clientDataJSON": createClientDataJSON(passkeyOrigin),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "clientDataJSON": btoa(JSON.stringify({ | |
| type: "webauthn.get", | |
| challenge: "TU9DSw", | |
| origin: "http://localhost:8103", // Doesn't match *.example.com | |
| origin: passkeyOrigin, // Doesn't match *.example.com | |
| crossOrigin: false | |
| })), | |
| "clientDataJSON": createClientDataJSON(passkeyOrigin), |
🤖 Prompt for AI Agents
In apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts
around lines 360 to 365, replace the inline btoa(JSON.stringify({...}))
construction with the existing createClientDataJSON helper to keep consistency
with other tests; call createClientDataJSON({ type: "webauthn.get", challenge:
"TU9DSw", origin: passkeyOrigin, crossOrigin: false }) in place of the inline
serialization so the test uses the shared helper logic.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts (2)
279-279: Remove unnecessary escape characters in regex pattern.Forward slashes don't need escaping in template literals passed to the RegExp constructor.
Apply this fix:
- expect(response.body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\/purchase\/[a-z0-9-_]+$`)); + expect(response.body.url).toMatch(new RegExp(`^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$`));
317-317: Remove unnecessary escape characters in regex pattern.Forward slashes don't need escaping in template literals passed to the RegExp constructor.
Apply this fix:
- expect(body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\/purchase\/[a-z0-9-_]+$`)); + expect(body.url).toMatch(new RegExp(`^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$`));apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (2)
279-279: Remove unnecessary escape characters in regex pattern.Forward slashes don't need escaping in template literals passed to the RegExp constructor.
Apply this fix:
- expect(response.body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\\/purchase\\/[a-z0-9-_]+$`)); + expect(response.body.url).toMatch(new RegExp(`^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+$`));
318-318: Remove unnecessary escape characters in regex pattern.Forward slashes don't need escaping in template literals passed to the RegExp constructor. Keep the
\?escape as it's necessary for the special character.Apply this fix:
- expect(body.url).toMatch(new RegExp(`^https?:\\/\\/localhost:${withPortPrefix("01")}\\/purchase\\/[a-z0-9-_]+\?return_url=`)); + expect(body.url).toMatch(new RegExp(`^https?://localhost:${withPortPrefix("01")}/purchase/[a-z0-9-_]+\?return_url=`));apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts (1)
54-55: Fix the HTML assertion logic (still unresolved).The assertion at line 55 claims the entire HTML body equals a bare URL string, but line 58 immediately attempts to extract an
hrefattribute using regex, which only makes sense if the HTML contains actual markup. Line 148 in the second test confirms the HTML is a full document with DOCTYPE, tags, etc., not a plain URL.This issue was previously flagged and remains unresolved. Either:
- Extract the href first (as done at line 58) and assert that extracted value equals
expectedUnsubscribeLink, or- Assert the HTML contains the expected link (substring/includes check), or
- Use
.toMatchInlineSnapshot()as recommended in the coding guidelines and demonstrated in the second testApply this diff to fix the assertion:
- const expectedUnsubscribeLink = localhostUrl("02", "/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E"); - expect(sentEmail!.body?.html).toBe(expectedUnsubscribeLink); - - // Extract the unsubscribe link and fetch it - const unsubscribeLinkMatch = sentEmail!.body?.html.match(/href="([^"]+)"/); - expect(unsubscribeLinkMatch).toBeDefined(); - const unsubscribeUrl = unsubscribeLinkMatch![1]; + // Extract the unsubscribe link from HTML and verify it + const unsubscribeLinkMatch = sentEmail!.body?.html.match(/href="([^"]+)"/); + expect(unsubscribeLinkMatch).toBeDefined(); + const unsubscribeUrl = unsubscribeLinkMatch![1]; + const expectedUnsubscribeLink = localhostUrl("02", "/api/v1/emails/unsubscribe-link?code=%3Cstripped+query+param%3E"); + expect(unsubscribeUrl).toBe(expectedUnsubscribeLink);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts(10 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts(4 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/e2e/tests/backend/endpoints/api/v1/users.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/send-email.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/auth/passkey/wildcard-domains.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--purchase-session.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
In tests, prefer .toMatchInlineSnapshot where possible; refer to snapshot-serializer.ts for snapshot formatting and handling of non-deterministic values
Files:
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.tsapps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.tsapps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts
🧬 Code graph analysis (4)
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts (1)
apps/e2e/tests/helpers/ports.ts (2)
withPortPrefix(6-6)localhostUrl(11-12)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
withPortPrefix(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: lint_and_build (latest)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: all-good
- GitHub Check: docker
- GitHub Check: Vercel Agent Review
- GitHub Check: docker
- GitHub Check: Cursor Bugbot
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (6)
apps/e2e/tests/backend/endpoints/api/v1/payments/before-offer-to-product-rename/outdated--create-purchase-url.test.ts (1)
3-3: LGTM! Import aligns with dynamic port handling.The import of
withPortPrefixis necessary and appropriate for generating dynamic port numbers in the test assertions.apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts (1)
3-3: LGTM!The import of
withPortPrefixcorrectly enables dynamic port construction for the test assertions.apps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.ts (3)
3-3: LGTM!The import of port helpers is correct and aligns with the PR's objective to introduce customizable port prefixes.
13-13: LGTM!The dynamic port configuration correctly uses
withPortPrefix("29")to compute the SMTP port (8129 with default prefix), replacing the hardcoded 2500.
107-107: LGTM!The port configuration is correctly updated to use the dynamic port helper, consistent with the first test.
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts (1)
3-7: LGTM! Dynamic port configuration adopted correctly.The import and constant definition properly implement the new customizable port prefix system, making the test adaptable to different port configurations.
| config_override_string: JSON.stringify({ | ||
| 'domains.trustedDomains.prefix': { | ||
| baseUrl: 'http://api-*:8107', // Won't match localhost | ||
| baseUrl: `http://api-*:${oauthMockServerPort}`, // Won't match localhost |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the OAuth mock server port suffix.
The dynamic port makes the test more maintainable, but please confirm that the suffix "07" (resulting in port 8107 with default prefix) matches the actual OAuth mock server configuration. The PR objectives don't explicitly document the OAuth mock server port mapping.
🏁 Script executed:
#!/bin/bash
# Description: Find OAuth mock server port configuration to verify the suffix "07" is correct
# Search for OAuth mock server port definitions
rg -n --type=ts --type=js -C3 'oauth.*mock.*server.*port|mock.*oauth.*port' -g '!node_modules' -g '!*.test.*'
# Also search for port "8107" or suffix "07" in port configurations
rg -n --type=ts --type=js -C3 '8107|withPortPrefix.*["\']07["\']' -g '!node_modules'
# Check for OAuth mock server startup/configuration files
fd -e ts -e js 'mock.*oauth|oauth.*mock' -x cat {}Length of output: 217
🏁 Script executed:
#!/bin/bash
# Description: Find OAuth mock server port configuration
# Search for oauthMockServerPort definition
rg -n 'oauthMockServerPort' --type ts --type js
# Search for port configuration patterns with "07" suffix
rg -n 'withPortPrefix.*07|portSuffix.*07' --type ts --type js
# Look for port setup in test helpers or configuration
fd -type f -name '*.ts' -o -name '*.js' | xargs grep -l 'mock.*server' | head -20Length of output: 591
🏁 Script executed:
#!/bin/bash
# Description: Find withPortPrefix function and port mapping
# Search for withPortPrefix function definition
rg -n 'function withPortPrefix|const withPortPrefix|export.*withPortPrefix' -A5
# Search for port mapping configuration, especially looking for "07" or 8107
rg -n 'portPrefix|portSuffix|8107' -B2 -A2
# Look for test setup files or fixtures that might define port mappings
fd -type f \( -name '*setup*' -o -name '*fixture*' -o -name '*config*' -o -name '*helper*' \) -path '*/e2e/*' | head -10Length of output: 281823
Update port suffix from "07" to "14" to match OAuth mock server configuration.
The test uses withPortPrefix("07") for the OAuth mock server, but the dev-launchpad configuration shows the OAuth mock server is assigned portSuffix: "14". Port suffix "07" is actually for Jaeger UI. Update line 6 to use the correct suffix:
-const oauthMockServerPort = withPortPrefix("07");
+const oauthMockServerPort = withPortPrefix("14");This ensures the test uses the correct port (8114) that matches the actual OAuth mock server configuration.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/wildcard-domains.test.ts
around line 290, the test is using the wrong dev-launchpad port suffix ("07")
for the OAuth mock server; change the withPortPrefix call from "07" to "14" so
the oauthMockServerPort resolves to the correct port (8114) that matches the
OAuth mock server configuration.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/e2e-custom-base-port-api-tests.yaml (2)
98-101: Port mismatch in wait-on commands for Inbucket and Svix (critical issue persists from prior review).The workflow sets
STACK_PORT_PREFIX: "67"but uses hardcoded default ports in wait-on commands instead of computing them from the prefix. Services run on 67xx ports, but these commands wait on 81xx ports, causing timeouts.With prefix
67:
- Inbucket SMTP:
67 + 29 = 6729(not8129)- Svix:
67 + 13 = 6713(not8113)Apply this diff:
- name: Wait on Inbucket - run: npx wait-on tcp:localhost:8129 + run: npx wait-on tcp:localhost:6729 - name: Wait on Svix - run: npx wait-on tcp:localhost:8113 + run: npx wait-on tcp:localhost:6713
106-132: Port mismatch in service startup wait-on URLs (critical issue persists from prior review).All three background service start commands use hardcoded port
8102, but withSTACK_PORT_PREFIX: "67", they should target different 67xx ports:
- Backend:
8102→6702(67 + 02)- Dashboard:
8102→6701(67 + 01)- Mock OAuth:
8102→6707(67 + 07)This mismatch will cause workflow failures as services run on the correct 67xx ports but wait-on commands target the wrong 81xx port.
Apply this diff:
- name: Start stack-backend in background uses: JarvusInnovations/background-action@v1.0.7 with: run: pnpm run start:backend --log-order=stream & wait-on: | - http://localhost:8102 + http://localhost:6702 tail: true wait-for: 30s log-output-if: true - name: Start stack-dashboard in background uses: JarvusInnovations/background-action@v1.0.7 with: run: pnpm run start:dashboard --log-order=stream & wait-on: | - http://localhost:8102 + http://localhost:6701 tail: true wait-for: 30s log-output-if: true - name: Start mock-oauth-server in background uses: JarvusInnovations/background-action@v1.0.7 with: run: pnpm run start:mock-oauth-server --log-order=stream & wait-on: | - http://localhost:8102 + http://localhost:6707 tail: true wait-for: 30s log-output-if: true
🧹 Nitpick comments (2)
.github/workflows/e2e-custom-base-port-api-tests.yaml (2)
55-89: Refactor repetitive .env.test.local copy pattern.Lines 55–89 contain nine nearly identical steps copying
.env.developmentto.env.test.localfor different apps. This repetition could be consolidated into a single script or loop.Consider replacing these steps with a single loop:
for app in backend dashboard e2e docs cjs-test demo docs-examples e-commerce middleware partial-prerendering supabase convex; do mkdir -p "$([ "$app" = "backend" ] && echo "apps/$app" || echo "examples/$app" || echo "docs")" cp "$([ "$app" = "backend" ] && echo "apps/$app" || echo "examples/$app" || echo "docs")/.env.development" \ "$([ "$app" = "backend" ] && echo "apps/$app" || echo "examples/$app" || echo "docs")/.env.test.local" doneOr use a reusable GitHub Action to centralize this logic across workflows.
30-38: Update GitHub Actions to latest versions (nice-to-have).The workflow uses outdated action versions for checkout@v3 and setup-node@v3. While these still function, consider updating to v4 for latest security patches and improvements.
- - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/setup-node@v3 + uses: actions/setup-node@v4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.claude/settings.json(1 hunks).github/workflows/e2e-custom-base-port-api-tests.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/e2e-custom-base-port-api-tests.yaml
16-16: label "ubicloud-standard-8" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
28-28: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
31-31: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Checkov (3.2.334)
.github/workflows/e2e-custom-base-port-api-tests.yaml
[medium] 20-21: Basic Auth Credentials
(CKV_SECRET_4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: docker
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: Cursor Bugbot
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (3)
.claude/settings.json (1)
24-24: Verify backend port alignment with service configuration.The dynamic port is calculated as
${STACK_PORT_PREFIX:-81}02, defaulting to 8102. Confirm this aligns with the actual backend service port defined elsewhere in the codebase (e.g., Docker Compose, environment configuration)..github/workflows/e2e-custom-base-port-api-tests.yaml (2)
20-21: Environment setup correctly implements port prefix logic.The
STACK_DIRECT_DATABASE_CONNECTION_STRINGcorrectly uses port6728(prefix67+ offset28), confirming the port computation pattern.
134-152: Test execution and verification steps look appropriate.The workflow correctly implements test retry logic on main/dev, data integrity verification, and comprehensive logging on failure.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/e2e/tests/snapshot-serializer.ts (1)
103-106: Fix replacement string; drop angle brackets and extra '$'; preserve 2‑digit suffix."localhost:<$$STACK_PORT_PREFIX>" renders as "localhost:<$STACK_PORT_PREFIX>" due to $$ → $ in JS replacement strings, which doesn’t match the intended "localhost:STACK_PORT_PREFIXdd". Also, make the suffix explicit for clarity.
Apply:
- [new RegExp(`localhost\\:${getPortPrefix()}`, "gi"), "localhost:<$$STACK_PORT_PREFIX>"], + // Replace 'localhost:<prefix><dd>' with 'localhost:STACK_PORT_PREFIX<dd>' + [new RegExp(`(localhost\\:)${getPortPrefix()}(\\d{2})\\b`, "gi"), "localhost:STACK_PORT_PREFIX$2"],Optionally support 127.0.0.1 too:
- [new RegExp(`(localhost\\:)${getPortPrefix()}(\\d{2})\\b`, "gi"), "localhost:STACK_PORT_PREFIX$2"], + [new RegExp(`((?:localhost|127\\.0\\.0\\.1)\\:)${getPortPrefix()}(\\d{2})\\b`, "gi"), "localhost:STACK_PORT_PREFIX$2"],
🧹 Nitpick comments (2)
apps/e2e/tests/snapshot-serializer.ts (2)
6-6: Be mindful of import-time env read.getPortPrefix() runs at module load. If tests mutate process.env.STACK_PORT_PREFIX after this file is imported, replacements won’t reflect it. Ensure the env var is set before the test runner loads this module, or compute the regex lazily inside overrides.
66-70: Confirm intent: stripping every field named "port".Adding "port" to stripFields will redact any property named exactly "port" (numbers and strings). This may hide meaningful values in snapshots (e.g., verifying 443 vs 80, or ephemeral ports). If the goal is only to stabilize stack-prefixed dev ports, consider narrowing scope (e.g., only string ports with the stack prefix) or moving to stripFieldsIfString.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/oauth.test.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(4 hunks)apps/e2e/tests/helpers/ports.ts(1 hunks)apps/e2e/tests/snapshot-serializer.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/oauth.test.ts
- apps/e2e/tests/helpers/ports.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
In tests, prefer .toMatchInlineSnapshot where possible; refer to snapshot-serializer.ts for snapshot formatting and handling of non-deterministic values
Files:
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/users.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/users.test.tsapps/e2e/tests/snapshot-serializer.ts
🧬 Code graph analysis (2)
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
localhostUrl(11-12)
apps/e2e/tests/snapshot-serializer.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
getPortPrefix(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: Vercel Agent Review
- GitHub Check: Cursor Bugbot
- GitHub Check: setup-tests
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (6)
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.ts (1)
17-23: LGTM! Snapshot correctly updated for dynamic port format.The inline snapshot has been properly updated to use the dynamic port prefix
<$STACK_PORT_PREFIX>01instead of a hardcoded port, aligning with the PR's port customization mechanism. The test continues to use.toMatchInlineSnapshotas recommended by the coding guidelines.apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (3)
4-4: LGTM! Import correctly added for port customization support.The
localhostUrlhelper enables dynamic port resolution using theSTACK_PORT_PREFIXenvironment variable, aligning with the PR's customizable ports objective.
696-724: LGTM! Profile image URL tests correctly updated for customizable ports.Both test cases properly handle the port migration:
- Line 702: Validates that client access correctly rejects localhost URLs (with dynamic port)
- Line 723: Snapshot format
<$STACK_PORT_PREFIX>21correctly uses the snapshot serializer for non-deterministic port valuesAs per coding guidelines: The snapshot format aligns with snapshot-serializer.ts for handling dynamic values.
1938-1948: LGTM! Server access profile image URL test correctly migrated.The test properly validates that server access can set localhost profile image URLs, with both the request body (line 1944) and assertion (line 1947) consistently using the
localhostUrlhelper for dynamic port resolution.apps/e2e/tests/snapshot-serializer.ts (2)
108-108: No-op whitespace change.
165-165: LGTM: broadened URL heuristic.Allowing '<' and '>' avoids truncating sanitized placeholders embedded in URLs. Looks good.
If you see false positives, consider adding a word boundary before the scheme or tightening the trailing class with a lookahead, but not necessary now.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts (1)
53-53: LGTM!The snapshot placeholder format is correct and aligns with the snapshot serializer's replacement pattern. The past review concern about placeholder mismatches appears to be resolved—runtime assertions now use
localhostUrl()for resolved URLs, while snapshots correctly use placeholders.
🧹 Nitpick comments (1)
apps/e2e/tests/snapshot-serializer.ts (1)
105-106: Regex patterns work correctly for the current port format.The regex patterns correctly replace port prefixes with placeholders:
localhost:8101→localhost:<$STACK_PORT_PREFIX>01- URL-encoded variant handled on Line 106
The static analysis ReDoS warning is a false positive—the constructed regex is simple with no backtracking risk.
Note: The patterns rely on the port prefix always being at the start of the port number. If port formats ever diverge (e.g.,
localhost:810Xwhere X is not the suffix), the regex could produce unexpected results. Consider adding word boundaries if port formats may vary in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/oauth.test.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.ts(5 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(4 hunks)apps/e2e/tests/helpers/ports.ts(1 hunks)apps/e2e/tests/snapshot-serializer.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/e2e/tests/backend/endpoints/api/v1/users.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/payments/create-purchase-url.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/transfer.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/oauth.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
In tests, prefer .toMatchInlineSnapshot where possible; refer to snapshot-serializer.ts for snapshot formatting and handling of non-deterministic values
Files:
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.tsapps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.tsapps/e2e/tests/helpers/ports.tsapps/e2e/tests/snapshot-serializer.ts
🧬 Code graph analysis (2)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
localhostUrl(11-12)
apps/e2e/tests/snapshot-serializer.ts (1)
apps/e2e/tests/helpers/ports.ts (1)
getPortPrefix(1-1)
🪛 ast-grep (0.39.6)
apps/e2e/tests/snapshot-serializer.ts
[warning] 105-105: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(localhost\%3A${getPortPrefix()}, "gi")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: Vercel Agent Review
- GitHub Check: Cursor Bugbot
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (7)
apps/e2e/tests/helpers/ports.ts (1)
1-12: LGTM!The port helper utilities are well-structured and provide a clean abstraction for dynamic port configuration across tests. The implementation correctly concatenates the prefix with suffixes to form complete port numbers (e.g., "81" + "01" = "8101").
apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/transfer.test.ts (1)
20-20: LGTM!The snapshot correctly uses the dynamic port placeholder format, aligning with the snapshot serializer's replacement pattern.
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts (2)
8-12: LGTM!The refactor to parse the Location header and validate origin/pathname separately is the correct approach. Using
localhostUrl("14")ensures the assertion matches the actual runtime port rather than a placeholder.
19-23: LGTM!Consistent with the first test—correctly uses URL parsing with dynamic port resolution.
apps/e2e/tests/snapshot-serializer.ts (2)
69-69: LGTM!Adding "port" to the strip fields ensures port values are consistently masked in snapshots.
166-166: LGTM!Adding angle brackets to the URL regex heuristic correctly handles placeholder URLs that appear in snapshots after serialization.
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth.test.ts (1)
78-78: LGTM!All snapshot updates consistently use the dynamic port placeholder format
<$STACK_PORT_PREFIX>02, aligning with the snapshot serializer's replacement pattern and the broader port customization scheme.Also applies to: 84-84, 86-86, 96-96, 98-98, 133-133, 141-141, 172-172, 181-181
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| <script> | ||
| const derivePrefixFromLocation = () => { | ||
| const port = window.location.port; | ||
| if (!port || port.length < 2) return "81"; |
There was a problem hiding this comment.
| if (!port || port.length < 2) return "81"; | |
| if (!port || port.length <= 2) return "81"; |
The port prefix extraction logic fails for 2-digit ports because it only checks for ports with fewer than 2 digits, not exactly 2-digit ports. If the dev launchpad is served on a port like "81", the extraction would return an empty string instead of the correct prefix.
View Details
Analysis
Port prefix extraction returns empty string for 2-digit ports
What fails: derivePrefixFromLocation() in apps/dev-launchpad/public/index.html line 108 returns "" instead of fallback "81" for 2-digit ports due to incorrect condition port.length < 2
How to reproduce:
// Set window.location.port to a 2-digit value
window.location.port = "81";
const result = derivePrefixFromLocation(); // returns "" instead of "81"Result: Returns empty string "", causing withPrefix("01") to produce "01" instead of expected "8101"
Expected: Should return fallback "81" for any port with fewer than 3 digits, ensuring consistent port construction pattern
apps/backend/src/prisma-client.tsx
Outdated
| } | ||
|
|
||
| function getSchemaFromConnectionString(connectionString: string) { | ||
| console.log(connectionString, process.env); |
There was a problem hiding this comment.
Remove the debug console.log; logging the connection string and entire process.env may expose sensitive information.
| console.log(connectionString, process.env); |
| if (!input) return input; | ||
| const prefix = process.env.NEXT_PUBLIC_STACK_PORT_PREFIX; | ||
| return prefix ? input.replace(/\$\{NEXT_PUBLIC_STACK_PORT_PREFIX:-81\}/g, prefix) as T : input; | ||
| }; |
There was a problem hiding this comment.
Bug: Default Port Expansion Fails in Local Dev
The replaceStackPortPrefix function doesn't correctly expand the ${NEXT_PUBLIC_STACK_PORT_PREFIX:-81} template when NEXT_PUBLIC_STACK_PORT_PREFIX is undefined, failing to use "81" as a default. This, along with removing the explicit baseUrl from StackClientApp, causes local development to incorrectly connect to the production API.
Additional Locations (1)
| import Provider, { errors } from 'oidc-provider'; | ||
|
|
||
| const port = process.env.PORT || 8114; | ||
| const stackPortPrefix = process.env.NEXT_PUBLIC_STACK_PORT_PREFIX ?? "81"; |
There was a problem hiding this comment.
The OAuth redirect URIs for the mock-oauth-server are incomplete. The code defines emulatorBackendPort but doesn't use it in the redirect_uris array, which causes OAuth to fail in emulator environments where the backend runs on port 32102 instead of 8102.
View Details
📝 Patch Details
diff --git a/apps/mock-oauth-server/src/index.ts b/apps/mock-oauth-server/src/index.ts
index 2f3e5a24..8890e672 100644
--- a/apps/mock-oauth-server/src/index.ts
+++ b/apps/mock-oauth-server/src/index.ts
@@ -24,6 +24,7 @@ const clients = providerIds.map((id) => ({
client_secret: 'MOCK-SERVER-SECRET',
redirect_uris: [
`http://localhost:${backendPortForRedirects}/api/v1/auth/oauth/callback/${id}`,
+ `http://localhost:${emulatorBackendPort}/api/v1/auth/oauth/callback/${id}`,
...(process.env.STACK_MOCK_OAUTH_REDIRECT_URIS ? [process.env.STACK_MOCK_OAUTH_REDIRECT_URIS.replace("{id}", id)] : [])
],
grant_types: ['authorization_code', 'refresh_token'],
Analysis
Mock OAuth server missing emulator backend redirect URI causes callback failures
What fails: OAuth providers redirect to incorrect port in emulator environment - mock-oauth-server only provides http://localhost:8102/api/v1/auth/oauth/callback/{id} but backend runs on port 32102
How to reproduce:
# Start emulator environment (docker-compose)
docker-compose -f docker/emulator/docker.compose.yaml up -d
# Backend runs on port 32102, mock-oauth-server on 32202
# OAuth callback fails when provider redirects to port 8102 instead of 32102Result: OAuth callbacks fail with connection refused or 404 errors because providers redirect to http://localhost:8102/api/v1/auth/oauth/callback/{id} but the backend is listening on port 32102
Expected: Mock OAuth server should provide redirect URIs for both normal (8102) and emulator (32102) backend ports to support both environments
Evidence: Docker configuration shows NEXT_PUBLIC_STACK_API_URL: "http://localhost:32102" but emulatorBackendPort variable is defined but unused in redirect_uris array (line 10 vs lines 25-28 in apps/mock-oauth-server/src/index.ts)
| if (!input) return input; | ||
| const prefix = process.env.NEXT_PUBLIC_STACK_PORT_PREFIX; | ||
| return prefix ? input.replace(/\$\{NEXT_PUBLIC_STACK_PORT_PREFIX:-81\}/g, prefix) as T : input; | ||
| }; |
There was a problem hiding this comment.
Bug: Unresolved URL Placeholders Break API Connections
The replaceStackPortPrefix function doesn't default NEXT_PUBLIC_STACK_PORT_PREFIX, leaving URL placeholders unresolved. With the explicit baseUrl configuration removed, this can prevent the client from correctly determining the API base URL, breaking connections when custom port prefixes are used.
Additional Locations (1)
|
|
||
| db: | ||
| image: postgres:15 | ||
| command: ["postgres", "-c"] |
There was a problem hiding this comment.
| command: ["postgres", "-c"] | |
| # Removed incomplete -c flag - see git history for original port configuration |
PostgreSQL command is incomplete and will cause the container to fail on startup. The -c flag requires a configuration parameter but none is provided.
View Details
Analysis
Incomplete PostgreSQL -c flag causes container startup failure
What fails: PostgreSQL container in docker/emulator/docker.compose.yaml line 84 has incomplete command ["postgres", "-c"] causing startup failure with "postgres: option requires an argument -- 'c'" error
How to reproduce:
# Start the Docker compose stack:
cd docker/emulator && docker-compose up dbResult: Container exits immediately with non-zero exit code. PostgreSQL fails with "postgres: option requires an argument -- 'c'" per PostgreSQL server documentation
Expected: Container should start successfully. Git history shows this was originally ["postgres", "-c", "port=8128"] but port=8128 parameter was accidentally removed while leaving the incomplete -c flag
|
|
||
| db: | ||
| image: postgres:15 | ||
| command: ["postgres", "-c"] |
There was a problem hiding this comment.
Bug: Missing Configuration Parameter Causes PostgreSQL Failure
The db service's postgres command is incomplete. The -c flag expects a configuration parameter, but none is provided, causing the PostgreSQL container to fail to start.
|
|
||
| db: | ||
| image: postgres:15 | ||
| command: ["postgres", "-c"] |
There was a problem hiding this comment.
Bug: PostgreSQL Container Missing Configuration Parameter
The PostgreSQL container's command: ["postgres", "-c"] is incomplete. The -c flag expects a configuration parameter, but none is provided, which prevents PostgreSQL from starting.
|
|
||
| db: | ||
| image: postgres:15 | ||
| command: ["postgres", "-c"] |
There was a problem hiding this comment.
Bug: Incomplete PostgreSQL Configuration Command
The db service's command: ["postgres", "-c"] is incomplete. The -c flag expects a configuration parameter, but none is provided, which causes the PostgreSQL container to fail to start.
| if (!input) return input; | ||
| const prefix = process.env.NEXT_PUBLIC_STACK_PORT_PREFIX; | ||
| return prefix ? input.replace(/\$\{NEXT_PUBLIC_STACK_PORT_PREFIX:-81\}/g, prefix) as T : input; | ||
| }; |
There was a problem hiding this comment.
Bug: Default Value Not Applied to Environment Variable
The replaceStackPortPrefix function doesn't correctly apply the default value for NEXT_PUBLIC_STACK_PORT_PREFIX. When the environment variable is undefined or empty, it leaves the ${NEXT_PUBLIC_STACK_PORT_PREFIX:-81} pattern unreplaced instead of defaulting to "81". This can result in StackClientApp using an incorrect base URL and experiencing connection issues.
Additional Locations (1)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-source-of-truth-api-tests.yaml (1)
113-135: Hardcoded port 8102 doesn't support dynamic port prefixes.Lines 114, 123, and 132 hardcode port 8102 for service wait-on checks. However, this workflow doesn't set
NEXT_PUBLIC_STACK_PORT_PREFIX, which means:
- The workflow cannot run with different port configurations
- It contradicts the PR's objective of enabling customizable port prefixes for parallel Stack instances
- If someone sets a different
NEXT_PUBLIC_STACK_PORT_PREFIXin their environment, this workflow will failAdd
NEXT_PUBLIC_STACK_PORT_PREFIXto the workflow env section and calculate the dynamic ports:env: NODE_ENV: test + NEXT_PUBLIC_STACK_PORT_PREFIX: 81 STACK_ENABLE_HARDCODED_PASSKEY_CHALLENGE_FOR_TESTING: yes STACK_OVERRIDE_SOURCE_OF_TRUTH: ${{ secrets.STACK_OVERRIDE_SOURCE_OF_TRUTH }} STACK_TEST_SOURCE_OF_TRUTH: true STACK_DIRECT_DATABASE_CONNECTION_STRING: ${{ secrets.STACK_DIRECT_DATABASE_CONNECTION_STRING }}Then update the wait-on conditions to derive the port from the prefix:
- name: Start stack-backend in background uses: JarvusInnovations/background-action@v1.0.7 with: run: pnpm run start:backend --log-order=stream & wait-on: | - http://localhost:8102 + http://localhost:${{ env.NEXT_PUBLIC_STACK_PORT_PREFIX }}02 tail: true wait-for: 30s log-output-if: true - name: Start stack-dashboard in background uses: JarvusInnovations/background-action@v1.0.7 with: run: pnpm run start:dashboard --log-order=stream & wait-on: | - http://localhost:8102 + http://localhost:${{ env.NEXT_PUBLIC_STACK_PORT_PREFIX }}02 tail: true wait-for: 30s log-output-if: true - name: Start mock-oauth-server in background uses: JarvusInnovations/background-action@v1.0.7 with: run: pnpm run start:mock-oauth-server --log-order=stream & wait-on: | - http://localhost:8102 + http://localhost:${{ env.NEXT_PUBLIC_STACK_PORT_PREFIX }}02 tail: true wait-for: 30s log-output-if: true
♻️ Duplicate comments (1)
apps/dev-launchpad/public/index.html (1)
106-113: Define prefix derivation with correct boundary condition.The
derivePrefixFromLocation()function attempts to extract the port prefix from the current page's location. However, the condition on line 108 uses< 2when it should use<= 2.When the dev launchpad is served on a short port (e.g., "81"), the current condition fails:
port.length < 2is false when port.length equals 2, so the fallback isn't triggered. Callingport.slice(0, -2)on a 2-character port returns an empty string.Apply this fix:
const derivePrefixFromLocation = () => { const port = window.location.port; - if (!port || port.length < 2) return "81"; + if (!port || port.length <= 2) return "81"; return port.slice(0, -2); };This issue was flagged in a previous review on line 108 and remains unfixed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/e2e-api-tests.yaml(2 hunks).github/workflows/e2e-source-of-truth-api-tests.yaml(2 hunks)apps/backend/package.json(2 hunks)apps/dashboard/package.json(1 hunks)apps/dev-launchpad/package.json(1 hunks)apps/dev-launchpad/public/index.html(14 hunks)docs/package.json(1 hunks)examples/cjs-test/package.json(1 hunks)examples/convex/package.json(1 hunks)examples/demo/package.json(1 hunks)examples/docs-examples/package.json(1 hunks)examples/e-commerce/package.json(1 hunks)examples/js-example/package.json(1 hunks)examples/middleware/package.json(1 hunks)examples/react-example/package.json(1 hunks)examples/supabase/package.json(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- examples/docs-examples/package.json
- apps/dashboard/package.json
- examples/js-example/package.json
- docs/package.json
- examples/demo/package.json
- examples/supabase/package.json
- examples/cjs-test/package.json
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/e2e-api-tests.yaml
[medium] 20-21: Basic Auth Credentials
(CKV_SECRET_4)
.github/workflows/e2e-source-of-truth-api-tests.yaml
[medium] 20-21: Basic Auth Credentials
(CKV_SECRET_4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Cursor Bugbot
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: all-good
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: Security Check
🔇 Additional comments (15)
examples/e-commerce/package.json (1)
6-6: Port-prefix pattern applied correctly.The dev and start scripts correctly implement the port-prefix mechanism. The default yields 8111 as expected.
Also applies to: 8-8
examples/react-example/package.json (1)
7-7: Port-prefix pattern applied correctly.The dev script correctly derives the port from the environment variable with the expected default of 8120.
examples/middleware/package.json (1)
6-6: Port-prefix pattern applied correctly.The dev and start scripts correctly implement the port-prefix mechanism with the expected default of 8112.
Also applies to: 8-8
package.json (1)
22-22: Port-prefix mechanism correctly applied to dev orchestration.The restart-dev-in-background wait condition (suffix 02 for backend), Docker Compose project naming with prefix, and PostgreSQL readiness check (suffix 28 → 8128) all correctly implement the dynamic port-prefix pattern.
Also applies to: 27-27, 29-29
apps/dev-launchpad/package.json (1)
6-6: Verify env-config.js generation script.The dev script now calls
write-env-config.jsto dynamically generate the env-config.js file before serving. This is necessary for the index.html script to load environment variables at runtime.Please confirm that
apps/dev-launchpad/scripts/write-env-config.jsis properly implemented to generateenv-config.jswith the requiredNEXT_PUBLIC_STACK_PORT_PREFIXvariable accessible as a global in the browser (likelywindow.NEXT_PUBLIC_STACK_PORT_PREFIX).examples/convex/package.json (1)
8-8: Port-prefix mechanism correctly applied with good variable extraction pattern.The dev:frontend script uses suffix 27, and the set-up-env-vars script correctly extracts the prefix into a variable for constructing the backend URL. This pattern is well-suited for complex scripts requiring multiple port-derived values.
Also applies to: 11-11, 13-13
apps/backend/package.json (1)
10-10: Port-prefix mechanism correctly applied across backend scripts.The dev, start, and prisma-studio scripts all correctly use the dynamic port-prefix pattern. Port 8102 for the API and 8106 for Prisma Studio match the dev-launchpad configuration.
Also applies to: 15-15, 24-24
apps/dev-launchpad/public/index.html (4)
5-5: Verify env-config.js is properly sourced before script execution.The env-config.js script is loaded in the head, which ensures it runs before the inline script below. Ensure that the
apps/dev-launchpad/scripts/write-env-config.jsscript properly generates this file with the correct environment variables accessible as globals.
115-133: Background services rendering correctly maps port suffixes.The background services array properly defines service definitions with port suffixes and renders them dynamically using the
withPrefix()helper. Port assignments (28 for PostgreSQL, 29 for SMTP, 30 for POP3, 31 for OTel) align with the PR objectives.
135-310: App registry correctly updated to use portSuffix pattern.All app entries have been migrated from hardcoded port values to a
portSuffixproperty. The suffixes are consistent with the port assignments across the codebase (e.g., 01 for dashboard, 02 for backend, 20 for React example). This enables dynamic port derivation at runtime.
321-324: Dynamic port construction consistently applied across app rendering.The app href, port display, and favicon src all use
withPrefix(app.portSuffix)for consistent runtime port derivation. This ensures all links and resource references adapt correctly when the port prefix changes..github/workflows/e2e-api-tests.yaml (2)
94-94: Port update for Inbucket aligns with port-prefix system.The change from
tcp:localhost:2500totcp:localhost:8129correctly reflects the new port assignment (port-prefix81+ suffix29). This is consistent with the PR objectives for the customizable port-prefix system.
20-20: Original review comment is incorrect—credentials are intentionally hardcoded for local containerized testing.This is not a security concern. The
PASSWORD-PLACEHOLDER--uqfEC1hmmvcredential is:
- Consistent across the stack: Present in
docker/dependencies/docker.compose.yaml, all.env.developmentfiles (apps/backend,apps/e2e, etc.), and the CI workflow—this is intentional design.- Test-only credentials: The "PASSWORD-PLACEHOLDER" prefix explicitly marks these as development/testing credentials, not production secrets.
- Local Docker services only: Connections are to
localhost:8128(containerized PostgreSQL), not external resources requiring authentication protection.- Architecturally sound: Docker Compose and the CI workflow must use matching credentials; using GitHub secrets here would require hardcoding them elsewhere anyway.
Checkov's CKV_SECRET_4 flag is a false positive for local containerized development. No changes needed.
Likely an incorrect or invalid review comment.
.github/workflows/e2e-source-of-truth-api-tests.yaml (2)
95-96: Inbucket port update looks good.The port change from 2500 to 8129 is correct and aligns with the PR's port-prefix scheme.
92-104: PostgreSQL port is correctly configured in wait script, but via environment variable default.The
wait-until-postgres-is-ready:pg_isreadyscript uses the pattern${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}28, which defaults to port 8128 when the environment variable is not set. This matches the explicit port 8128 used in the psql commands on lines 103–104. The configuration is correct, though it relies on theNEXT_PUBLIC_STACK_PORT_PREFIXenvironment variable being either unset or properly managed to ensure the port resolves to 8128.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/e2e-custom-base-port-api-tests.yaml (1)
108-108: Verify service ports and check for incomplete fix from previous review.All three services (backend, dashboard, mock-oauth-server) wait on the identical URL
http://localhost:6702. This appears inconsistent with typical multi-service architectures where each service runs on a distinct port. Previous review comments suggested backend on 6702, dashboard on 6701, and mock-oauth-server on 6707. Please confirm whether services should have different ports or if they intentionally share a port (e.g., via different paths).Also applies to: 117-117, 126-126
🧹 Nitpick comments (1)
.github/workflows/e2e-custom-base-port-api-tests.yaml (1)
28-28: Update deprecated GitHub Actions versions.Lines 28 and 31 use outdated action versions (
actions/checkout@v3andactions/setup-node@v3). GitHub has deprecated v3 runners. Update to v4 to ensure compatibility with current GitHub Actions infrastructure.- - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/setup-node@v3 + uses: actions/setup-node@v4Also applies to: 31-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-custom-base-port-api-tests.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/e2e-custom-base-port-api-tests.yaml
16-16: label "ubicloud-standard-8" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
28-28: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
31-31: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Checkov (3.2.334)
.github/workflows/e2e-custom-base-port-api-tests.yaml
[medium] 20-21: Basic Auth Credentials
(CKV_SECRET_4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Cursor Bugbot
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: Security Check
🔇 Additional comments (2)
.github/workflows/e2e-custom-base-port-api-tests.yaml (2)
16-16: Custom runner label may not be recognized; verify availability.The runner label
ubicloud-standard-8(line 16) is not in GitHub's standard label set. If this is a custom self-hosted runner, ensure it is properly registered in repository or organization settings. Otherwise, consider using a standard label likeubuntu-latest-8-cores.
20-20: The review comment is incorrect. There is no security risk here.The
PASSWORD-PLACEHOLDER--uqfEC1hmmvstring is not a real credential but an intentionally designed placeholder for the local CI/CD testing environment. The Docker Compose configuration explicitly uses this same placeholder value for all services (PostgreSQL, PgHero, PgAdmin, Supabase Studio, Svix), confirming it is by design. All connections are tolocalhostor internal Docker service hostnames—never to production. The codebase already properly uses GitHub Secrets for actual sensitive data (API keys, Docker credentials, etc.), demonstrating that real credentials are handled correctly elsewhere. Migrating this placeholder to a secret would be unnecessary and could break the local test infrastructure.Likely an incorrect or invalid review comment.
High-level PR Summary
This PR changes the default development ports for several background services to avoid conflicts. PostgreSQL moves from port
5432to8128, Inbucket SMTP from2500to8129, Inbucket POP3 from1100to8130, and the OpenTelemetry collector from4318to8131. All references across configuration files, Docker Compose setups, environment files, CI/CD workflows, test files, and documentation have been updated to reflect these new port assignments. A knowledge base document has been added to document the new port mappings.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
claude/CLAUDE-KNOWLEDGE.mdapps/dev-launchpad/public/index.htmldocker/dependencies/docker.compose.yamldocker/emulator/docker.compose.yamlapps/backend/.envapps/backend/.env.developmentdocker/server/.env.examplepackage.json.devcontainer/devcontainer.jsonapps/e2e/.env.development.github/workflows/check-prisma-migrations.yaml.github/workflows/docker-server-test.yaml.github/workflows/e2e-api-tests.yaml.github/workflows/e2e-source-of-truth-api-tests.yaml.github/workflows/restart-dev-and-test.yamlapps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/email.test.tsapps/e2e/tests/backend/endpoints/api/v1/send-email.test.tsapps/e2e/tests/backend/endpoints/api/v1/unsubscribe-link.test.tsapps/e2e/tests/backend/workflows.test.tsdocs/templates/others/self-host.mdxImportant
This PR introduces customizable development ports using
NEXT_PUBLIC_STACK_PORT_PREFIX, updating configurations, documentation, and tests accordingly.NEXT_PUBLIC_STACK_PORT_PREFIX.5432to${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}28.2500to${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}29.1100to${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}30.4318to${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}31.docker.compose.yamlto use new port variables for services like PostgreSQL, Inbucket, and OpenTelemetry.apps/backend,apps/dashboard, andapps/e2eupdated to useNEXT_PUBLIC_STACK_PORT_PREFIX.package.jsonscripts updated to reflect new port configurations.CLAUDE-KNOWLEDGE.mdto document new port mappings.self-host.mdxto reflect new port configurations.apps/e2e/teststo use new port configurations.helpers/ports.tsfor port-related utilities in tests.This description was created by
Summary by CodeRabbit