feat: Enhance copy_env_file to support directory and destination path#7
feat: Enhance copy_env_file to support directory and destination path#7FlamesCN wants to merge 2 commits intonotdp:mainfrom
Conversation
FlamesCN
commented
Nov 1, 2025
- Allow copy_env_file to copy directories.
- Support specifying a destination path.
- Add a message for missing source files.
- Allow copy_env_file to copy directories. - Support specifying a destination path. - Add a message for missing source files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| if [ -z "$effective_port" ] && [ -n "$port_candidate" ]; then | ||
| if [ "$port_candidate" -lt 1024 ]; then | ||
| skip_dev=1 | ||
| skip_dev_port="$port_candidate" |
There was a problem hiding this comment.
Restore skip logic when no port can be inferred
The new condition now executes only when port_candidate is non-empty, so names like wt add docs leave effective_port and port_candidate empty and the block never sets skip_dev/skip_dev_reason. As a result the dev server is started with no PORT set, falling back to whatever default the command uses and potentially colliding with other services, whereas previously cmd_add explicitly skipped the dev command and printed dev_skipped_no_port. Consider reintroducing the skip_dev branch for the empty-port case so default ports are not started unexpectedly.
Useful? React with 👍 / 👎.
|
The local worktree is designed to be deleted immediately after use, so it only needs to copy one environment variable file to support local application startup. If not started locally, this environment variable is not even needed. The existing The Additionally, Codex's review comments pointed out regression issues introduced by this PR. |