fix(server): persist worktree runtime port when ambient PORT does not match (#1849)

[manavshrivastavagit]

What was done

Replaced the strict !nonEmpty(process.env.PORT) guard in maybePersistWorktreeRuntimePorts with a new isPortPinnedByRuntimeEnv helper function. This function checks if process.env.PORT is set, but only suppresses persisting the port to configuration if the ambient PORT matches the newly allocated selectedPort.

Why it matters

Fixes issue #1849. Previously, if an ambient PORT environment variable was exported globally (like inheriting from the shell running the parent workspace), worktrees would silently fail to write their collision-avoiding ports (e.g. 3103 instead of 3100) back to their respective local config.json files. This resulted in orphaned sub-worktrees and lost port tracking on reboot. With this fix, worktrees correctly persist their assigned ports even while nested under an inherited environment variables stack, while continuing to respect manual, explicit pinning.

How to verify

1. Export a port in the shell explicitly: export PORT=3100.
2. Launch a sub-worktree instance which receives an auto-assigned free port (e.g., 3103).
3. View the underlying config.json for that worktree inside .paperclip/worktrees/.
4. The config file should correctly contain {"server": {"port": 3103}} rather than dropping the write operation.

Risks

None expected. The Number() and Number.isInteger() checks handle parsing edge cases cleanly, defaulting robustly to preventing writes if process.env.PORT is somehow malformed (e.g., set to a non-integer), ensuring absolute safety during misconfigurations.

[manavshrivastavagit]

cc @devinfoley @cryppadotta: This fixes issue #1849 regarding the missing runtime port persistence during worktree spawns when bounded by an ambient PORT. Could you please take a look, approve, and merge it when you have a moment?

[greptile-apps]

Greptile Summary

This PR fixes a bug (#1849) where worktrees would silently skip persisting their auto-assigned collision-avoiding ports to config.json whenever an ambient PORT environment variable was present in the shell — even when that ambient PORT differed from the port actually selected for the worktree. The fix replaces a coarse !nonEmpty(process.env.PORT) guard with a new isPortPinnedByRuntimeEnv helper that only suppresses the write when the ambient PORT exactly matches the selected port (i.e., the port is genuinely pinned), allowing collision-avoidance assignments to be persisted correctly in inherited-environment scenarios.\n\nKey changes:\n- New private isPortPinnedByRuntimeEnv(rawValue, selectedPort) helper correctly handles all edge cases: empty/unset (false → allow write), malformed/non-integer (true → block write as a safe default), matching integer (true → block write, already pinned), non-matching integer (false → allow write, this is the bug fix).\n- maybePersistWorktreeRuntimePorts updated to use the new helper in place of the old blunt guard.\n\nOne note for the PR author: The PR description does not include a thinking path as described in CONTRIBUTING.md. The description has "What was done", "Why it matters", "How to verify", and "Risks" sections, which is great — but please also add a top-level thinking path that walks from the overall system down to the specific issue being fixed (see examples in CONTRIBUTING.md).\n\nMinor discrepancy: The PR description states "the regex handles parsing edge cases cleanly" — the implementation actually uses Number() and Number.isInteger(), not a regex. The logic is correct; the description just needs a small update to reflect the actual approach.

Confidence Score: 5/5

Safe to merge — the change is small, well-scoped, and all relevant edge cases (empty, malformed, matching, non-matching PORT) are correctly handled.

All remaining findings are P2 (description accuracy, missing thinking path). The code logic is correct and the fix directly addresses the described bug without touching unrelated paths.

No files require special attention.

Important Files Changed

Filename	Overview
server/src/worktree-config.ts	Adds isPortPinnedByRuntimeEnv helper to replace a blanket !nonEmpty(process.env.PORT) guard; logic correctly handles empty, malformed, matching, and non-matching PORT values

Prompt To Fix All With AI

This is a comment left during a code review.
Path: server/src/worktree-config.ts
Line: 443-449

Comment:
**PR description mentions regex — none exists in implementation**

The PR description states _"The regex handles parsing edge cases cleanly"_, but the actual implementation uses `Number()` + `Number.isInteger()` — there is no regex involved. The code is fine as written, but the description is misleading. Consider updating the PR description to reflect the actual parsing approach (`Number()` / `Number.isInteger()`) so reviewers and future maintainers aren't confused when auditing the change.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(server): persist worktree runtime po..." | Re-trigger Greptile}

[manavshrivastavagit]

Good catch! I've updated the PR description to be accurate—thanks for catching that typo. The code implementation correctly handles parsing with Number() and Number.isInteger(), so this was purely a documentation fix. The PR description now accurately pairs with the code.