transport/remotehelper: Add transport for Git remote helpers by pjbgf · Pull Request #14 · go-git/x

pjbgf · 2026-10-31T08:26:21Z

Add a go-git transport that runs a Git remote helper program (git-remote-) and talks to it through the connect capability. The helper only carries the pack protocol, which go-git still speaks itself, so v0, v1 and v2 all work.

Helpers are opt-in per name and per operation, registered with client.WithTransport. The ext helper is refused. The program is resolved to an absolute path once, at construction. Requests are limited to the Git services and addresses starting with "-" are rejected. Git's repository environment variables are stripped from the helper's environment. Handshake lines and stderr are bounded. When the context is done the helper is killed and its pipes are closed, so a process it started cannot keep a read blocked. A helper that does not exit after Close is killed.

Add a go-git transport that runs a Git remote helper program (git-remote-<name>) and talks to it through the connect capability. The helper only carries the pack protocol, which go-git still speaks itself, so v0, v1 and v2 all work. Helpers are opt-in per name and per operation, registered with client.WithTransport. The ext helper is refused. The program is resolved to an absolute path once, at construction. Requests are limited to the Git services and addresses starting with "-" are rejected. Git's repository environment variables are stripped from the helper's environment. Handshake lines and stderr are bounded. When the context is done the helper is killed and its pipes are closed, so a process it started cannot keep a read blocked. A helper that does not exit after Close is killed. Assisted-by: Claude Opus 5.5 <[email protected]> Signed-off-by: Paulo Gomes <[email protected]>

Copilot

🟡 Changes recommended

Forced-helper URLs without :// bypass the registered transport, and failed connections return a non-nil interface that can panic during cleanup.

2 open issues

What changed in this PR

Adds an opt-in remote-helper transport that lets go-git exchange Git pack protocols through external git-remote-<name> programs.

Changes:

Implements capability negotiation and raw connections through the helper’s connect capability.
Adds request validation, environment filtering, bounded diagnostics, and process cleanup.
Adds integration tests for protocol versions, clone/push, error handling, and cancellation.

File	Description
`transport/remotehelper/remotehelper.go`	Implements the transport and helper lifecycle safeguards.
`transport/remotehelper/remotehelper_test.go`	Tests transport behavior and end-to-end operations.
`transport/remotehelper/helper_test.go`	Provides a subprocess helper with service and failure modes.
`transport/remotehelper/go.sum`	Records dependency checksums.
`transport/remotehelper/go.mod`	Defines the module and its dependencies.

_{Review effort: Balanced}

+go 1.26.0
+
+require (
+	github.com/go-git/go-git/v6 v6.0.0-beta.1.0.20261030085734-db474d339548


+//
+//nolint:ireturn // transport.Connector requires returning the Conn interface.
+func (t *Transport) Connect(ctx context.Context, req *transport.Request) (transport.Conn, error) {
+	return t.connect(ctx, req)


hiddeco

There are a few things I think need addressing (see the inline comments), most importantly Close failing for helpers that leave a process behind, and the version of go-git this module requires.

Note that I also left two design questions on go-git/go-git#2546 (submodules, and a separate registry for helpers), which may change how this transport is registered.

hiddeco · 2026-11-03T16:16:57Z

+go 1.26.0
+
+require (
+	github.com/go-git/go-git/v6 v6.0.0-beta.1.0.20261030085734-db474d339548


This version predates go-git/go-git#2546. With it, foo::<address> only reaches the helper when the address happens to contain ://, while foo::/path or foo::host/repo parses as SCP-like SSH and dials host foo. As Go selects the minimum required version, a user who only adds this module gets that behaviour.

The tests do not catch this, as they build the helper URL by hand (helperURL) and clone through test://. I would like to see #2546 merged first, this bumped to a version that includes it, and a clone (and push) through git.CloneContext with a test::<address> URL that asserts the arguments of the helper. This to have at least one test that exercises both repositories together.

hiddeco · 2026-11-03T16:16:57Z

+	}
+
+	stderr := &boundedBuffer{buf: nil, limit: maxStderrSize, mu: sync.Mutex{}}
+	cmd.Stderr = stderr


As stderr is not an *os.File, os/exec creates a pipe and Wait waits for the copy to finish. When the helper leaves a process behind that holds stderr (e.g. ssh with ControlPersist), Wait returns exec.ErrWaitDelay after 5s. Close then returns an error and the clone fails, while the refs and objects have already been written. A probe with a helper that starts a sleep child failed after ~6s with "exec: WaitDelay expired before I/O complete".

The smallest fix is to treat exec.ErrWaitDelay as success in wait() when c.cmd.ProcessState.Success(). Reading stderr through an os.Pipe with a goroutine Wait does not wait for would avoid the delay as well. Given this was not caught, a test with a child that holds stderr would be good to have.

hiddeco · 2026-11-03T16:16:57Z

+
+// Stderr returns what the helper has written to stderr so far.
+// StreamSession reports it in place of a failed operation's error.
+func (c *conn) Stderr() io.Reader {


Because conn implements Stderr(), StreamSession.wrapStderr replaces every Fetch, Push and Command error with what the helper wrote to stderr. Helpers write warnings and progress to stderr routinely, and a probe with a helper that writes a single warning got that warning back in place of context.DeadlineExceeded. As Close and the handshake already append stderr through withStderr, I think this can return nil (or only return stderr once the helper has exited unsuccessfully).

hiddeco · 2026-11-03T16:16:57Z

+		return nil, err
+	}
+
+	sess, err := transport.NewStreamSession(helper, req.Command)


When the context is done while NewStreamSession reads the advertisement, this returns a bare "read |0: file already closed". connect wraps ctx.Err() for this case, I think the same should happen here so callers can tell it was a cancellation.

hiddeco · 2026-11-03T16:16:57Z

+		return nil, fmt.Errorf("%w: %q", ErrInvalidName, name)
+	}
+
+	if name == "ext" {


The package documentation says NewTransport always refuses ext, but this only checks the name. NewTransport("foo", Options{Program: "<exec-path>/git-remote-ext"}) is accepted, and runs its address as a command. I would either refuse when the base name of the resolved program is git-remote-ext (case-insensitive, without .exe), or narrow the documentation to "refuses the name ext".

hiddeco · 2026-11-03T16:16:58Z

+	// GitDir is exported to the helper as GIT_DIR, as Git does for the
+	// repository it runs in. A relative path is made absolute. When
+	// empty, GIT_DIR is not set.
+	GitDir string


One Transport serves every repository the ClientOptions are used for, including submodules, so their helpers get the GIT_DIR of the superproject. Git sets it per repository (transport-helper.c:151-153). This can not be fixed from here without the repository being on transport.Request, so for now I would document that it is wrong for submodules.

hiddeco · 2026-11-03T16:16:58Z

+	assert.NotContains(t, dump.Env, "GIT_PROTOCOL")
+}
+
+func TestConnectContextCancelKillsHelper(t *testing.T) {


This covers cancellation during the capabilities handshake only. Replacing exec.CommandContext with exec.Command keeps the whole suite green, as does deregistering the AfterFunc once connect succeeds. I would like to see a test that cancels the context after a successful Connect (also with a child holding stdout), and asserts that reads end and the helper is gone.

hiddeco · 2026-11-03T16:16:58Z

+	return dump
+}
+
+func TestConnectEnvironment(t *testing.T) {


The appended GIT_DIR= and GIT_PROTOCOL= values win over the inherited ones here, so the stripping of those two is never observed. Removing them from isLocalRepoEnv keeps the test green. Setting both in the parent environment, and making a V0 request with an empty GitDir, would cover it.

hiddeco · 2026-11-03T16:16:58Z

+//		ClientOptions: []client.Option{client.WithTransport("foo", tr)},
+//	})
+//
+// The helper is invoked as Git invokes it for a URL given on the command


Nit: the helper is invoked mostly as Git invokes it, but there are deliberate differences. Git passes GIT_CONFIG_PARAMETERS and the other repository variables through and never sets GIT_PROTOCOL for a helper, and for foo:// URLs the helper gets the re-encoded url.URL form rather than the string the user wrote. I would mention these here, so the comment does not suggest parity.

hiddeco · 2026-11-03T16:16:58Z

+}
+
+// isURLSchemeChar mirrors Git's is_urlschemechar. See
+// https://github.com/git/git/blob/v2.56.0/url.c#L5-L19.


Nit: at v2.56.0, is_urlschemechar is at L19-L32.

Suggested change

// https://github.com/git/git/blob/v2.56.0/url.c#L5-L19.

// https://github.com/git/git/blob/v2.56.0/url.c#L19-L32.

Copilot AI balanced review requested due to automatic review settings October 31, 2026 08:26

Copilot started reviewing on behalf of pjbgf October 31, 2026 08:26

View session

Copilot AI reviewed Oct 31, 2026

View reviewed changes

hiddeco mentioned this pull request Nov 3, 2026

plumbing: transport, Parse remote helper URLs as Git does go-git/go-git#2546

Draft

hiddeco reviewed Nov 3, 2026

View reviewed changes

Navigation Menu

Repository navigation

transport/remotehelper: Add transport for Git remote helpers - #14

transport/remotehelper: Add transport for Git remote helpers#14
pjbgf wants to merge 1 commit into
mainfrom
remote-helper

pjbgf commented Oct 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

hiddeco left a comment

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

hiddeco Nov 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// https://github.com/git/git/blob/v2.56.0/url.c#L5-L19.
	// https://github.com/git/git/blob/v2.56.0/url.c#L19-L32.

Conversation

pjbgf commented Oct 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

hiddeco left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants