Skip to content

Conversation

sigmachirality
Copy link
Member

Mostly follows the existing implementation of sf vms logs and sf vms ssh with some differences:

More spinners/communication to the user, which solves the issue where the command appeared to "hang" for 10 seconds sometimes.

Adds --json option to sf vms ssh

Gracefully fails from id with vm_ -> node name -> passthrough whatever was passed in.

Copy link

semanticdiff-com bot commented Oct 9, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  src/lib/nodes/list.tsx  30% smaller
  src/lib/nodes/utils.ts  17% smaller
  src/lib/nodes/index.ts  16% smaller
  deno.lock Unsupported file format
  src/lib/nodes/logs.ts  0% smaller
  src/lib/nodes/ssh.ts  0% smaller

@sigmachirality sigmachirality requested a review from joshi4 October 9, 2025 16:32
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

This PR implements two new commands for the SF Compute CLI: `sf nodes logs` and `sf nodes ssh`. These commands extend the existing VM-focused functionality (`sf vms logs` and `sf vms ssh`) to work with node-level abstractions, providing users with a more intuitive way to interact with their compute resources.

The implementation introduces several key improvements:

Node-to-VM Resolution: Both commands can accept either node names/IDs or direct VM IDs, with intelligent fallback logic. When a node name is provided, the system first attempts to resolve it to a node, then identifies the current VM for that node. If no node is found, it gracefully falls back to treating the input as a VM ID.

Enhanced User Experience: The commands address a significant UX issue where operations would appear to "hang" for 10+ seconds without feedback. New spinner indicators provide clear communication during network operations, showing users when the system is fetching node information, resolving VMs, or establishing connections.

Code Organization: The implementation extracts shared VM selection logic into a reusable getLastVM utility function in src/lib/nodes/utils.ts. This function prioritizes the current_vm field when available, then falls back to selecting the most recently started or updated VM based on timestamps.

Consistency with Existing Patterns: The new commands follow established CLI patterns in the codebase, including command registration, help text formatting, and error handling. The sf nodes ssh command also adds a --json output option for machine-readable results, maintaining consistency with other CLI commands.

The changes integrate seamlessly with the existing codebase architecture, reusing the established nodesClient() and VM SDK patterns while extending functionality to bridge the gap between node-level management and VM-level operations.

Changed Files
Filename Score Overview
src/lib/nodes/list.tsx 5/5 Refactors VM selection logic to use new getLastVM utility function
src/lib/nodes/utils.ts 5/5 Adds reusable getLastVM utility function for consistent VM selection
src/lib/nodes/index.ts 4/5 Registers new SSH and logs commands with comprehensive help documentation
src/lib/nodes/logs.ts 4/5 Implements new sf nodes logs command with node-to-VM resolution and enhanced UX
src/lib/nodes/ssh.ts 4/5 Implements new sf nodes ssh command with fallback logic and JSON output option

Confidence score: 4/5

  • This PR is safe to merge with minimal risk as it adds new functionality without modifying existing command behavior
  • Score reflects solid implementation following established patterns, though the complex node-to-VM resolution logic requires attention
  • Pay close attention to the new SSH and logs command files for potential edge cases in the fallback logic

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as "SF CLI"
    participant NodesAPI as "Nodes API"
    participant VMSAPI as "VMs API"
    participant SSH as "SSH Process"

    User->>CLI: "sf nodes logs my-node"
    CLI->>NodesAPI: "GET /nodes/{node}"
    NodesAPI-->>CLI: "Node details with current_vm"
    CLI->>VMSAPI: "GET /v0/vms/logs2?instance_id={vm_id}"
    VMSAPI-->>CLI: "Log entries array"
    CLI-->>User: "Display formatted logs"

    alt Follow mode
        User->>CLI: "sf nodes logs my-node --follow"
        CLI->>NodesAPI: "GET /nodes/{node}"
        NodesAPI-->>CLI: "Node details"
        CLI->>VMSAPI: "GET /v0/vms/logs2 (initial)"
        VMSAPI-->>CLI: "Initial logs"
        CLI-->>User: "Display initial logs"
        loop Every 2 seconds
            CLI->>VMSAPI: "GET /v0/vms/logs2?since_seqnum={last_seqnum}"
            VMSAPI-->>CLI: "New log entries"
            CLI-->>User: "Display new logs"
        end
    end

    User->>CLI: "sf nodes ssh my-node"
    CLI->>NodesAPI: "GET /nodes/{node}"
    NodesAPI-->>CLI: "Node with current_vm"
    CLI->>VMSAPI: "GET /vms/ssh?vm_id={vm_id}"
    VMSAPI-->>CLI: "SSH connection details"
    CLI->>SSH: "spawn ssh command with host keys"
    SSH-->>User: "Interactive SSH session"
Loading

5 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

while (true) {
const newParams: VMLogsParams = {
instance_id: vmId,
limit: 2500,
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Extract hardcoded 2500 limit into a named constant

Context Used: Rule from dashboard - Extract magic numbers into named constants at the top of files with documentation comments explainin... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/nodes/logs.ts
Line: 239:239

Comment:
**style:** Extract hardcoded 2500 limit into a named constant

**Context Used:** Rule from `dashboard` - Extract magic numbers into named constants at the top of files with documentation comments explainin... ([source](https://app.greptile.com/review/custom-context?memory=c50759b8-8cbe-4751-8359-e0fc9aa1254f))

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

1;
}

await new Promise((resolve) => setTimeout(resolve, 2000));
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Extract 2000ms polling interval into a named constant

Context Used: Rule from dashboard - Extract magic numbers into named constants at the top of files with documentation comments explainin... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/nodes/logs.ts
Line: 255:255

Comment:
**style:** Extract 2000ms polling interval into a named constant

**Context Used:** Rule from `dashboard` - Extract magic numbers into named constants at the top of files with documentation comments explainin... ([source](https://app.greptile.com/review/custom-context?memory=c50759b8-8cbe-4751-8359-e0fc9aa1254f))

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

if (sshUsername !== undefined) {
sshDestination = `${sshUsername}@${sshDestination}`;
}
if (sshPort !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: SSH port check is redundant since sshPort is typed as number and will never be undefined

Suggested change
if (sshPort !== undefined) {
sshDestination = `${sshDestination}:${sshPort}`;
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/nodes/ssh.ts
Line: 149:149

Comment:
**logic:** SSH port check is redundant since `sshPort` is typed as `number` and will never be `undefined`

```suggestion
      sshDestination = `${sshDestination}:${sshPort}`;
```

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

sigmachirality and others added 3 commits October 9, 2025 12:36
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
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