# Simplify remote-helper transport after review

`c4495b4`→[main](/content/gh/entireio/git-sync/commits/main/index.html)·

Soph·4w ago·3 files·+87 added/-67 removed

Post-review cleanup of the remote-helper transport, no behavior change:

- Extract a shared readRawPktLine primitive in pktline.go and route both readAdvertisement and responseEndReader through it, replacing two hand-rolled copies of pkt-line header parsing. This also adds the pktline.MaxSize bound both copies were missing, and lets responseEndReader reuse one grow-only buffer instead of allocating per packet — relaying a multi-GB pack now costs no per-packet allocation.
- Drop the unreachable os.ErrClosed guard in helperProcess.finish (the sync.Once already makes a redundant close impossible) and collapse it to a single errors.Join.
- Merge RequestInfoRefs's twin error branches into one errors.Join.
- Fold newConn's scheme dispatch into a single switch so the native scheme set is named once instead of split between a switch and a trailing if.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

## Sessions

de61fb4754bbView transcript

[?\
❯ GITSYNC_MAX_REF_UPDATES_PER_PUSH=5000 go run ./cmd/git-sync replicate --all-refs --stats --verbose \Claude Code·Opus 4.8[1m]·5 steps](/content/gh/entireio/git-sync/session/4f8f5e9b-9f90-4c22-88b9-988c55f45833#timeline-de61fb4754bb/index.html)

## Changes

3

- internal

- gitproto

- Mhelper.go+28/-59

- Mpktline.go+48

- syncer

- Msyncer.go+11/-8

```
42 unmodified lines

43
44
45
46
47
48
49
50
51
52
53
54
44 unmodified lines

99
100
101
96
97
98
99
100
101
102
103
104
105
106
107
108
89 unmodified lines

198
199
200
197
198
201
202
203
204
205
31 unmodified lines

237
238
239
236
237
238
239
240
240
241
242
243
9 unmodified lines

253
254
255
256
256
257
258
258
259
260
261
261
262
263
262
263
264
265
265
266
266
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
267
268
269
1 unmodified line

271
272
273
290
274
275
276
277
293
278
279
280
281
282
16 unmodified lines

299
300
301
316
317
302
303
304
305
320
321
306
307
308
324
325
326
309
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
310
311
312
313
314

42 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.
//
// Each Conn operation spawns its own helper process and tears it down when the
// operation completes. This is deliberate, not lazy: the helper services
// exactly one stateless-connect session per process (its protocol loop returns
44 unmodified lines

return nil, err
	}
	adv, readErr := readAdvertisement(proc.out)
	finishErr := proc.finish()
	if readErr != nil {
		return nil, fmt.Errorf("%s advertisement: %w", service, errors.Join(readErr, finishErr))
	}
	if finishErr != nil {
		return nil, fmt.Errorf("%s advertisement: %w", service, finishErr)
	// errors.Join drops nils, so this covers the readErr-only, finishErr-only,
	// and both-set cases in one branch.
	if err := errors.Join(readErr, proc.finish()); err != nil {
		return nil, fmt.Errorf("%s advertisement: %w", service, err)
	}
	return adv, nil
}
```

// readAck consumes the helper's stateless-connect response line: an empty line
// means the connection is established, "fallback" means it can't speak the
// smart protocol, and anything else is an error (with captured stderr).
// means the connection is established, "fallback" means it can't proxy this
// service, and anything else is an error (with captured stderr).
func (p *helperProcess) readAck() error {
	line, err := p.out.ReadString('\n')
	if err != nil {
31 unmodified lines

func (p *helperProcess) finish() error {
	closeErr := p.closeStdin()
	_, _ = io.Copy(io.Discard, p.out) //nolint:errcheck // best-effort drain before Wait; exit status is authoritative
	waitErr := p.wait()
	if closeErr != nil && !errors.Is(closeErr, os.ErrClosed) {
		return errors.Join(closeErr, waitErr)
	}
	return waitErr
	return errors.Join(closeErr, p.wait())
}
// cleanup force-tears-down the process on an error path.
9 unmodified lines

// it as the section terminator.
func readAdvertisement(br *bufio.Reader) ([]byte, error) {
	var buf bytes.Buffer
	var header [4]byte
	var scratch []byte
	for {
		if _, err := io.ReadFull(br, header[:]); err != nil {
			kind, frame, err := readRawPktLine(br, scratch)
			if err != nil {
				return nil, fmt.Errorf("read advertisement pkt-line: %w", err)
			}
			buf.Write(header[:])
			switch string(header[:]) {
			case "0000":
				buf.Write(frame)
				scratch = frame // reuse the (possibly grown) backing buffer next iteration
				if kind == PacketFlush {
					return buf.Bytes(), nil
				}
			case "0001", "0002":
				continue
			}
			n, err := parseHexLength(header)
			if err != nil {
				return nil, fmt.Errorf("advertisement pkt-line: %w", err)
			}
			if n < 4 {
				return nil, fmt.Errorf("advertisement pkt-line: invalid length %d", n)
			}
			if n == 4 {
				continue
			}
			payload := make([]byte, n-4)
			if _, err := io.ReadFull(br, payload); err != nil {
				return nil, fmt.Errorf("read advertisement payload: %w", err)
			}
			buf.Write(payload)
		}
}
}

1 unmodified line

// io.EOF when it reaches the stateless-connect response-end (0002) packet. The
// 0002 is consumed but never forwarded, so the downstream protocol parser sees
// exactly the same bytes it would from an HTTP response body (which ends at the
// connection's EOF instead).
// connection's EOF instead). The frame buffer is reused across packets, so
// relaying a multi-GB pack costs no per-packet allocation.
type responseEndReader struct {
	src     *bufio.Reader
	pending []byte
	buf     []byte // reused backing for the current frame
	pending []byte // unread bytes of the current frame (sub-slice of buf)
	done    bool
}

16 unmodified lines

}

func (r *responseEndReader) fill() error {
	var header [4]byte
	if _, err := io.ReadFull(r.src, header[:]); err != nil {
		kind, frame, err := readRawPktLine(r.src, r.buf)
		if err != nil {
			return fmt.Errorf("read response pkt-line: %w", err)
		}
		switch string(header[:]) {
		case "0002":
			if kind == PacketResponseEnd {
				r.done = true
				return io.EOF
			}
		case "0000", "0001":
			r.pending = append([]byte(nil), header[:]...)
			return nil
		}
		n, err := parseHexLength(header)
		if err != nil {
			return fmt.Errorf("response pkt-line: %w", err)
		}
		if n < 4 {
			return fmt.Errorf("response pkt-line: invalid length %d", n)
		}
		buf := make([]byte, n)
		copy(buf, header[:])
		if n > 4 {
			if _, err := io.ReadFull(r.src, buf[4:]); err != nil {
				return fmt.Errorf("read response payload: %w", err)
			}
		}
		r.pending = buf
		r.buf = frame // retain the grown backing for the next fill
		r.pending = frame
		return nil
	}
}
```

Minternal/gitproto/helper.go+28/-59

```
79 unmodified lines

80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133

79 unmodified lines

return PacketData, pr.buf, nil
}

// readRawPktLine reads one pkt-line and returns its type plus the verbatim
// on-wire bytes — the 4-byte length header followed by any payload — so callers
// that must relay or accumulate the exact wire framing (unlike ReadPacket,
// which yields only the payload) can do so. scratch is reused as backing
// storage and grown as needed; pass the slice returned by the previous call to
// avoid per-packet allocation. The returned slice aliases scratch and is valid
// only until the next call.
func readRawPktLine(br *bufio.Reader, scratch []byte) (PacketType, []byte, error) {
	if cap(scratch) < 4 {
		scratch = make([]byte, 4)
	}
	frame := scratch[:4]
	if _, err := io.ReadFull(br, frame); err != nil {
		return PacketData, nil, fmt.Errorf("read pktline header: %w", err)
	}
	switch string(frame) {
	case "0000":
		return PacketFlush, frame, nil
	case "0001":
		return PacketDelim, frame, nil
	case "0002":
		return PacketResponseEnd, frame, nil
	}

var header [4]byte
	copy(header[:], frame)
	n, err := parseHexLength(header)
	if err != nil {
		return PacketData, nil, err
	}
	if n < 4 || n > pktline.MaxSize {
		return PacketData, nil, pktline.ErrInvalidPktLen
	}
	if n > cap(scratch) {
		grown := make([]byte, n)
		copy(grown, frame)
		scratch = grown
	} else {
		scratch = scratch[:n]
	}
	if n > 4 {
		if _, err := io.ReadFull(br, scratch[4:n]); err != nil {
			return PacketData, nil, fmt.Errorf("read pktline payload: %w", err)
		}
	}
	return PacketData, scratch[:n], nil
}

func parseHexLength(header [4]byte) (int, error) {
	var n int
	for _, b := range header {