Skip to content

Support session#11

Merged
InftyAI-Agent merged 13 commits into
InftyAI:mainfrom
kerthcet:cleanup/support-session
Jun 3, 2026
Merged

Support session#11
InftyAI-Agent merged 13 commits into
InftyAI:mainfrom
kerthcet:cleanup/support-session

Conversation

@kerthcet
Copy link
Copy Markdown
Member

@kerthcet kerthcet commented Jun 2, 2026

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Special notes for your reviewer

Does this PR introduce a user-facing change?


kerthcet added 3 commits June 2, 2026 15:53
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Copilot AI review requested due to automatic review settings June 2, 2026 08:04
@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 2, 2026
Copy link
Copy Markdown
Contributor

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 first-class interactive PTY sessions end-to-end (daemon ⇄ server ⇄ Python), renaming the previous “shell” concept to “session” across the protocol, Rust code, Python bindings, tests, and docs.

Changes:

  • Replaced Shell* protocol/messages and APIs with Session* equivalents, including a new SessionClose message.
  • Implemented PTY-backed session management in the daemon and exposed a Python Session handle with write/read/resize/close (+ optional interactive terminal mode).
  • Updated examples, docs, and tests to use server.exec() and server.new_session().

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
server/src/server.rs Forwards SessionOutput/SessionExit from daemon to Python-side session streams.
server/src/registry.rs Renames shell session routing map to sessions and updates routing methods.
server/src/protocol.rs Renames shell message variants to session variants and adds SessionClose. Updates protocol tests accordingly.
server/src/lib.rs Renames Python binding execute_commandexec, start_shellnew_session; introduces Session pyclass and adds close().
sandd/src/session.rs Adds daemon-side PTY session implementation (start/read/write/resize/close).
sandd/src/protocol.rs Mirrors protocol rename from shell messages to session messages.
sandd/src/main.rs Wires SessionManager into daemon message handling for start/input/resize/close.
README.md Updates project messaging and example code to exec() and “Interactive Sessions”.
python/tests/test_unit.py Updates unit tests to use exec() and new_session().
python/tests/test_integration.py Updates integration tests to exec(); comments out file-transfer tests.
python/tests/test_e2e.py Updates to exec() and adds E2E coverage for interactive sessions.
python/sandd/init.py Updates Python wrapper API (exec, new_session) and adds interactive terminal loop.
pyproject.toml Updates package description string.
Makefile Renames docker helper target (docker-up) and removes test-all.
examples/interactive_session.py New interactive session example script.
examples/install_htop.py Updates to exec() and adjusts guidance for interactive usage.
examples/exec_commands.py Updates to exec() and refreshes script description.
examples/agent_example.py Removes legacy example script.
docs/QUICKSTART.md Updates docs snippets from execute_command() to exec() and shell→session.
docs/PROTOCOL.md Updates protocol documentation to session terminology and adds SessionClose.
docs/DEVELOP.md Updates developer docs to reference session implementation and new API names.
docs/ARCHITECTURE.md Updates terminology from shell output to session output.
.github/workflows/rust-ci.yaml Removes E2E tests from CI, leaving make test only.
Comments suppressed due to low confidence (4)

sandd/src/session.rs:7

  • reader.read(&mut buffer) relies on the std::io::Read trait, but only Write is imported. This will fail to compile unless the reader type provides an inherent read() (portable-pty readers typically implement Read).
    sandd/src/session.rs:199
  • close_session() removes the handle but never stops the spawned reader task. Dropping the JoinHandle does not cancel it, so the task can continue reading/sending after a session is “closed”. Abort the task when closing the session.
    sandd/src/session.rs:100
  • The PTY reader loop performs a blocking std::io::Read::read() inside tokio::spawn. This can block a Tokio worker thread per active session (and send_input()/resize() also perform blocking Write/resize calls under async locks), which can degrade throughput or stall other async tasks under load. Consider moving PTY I/O to spawn_blocking/dedicated threads and forwarding data over async channels.
    python/sandd/init.py:160
  • Renaming execute_command()exec() (and start_shell()new_session()) is a breaking change for existing Python consumers. If backward compatibility matters, consider keeping the old method names as thin wrappers (deprecated) for at least one release, or bumping the major version and calling this out in release notes/docs.
    def exec(
        self,
        daemon_id: str,
        command: str,
        timeout: int = 300,
        env: Optional[Dict[str, str]] = None,
        cwd: Optional[str] = None,
    ) -> CommandResult:

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

Comment thread server/src/lib.rs Outdated
Comment thread server/src/registry.rs Outdated
Comment thread python/sandd/__init__.py
Comment thread python/sandd/__init__.py
Comment thread python/sandd/__init__.py Outdated
Comment thread python/tests/test_integration.py
Comment thread README.md
Comment thread server/src/lib.rs Outdated
Comment on lines +133 to +141
let msg = Message::StartSession {
session_id: session_id.clone(),
rows,
cols,
term,
};

conn.send_message(msg)
.map_err(|e| PyRuntimeError::new_err(format!("Failed to start shell: {}", e)))?;
.map_err(|e| PyRuntimeError::new_err(format!("Failed to start session: {}", e)))?;
Comment thread docs/PROTOCOL.md
Comment thread docs/PROTOCOL.md
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
kerthcet added 5 commits June 2, 2026 22:47
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
@kerthcet
Copy link
Copy Markdown
Member Author

kerthcet commented Jun 3, 2026

/kind feature

@InftyAI-Agent InftyAI-Agent added feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Jun 3, 2026
kerthcet added 3 commits June 3, 2026 11:14
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
@kerthcet
Copy link
Copy Markdown
Member Author

kerthcet commented Jun 3, 2026

/lgtm

@InftyAI-Agent InftyAI-Agent added the lgtm Looks good to me, indicates that a PR is ready to be merged. label Jun 3, 2026
Signed-off-by: kerthcet <kerthcet@gmail.com>
@kerthcet
Copy link
Copy Markdown
Member Author

kerthcet commented Jun 3, 2026

/lgtm

@InftyAI-Agent InftyAI-Agent added lgtm Looks good to me, indicates that a PR is ready to be merged. and removed lgtm Looks good to me, indicates that a PR is ready to be merged. labels Jun 3, 2026
Copy link
Copy Markdown
Member

@InftyAI-Agent InftyAI-Agent left a comment

Choose a reason for hiding this comment

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

Approved: PR has both lgtm and approved labels

Copy link
Copy Markdown
Member

@InftyAI-Agent InftyAI-Agent left a comment

Choose a reason for hiding this comment

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

Approved: PR has both lgtm and approved labels

Copy link
Copy Markdown
Member

@InftyAI-Agent InftyAI-Agent left a comment

Choose a reason for hiding this comment

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

Approved: PR has both lgtm and approved labels

@InftyAI-Agent InftyAI-Agent merged commit 917e997 into InftyAI:main Jun 3, 2026
16 of 17 checks passed
@kerthcet kerthcet deleted the cleanup/support-session branch June 3, 2026 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. feature Categorizes issue or PR as related to a new feature. lgtm Looks good to me, indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants