Skip to content

fix(execution): unblock git-over-SSH transport #1740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NathanFlurry wants to merge 1 commit into
base: stack/feat-ssh-real-openssh-client-10-4p1-without-openssl-for-wasm32-wasip1-git-over-ssh-mvlvqmkp
Choose a base branch
from
Open

fix(execution): unblock git-over-SSH transport #1740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 49 additions & 22 deletions crates/execution/assets/runners/wasm-runner.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3299,14 +3299,16 @@ const hostNetImport = {
net_poll(fdsPtr, nfds, timeoutMs, retReadyPtr) {
const n = Number(nfds) >>> 0;
const base0 = Number(fdsPtr) >>> 0;
// The patched wasi sysroot's effective poll bits (bits/poll.h): POLLIN=POLLRDNORM=0x1,
// POLLOUT=POLLWRNORM=0x2 (NOT the 0x004 in legacy poll.h). Guests (X server + libxcb) use
// these, so net_poll must match or POLLOUT readiness is never reported and writers block.
// Match the owned sysroot ABI in __header_poll.h exactly. Its WASI event
// representation uses POLLIN=0x1, POLLOUT=0x2 and the widened exceptional
// bits below, rather than Linux's numeric values. poll(2) defines the
// behavior; the sysroot header defines the guest-visible wire values.
// https://man7.org/linux/man-pages/man2/poll.2.html
const POLLIN = 0x001;
const POLLOUT = 0x002;
const POLLERR = 0x008;
const POLLHUP = 0x010;
const POLLNVAL = 0x020;
const POLLERR = 0x1000;
const POLLHUP = 0x2000;
const POLLNVAL = 0x4000;
const t = Number(timeoutMs) | 0;
const deadline = t < 0 ? null : Date.now() + Math.max(0, t);
const kernelManagedStdio =
Expand All @@ -3322,6 +3324,7 @@ const hostNetImport = {
// comes from a batched __kernel_poll below, which doubles as the wait slice.
const kernelTargets = [];
const kernelEntries = [];
let hasHostNetWaitTarget = false;
for (let i = 0; i < n; i++) {
const base = base0 + i * 8;
const fd = view.getInt32(base, true);
Expand All @@ -3330,6 +3333,7 @@ const hostNetImport = {
const socket = getHostNetSocket(fd);
const handle = fd >= 0 ? lookupFdHandle(fd >>> 0) : undefined;
if (socket && !socket.closed) {
hasHostNetWaitTarget = true;
if (socket.serverId) {
if (events & POLLIN) {
// Report the listener readable only when a connection is actually pending.
Expand All @@ -3344,6 +3348,16 @@ const hostNetImport = {
if (events & POLLIN && socket.readChunks && socket.readChunks.length > 0) {
revents |= POLLIN;
}
// poll(2) reports peer shutdown as POLLHUP even when it was not
// requested, and a read after the queued data drains must return
// EOF without blocking. OpenSSH waits on this transition before
// exiting after the remote command closes its connection.
// https://man7.org/linux/man-pages/man2/poll.2.html
if (socket.readableEnded) {
revents |= POLLHUP;
if (events & POLLIN) revents |= POLLIN;
}
if (socket.lastError) revents |= POLLERR;
if (events & POLLOUT) revents |= POLLOUT;
}
} else if (handle?.kind === 'pipe-read') {
Expand All @@ -3360,21 +3374,28 @@ const hostNetImport = {
}
} else if (handle?.kind === 'pipe-write') {
if (events & POLLOUT) revents |= POLLOUT;
} else if (
fd >= 0 &&
fd <= 2 &&
kernelManagedStdio &&
(!handle || (handle.kind === 'passthrough' && handle.targetFd === fd))
) {
// Kernel-managed stdio (PTY slave / stdio pipes): ask the kernel, like a
// native poll(2) on the terminal fd.
} else if (fd >= 0 && kernelManagedStdio && (
(!handle && fd <= 2) ||
(handle?.kind === 'passthrough' && Number(handle.targetFd) >= 0 &&
Number(handle.targetFd) <= 2)
)) {
// poll(2): readiness means the requested operation will not block.
// https://man7.org/linux/man-pages/man2/poll.2.html
// Kernel-managed stdio (PTY slave / stdio pipes), including dup'd
// aliases: ask the kernel instead of treating a high alias like a
// regular file that is always ready. A false POLLIN here makes a
// guest block on an empty stdin pipe before it services another
// ready fd (for example OpenSSH flushing an exec request).
const kernelFd = handle?.kind === 'passthrough'
? Number(handle.targetFd) >>> 0
: fd;
kernelTargets.push({
fd,
fd: kernelFd,
events:
((events & POLLIN) !== 0 ? KERNEL_POLLIN : 0) |
((events & POLLOUT) !== 0 ? KERNEL_POLLOUT : 0),
});
kernelEntries.push({ base, fd, events });
kernelEntries.push({ base, fd, kernelFd, events });
} else if (handle) {
// Regular files / other VFS-backed fds: always ready, as on Linux.
revents |= events & (POLLIN | POLLOUT);
Expand All @@ -3391,10 +3412,13 @@ const hostNetImport = {

if (kernelTargets.length > 0) {
// If something is already ready (or this is a non-blocking poll), probe the
// kernel without waiting; otherwise let the kernel wait one slice for us.
// kernel without waiting. Mixed host-net + kernel polls must also keep
// this probe nonblocking: __kernel_poll cannot wake for a host socket,
// so sleeping here starves each queued SSH packet for a full 10s slice.
// The socket pump below supplies the bounded wait in that case.
const remaining = deadline == null ? Infinity : deadline - Date.now();
const sliceMs =
ready > 0 || t === 0
ready > 0 || t === 0 || hasHostNetWaitTarget
? 0
: Math.max(0, Math.min(KERNEL_WAIT_SLICE_MS, remaining));
let response = null;
Expand All @@ -3406,7 +3430,7 @@ const hostNetImport = {
const responseEntries = Array.isArray(response?.fds) ? response.fds : [];
for (const entry of kernelEntries) {
const responseEntry = responseEntries.find(
(item) => (Number(item?.fd) >>> 0) === (entry.fd >>> 0),
(item) => (Number(item?.fd) >>> 0) === (entry.kernelFd >>> 0),
);
const kernelRevents = Number(responseEntry?.revents) >>> 0;
let revents = 0;
Expand Down Expand Up @@ -5257,11 +5281,14 @@ wasiImport.fd_read = (fd, iovs, iovsLen, nreadPtr) => {
}

if (
numericFd === 0 &&
handle?.kind === 'passthrough' &&
handle.targetFd === 0 &&
passthroughHandles.get(0) === handle
handle.targetFd === 0
) {
// dup(2) aliases share the same open file description as fd 0. In a
// sidecar-managed process they must therefore read the kernel stdin pipe,
// not the runner process's unrelated host stdin. OpenSSH duplicates stdin
// before its poll/read loop, so splitting these paths loses pipe EOF.
// https://man7.org/linux/man-pages/man2/dup.2.html
const sidecarManagedProcess =
typeof process?.env?.AGENTOS_SANDBOX_ROOT === 'string' &&
process.env.AGENTOS_SANDBOX_ROOT.length > 0;
Expand Down
2 changes: 1 addition & 1 deletion crates/execution/src/node_import_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const NODE_IMPORT_CACHE_PATH_ENV: &str = "AGENTOS_NODE_IMPORT_CACHE_PATH";
const NODE_IMPORT_CACHE_LOADER_PATH_ENV: &str = "AGENTOS_NODE_IMPORT_CACHE_LOADER_PATH";
const NODE_IMPORT_CACHE_SCHEMA_VERSION: &str = "1";
const NODE_IMPORT_CACHE_LOADER_VERSION: &str = "8";
const NODE_IMPORT_CACHE_ASSET_VERSION: &str = "98";
const NODE_IMPORT_CACHE_ASSET_VERSION: &str = "104";
const NODE_IMPORT_CACHE_DIR_PREFIX: &str = "agentos-node-import-cache";
const DEFAULT_NODE_IMPORT_CACHE_MATERIALIZE_TIMEOUT: Duration = Duration::from_secs(30);
const PYODIDE_DIST_DIR: &str = "pyodide-dist";
Expand Down
4 changes: 2 additions & 2 deletions docs-internal/registry-parity-worklist.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ breadth; (2) `git` is **rare inside agent turns** (harnesses extract the diff
out-of-band) but stays essential; (3) a **long tail of project-specific CLIs**
(`dvc`, `sqlglot`, `sanic`, …) comes from pip/npm install, not the registry.

**Requested (add):** ssh 🟡, rsync 🟡, tmux/screen 🟡 (PTY — session persistence),
**Requested (add):** ssh ✅, rsync 🟡, tmux/screen 🟡 (PTY — session persistence),
gpg 🟡, ffmpeg 🟡 (media transcode — heavy but headless), jj 🟢, dig 🟡,
nslookup 🟡, less ⭐🟡 (pager), openssl ⭐🟡 (TLS/certs/keys/hashing).
tail/head/cat are already in coreutils — confirm present.
Expand All @@ -825,7 +825,7 @@ tail/head/cat are already in coreutils — confirm present.
big C runtime but real; 563 uses in history), miller `mlr` 🟢 (CSV/JSON),
xmlstarlet 🟢, pcre2grep 🟢. (jq/yq/sed/awk/grep/head/tail already covered.)

**Networking (host TCP/DNS bridge only):** openssl ⭐🟡, ssh 🟡, nc/netcat 🟡
**Networking (host TCP/DNS bridge only):** openssl ⭐🟡, ssh ✅, nc/netcat 🟡
(TCP/UDP), socat 🟡, whois 🟢, dig/nslookup 🟡, redis-cli / psql client 🟡,
aria2 🟡 (C++ downloader), sshpass 🟢 (ssh password helper).

Expand Down
25 changes: 6 additions & 19 deletions software/ssh/test/ssh.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ function installGitExecHandler(client: Connection, repoRoot: string) {
const stream = acceptExec();
const child = spawn('git', [service.replace(/^git-/, ''), repoPath]);
stream.pipe(child.stdin);
child.stdout.pipe(stream);
child.stderr.pipe(stream.stderr);
child.stdout.pipe(stream, { end: false });
child.stderr.pipe(stream.stderr, { end: false });
child.on('close', (code) => {
stream.exit(code ?? 1);
stream.end();
Expand Down Expand Up @@ -356,22 +356,9 @@ describeIf(hasSsh, 'ssh command', () => {
// which tunnels git-upload-pack / git-receive-pack to the host-side bare
// repo behind the ssh2 server.
//
// Opt-in (AGENTOS_SSH_GIT_E2E=1). The ssh client itself is correct on this
// path — it connects, completes curve25519/ed25519 KEX, authenticates, and
// opens the session channel. The blocker is in the RUNTIME, not the port:
// when git spawns ssh as a child_process, ssh's stdio are the sidecar's
// child-process "synthetic" pipes and ssh dups its channel I/O onto high
// synthetic fds (>= 1<<20) which it then polls alongside the host-net
// socket. In that configuration the exec channel-request is queued by ssh
// but never flushed to the socket (the sidecar's net.write for the exec is
// never issued), so the server never runs upload-pack and both sides wait
// forever. The SAME ssh binary delivers the exec correctly for every other
// invocation shape, including a cross-process pipe to a sibling reader
// (`sleep | ssh … 'git-upload-pack …' | head` returns the ref
// advertisement). This is a child_process-synthetic-pipe + host-net + guest
// poll() interaction in crates/execution/assets/runners/wasm-runner.mjs /
// the sidecar, tracked separately; the cases below pass once it is fixed.
describeIf(hasGit && hasHostGit && process.env.AGENTOS_SSH_GIT_E2E === '1', 'git-over-ssh clone/push', () => {
// This also regresses mixed polling in the runtime: ssh polls a dup'd stdin
// pipe alongside its host-net socket while Git waits for the remote helper.
describeIf(hasGit && hasHostGit, 'git-over-ssh clone/push', () => {
let keys: TestKeys;
let server: SshServer;
let port: number;
Expand Down Expand Up @@ -436,7 +423,7 @@ describeIf(hasSsh, 'ssh command', () => {
const url = `ssh://${SSH_USER}@127.0.0.1:${port}/origin.git`;

const cloned = await kernel.exec(git(`clone ${url} /tmp/clone`));
expect(cloned.exitCode).toBe(0);
expect(cloned.exitCode, cloned.stderr).toBe(0);
const readme = new TextDecoder().decode(
await kernel.readFile('/tmp/clone/README.md'),
);
Expand Down