Skip to content

Remove the remote-control web terminal feature#5105

Merged
saddlepaddle merged 1 commit into
mainfrom
remove-remote-control
Jun 4, 2026
Merged

Remove the remote-control web terminal feature#5105
saddlepaddle merged 1 commit into
mainfrom
remove-remote-control

Conversation

@saddlepaddle

@saddlepaddle saddlepaddle commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

What

Fully removes the anonymous-viewer remote control share-link feature (PR #4345 + #4652 + #4657). Web terminal access is authed-only now (the /workspaces route streams through the host-service over the relay's JWT-gated Path A), so this code path was dead.

Removed

  • sharedremote-control-protocol module + WEB_REMOTE_CONTROL_ACCESS flag + package export
  • host-service — WS route, session-manager, host tRPC sub-router, and the viewer/tailRing subsystem in terminal.ts (attachTerminalViewer, terminalSessionExists, pty.writeBytes, notifyViewers*, tail ring) — all introduced by feat: browser-based remote control for v2 desktop terminals #4345
  • cloud tRPCremoteControl router (removed from appRouter)
  • web(public)/agents/remote-control page subtree + proxy allowlist entry
  • desktopTerminalRemoteControlButton + the now-empty TerminalHeaderExtras
  • relay/remote-control/* JWT-skip branch + remoteControlToken log-redaction
  • dbv2_remote_control_sessions table + enums (drop migration 0057; original 0050 kept as history)
  • ci — restored bunx sherif (the -i @xterm/* ignores feat: browser-based remote control for v2 desktop terminals #4345 added for a web/desktop version split that no longer exists)

Pruned the now-dead hostServiceSecret field passed to createApp (the HOST_SERVICE_SECRET env var stays — it's the host-auth PSK / relay-tunnel secret).

Faithfulness vs the original PRs

Diffed every file #4345/#4652/#4657 touched against this revert: every added file is deleted (MobileToolbar was already removed pre-branch; the 0050 migration is intentionally preserved). xterm deps, the next.config.ts relay CSP entry, and MobileTerminalInput are retained because the authed /workspaces WebTerminal now consumes them. No over-removal — the deep-cleanup symbols all originated in #4345.

Verification

  • lint clean · typecheck 28/28 · host-service terminal tests 65/65 · shared 625/625 · sherif clean
  • Pre-existing unrelated failure in v2-project.test.ts (verifyOrgOwner transpile quirk) — confirmed independent of this PR.

Follow-ups (not code)

  • Apply migration 0057 per environment.
  • Archive the web-remote-control-access PostHog flag.

Summary by CodeRabbit

  • Chores
    • Removed web/desktop remote-control feature (UI, viewer workflow, token/session APIs, and related tests)
    • Dropped remote-control DB table, enums, and migration journal entry
    • Removed remote-control protocol exports and package subpath
    • Simplified relay/host routing and host-service wiring; terminal output handling refactored
    • Removed web feature flag and updated CI step configuration

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR removes the browser remote-control feature end-to-end: UI components and pages are deleted, host-service remote-control runtime and tRPC endpoints are removed, terminal I/O is refactored to a buffered WS model, relay/auth routing is tightened, database remote-control tables/enums are dropped, and shared protocol exports are removed.

Changes

Remote Control Feature Removal

Layer / File(s) Summary
Desktop terminal UI removal
apps/desktop/src/renderer/.../TerminalPane/*
Removes TerminalHeaderExtras and TerminalRemoteControlButton; terminal pane title rendering updated to use notification indicator.
Web public viewer removal
apps/web/src/app/(public)/agents/remote-control/*, apps/web/src/proxy.ts
Deletes RemoteTerminal viewer loader/page and removes /agents/remote-control/ from publicRoutes allowlist.
Relay routing & logging
apps/relay/src/index.ts
Redacts only token in logs and applies authMiddleware unconditionally for host routes (removes former /remote-control bypass).
Host-service config & app unwiring
packages/host-service/src/app.ts, packages/host-service/src/serve.ts
Stops forwarding hostServiceSecret via createApp config and removes remote-control bootstrap/teardown imports and calls.
Terminal session I/O refactor
packages/host-service/src/terminal/terminal.ts
Refactors from tail-ring/viewer model to byte FIFO buffering + WS broadcast/backpressure; removes raw-byte write path and viewer lifecycle hooks.
tRPC & shared contract cleanup
packages/trpc/src/*, packages/shared/*
Removes remoteControl routers/re-exports from trpc root and router index; removes ./remote-control-protocol export and WEB_REMOTE_CONTROL_ACCESS feature flag.
DB schema & migration
packages/db/drizzle/*, packages/db/src/schema/*
Adds migration to drop v2_remote_control_sessions table and removes related enum exports; updates migration journal.
CI workflow: sherif step
.github/workflows/ci.yml
Runs bunx sherif without previous -i exclusions.

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

"🐰 With twitching whiskers, I hop and cheer,
Bye-bye tokens, routes, and viewers near;
Buffers now hum and CI runs light,
A cleaner stack to sleep tonight.
🥕✨"

🚥 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the primary change: removing the remote-control web terminal feature. It is specific, directly related to the main changeset, and avoids vague terms.
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.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering what was removed, verification steps, and follow-ups.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-remote-control

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@stage-review

stage-review Bot commented Jun 4, 2026

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fully removes the anonymous-viewer remote-control web terminal feature, which was already dead code. All related code paths — the relay JWT-skip bypass, host-service WS route and session manager, cloud tRPC router, web public page, desktop UI button, and the database table — are deleted cleanly.

  • Relay: Replaces the custom per-path middleware (which had a JWT-auth bypass for /remote-control/ paths) with a simple app.use("/hosts/:hostId/*", authMiddleware). The SENSITIVE_QUERY_RE log-redaction regex is trimmed to remove the now-unused remoteControlToken term. Cross-region replay and tunnel-presence checks are preserved because authMiddleware already handles them.
  • Database: Adds drop migration 0057 that removes v2_remote_control_sessions and its two dependent enum types via CASCADE.
  • Shared/host-service/trpc/web/desktop: All related modules, routes, UI components, and feature-flag constant (WEB_REMOTE_CONTROL_ACCESS) are deleted; hostServiceSecret is pruned from createApp's config object (the underlying HOST_SERVICE_SECRET env var is retained as the relay-tunnel PSK).

Confidence Score: 5/5

Safe to merge — this is a dead-code removal with no live consumers of the deleted paths.

The relay change is the most load-bearing: replacing a custom per-path middleware (which had an intentional auth bypass for remote-control viewers) with the standard authMiddleware is correct because the bypassed paths no longer exist. Cross-region replay and tunnel-presence checks are already handled inside authMiddleware, so no behavior is lost for the remaining routes. DB migration uses CASCADE appropriately. The only finding is a missing newline at EOF in the SQL file.

No files require special attention. The relay middleware swap and DB drop migration are the highest-impact changes and both look correct.

Important Files Changed

Filename Overview
apps/relay/src/index.ts Replaces complex per-path middleware (with JWT-skip for /remote-control/) with a single authMiddleware; trimmed log-redaction regex. Correct: tunnel-presence replay is preserved inside authMiddleware.
packages/db/drizzle/0057_drop_remote_control_sessions.sql Drops the v2_remote_control_sessions table and two enum types with CASCADE. Missing trailing newline.
packages/host-service/src/app.ts Removed hostServiceSecret field from CreateAppOptions.config; no other remote-control imports remain.
packages/host-service/src/terminal/terminal.ts Viewer/tail-ring subsystem removed; normal renderer-socket path untouched.
packages/trpc/src/root.ts remoteControl router removed from appRouter; no remaining references.
apps/web/src/proxy.ts Removed /agents/remote-control/ from publicRoutes allowlist; route now requires auth.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph BEFORE["Before (removed)"]
        B1["Browser viewer\n(no Superset JWT)"] -->|"WS /hosts/:hostId/remote-control/*\n?remoteControlToken=HMAC"| R1["Relay\n(JWT-skip branch)"]
        R1 -->|"Bypass auth,\ncheck tunnel only"| HS1["host-service\n/remote-control/:sessionId"]
        HS1 --> SM1["SessionManager\n+ tail-ring viewer"]
        WEB1["Web public page\n(public)/agents/remote-control/[sessionId]"] --> TRPC1["cloud tRPC\nremoteControl router"]
        DB1["v2_remote_control_sessions\ntable + enums"]
    end

    subgraph AFTER["After (current)"]
        B2["Authenticated user\n(Superset JWT)"] -->|"WS /hosts/:hostId/...\n?token=JWT"| R2["Relay\nauthMiddleware only"]
        R2 -->|"Full JWT + access check"| HS2["host-service\n/terminal/:terminalId"]
        HS2 --> SS2["Normal renderer\nsocket path"]
    end

    BEFORE -.->|"PR removes"| AFTER

    style BEFORE fill:#fee2e2,stroke:#ef4444
    style AFTER fill:#dcfce7,stroke:#22c55e
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/db/drizzle/0057_drop_remote_control_sessions.sql:3
The file is missing a trailing newline. Most editors and POSIX tools expect text files to end with a newline, and some linters (e.g. prettier) will flag this.

```suggestion
DROP TYPE "public"."remote_control_session_status";
```

Reviews (1): Last reviewed commit: "refactor: fully remove the remote-contro..." | Re-trigger Greptile

@@ -0,0 +1,3 @@
DROP TABLE "v2_remote_control_sessions" CASCADE;--> statement-breakpoint
DROP TYPE "public"."remote_control_session_mode";--> statement-breakpoint
DROP TYPE "public"."remote_control_session_status"; No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The file is missing a trailing newline. Most editors and POSIX tools expect text files to end with a newline, and some linters (e.g. prettier) will flag this.

Suggested change
DROP TYPE "public"."remote_control_session_status";
DROP TYPE "public"."remote_control_session_status";
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/db/drizzle/0057_drop_remote_control_sessions.sql
Line: 3

Comment:
The file is missing a trailing newline. Most editors and POSIX tools expect text files to end with a newline, and some linters (e.g. prettier) will flag this.

```suggestion
DROP TYPE "public"."remote_control_session_status";
```

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/host-service/src/terminal/terminal.ts (1)

569-574: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Flush timed-out shell-ready bytes to live sockets too.

These bytes are currently only enqueued into the replay FIFO. If a renderer is already attached when the OSC 133 scan times out, the user never sees them in the active terminal; they only show up after a later reattach. This should follow the normal output path: broadcast first, then buffer only if nothing was sent.

Proposed fix
 	if (session.scanState.heldBytes.length > 0) {
 		const heldBytes = Uint8Array.from(session.scanState.heldBytes);
 		session.modeTracker.feed(heldBytes);
-		bufferOutput(session, heldBytes);
+		if (broadcastBytes(session, heldBytes) === 0) {
+			bufferOutput(session, heldBytes);
+		}
 		session.scanState.heldBytes.length = 0;
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/host-service/src/terminal/terminal.ts` around lines 569 - 574, The
held marker bytes in session.scanState.heldBytes are only being fed into
session.modeTracker and enqueued via bufferOutput(session, heldBytes), so
attached renderers never see them immediately; change the flow to first feed
session.modeTracker.feed(heldBytes), then attempt to send the same heldBytes
down the live-renderer path (use the module's existing live-send/broadcast
function used elsewhere in this file — e.g., the function used for real-time
output to clients/renderers), and only call bufferOutput(session, heldBytes) if
that live send indicates nothing was delivered (zero/false/no-clients). Ensure
you still clear session.scanState.heldBytes.length = 0 after handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/host-service/src/terminal/terminal.ts`:
- Around line 569-574: The held marker bytes in session.scanState.heldBytes are
only being fed into session.modeTracker and enqueued via bufferOutput(session,
heldBytes), so attached renderers never see them immediately; change the flow to
first feed session.modeTracker.feed(heldBytes), then attempt to send the same
heldBytes down the live-renderer path (use the module's existing
live-send/broadcast function used elsewhere in this file — e.g., the function
used for real-time output to clients/renderers), and only call
bufferOutput(session, heldBytes) if that live send indicates nothing was
delivered (zero/false/no-clients). Ensure you still clear
session.scanState.heldBytes.length = 0 after handling.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2b5cfbf4-289a-4920-ab44-f6b5f62bb8c5

📥 Commits

Reviewing files that changed from the base of the PR and between 30ff989 and 2e71949.

📒 Files selected for processing (33)
  • apps/desktop/src/main/host-service/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/TerminalHeaderExtras.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/TerminalRemoteControlButton.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
  • apps/relay/src/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/RemoteTerminal.tsx
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/RemoteTerminalLoader.tsx
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/page.tsx
  • apps/web/src/proxy.ts
  • packages/db/drizzle/0057_drop_remote_control_sessions.sql
  • packages/db/drizzle/meta/0057_snapshot.json
  • packages/db/drizzle/meta/_journal.json
  • packages/db/src/schema/enums.ts
  • packages/db/src/schema/schema.ts
  • packages/host-service/src/app.ts
  • packages/host-service/src/serve.ts
  • packages/host-service/src/terminal/remote-control/route.ts
  • packages/host-service/src/terminal/remote-control/session-manager.test.ts
  • packages/host-service/src/terminal/remote-control/session-manager.ts
  • packages/host-service/src/terminal/terminal.ts
  • packages/host-service/src/trpc/router/terminal/remote-control.ts
  • packages/host-service/src/trpc/router/terminal/terminal.ts
  • packages/shared/package.json
  • packages/shared/src/constants.ts
  • packages/shared/src/remote-control-protocol.ts
  • packages/trpc/src/root.ts
  • packages/trpc/src/router/remote-control/index.ts
  • packages/trpc/src/router/remote-control/remote-control.ts
  • plans/20260511-remote-control-remaining-work.md
💤 Files with no reviewable changes (28)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/RemoteTerminalLoader.tsx
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/index.ts
  • packages/host-service/src/trpc/router/terminal/remote-control.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/page.tsx
  • packages/host-service/src/terminal/remote-control/session-manager.test.ts
  • packages/trpc/src/router/remote-control/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/index.ts
  • packages/host-service/src/terminal/remote-control/route.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/TerminalHeaderExtras.tsx
  • packages/trpc/src/root.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/TerminalRemoteControlButton.tsx
  • plans/20260511-remote-control-remaining-work.md
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
  • packages/host-service/src/serve.ts
  • packages/shared/src/remote-control-protocol.ts
  • packages/db/src/schema/enums.ts
  • packages/host-service/src/terminal/remote-control/session-manager.ts
  • packages/host-service/src/app.ts
  • packages/trpc/src/router/remote-control/remote-control.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/RemoteTerminal.tsx
  • packages/shared/src/constants.ts
  • packages/shared/package.json
  • apps/desktop/src/main/host-service/index.ts
  • apps/web/src/proxy.ts
  • packages/host-service/src/trpc/router/terminal/terminal.ts
  • packages/db/src/schema/schema.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 33 files

Architecture diagram
sequenceDiagram
    participant Client as Desktop App (Renderer)
    participant Relay as Relay Server
    participant Host as Host Service
    participant Term as Terminal Session (PTY)
    participant Cloud as Cloud API (tRPC)
    participant Web as Web Browser (Public Viewer)

    Note over Client,Web: Authenticated terminal access (unchanged)
    Client->>Relay: WS upgrade /workspaces/... with JWT
    Relay->>Relay: CHANGED: No JWT-skip branch — always verify
    Relay->>Host: Forward WS (over tunnel, JWT-verified)
    Host->>Term: Create session / stream data

    Note over Term: REMOVED: viewer/tailRing subsystem in terminal.ts
    Note over Term: REMOVED: attachTerminalViewer, notifyViewers*, outputSequence, tailRing
    Note over Host: REMOVED: /remote-control/:sessionId WS route and session-manager

    alt Public viewer (REMOVED)
        Note over Web: REMOVED: (public)/agents/remote-control/[sessionId] page
        Note over Web: REMOVED: RemoteTerminal component
        Web->>Cloud: (was) remoteControl.get mutation
        Web->>Relay: (was) WS upgrade with token in query param
        Relay->>Relay: (was) JWT-skip for /remote-control/* paths — REMOVED
        Relay->>Host: (was) Forward to /remote-control/:sessionId
        Host->>Term: (was) attachTerminalViewer, send snapshot/exit
        Note over Host: (was) viewer count/input/resize
    end

    Note over Client,Cloud: Cloud tRPC (REMOVED)
    Note over Cloud: REMOVED: remoteControl router
    Note over Cloud: REMOVED: remoteControl.create, .get, .revoke, .listForWorkspace

    Note over Client: Desktop (REMOVED)
    Note over Client: REMOVED: TerminalRemoteControlButton
    Note over Client: REMOVED: TerminalHeaderExtras

    Note over Host: Host service (cleaned up)
    Note over Host: REMOVED: tRPC terminal.remoteControl sub-router
    Note over Host: REMOVED: revokeSessionsForTerminal on session dispose
    Note over Host: REMOVED: hostServiceSecret config option (env var retained)

    Note over Cloud: Database (REMOVED)
    Note over Cloud: REMOVED: v2_remote_control_sessions table
    Note over Cloud: REMOVED: remote_control_session_mode/status enums

    Note over Cloud: Relay (simplified)
    Note over Cloud: REMOVED: remoteControlToken log redaction term
    Note over Relay: CHANGED: authMiddleware now covers all /hosts/:hostId/*

    Note over Client,Relay: All terminal access now goes through JWT-gated Path A only.
Loading

Re-trigger cubic

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch

Thank you for your contribution! 🎉

Removes the anonymous-viewer "remote control" share-link feature end to end.
Web terminal access is authed-only (via the /workspaces route over the relay's
JWT-gated Path A), so this code was dead.

Removed:
- shared: remote-control-protocol + WEB_REMOTE_CONTROL_ACCESS flag
- host-service: WS route, session-manager, host tRPC sub-router, and the
  viewer/tailRing subsystem in terminal.ts (attachTerminalViewer,
  terminalSessionExists, pty.writeBytes, notifyViewers*)
- cloud trpc: remoteControl router (removed from appRouter)
- web: (public)/agents/remote-control page + proxy public-route entry
- desktop: TerminalRemoteControlButton + TerminalHeaderExtras
- relay: /remote-control/* JWT-skip branch + token log-redaction
- db: v2_remote_control_sessions table + enums (drop migration 0057)

Pruned the now-dead hostServiceSecret field passed to createApp
(HOST_SERVICE_SECRET env var retained as the host-auth PSK / tunnel secret).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)

1-9: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add least-privilege permissions for GITHUB_TOKEN in CI

.github/workflows/ci.yml has no top-level permissions: block, so the workflow uses GitHub defaults; set an explicit least-privilege scope (e.g., contents: read) between on: and jobs:.

Suggested patch
 name: CI
 
 on:
   push:
     branches: [main]
   pull_request:
     types: [opened, synchronize]
+
+permissions:
+  contents: read
 
 jobs:
   sherif:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 1 - 9, Add an explicit top-level
GitHub Actions permissions block to enforce least-privilege for GITHUB_TOKEN:
insert a permissions: section between the existing on: and jobs: blocks and set
minimal scopes (e.g., permissions: contents: read) so the workflow no longer
relies on GitHub defaults; ensure the new block is at the same indentation level
as on: and jobs: and include any other required minimal permissions if the
workflow needs them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1-9: Add an explicit top-level GitHub Actions permissions block to
enforce least-privilege for GITHUB_TOKEN: insert a permissions: section between
the existing on: and jobs: blocks and set minimal scopes (e.g., permissions:
contents: read) so the workflow no longer relies on GitHub defaults; ensure the
new block is at the same indentation level as on: and jobs: and include any
other required minimal permissions if the workflow needs them.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 147da7ec-7925-4c7a-94b6-80d65da5f63e

📥 Commits

Reviewing files that changed from the base of the PR and between 2e71949 and fcb4fc4.

📒 Files selected for processing (34)
  • .github/workflows/ci.yml
  • apps/desktop/src/main/host-service/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/TerminalHeaderExtras.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/TerminalRemoteControlButton.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
  • apps/relay/src/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/RemoteTerminal.tsx
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/RemoteTerminalLoader.tsx
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/page.tsx
  • apps/web/src/proxy.ts
  • packages/db/drizzle/0057_drop_remote_control_sessions.sql
  • packages/db/drizzle/meta/0057_snapshot.json
  • packages/db/drizzle/meta/_journal.json
  • packages/db/src/schema/enums.ts
  • packages/db/src/schema/schema.ts
  • packages/host-service/src/app.ts
  • packages/host-service/src/serve.ts
  • packages/host-service/src/terminal/remote-control/route.ts
  • packages/host-service/src/terminal/remote-control/session-manager.test.ts
  • packages/host-service/src/terminal/remote-control/session-manager.ts
  • packages/host-service/src/terminal/terminal.ts
  • packages/host-service/src/trpc/router/terminal/remote-control.ts
  • packages/host-service/src/trpc/router/terminal/terminal.ts
  • packages/shared/package.json
  • packages/shared/src/constants.ts
  • packages/shared/src/remote-control-protocol.ts
  • packages/trpc/src/root.ts
  • packages/trpc/src/router/remote-control/index.ts
  • packages/trpc/src/router/remote-control/remote-control.ts
  • plans/20260511-remote-control-remaining-work.md
💤 Files with no reviewable changes (28)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/index.ts
  • packages/trpc/src/router/remote-control/index.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/index.ts
  • packages/shared/package.json
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminalLoader/RemoteTerminalLoader.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalRemoteControlButton/TerminalRemoteControlButton.tsx
  • packages/trpc/src/root.ts
  • packages/shared/src/constants.ts
  • packages/db/src/schema/enums.ts
  • apps/web/src/proxy.ts
  • plans/20260511-remote-control-remaining-work.md
  • packages/host-service/src/terminal/remote-control/session-manager.test.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalHeaderExtras/TerminalHeaderExtras.tsx
  • packages/trpc/src/router/remote-control/remote-control.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/components/RemoteTerminal/RemoteTerminal.tsx
  • packages/db/src/schema/schema.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
  • packages/host-service/src/app.ts
  • packages/host-service/src/trpc/router/terminal/remote-control.ts
  • apps/desktop/src/main/host-service/index.ts
  • packages/shared/src/remote-control-protocol.ts
  • packages/host-service/src/terminal/remote-control/route.ts
  • packages/host-service/src/terminal/remote-control/session-manager.ts
  • apps/web/src/app/(public)/agents/remote-control/[sessionId]/page.tsx
  • packages/host-service/src/serve.ts
  • packages/host-service/src/trpc/router/terminal/terminal.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/db/drizzle/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/relay/src/index.ts
  • packages/db/drizzle/0057_drop_remote_control_sessions.sql

@saddlepaddle saddlepaddle merged commit ed58c0c into main Jun 4, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant