Skip to content

Conversation

@khaliqgant
Copy link
Collaborator

No description provided.

khaliqgant and others added 6 commits January 6, 2026 23:04
Codex and other AI agents were adding text like "I'm Codex (GPT-5) running
in the Codex CLI as agent 'Lead'." to every relay message. This happened
because agents interpreted the startup instruction as a cue to self-identify.

Changes:
- Add explicit instruction in tmux-wrapper.ts: "Do NOT include
  self-identification or preamble in messages. Start with your actual
  response content."
- Add rule to agent-relay-snippet.md documentation reinforcing this
- Add OpenAI/Codex auth check endpoint for CLI helper

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@khaliqgant khaliqgant requested a review from Copilot January 7, 2026 10:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements an interactive terminal (xterm.js) for provider authentication setup and improves the Codex OAuth flow with SSH tunneling. Key changes include:

  • Added interactive terminal mode to bypass auto-injection features during auth flows
  • Implemented SSH tunneling for Codex OAuth callback forwarding in cloud/container environments
  • Unified provider connection UI across dashboard pages with CLI and API key options
  • Enhanced UI consistency: cyan branding, horizontal tabs in settings, smart billing recommendations

Reviewed changes

Copilot reviewed 58 out of 59 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/wrapper/pty-wrapper.ts Added interactive mode guards to disable auto-injection features (hooks, continuity, auto-accept prompts)
src/wrapper/tmux-wrapper.ts Added instruction to avoid self-identification preambles in agent messages
src/dashboard/react-components/XTermInteractive.tsx New interactive terminal component with user input support via WebSocket
src/dashboard/react-components/TerminalProviderSetup.tsx New terminal-based provider auth component with URL detection and cleanup
src/dashboard/react-components/AgentList.tsx Filters out temporary setup agents from display
src/dashboard/react-components/settings/*.tsx UI improvements: horizontal tabs, cyan branding, smart plan recommendations
src/cloud/services/ssh-security.ts New deterministic SSH password derivation using workspace ID + salt
src/cloud/server.ts WebSocket proxy for agent logs, security validation on startup
src/cloud/api/workspaces.ts Agent management endpoints (spawn, list, delete) proxied to workspace
src/cloud/api/codex-auth-helper.ts SSH tunnel info endpoints for CLI-based Codex authentication
src/cloud/provisioner/index.ts SSH service configuration for Fly.io and Docker workspace containers
src/daemon/cli-auth.ts OAuth callback forwarding to workspace, auth status checking
src/cli/index.ts Rewritten codex-auth command to use SSH tunneling instead of local server
deploy/workspace/entrypoint.sh SSH server setup, Codex config management, environment hardening

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


import * as crypto from 'crypto';

const DEFAULT_SALT = 'default-salt-change-in-prod';
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The default salt value 'default-salt-change-in-prod' is insecure. While there is validation logic warning about this, consider using a randomly generated default or refusing to start in production without SSH_PASSWORD_SALT set.

Copilot uses AI. Check for mistakes.
const handleComplete = useCallback(async () => {
// Mark provider as connected in the database
// Use provider.name (anthropic/openai) not provider.id (claude/codex)
const providerName = provider.name || provider.id;
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The comment at line 345 states 'Use provider.name (anthropic/openai) not provider.id (claude/codex)' but the fallback to provider.id contradicts this guidance. Consider either removing the fallback or clarifying when provider.id would be acceptable.

Copilot uses AI. Check for mistakes.
<h3 className="text-white font-medium mb-2">How this works:</h3>
<ol className="text-sm text-text-muted space-y-1 list-decimal list-inside">
<li>The terminal above is interactive - respond to any prompts by typing directly</li>
<li>When a login URL appears, we&apos;ll detect it and show a popup to help you open it</li>
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The help text mentions 'popup' but the actual implementation uses a modal overlay (not a browser popup window). Consider updating to 'we'll show a modal to help you open it' for accuracy.

Suggested change
<li>When a login URL appears, we&apos;ll detect it and show a popup to help you open it</li>
<li>When a login URL appears, we&apos;ll detect it and show a modal to help you open it</li>

Copilot uses AI. Check for mistakes.
}
userId = cliToken.userId;
// Don't delete token - it's also used for auth-status polling
// Cleanup interval will remove it after 10 minutes
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The comment mentions 10 minutes, but the cleanup interval check at line 57 uses different timing. Ensure consistency or clarify the distinction between token lifetime and cleanup schedule.

Suggested change
// Cleanup interval will remove it after 10 minutes
// Periodic cleanup will eventually remove it once it expires

Copilot uses AI. Check for mistakes.
username: tunnelInfo.user,
password: tunnelInfo.password,
readyTimeout: 10000,
// Disable host key checking for simplicity (workspace containers)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Disabling host key verification (hostVerifier: () => true) bypasses SSH security. While acceptable for workspace containers with derived passwords, add a comment explaining this security trade-off to prevent misuse in other contexts.

Suggested change
// Disable host key checking for simplicity (workspace containers)
// SECURITY NOTE:
// This disables SSH host key verification and is only acceptable here because
// we connect to ephemeral workspace containers in a controlled environment
// using derived credentials. Do NOT reuse this pattern for general SSH clients
// or production systems; instead, implement strict host key verification.

Copilot uses AI. Check for mistakes.
# Expose ports
EXPOSE 3888 3889
# 3888: Dashboard/API, 3889: Daemon, 1455: Codex OAuth callback, 2222: SSH (for tunneling)
EXPOSE 3888 3889 1455 2222
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Port 1455 is exposed but the comment at line 71 doesn't mention it. Update the comment to explain all four ports: 3888 (Dashboard/API), 3889 (Daemon), 1455 (Codex OAuth callback), 2222 (SSH tunneling).

Copilot uses AI. Check for mistakes.
{
"name": "agent-relay",
"version": "1.2.3",
"version": "1.3.0",
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Version bump from 1.2.3 to 1.3.0 follows semantic versioning for new features. Ensure CHANGELOG.md is updated to document the interactive terminal and SSH tunneling features.

Suggested change
"version": "1.3.0",
"version": "1.2.3",

Copilot uses AI. Check for mistakes.
@khaliqgant khaliqgant merged commit 8e0bff9 into main Jan 7, 2026
9 checks passed
@khaliqgant khaliqgant deleted the xterm-display branch January 7, 2026 10:53
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.

2 participants