fix(link): require owned listener before tunnel startup

[luvs01]

Summary

• The link supervisor was started unconditionally when the optional listener set started. If the link listener failed to bind (port busy, permission), tunnels could start against a listener this process does not own.
• linkListenerOwnsTarget now gates supervisor startup on an owned, bound listener, and issue() calls supervisor.ensureStarted() so a listener that binds later still recovers the supervisor before issuing a link.
• Relationship to fix(link): Remote Link loads and probes SSH hosts from the local dashboard #5928: complementary, no overlap. fix(link): Remote Link loads and probes SSH hosts from the local dashboard #5928 (maintainer) improves Remote Link host probing/GUI; this PR fixes startup ordering on the hub side. The only shared file is src/server/management/link-routes.ts, where this adds one line in issue() away from fix(link): Remote Link loads and probes SSH hosts from the local dashboard #5928's probing changes.

Verification

• tests/server/link-listener-lifecycle.test.ts and tests/server/link-management-routes.test.ts cover the owned-listener gate and the ensureStarted recovery path.
• Full fork CI green at head codex/fix-link-tunnels-startup-on-bind-failure (test shards 1-4, hygiene, structure gate all SUCCESS).

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

• Bug Fixes
  • Link supervision now starts only when the listener is active, and link requests can recover from a failed listener before returning a successful response.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 725f7e92-e5b9-400f-88b3-0b5e7e027a8a

📥 Commits

Reviewing files that changed from the base of the PR and between 0c8e483 and 674171a.

📒 Files selected for processing (6)

• src/server/index/link-listener.ts
• src/server/index/optional-listeners.ts
• src/server/management/link-routes.ts
• structure/remote-link.md
• tests/server/link-listener-lifecycle.test.ts
• tests/server/link-management-routes.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The link listener now exposes whether it owns a reverse-forward target. Optional startup uses that status to decide whether to start the supervisor. Link issuance starts the supervisor after confirming that the listener is listening.

Changes

Link listener and supervisor startup

Layer / File(s)	Summary
Determine target ownership src/server/index/link-listener.ts (lines 32–35), src/server/index/optional-listeners.ts (lines 9, 86–88), tests/server/link-listener-lifecycle.test.ts (lines 6, 80–85)	Adds linkListenerOwnsTarget, which returns true only when the listener is listening and has a non-null port. Optional startup uses this predicate before starting the supervisor. The lifecycle test covers listening, failed, and off states.
Order listener and supervisor startup src/server/management/link-routes.ts (line 365), tests/server/link-management-routes.test.ts (lines 218–233)	Link issuance starts the supervisor after confirming that the listener is listening. The test checks recovery from a failed listener and verifies that listener startup occurs before supervisor startup.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 67417

Link issuance can recover the listener before returning credentials, with no established user-visible regression in the changed startup flow.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 67417

The new startup order reduces the chance of starting tunnels without a listener this process owns. An issuance and removal happening at the same time may still produce a successful response for a link that has already been removed; no new unauthorized access was established.

Retained concerns

• Low · reliability · inferred: Issuance can return a key and listener port after concurrent removal has revoked that link and closed the last listener. The added supervisor await creates a pause after the ownership-status check but before the success response.

Security review details

Security Blast Radius

• inferred — The affected target is the hub's loopback link listener and the reverse forwards aimed at its stored port, rather than a newly exposed management route. The available evidence does not establish wider tenant or deployment exposure.

Security Findings and Attack Paths

• inferred — No introduced unauthorized tunnel path was established: issuance is admin-gated, and its client-initiated record does not pass the supervisor's hub-initiated spawn condition. The concurrent-removal concern can instead return an already-revoked credential or an unowned listener port.

Trust Boundaries and Controls

• observed — The link-issue handler checks admin authority before processing request-controlled alias and tunnel port. Supervisor tunnel creation separately requires a hub-initiated record and a stored listener port.

Resilience and Maintainability Implications

• observed — Failed issuance attempts use revoke-first compensation; cleanup failures are represented as compensation failures rather than silently dropping residual records. This does not make the listener-status check atomic with the eventual success response.

Hardening Proposals

• proposed — Coordinate issuance and removal across the listener-ownership and response-commit transition, so a successful issuance cannot race with deletion of its record and closure of its target.

🚥 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. (1 skipped: 1 …	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 summarizes the main change: tunnel startup now requires an owned, bound listener. It matches the gating and recovery changes described in the pull request.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

리뷰 · 우선순위 22 / 80

허브가 켜질 때 링크 포트를 이 프로그램이 못 열어도, 터널은 바로 시작됐습니다. 터널은 파일에 적힌 포트로 들어갑니다. 그 포트는 다른 프로그램이 이미 열고 있을 수 있습니다.

이 PR은 이 프로그램이 포트를 연 뒤에만 터널을 시작합니다. 상태는 listening이고 포트 번호가 있어야 합니다. 시작할 때 포트를 못 열면 터널은 꺼 둡니다. 나중에 링크를 발급하는 issue()가 포트를 다시 열고, 그때 터널을 켭니다. 허브가 상대에게 붙이는 apply()는 원래 그 순서입니다. 바탕 브랜치는 dev입니다. types.ts와 config.ts를 나누는 변경은 아닙니다. #5928은 화면에서 SSH 호스트를 읽고 확인하는 수정이라, 겹치는 파일은 link-routes.ts 하나이고 고친 줄은 다릅니다.

라인 - src/server/management/link-routes.ts 438줄. 허브가 만든 링크를 지우다가 상대 끊기가 실패하면 restartTunnel이 supervisor.ensureStarted()를 부릅니다. 포트가 이 프로그램 것인지 보지 않습니다. 시작 때 터널을 건너뛴 뒤 여기서 지우기가 실패하면, 파일의 포트로 터널이 다시 켜집니다. 그 포트를 이 프로그램이 연 것이 아닙니다.

라인 - tests/server/link-listener-lifecycle.test.ts 80줄. 새 테스트는 가짜 상태를 넣어 linkListenerOwnsTarget이 참인지 거짓인지만 봅니다. optional-listeners.ts 86줄에서 supervisor.start()를 건너뛰는 코드는 테스트가 없습니다. 발급 실패 테스트도, 포트가 계속 실패면 supervisor 기록이 없는지 확인하지 않습니다.

메인테이너의 판단이 필요한 지점

시작할 때 포트를 못 열면, 이미 있던 허브 터널은 다음 issue()나 apply()까지 꺼져 있습니다. 그 사이에 포트를 다시 여는 반복 작업은 없습니다. 그 대기로 충분한지 정하면 됩니다. 438줄도 포트가 listening인 뒤에만 터널을 다시 켤지 같이 정하면 됩니다. #5928과 이 PR은 고치는 곳이 달라서, 이 PR을 중복으로 닫을 이유는 없습니다.

너의 추천

포트를 연 뒤에만 터널을 켜는 시작 조건은 맞습니다. 발급이 포트를 복구한 뒤 365줄에서 터널을 켜는 순서도 테스트와 맞습니다. 머지 전에 438줄은 포트가 열린 뒤에만 ensureStarted()를 부르게 하면, 지우기 실패가 예전 구멍을 다시 열지 않습니다. types.ts/config.ts 분할 때문에 이 PR을 닫을 이유는 없습니다.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Thanks! This landed on dev through bug-PR merge train batch 9C, #5987 (merge 81aea0f). Your change is one commit on dev with you as the author and a Co-authored-by trailer. A regression was added on top for a failed optional bind (the supervisor does not start) followed by successful recovery. Closing since the content is now on dev.