Skip to content

[codex] Use Conductor port range for worktree defaults#294

Merged
michaelmwu merged 11 commits into
mainfrom
michaelmwu/conductor-port-range
May 20, 2026
Merged

[codex] Use Conductor port range for worktree defaults#294
michaelmwu merged 11 commits into
mainfrom
michaelmwu/conductor-port-range

Conversation

@michaelmwu
Copy link
Copy Markdown
Member

@michaelmwu michaelmwu commented May 19, 2026

Summary

  • Use CONDUCTOR_PORT as the base for unset worktree port defaults inside Conductor workspaces.
  • Map services into the assigned 10-port range: Redis +0, Postgres +1, Compose web +2, MinIO API +3, MinIO console +4, host-run web/API +5, and bot health +6.
  • Keep existing explicit port override behavior, and fail loudly if the Conductor-derived HTTP defaults would land on browser-unsafe ports.
  • Add scripts/archive-workspace.sh with --dry-run support to stop workspace-scoped host dev processes and run ./scripts/docker-compose.sh down --remove-orphans.
  • Teach ./scripts/dev.sh web and ./scripts/dev.sh discord-bot to reclaim occupied dev ports from same-service processes in the same Conductor repo workspace group before startup.
  • Document the Conductor port mapping and archive command in README/ENVIRONMENT.

Why

Conductor assigns each workspace a narrow 10-port range via CONDUCTOR_PORT. The previous worktree defaults spread service ports across broad deterministic ranges, which made Conductor port allocation less useful and increased the chance of workspace port mismatch.

Archived or stale workspaces can also leave host-run dev processes behind. The archive helper scopes cleanup to this workspace by command path, cwd, descendant process tree, or this workspace's assigned Conductor listener ports. The dev run path now also handles the common startup failure where a stale same-service listener from a sibling 508-workflows Conductor workspace still owns the desired port.

Validation

  • /bin/sh -n scripts/worktree-env.sh scripts/dev.sh scripts/docker-compose.sh
  • /bin/bash -n scripts/archive-workspace.sh
  • ./scripts/archive-workspace.sh --help
  • CONDUCTOR_PORT=45000 ./scripts/archive-workspace.sh --dry-run --skip-docker
  • CONDUCTOR_PORT=45000 ./scripts/archive-workspace.sh --dry-run
  • uv run pytest tests/unit/test_worktree_env.py
  • uv run pytest tests/unit/test_dev_mux.py tests/unit/test_worktree_env.py
  • uv run ruff check scripts/dev_mux.py tests/unit/test_dev_mux.py
  • uv run mypy scripts/dev_mux.py tests/unit/test_dev_mux.py
  • Manual CONDUCTOR_PORT=45000 ./scripts/dev.sh ports
  • Manual CONDUCTOR_PORT=45000 ./scripts/docker-compose.sh print-ports
  • Manual python3 scripts/dev_mux.py --ensure-port web and --ensure-port discord-bot with resolved dev env

Summary by CodeRabbit

Release Notes

  • New Features

    • Added workspace archival workflow to cleanly shut down development environments (with dry-run support).
    • Automatic port reclamation ensures services start without conflicts.
  • Documentation

    • Clarified port assignment behavior and defaults in Conductor environments.
    • Enhanced environment variable documentation with explicit offset guidance.

Review Change Stack

Copilot AI review requested due to automatic review settings May 19, 2026 22:45
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Warning

Rate limit exceeded

@michaelmwu has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 1 second before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dc017482-e0b4-440d-85c0-e4c4328e5b4a

📥 Commits

Reviewing files that changed from the base of the PR and between 0979efc and 5101471.

📒 Files selected for processing (7)
  • README.md
  • docs/configuration.md
  • scripts/archive-workspace.sh
  • scripts/dev.sh
  • scripts/dev_mux.py
  • tests/unit/test_dev_mux.py
  • tests/unit/test_worktree_env.py
📝 Walkthrough

Walkthrough

This PR integrates Conductor-aware port management across the dev environment: CONDUCTOR_PORT becomes the base of a deterministic 10-port workspace range; services auto-reclaim their ports before startup by stopping previous instances; and a new archival script cleanly terminates all host processes and Docker services for a workspace.

Changes

Conductor Port Scheme and Workspace Management

Layer / File(s) Summary
Documentation of Conductor port scheme
ENVIRONMENT.md, README.md
Port offset documentation added: CONDUCTOR_PORT + 0 (Redis), +1 (Postgres), +2 (Webhook), +3/+4 (MinIO API/console), +5 (Webhook port), +6 (healthcheck); fallback to deterministic per-worktree slot offsets when Conductor unavailable; includes guidance on browser-unsafe ports and pinning.
Conductor port base validation and default computation
scripts/worktree-env.sh
New worktree_env_decimal_value for numeric normalization; worktree_env_resolve_conductor_port_base validates CONDUCTOR_PORT and reserves 10-port range; worktree_env_load conditionally derives defaults from Conductor base or falls back to slot-based offsets; browser-safe port finalization rejects conductor defaults that would map to unsafe ports.
Process discovery and port reclaim helpers
scripts/dev_mux.py
New ProcessInfo NamedTuple; suite of helpers discover PID CWD, build process trees via ps, match processes by workspace/service scope, identify reclaimable PIDs; _ensure_ports_available now workspace-aware: identifies listeners, determines reclamability by service/workspace match, stops related PIDs (ancestors/descendants), re-checks port; main accepts optional argv and supports --ensure-port web|discord-bot mode.
Service port reclaim during startup
scripts/dev.sh
New reclaim_service_port() helper invokes dev_mux.py --ensure-port; integrated before migrations and service startup for web and discord-bot to ensure ports are free and prior instances cleanly terminated.
Workspace archival and manual cleanup
scripts/archive-workspace.sh
New Bash script with embedded Python process discovery: resolves workspace root from CONDUCTOR_WORKSPACE_PATH, git, or cwd; validates scripts/docker-compose.sh executability; discovers processes by command/path and Conductor port range via lsof; applies graceful TERM then forceful KILL; runs or previews Docker Compose shutdown; supports --dry-run and --skip-docker options.
Tests for port reclaim behavior
tests/unit/test_dev_mux.py
Five new tests cover _ensure_ports_available reclaim scenarios: reclaiming same service within conductor workspace, rejecting different service, rejecting unrelated same-worktree listeners, reclaiming via parent process tree, and rejecting prefix-sibling workspace conflicts; all use monkeypatched process/port helpers.
Tests for Conductor port defaults and validation
tests/unit/test_worktree_env.py
Ten new tests validate conductor port scheme: derives 10-port defaults from CONDUCTOR_PORT, normalizes leading-zero input, preserves explicit overrides, rejects when range unavailable, rejects browser-unsafe conductor defaults; new _base_env() helper sanitizes subprocess environment for isolation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • 508-dev/508-workflows#238: Both PRs modify scripts/dev_mux.py port-availability and reclaim logic and scripts/worktree-env.sh deterministic per-worktree port defaults; this PR's Conductor-aware reclaim and defaulting is a direct evolution of the same code paths.
  • 508-dev/508-workflows#277: Both PRs modify scripts/worktree-env.sh WEB_PORT/WEB_HOST_PORT resolution; one adds Conductor-based deterministic default/offset behavior, the other handles preferred port handling with deprecated fallbacks.

Poem

🐰 The rabbit hops through ten-port slots so bright,
Each service claims its Conductor-based light,
Old processes gracefully exit the stage,
While archival scripts clean the workspace page,
Port harmony reigns from the darkness of night! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: using Conductor's port range for worktree default port assignments, which is the primary objective of this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch michaelmwu/conductor-port-range

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the worktree environment port-defaulting logic to honor Conductor’s CONDUCTOR_PORT-assigned 10-port workspace range when available, keeping existing explicit override precedence and adding a hard failure for browser-unsafe Conductor-derived HTTP defaults.

Changes:

  • Add CONDUCTOR_PORT-based default port mapping (+0..+6) in worktree_env_load and fail loudly on browser-unsafe Conductor defaults.
  • Update unit tests to cover Conductor default mapping, override precedence, range validation, and unsafe-port rejection.
  • Document the Conductor port mapping and updated default rules in README.md and ENVIRONMENT.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
scripts/worktree-env.sh Introduces Conductor-aware default port computation and stricter handling for browser-unsafe Conductor-derived defaults.
tests/unit/test_worktree_env.py Adds test coverage for Conductor port-range defaults, override behavior, and failure modes.
README.md Documents Conductor port mapping and updates default-port descriptions for impacted variables.
ENVIRONMENT.md Mirrors README’s Conductor mapping/docs for environment variable defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/worktree-env.sh Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread scripts/archive-workspace.sh Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50b5e46b31

ℹ️ 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".

Comment thread scripts/archive-workspace.sh Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0979efc1ec

ℹ️ 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".

Comment thread scripts/dev_mux.py Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20e98caf37

ℹ️ 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".

Comment thread scripts/dev_mux.py Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread scripts/dev_mux.py Outdated
Comment thread scripts/archive-workspace.sh Outdated
@michaelmwu michaelmwu merged commit 2cb6f97 into main May 20, 2026
7 checks passed
@michaelmwu michaelmwu deleted the michaelmwu/conductor-port-range branch May 20, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants