Probe helper capabilities for a clear unsupported error · Entire
Probe helper capabilities for a clear unsupported error
4ff9494→main·
Soph·4w ago·2 files·+70 added/-9 removed
Before opening a stateless-connect session, dial now issues the remote helper's
capabilities command and checks that stateless-connect is advertised. A helper
that lacks it (e.g. one offering only the legacy connect capability) fails
with an actionable error naming the helper and its advertised capabilities,
instead of a cryptic mid-protocol failure. The probe is a local exchange — no
network round trip happens until stateless-connect — so it adds no meaningful
cost.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Sessions
53d1e0ac66e6View transcript
Changes
2
internal/gitproto
Mhelper.go+40/-5
Mhelper_test.go+30/-4
9 unmodified lines
10
11
12
13
14
15
16
27 unmodified lines
44
45
46
46
47
48
49
50
47
48
49
50
51
52
53
54
55
123 unmodified lines
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
14 unmodified lines
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
9 unmodified lines
"net/url"
"os"
"os/exec"
"slices"
"strings"
"sync"
)
27 unmodified lines
// no native transport for (e.g. entire://) by delegating auth and the actual
// network I/O to the helper, while still running the wire protocol itself.
//
// It assumes the helper supports stateless-connect (the modern v2 bridge) for
// both upload-pack and receive-pack — it issues stateless-connect directly
// rather than negotiating via the capabilities handshake, and treats a
// "fallback" reply as an error. git-remote-entire satisfies this; a helper that
// only offers the legacy `connect` capability is not supported.
// It requires the helper to support stateless-connect (the modern v2 bridge)
// for both upload-pack and receive-pack. It probes the helper's advertised
// capabilities first and fails with a clear error if stateless-connect is
// absent; it does not fall back to the legacy `connect` capability. A
// per-service "fallback" reply is also treated as an error. git-remote-entire
// satisfies this.
//
// Each Conn operation spawns its own helper process and tears it down when the
// operation completes. This is deliberate, not lazy: the helper services
123 unmodified lines
out: bufio.NewReaderSize(stdout, 65536),
stderr: stderr,
}
// Probe capabilities first so an incompatible helper produces a clear error
// instead of a cryptic mid-protocol failure. This is a local exchange — no
// network round trip happens until stateless-connect below.
caps, err := proc.readCapabilities()
if err != nil {
return nil, fmt.Errorf("probe remote helper %s: %w", c.HelperPath, errors.Join(err, proc.cleanup()))
}
if !slices.Contains(caps, "stateless-connect") {
unsupported := fmt.Errorf("remote helper %s does not support stateless-connect (advertises: %s); git-sync needs a stateless-connect-capable helper such as git-remote-entire",
c.HelperPath, strings.Join(caps, " "))
return nil, errors.Join(unsupported, proc.cleanup())
}
if _, err := io.WriteString(stdin, "stateless-connect "+service+"\n"); err != nil {
return nil, fmt.Errorf("request stateless-connect %s: %w", service, errors.Join(err, proc.cleanup()))
}
14 unmodified lines
stdinOnce sync.Once
}
// readCapabilities issues the helper's `capabilities` command and returns the
// advertised capability names (the token before any argument on each line),
// reading until the blank terminator line.
func (p *helperProcess) readCapabilities() ([]string, error) {
if _, err := io.WriteString(p.stdin, "capabilities\n"); err != nil {
return nil, fmt.Errorf("request capabilities: %w", err)
}
var caps []string
for {
line, err := p.out.ReadString('\n')
if err != nil {
return nil, fmt.Errorf("read capabilities: %w", p.stderr.wrap(err))
}
if line = strings.TrimRight(line, "\r\n"); line == "" {
return caps, nil
}
name, _, _ := strings.Cut(line, " ")
caps = append(caps, name)
}
}
// readAck consumes the helper's stateless-connect response line: an empty line
// means the connection is established, "fallback" means it can't proxy this
// service, and anything else is an error (with captured stderr).
Minternal/gitproto/helper.go+40/-5
28 unmodified lines
29
30
31
32
33
34
32
33
34
35
36
36
37
38
39
40
41
1 unmodified line
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
151 unmodified lines
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
28 unmodified lines
os.Exit(0)
}
// runFakeHelper emulates git-remote-entire's stateless-connect handling closely
// enough to drive HelperConn: it reads the stateless-connect command line,
// writes the empty-line ack, emits a canned advertisement, then services one
// request. upload-pack frames the response with a trailing 0002 (v2 stateless);
// receive-pack streams the response and exits (v0 connect).
// receive-pack streams the response and exits (v0 connect). The "no-stateless"
// mode advertises only `connect` so HelperConn's capability probe rejects it.
func runFakeHelper(mode string, stdin io.Reader, stdout io.Writer) error {
write := func(s string) error {
_, err := io.WriteString(stdout, s)
1 unmodified line
}
br := bufio.NewReader(stdin)
if line, err := br.ReadString('\n'); err != nil {
return fmt.Errorf("read capabilities command: %w", err)
} else if got := strings.TrimRight(line, "\r\n"); got != "capabilities" {
return fmt.Errorf("expected capabilities, got %q", got)
}
if mode == "no-stateless" {
return write("connect\noption\n\n") // no stateless-connect: probe must reject
}
if err := write("stateless-connect\npush\noption\n\n"); err != nil {
return err
}
cmd, err := br.ReadString('\n')
if err != nil {
return fmt.Errorf("read command: %w", err)
151 unmodified lines
}
}
func TestHelperConn_NoStatelessConnect_MeaningfulError(t *testing.T) {
c := fakeHelperConn(t, "no-stateless")
_, err := c.RequestInfoRefs(context.Background(), "git-upload-pack", GitProtocolV2)
if err == nil {
t.Fatal("expected error when helper lacks stateless-connect")
}
if !strings.Contains(err.Error(), "stateless-connect") {
t.Fatalf("error = %v, want it to explain the missing stateless-connect capability", err)
}
}
func TestReadAdvertisement_StopsAtFlush(t *testing.T) {
in := FormatPktLine("version 2\n") + FormatPktLine("agent=x\n") + "0000" + "extra-bytes"
br := bufio.NewReader(strings.NewReader(in))