Skip to content

Conversation

sestinj
Copy link
Contributor

@sestinj sestinj commented Sep 10, 2025

Description

cn ls and /resume now show remote sessions if you are logged in, and will start in remote mode


Summary by cubic

Show remote sessions in cn ls and /resume. Selecting a remote session launches Remote Mode automatically.

  • New Features

    • List remote sessions alongside local ones in cn ls and the /resume session picker.
    • Selecting a remote session opens it via the remote command (agent URL), no extra steps.
    • JSON output now includes isRemote and remoteId fields.
    • Added /title slash command to set the current session title.
    • Session selector: scrollable list with remote/local labels and a loading state.
  • Bug Fixes

    • Slash command dropdown hides when a full command with arguments is typed, so Enter executes the command (affects /title).

@sestinj sestinj requested a review from a team as a code owner September 10, 2025 22:26
@sestinj sestinj requested review from Patrick-Erichsen and removed request for a team September 10, 2025 22:26
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 10, 2025
Copy link
Contributor

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

Choose a reason for hiding this comment

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

5 issues found across 14 files

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@sestinj sestinj marked this pull request as draft September 11, 2025 02:33
@sestinj sestinj marked this pull request as ready for review September 13, 2025 02:06
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 13, 2025
sestinj and others added 3 commits September 12, 2025 19:09
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
sestinj and others added 2 commits September 12, 2025 19:10
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Copy link
Contributor

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

Choose a reason for hiding this comment

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

23 issues found across 25 files

Prompt for AI agents (all 23 issues)

Understand the root cause of the following 23 issues and fix them.


<file name="extensions/cli/src/ui/hooks/useChat.remote.helpers.ts">

<violation number="1" location="extensions/cli/src/ui/hooks/useChat.remote.helpers.ts:272">
Embedding diffContent in a shell command enables command injection and is fragile across platforms; pass the patch via stdin to git apply instead.</violation>
</file>

<file name="extensions/cli/src/ui/hooks/useChat.helpers.ts">

<violation number="1" location="extensions/cli/src/ui/hooks/useChat.helpers.ts:268">
The logic for remote commands `/diff` and `/apply` is duplicated across two files. `useChat.helpers.ts` contains `handleRemoteApplyCommand` for the TUI, while `useChat.remote.helpers.ts` has a nearly identical `handleRemoteApply` function. This violates DRY principles and fragments command handling, increasing maintenance overhead and risk of inconsistent behavior.</violation>

<violation number="2" location="extensions/cli/src/ui/hooks/useChat.helpers.ts:292">
Shelling out with echo and an interpolated diff enables command injection and may break patch newlines; avoid the shell and pass diff via stdin to git apply.</violation>
</file>

<file name="extensions/cli/src/slashCommands.remote.test.ts">

<violation number="1" location="extensions/cli/src/slashCommands.remote.test.ts:8">
The new test file `slashCommands.remote.test.ts` tests `handleSlashCommands` from `slashCommands.ts` for `/diff` and `/apply` command handling. However, the implementation for these commands is not in `slashCommands.ts`, but is split between `useChat.helpers.ts` and `useChat.remote.helpers.ts`. The tests are therefore testing the wrong component and provide a false sense of security, leaving the actual implementation untested.</violation>
</file>

<file name="extensions/cli/src/commands/commands.ts">

<violation number="1" location="extensions/cli/src/commands/commands.ts:108">
Comment says only exit is shown in remote mode, but new remote commands &#39;diff&#39; and &#39;apply&#39; were added; update the comment or adjust logic for consistency.</violation>
</file>

<file name="extensions/cli/src/ui/SessionSelector.tsx">

<violation number="1" location="extensions/cli/src/ui/SessionSelector.tsx:81">
Pressing Up with an empty list sets selectedIndex to -1; guard for sessions.length === 0 to avoid negative index.</violation>
</file>

<file name="extensions/cli/build.mjs">

<violation number="1" location="extensions/cli/build.mjs:93">
dist/cn.js is written twice; the later write overwrites this new wrapper and doesn’t call runCli, so the CLI won’t start.</violation>
</file>

<file name="extensions/cli/src/ui/components/SessionSelectorWithLoading.tsx">

<violation number="1" location="extensions/cli/src/ui/components/SessionSelectorWithLoading.tsx:18">
Add an unmount guard or cancellation in this async effect to avoid setting state after unmount and to prevent duplicate/stale updates.</violation>

<violation number="2" location="extensions/cli/src/ui/components/SessionSelectorWithLoading.tsx:25">
Rule violated: **Don&#39;t use console.log**

Replace console.error with the project&#39;s logger (e.g., logger.error) per logging guidelines instead of using console.*</violation>
</file>

<file name="extensions/cli/src/ui/__tests__/TUIChat.slashCommands.test.tsx">

<violation number="1" location="extensions/cli/src/ui/__tests__/TUIChat.slashCommands.test.tsx:110">
Test name claims the dropdown is hidden, but assertions never verify hidden state; either add an assertion for hidden dropdown or rename the test to match what is asserted.</violation>

<violation number="2" location="extensions/cli/src/ui/__tests__/TUIChat.slashCommands.test.tsx:117">
Using fixed timeouts can make tests flaky; prefer waiting for a specific UI condition (e.g., vi.waitFor) instead of arbitrary delays.</violation>

<violation number="3" location="extensions/cli/src/ui/__tests__/TUIChat.slashCommands.test.tsx:143">
Test name asserts Enter executes with dropdown hidden, but the test does not verify either; either add assertions for execution/hidden state or rename to reflect current checks.</violation>

<violation number="4" location="extensions/cli/src/ui/__tests__/TUIChat.slashCommands.test.tsx:159">
Avoid arbitrary 300ms sleep; wait for a concrete post-Enter condition (e.g., input cleared or command output) using vi.waitFor or a helper.</violation>
</file>

<file name="extensions/cli/src/ui/TUIChat.tsx">

<violation number="1" location="extensions/cli/src/ui/TUIChat.tsx:201">
Clearing statusMessage via untracked setTimeout can race and clear a newer message; also no cleanup on unmount.</violation>

<violation number="2" location="extensions/cli/src/ui/TUIChat.tsx:341">
Status messages are always green, so failures (prefixed with ✗) render as success color.</violation>
</file>

<file name="extensions/cli/src/slashCommands.test.ts">

<violation number="1" location="extensions/cli/src/slashCommands.test.ts:301">
Consider asserting that updateSessionTitle was not called to ensure empty titles never trigger a session update.</violation>

<violation number="2" location="extensions/cli/src/slashCommands.test.ts:311">
Add an assertion that updateSessionTitle was not called for whitespace-only input to prevent unintended updates.</violation>

<violation number="3" location="extensions/cli/src/slashCommands.test.ts:323">
Use mockImplementationOnce to keep the throwing behavior scoped to this test and avoid leaking state across tests.</violation>
</file>

<file name="extensions/cli/src/session.ts">

<violation number="1" location="extensions/cli/src/session.ts:311">
Email domain check is case-sensitive; uppercase emails could bypass/disable remote sessions unintentionally. Normalize before endsWith.</violation>

<violation number="2" location="extensions/cli/src/session.ts:334">
Potential runtime error if create_time_ms is undefined/invalid; add a safe fallback to avoid toISOString throwing.</violation>

<violation number="3" location="extensions/cli/src/session.ts:389">
Guard against invalid/missing dateCreated in comparator to ensure deterministic ordering.</violation>
</file>

<file name="extensions/cli/src/commands/ls.ts">

<violation number="1" location="extensions/cli/src/commands/ls.ts:110">
Rule violated: **Don&#39;t use console.log**

Use logger.error instead of console.error for consistent, centralized logging.</violation>
</file>

<file name="extensions/cli/src/index.ts">

<violation number="1" location="extensions/cli/src/index.ts:329">
Rule violated: **Don&#39;t use console.log**

Replace console.error with logger.error for consistent structured logging per project guidelines.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

1 participant