Repository navigation
feat(link): add the #remote Remote Link page - #5807
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (128)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
the
intake: hygiene-blocked
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01e9f06da6
ℹ️ 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".
|
|
||
| <dialog ref={sheetRef} className="remote-link-sheet" aria-labelledby="remote-link-sheet-title" onCancel={event => { event.preventDefault(); closeSheet(); }}> | ||
| <div className="remote-link-sheet-head"><h3 id="remote-link-sheet-title">{t("link.sheetTitle")}</h3><button type="button" className="btn btn-ghost btn-icon" onClick={closeSheet} aria-label={t("link.close")}><IconX /></button></div> | ||
| <div className="remote-link-sheet-body"><div><h4>{t("link.candidates")}</h4>{busy === "candidates" ? <p className="remote-link-info">{t("link.loading")}</p> : candidates.length > 0 ? <div className="remote-link-candidates">{candidates.map(candidate => <button type="button" className="remote-link-candidate" key={`${candidate.source}:${candidate.alias}`} onClick={() => setAlias(candidate.alias)}><span>{candidate.alias}</span><small>{candidate.source}</small></button>)}</div> : <p className="remote-link-info">{t("link.noCandidates")}</p>}</div><div className="remote-link-form"><label htmlFor="remote-link-alias">{t("link.alias")}</label><input id="remote-link-alias" value={alias} onChange={event => setAlias(event.target.value)} placeholder={t("link.aliasPlaceholder")} autoComplete="off" /><button type="button" className="btn btn-ghost" onClick={() => void runProbe()} disabled={!alias.trim() || busy !== null}><IconLink />{busy === "probe" ? t("link.probing") : t("link.probe")}</button></div>{probe && <div className="remote-link-panel"><div><span className="remote-link-info">{t("link.hostFingerprint")}</span><p className="remote-link-fingerprint"><code>{probe.fingerprint}</code></p><span className="remote-link-info">{probe.keyType}</span></div><label><input type="checkbox" checked={checkedFingerprint} onChange={event => setCheckedFingerprint(event.target.checked)} /> {t("link.confirmFingerprint")}</label><button type="button" className="btn btn-primary" onClick={() => void confirmHost()} disabled={!checkedFingerprint || busy !== null}>{busy === "confirm" ? t("link.confirming") : t("link.confirm")}</button></div>}{confirmation && <div className="remote-link-panel"><p className="remote-link-info">{t("link.ocxVersion", { version: confirmation.ocxVersion })}</p><button type="button" className="btn btn-primary" onClick={() => void applyLink()} disabled={busy !== null}>{busy === "apply" ? t("link.applying") : t("link.apply")}</button></div>}{actionError && <Notice tone="err"><span className="remote-link-error">{t(actionError)}</span></Notice>}<div className="remote-link-sheet-actions"><button type="button" className="btn btn-ghost" onClick={closeSheet}>{t("link.cancel")}</button></div></div> |
There was a problem hiding this comment.
After probing host A, selecting candidate B or editing the alias only calls setAlias; the existing probe, confirmation, and fingerprint checkbox remain bound to A. The form therefore appears to target B while Confirm host and Connect child still authenticate and apply A, potentially issuing a link key to the wrong machine. Clear all host-proof state whenever the alias changes, or prevent alias changes once probing begins.
AGENTS.md reference: AGENTS.md:L437-L443
Useful? React with 👍 / 👎.
| {page === "usage" && <Usage apiBase={sharedBase} connected={targets.connected} apiKeyId={targets.apiKeyId} />} | ||
| {page === "storage" && <Storage apiBase={sharedBase} />} | ||
| {page === "remote" && <RemoteWorkspace apiBase={sharedBase} hubOrigin={targets.shared.serverOrigin} />} | ||
| {page === "remote" && <RemoteLink apiBase={sharedBase} sessionReady={targets.connected && sharedSessionReady} workspaceAvailable={remoteWorkspaceAvailable} onOpenWorkspace={() => navigateToPage("remote-workspace")} />} |
There was a problem hiding this comment.
For normal standalone and hub/Home dashboards, targets.connected is false because it denotes only the connected-client runtime, so this expression passes sessionReady={false} even when sharedSessionReady contains a valid paired dashboard session. RemoteLink consequently renders only the sign-in warning and makes no /api/link/* requests, making the advertised Home workflow inaccessible on the machine that must create the link. Gate this on the actual shared session readiness rather than client topology.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| } else if (value.links.length === 0) { | ||
| setUiState(current => ["role-select", "adding-child", "confirming-host", "applying"].includes(current) ? current : "off"); | ||
| } else if (value.links.some(link => link.state === "failed")) setUiState("failed"); | ||
| else if (value.links.some(link => link.state === "reconnecting")) setUiState("reconnecting"); | ||
| else if (value.links.some(link => link.state === "connected")) setUiState("connected"); |
There was a problem hiding this comment.
When a persisted link restarts but the link listener cannot bind or persist its port, /api/link/status reports listener.state === "failed" while the link rows may still be idle or even retain another non-failed state. This state machine inspects only value.links, so the page can show no failure banner—and may show a connected-looking row—even though the ingress cannot serve the Child. Include the listener state in the failure derivation and render an actionable listener error.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
리뷰 · 우선순위 70 / 80이 PR은 원격 링크의 다섯 번째 층이다. 라인 - 라인 - 라인 - 라인 - 메인테이너의 판단이 필요한 지점 집 화면의 세션 조건을 별칭이 바뀌면 지문을 지울지, 시험이 시작된 뒤에는 별칭을 잠글지 정해 달라. 지문 확인이 이 층의 안전장치다. 리스너 실패를 이 화면에서 보여줄지, 링크 줄의 실패만 보여줄지 정해 달라. 너의 추천 방향은 맞다. 스택에 두고, #5803 이 이 댓글은 grok-bot이 작성했습니다 |
01e9f06 to
ac92998
Compare
github-actions
Bot
removed
the
intake: hygiene-blocked
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head a019d392d283a6d8a13bef32af78c9424aa54f9a. The lint/React Doctor cleanup is present, but three functional blockers from the current UI remain:
gui/src/App.tsx:548passessessionReady={targets.connected && sharedSessionReady}. A normal Home/standalone dashboard hastargets.connected === false, so even a valid paired session can never open the link workflow or call/api/link/*. Gate on the actual shared-session readiness, with any topology condition stated separately.gui/src/pages/RemoteLink.tsx:261changesaliaswithout clearingprobe,confirmation, orcheckedFingerprint. After probing host A, the visible field can point at host B while Confirm/Apply still use A's proof. Reset all proof state on every alias change, or lock the alias after probing.refreshStatusderives failure only from link rows. A persisted link can coexist withlistener.state === "failed", leaving the page without an actionable error and potentially showing a connected-looking child while ingress is unavailable. Include listener failure in the page state and rendering.
Please add regressions for Home-session admission, alias changes after probe/confirmation, and failed-listener status. This PR also remains stacked on #5803, whose current head has separate lifecycle blockers.
There was a problem hiding this comment.
Re-reviewed exact head 2b47b7054a960af3c74728f5d5713ffe045222a5. The new sharedSessionReady gate and Home/standalone route regression fix blocker 1 (focused GUI suites pass 17/17 under CPUQuota=200%, MemoryMax=4G, MemorySwapMax=0, TasksMax=128). Two blockers remain unchanged: (1) candidate clicks and alias input still call setAlias directly without clearing probe, confirmation, or checkedFingerprint, so displayed alias B can coexist with host proof and apply payload for A; (2) refreshStatus still derives UI failure only from link rows and ignores value.listener.state === "failed", so failed ingress can render a connected-looking link without an actionable listener error. Please reset or lock proof state on alias changes and surface listener failure, with regressions for both. This stacked PR also remains blocked by lower-layer #5803.
a0a5a24 to
1cefb16
Compare
#remote now opens Remote Link: a switch that starts off over a blurred preview, a Home/Child choice (Child waits for the find-home flow), the child list with labelled status, and an add-child dialog that probes an SSH host, shows its fingerprint for confirmation and applies the link. Removing a link offers a local-only removal when the child cannot be reached. Remote Workspace moves to #remote-workspace with a card on #remote when it is available. All ten GUI locales and eight docs-site locales are updated, and a fixture server reproduces the screenshots.
… route A failed probe or apply stays on screen and Retry repeats the failed step; polling aborts stale requests and never lets an older response win. #remote-workspace shows a fallback with a way back when Remote Workspace is unavailable. The role choice is a keyboard radiogroup, the disconnect dialog returns focus to its button, and tunnel failure reasons are translated.
Workspace visibility is derived from the session instead of reset in an effect, the status parser lives in remote-link-api.ts, status styling keys off data-state, and the first status read runs through an effect event.
The unavailable-workspace notice links to Remote Link with a link-btn button instead of a #remote fragment, the status-poll effect drops an unused dependency, the tests drop unused fetch parameters, and styles.css no longer re-imports the stylesheet RemoteLink.tsx already imports.
The page was gated on a connected-client target, so a Home or standalone dashboard, which has its own loopback session but no client connection, stayed on the sign-in notice. The page now needs only the shared session. The screenshot fixture served a fake client runtime, which hid this; it now serves a hub. An App-level test mounts both roles at #remote.
2b47b70 to
6fa1256
Compare
2 participants
Summary
Fifth layer of the remote link stack.
#remotenow opens Remote Link, the page that links this machine to others over SSH, using the/api/link/*routes from L4.compensation_failedwith a removal action).<dialog>with SSH host candidates and manual entry. Test connection probes the host and shows its fingerprint, which must be ticked before Confirm host. Connect child then applies the link.{force: true}) and tells the user to runocx disconnecton the child.#remote-workspace. When it is available,#remoteshows a card linking to it so old bookmarks are one click away. When it is not, the route shows a fallback with a way back.link-routes.tsemits, and a test fails if the server adds a code the GUI does not know. Tunnel failure reasons are translated.gui/scripts/remote-link-fixture.tsserves the built dashboard with fixed/api/link/*responses, so the screenshots below can be reproduced without SSH.Stack: #5788 (L1) ← #5798 (L2) ← #5801 (L3) ← #5803 (L4) ← this PR (L5) ← child-initiated flow (L6). Base is
codex/remote-link-4-api.Screenshots
Off (switch and blurred preview), then role choice:
Add child dialog, then host fingerprint confirmation:
Home with a connected child (desktop and mobile):
All 15 captures (en/ko desktop, en mobile) are under
pr-assets@df24377da4/260925-remote-link-home. They were retaken with the fixture serving a hub runtime; the earlier set was taken with a fake connected-client runtime.Verification
cd gui && bun test tests: 2,358 pass, 0 fail. Covers the error-code parity test, every UX state, off and role choice issuing no mutation, the force path, failed-state retry for probe and apply, cancel after a failed apply, stale polling responses, focus restoration, the radiogroup keys, and workspace gating.bun run lint,bun run lint:i18n,bun run build(existing chunk-size warning only): pass.bun run typecheck,bun run structure:check,bun run privacy:scan,tests/guiplus test-layout (457 pass).cd docs-site && bun run build: 513 pages, internal links checked.e604b7b): CI onac92998failedreact-doctor(a#remotefragment with no matching id, an unused effect dependency, two unused test parameters) and thefile-size ratchet(gui/src/styles.cssgrew one line from a duplicate@import). The notice now uses alink-btnbutton, the effect and tests drop the unused names, andRemoteLink.tsxremains the only importer ofstyles-remote-link.css. Localreact-doctor@0.9.11 --scope changed: no issues. The ratchet test, the Remote Link, Remote Workspace, and locale-parity GUI tests,bun run lint, andbun run buildpass.a019d39): with the ratchet fixed, the same CI shard reachedheadless GUI parity CLI, which requires every GUI management endpoint to map to a CLI resource./api/linknow maps toocx linkintests/cli/cli-headless-parity.test.ts(81 pass).2b47b70): the page was gated on a connected-client target (targets.connected), so a Home or standalone dashboard, which has its own loopback session but no client connection, stayed on the sign-in notice. The fixture served a fake client runtime, which hid it. The page now needs only the shared session, the fixture serves a hub, andgui/tests/remote-link-route.test.tsxmounts the whole App at#remotefor the standalone and hub roles (red before the fix, green after).cd gui && bun test tests: 2,360 pass; lint, React Doctor, and build pass.Checklist