[codex] Stabilize worktree compose project names - #11
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ 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 |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_312df7af-ca98-415b-b510-58fa84eaa4b6) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c21bbfe992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| load_port_reservations "$ENV_FILE" | ||
|
|
||
| eval "$(./scripts/worktree-ports.sh export)" |
There was a problem hiding this comment.
When a developer sets POSTGRES_HOST_PORT or REDIS_HOST_PORT in .env, this eval exports the generated values into the shell before the later docker compose --env-file ... --env-file .env call. Docker Compose interpolation precedence puts “variables from your shell environment” ahead of variables from --env-file (docs.docker.com/compose/how-tos/environment-variables/variable-interpolation/), so the local .env no longer has the final override authority promised by this wrapper and Compose will still bind the generated ports.
Useful? React with 👍 / 👎.
| print_env() { | ||
| prefix="$1" | ||
| calculate_ports | ||
| printf '%sCOMPOSE_PROJECT_NAME=%s\n' "$prefix" "$COMPOSE_PROJECT_NAME" |
There was a problem hiding this comment.
For direct ./scripts/worktree-ports.sh env consumers, this inserts COMPOSE_PROJECT_NAME before WEB_URL, violating the documented output contract in docs/development.md:18-22 and README.md:127-130 that keeps WEB_URL first so workspace URL scanners open the web surface. In environments that depend on the first emitted URL/key, the web URL is no longer discoverable as intended.
Useful? React with 👍 / 👎.
| env["FAKE_DOCKER_LOG"] = str(log_path) | ||
| env["FAKE_DOCKER_PS"] = "\n".join( | ||
| [ | ||
| f"same-port\told-project\t{alias_root}\t127.0.0.1:9540->5432/tcp\told-postgres", |
There was a problem hiding this comment.
This hard-coded 9540 only matches checkouts whose path hash assigns Postgres to that exact port; in other workspaces the wrapper looks for the generated POSTGRES_HOST_PORT instead, so the fake stale container is not reclaimed and assert "same-port" in log fails (for example this checkout assigns POSTGRES_HOST_PORT=9140). Derive the fake Ports value from ./scripts/worktree-ports.sh env or reserve the block in the test so it is deterministic across CI paths.
Useful? React with 👍 / 👎.
| case "${1:-}" in | ||
| up) |
There was a problem hiding this comment.
When an adapted repo invokes the wrapper with Docker Compose global options before the subcommand, such as ./scripts/docker-compose.sh --profile debug up -d (the Compose CLI documents --profile and other options at the docker compose level), $1 is not up, so the stale same-worktree cleanup is skipped and the old symlink-named containers can still hold the assigned Postgres/Redis ports. Parse past Compose global options before deciding whether this invocation is an up.
Useful? React with 👍 / 👎.
1 participant
Summary
COMPOSE_PROJECT_NAMEfromscripts/worktree-ports.shand have the Compose wrapper use itWhy
Launching dev scripts through a symlinked Conductor workspace path could produce a different Compose project name for the same real worktree. That made Docker try to start duplicate infra containers on the same assigned ports instead of reusing the existing stack.
Validation
sh -n scripts/worktree-ports.sh && sh -n scripts/docker-compose.sh && sh -n scripts/dev.sh./scripts/worktree-ports.sh env./scripts/docker-compose.sh config --servicesuv run pytest tests/test_worktree_ports.py tests/test_root_worktree_scripts.pyfromstacks/pythonNote
Low Risk
Changes are limited to local dev shell scripts and narrowly scoped Docker cleanup on
compose up; no app runtime, auth, or production paths.Overview
Symlinked worktree paths (e.g. Conductor aliases) now resolve to the same canonical git root as the real checkout, so port blocks and
COMPOSE_PROJECT_NAMEstay identical instead of spawning a second Compose project on the same host ports.worktree-ports.shaddscompose_project_name()and emitsCOMPOSE_PROJECT_NAMEinenv/export.dev.shanddocker-compose.shrecordWORKTREE_ROOTviapwd -Pand run from that directory; the Compose wrapper alsoexports worktree ports before invoking Docker.On
docker compose up, the wrapper reclaims stale containers only when Compose’s recorded working dir resolves to the same real path as the current worktree, the container is from an older project name, and it publishes this worktree’s Postgres/Redis host ports—helpers without host ports and sibling worktrees are skipped.Regression coverage lives in
stacks/python/tests/test_root_worktree_scripts.py(symlink env parity, project name through the wrapper, selectivedocker rm).Reviewed by Cursor Bugbot for commit c21bbfe. Bugbot is set up for automated code reviews on this repo. Configure here.