fix: validate serverUrl port before tmux pane spawn (fixes #2729)#2761
Merged
code-yeongyu merged 1 commit intodevfrom Mar 23, 2026
Merged
fix: validate serverUrl port before tmux pane spawn (fixes #2729)#2761code-yeongyu merged 1 commit intodevfrom
code-yeongyu merged 1 commit intodevfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 4/5
- This PR is likely safe to merge, with minimal risk: the only flagged item is a low-to-moderate test fragility issue (4/10) rather than a production behavior bug.
- In
src/features/tmux-subagent/manager.test.ts, an assertion hard-codes fallback port4096, so tests can fail whenOPENCODE_PORTis set, creating avoidable CI instability. - Pay close attention to
src/features/tmux-subagent/manager.test.ts- make the port assertion environment-aware to avoid brittle failures.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/tmux-subagent/manager.test.ts">
<violation number="1" location="src/features/tmux-subagent/manager.test.ts:250">
P2: The test assertion is brittle because it assumes the fallback port is always 4096. It will fail if `OPENCODE_PORT` is set in the environment.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const manager = new TmuxSessionManager(ctx, config, mockTmuxDeps) | ||
|
|
||
| // then | ||
| expect((manager as any).serverUrl).toBe('http://localhost:4096') |
There was a problem hiding this comment.
P2: The test assertion is brittle because it assumes the fallback port is always 4096. It will fail if OPENCODE_PORT is set in the environment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/tmux-subagent/manager.test.ts, line 250:
<comment>The test assertion is brittle because it assumes the fallback port is always 4096. It will fail if `OPENCODE_PORT` is set in the environment.</comment>
<file context>
@@ -226,6 +226,29 @@ describe('TmuxSessionManager', () => {
+ const manager = new TmuxSessionManager(ctx, config, mockTmuxDeps)
+
+ // then
+ expect((manager as any).serverUrl).toBe('http://localhost:4096')
+ })
})
</file context>
Suggested change
| expect((manager as any).serverUrl).toBe('http://localhost:4096') | |
| const expectedPort = process.env.OPENCODE_PORT ?? '4096' | |
| expect((manager as any).serverUrl).toBe(`http://localhost:${expectedPort}`) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When using
serve+attachmode,ctx.serverUrlcan containhttp://127.0.0.1:0/(port 0), causing tmux pane spawn to fail.Fix
Add port validation in
src/features/tmux-subagent/manager.ts— if port is 0 or invalid, fall back to default port lookup.Fixes #2729
Automated fix by Sisyphus (oh-my-opencode)
Summary by cubic
Validate the server URL port before spawning the tmux pane to prevent failures when
serve+attachsets port 0; if invalid, fall back to the default port fromOPENCODE_PORTor 4096. Fixes #2729.ctx.serverUrland treat port "0" as invalid; use a computedfallbackUrl.Written for commit 0810e37. Summary will update on new commits.