Skip to content

Add machine slugs and luv handover - #36

Open
nk-ag wants to merge 1 commit into
mainfrom
claude/ssh-tmux-handover-workflow-zcq3gr
Open

Add machine slugs and luv handover#36
nk-ag wants to merge 1 commit into
mainfrom
claude/ssh-tmux-handover-workflow-zcq3gr

Conversation

@nk-ag

@nk-ag nk-ag commented Jul 26, 2026

Copy link
Copy Markdown

Workspace numbers come from GitHub's issue counter, which each machine
computes on its own — so a laptop and a box routinely pick the same one
and push the same branch. Workspaces are now {repo}-{machine}-{number}
and branches luv-{machine}-{number}, where {machine} is a short name for
the machine that created them (config 'machine', default: hostname).
The slug is stamped once and never changes, so it records where a
workspace came from. Pre-slug folders and branches keep working: lookups
try our own slug, then any other machine's, then the plain name.

Because the slug belongs to whichever machine holds a workspace, the
dispatcher can no longer compute a remote folder name. It takes it from
the session registry, else asks the host over ssh (luv --where), else
falls back to the existing luv-pending rename-on-arrival path.

On top of that, luv handover moves a session between machines:

luv --local myrepo "start something"
luv handover myrepo 43 --to box

The workspace is copied byte for byte — .git, dirty tree, untracked and
gitignored files — and the agent's transcript goes with it, with the
absolute path recorded inside rewritten for the new machine, so
claude --resume and codex resume --last continue the thread instead of
starting over. Bytes are relayed through the machine running the command,
so the two ends need no credentials for each other.

Everything checkable is checked before the agent is stopped, and the
source folder is never deleted without --purge, so a failed transfer is
always recoverable where it was.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01UUYkpb2JohEeSpVK6TeeWf

Workspace numbers come from GitHub's issue counter, which each machine
computes on its own — so a laptop and a box routinely pick the same one
and push the same branch. Workspaces are now {repo}-{machine}-{number}
and branches luv-{machine}-{number}, where {machine} is a short name for
the machine that created them (config 'machine', default: hostname).
The slug is stamped once and never changes, so it records where a
workspace came from. Pre-slug folders and branches keep working: lookups
try our own slug, then any other machine's, then the plain name.

Because the slug belongs to whichever machine holds a workspace, the
dispatcher can no longer compute a remote folder name. It takes it from
the session registry, else asks the host over ssh (luv --where), else
falls back to the existing luv-pending rename-on-arrival path.

On top of that, luv handover moves a session between machines:

    luv --local myrepo "start something"
    luv handover myrepo 43 --to box

The workspace is copied byte for byte — .git, dirty tree, untracked and
gitignored files — and the agent's transcript goes with it, with the
absolute path recorded inside rewritten for the new machine, so
claude --resume and codex resume --last continue the thread instead of
starting over. Bytes are relayed through the machine running the command,
so the two ends need no credentials for each other.

Everything checkable is checked before the agent is stopped, and the
source folder is never deleted without --purge, so a failed transfer is
always recoverable where it was.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUYkpb2JohEeSpVK6TeeWf
@NiveditJain
NiveditJain requested review from hermes-exosphere and mayank6091 and removed request for mayank6091 July 26, 2026 04:46
@hermes-exosphere

Copy link
Copy Markdown
Contributor

Automated code review started - full review. Results will be posted here.

@hermes-exosphere

Copy link
Copy Markdown
Contributor

Automated code review in progress...

  • ✅ Phase 0: No existing review threads/comments to resolve
  • ✅ Phase 1: Full diff analyzed — 7 files, +1216/-105 lines
  • ✅ Phase 2: Deep analysis complete — all functions reviewed for security, logic, race conditions
  • ✅ Phase 3: All 73 tests pass (0 failures, 0.33s)
  • 🔍 Posting inline comments and summary now...

Comment thread luv/__init__.py
if old == new:
return "true"
return (f'for f in {files_expr}; do [ -f "$f" ] || continue; '
f'sed {shlex.quote(f"s|{old}|{new}|g")} "$f" > "$f.luvtmp" '

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.

🟡 Warning: The rewrite_script function uses | as the sed delimiter (s|OLD|NEW|g), which will silently produce incorrect results if either path contains a | character. While uncommon in file paths, it is not impossible (some CI systems use pipe-delimited identifiers).

Fix: Use # as the sed delimiter instead — # cannot appear in POSIX filesystem paths, making it inherently safe:

return (f'for f in {files_expr}; do [ -f "$f" ] || continue; '
        f'sed {shlex.quote(f"s#{old}#{new}#g")} "$f" > "$f.luvtmp" '
        f'&& mv "$f.luvtmp" "$f"; done')

@hermes-exosphere

Copy link
Copy Markdown
Contributor

🔍 Automated Code Review — PR #36: Add machine slugs and luv handover

📋 Executive Summary

This is a high-quality, well-designed PR that solves a real collision problem (two machines picking the same workspace number) and adds a powerful new luv handover feature. The naming change is backward-compatible, the handover implementation is careful about safety (checks everything first, leaves source intact), and the test coverage is excellent. 73/73 tests pass. One minor suggestion about sed delimiter safety — nothing blocking.


📊 Change Architecture

graph TD
    A["luv CLI (main)"]
    B[Machine Slugs]
    C[Workspace Naming]
    D["Handover (cmd_handover)"]
    E[Tar Relay]
    F[Agent State Transfer]
    G["Session Registry (sessions.json)"]
    H["Remote Dispatch (dispatch_remote)"]

    A -->|"new: --where, --paths"| B
    B -->|"slug in name"| C
    C -->|"backward compat"| G
    A -->|"new: handover subcommand"| D
    D -->|"stop agent + docker"| E
    E -->|"tar relay through laptop"| F
    F -->|"rewrite paths + resume"| H
    D -->|"remove old, record new"| G
    style A fill:#87CEEB
    style B fill:#90EE90
    style C fill:#FFD700
    style D fill:#90EE90
    style E fill:#90EE90
    style F fill:#90EE90
    style G fill:#87CEEB
    style H fill:#87CEEB
Loading

Legend: 🟢 New | 🔵 Modified | 🟡 Breaking Change Risk


🔴 Breaking Changes

No breaking changes detected. The naming change from {repo}-{number} to {repo}-{machine}-{number} is fully backward-compatible:

  • find_workspace() checks own slug first, then foreign slugs, then pre-slug folders
  • workspace_re() makes the slug group optional in the regex
  • branch_re() matches both luv-machine-N and luv-N forms
  • workspace_number() reads numbers from either format
  • resolve_remote_workspace() falls back to luv-pending when the host doesn't respond

⚠️ Issues Found

  1. 🟡 Warningluv/__init__.py:1522 — Sed delimiter | could conflict with paths containing pipe characters. Use # instead.

🔬 Logical / Bug Analysis

Naming System (lines 168-253):

  • sanitize_slug() correctly strips all non-alphanumeric chars and caps at 8 — prevents the separator ambiguity described in docs
  • machine_slug() falls back to hostname, then "local" — never returns empty
  • workspace_re() uses optional slug group — makes pre-slug folders match
  • find_workspace() has early-exit when PRS_DIR doesn't exist, and correctly handles ambiguity (dies with error when two foreign candidates exist)

Handover Flow (lines 1793-1900):

  • ✅ All checks done BEFORE stopping the agent — failures cost nothing
  • ✅ Source folder never deleted without --purge
  • ✅ Recovery command printed before copy starts
  • workspace_git_state() verification catches torn copies
  • ✅ Registry entry is removed after successful copy
  • ✅ Session is killed with tmux kill-session + explicit docker teardown

Tar Relay (lines 1449-1486):

  • stream_copy() properly closes send.stdout to propagate EPIPE
  • ✅ Both send.wait() and recv.wait() are checked
  • tar_send_argv() never uses -t (would corrupt binary stream)
  • stream_copy() is a no-op with empty members list

Agent State (lines 1489-1549):

  • claude_project_slug() matches Claude's own algorithm
  • rewrite_script() avoids sed -i (BSD/GNU incompatibility)
  • settle_agent_state() gracefully handles Codex vs Claude differences
  • ✅ Codex path discovery via grep -rlF is a reasonable best-effort approach
  • ✅ Warning printed (not error) when no transcript is found

Remote Resolution (lines 1322-1345):

  • resolve_remote_workspace() checks registry first, then SSH fallback, then returns None
  • ✅ Registry lookup is the fastest path and avoids an SSH round-trip

Backward Compatibility:

  • ✅ Pre-slug workspace folders (e.g., myrepo-42) continue to work
  • ✅ Pre-slug branches (luv-42) still match branch_re()
  • --clean reads branch from git (rev-parse --abbrev-ref) rather than reconstructing from folder name

🧪 Evidence — Build & Test Results

Test Results — 73 passed, 0 failed (0.33s)
tests/test_agents.py::AgentLaunchTests::test_claude_remains_default PASSED
tests/test_agents.py::AgentLaunchTests::test_codex_launch_uses_yolo_mode PASSED
tests/test_agents.py::AgentLaunchTests::test_codex_non_interactive_uses_exec PASSED
tests/test_agents.py::AgentLaunchTests::test_codex_resume_uses_last_session_in_yolo_mode PASSED
tests/test_agents.py::AgentLaunchTests::test_main_routes_codex_to_existing_workspace PASSED
tests/test_agents.py::RemoteDispatchTests::test_clean_dispatches_without_tmux PASSED
... (all 73 pass)
tests/test_agents.py::TransferTests::test_stream_copy_does_nothing_without_members PASSED

============================== 73 passed in 0.33s ==============================

New test classes added:

  • NamingTests (14 tests) — slugs, workspace names, branch names, fallbacks, ambiguity
  • HandoverTests (11 tests) — destination validation, source/destination rejection, replay of agent/model, purge, copy verification
  • TransferTests (7 tests) — tar argv, TTY avoidance, path rewriting, stream copy edge cases
  • Plus 3 new tests in RemoteDispatchTests and 1 in CleanGuardTests

Total: 36 new test cases (up from 37 to 73)


🔗 Issue Linkage

⚠️ No issue linked. Consider creating one for better traceability — especially given the scope of this change (1216 lines, 7 files).


👥 Human Review Feedback

No human review feedback on this PR (first review, no prior comments from team members).


💡 Suggestions

  1. claude_project_slug (line 1489): This duplicates Claude's internal cwd-to-slug algorithm. If Anthropic changes it, handovers silently break. Add a comment referencing the Claude version this was derived from (see inline comment).
  2. rewrite_script sed delimiter (line 1522): Switch from | to # for the sed delimiter — # can't appear in POSIX paths (see inline comment).
  3. Session registry for local sessions: start_local_session() at line 1703 records sessions with host=None (meaning local). The registry was designed for remote sessions only; consider whether local sessions surviving in the registry is intentional — they'll show up in luv ls.

🏆 Verdict

VERDICT: APPROVED


Automated code review · 2026-07-27 10:25:00 UTC

@hermes-exosphere hermes-exosphere 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.

Automated review: Approved. ✅

All 73 tests pass. The naming change is backward-compatible, the handover implementation is carefully defensive (checks before acting, leaves source intact on failure, verifies git state after copy). One minor suggestion about sed delimiter safety — non-blocking.

Excellent test coverage: 36 new tests covering naming, handover, transfer, and remote resolution edge cases.

@hermes-exosphere hermes-exosphere 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.

Automated review: Approved. ✅

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.

3 participants