can you fix the linting error
can you fix the linting error
No response requested.
can you also let me know what you think about this: 🤔 not a blocker, but I wonder if we could do a pass and handle these better. e.g. is this specific to github?
Took a look at shouldFallbackToV1AfterV2ProbeError at internal/gitproto/refs.go:91. You're right — the three substrings ("Invalid command:", "GIT_PROTOCOL='version=2'", "git-upload-pack") match GitHub's SSH gateway output specifically. Gitolite, GitLab Shell, and OpenSSH + custom forced-command shells all word their rejections differently, so a self-hosted source that strips GIT_PROTOCOL won't trip this path and will just fail loudly.
My take: in auto mode over SSH, drop the string match entirely and just fall back to v1 on any v2 probe error, joining both errors if v1 also fails. Rationale: "auto" already means "try the fast thing, then the slow thing" — the user has explicitly opted out of strict v2. The substring match adds brittleness without preventing anything bad: the worst case today is one wasted v1 attempt against a server that's genuinely broken, which is cheap. Tradeoff is that a real outage now produces two errors instead of one (mitigated by errors.Join), and we lose the breadcrumb that says "the server explicitly rejected v2" — but that breadcrumb is already host-specific.
If you want to keep some guardrail, scope the broad fallback to SSH-scheme connections only (HTTPS v2 rejection is much rarer and usually means real misconfig worth surfacing).
yeah, let's do the auto fallback with error join
No response requested.
can you look at the comments on the PR
Two PR comments:
1. cursor[bot] on internal/gitproto/ssh.go:54-56 (commit ceeddf92, "Medium severity")
"SSH process leak when stdin close fails... if
cmd.Stdin.Close()fails, the function returns early without callingcmd.wait()or closingcmd.Stdout."
Already fixed in your follow-up commit 9c50a33f ssh: clean up failed info-refs startup. The current requestInfoRefsWithCommand (ssh.go:59) now calls cleanupSSHCommand(cmd) which closes stdout and waits — exactly the pattern the bot was asking for. No action needed.
2. @nodo on internal/gitproto/refs.go (the shouldFallbackToV1AfterV2ProbeError substring match)
"🤔 not a blocker, but I wonder if we could do a pass and handle these better. e.g. is this specific to github?"
Addressed in this session: removed the substring match, broadened to any v2 probe error over SSH, and joined the v2 + v1 errors when fallback also fails. Changes are uncommitted on soph/ssh-transport — ready to commit + push whenever you want me to.
can you rebase onto latest main and fix the conflict
No response requested.
can you now also do a rebase for soph/ssh-e2e-docker
Rebase complete with no conflicts. The 2 docker-test commits (dec0ec04, 833353a9) now sit on top of the rewritten soph/ssh-transport. Tests and lint pass.
Branch state:
soph/ssh-transport: 16 commits ahead oforigin/main(11 commits ahead oforigin/soph/ssh-transportsince SHAs were rewritten)soph/ssh-e2e-docker: 2 commits ahead ofsoph/ssh-transport
Both branches have diverged from their origin/* counterparts and will need force-pushes when you're ready.
how do I run the full docker test again?
No response requested.
[test:ssh-docker] $ GITSYNC_E2E_SSH_DOCKER=1 go test ./internal/syncer -run TestRun_SSHDockerSync -v === RUN TestRun_SSHDockerSync ssh_docker_test.go:82: cleanup docker container: docker rm -f ebe681303772419569e0586265ad22688542534156dcaec3782c66ec04ba68c4 failed: context canceled ssh_docker_test.go:70: cleanup docker image: docker image rm -f git-sync-ssh-e2e:5906157617516596622 failed: context canceled --- PASS: TestRun_SSHDockerSync (6.46s) PASS ok entire.io/entire/git-sync/internal/syncer (cached)
Two things worth flagging from that output:
1. The result was cached. ok ... (cached) means Go served the previous test result from its test cache — it didn't re-execute. The verbose output you saw is replayed from the prior run, including those cleanup errors. To force a fresh run:
or go clean -testcache first.
2. The test leaks a Docker image. I checked: container is gone, but image git-sync-ssh-e2e:5906157617516596622 (35MB) is still around. The bug is in the cleanup at ssh_docker_test.go:68-72 and :80-84 — both pass t.Context() to runCommandBestEffort, but t.Context() is cancelled the moment the test function returns, so the cleanup subprocesses can't run. The container probably cleaned up because Docker daemon also retried/aborted gracefully, but image rm failed.
Clean up the orphan and the cache:
Want me to fix the cleanup-context bug? It's a one-line change per cleanup: use context.Background() (or a fresh context with a short timeout) instead of t.Context().
[test:ssh-docker] $ GITSYNC_E2E_SSH_DOCKER=1 go test ./internal/syncer -run TestRun_SSHDockerSync -v === RUN TestRun_SSHDockerSync ssh_docker_test.go:82: cleanup docker container: docker rm -f 89bb5e5efd2c0eb3fc01283df21a072d0ebb8845a002439cbd7c6c142eec1f2b failed: context canceled ssh_docker_test.go:70: cleanup docker image: docker image rm -f git-sync-ssh-e2e:3956721581956427359 failed: context canceled --- PASS: TestRun_SSHDockerSync (1.22s) PASS ok entire.io/entire/git-sync/internal/syncer 1.487s
Test passes cleanly, no more cleanup errors, no leaked images or containers. The fix replaces t.Context() (cancelled when the test returns) with a fresh context.WithTimeout(context.Background(), 30*time.Second) in both t.Cleanup callbacks at ssh_docker_test.go:68 and :80.
can you commit
Committed as 8adeadaf on soph/ssh-e2e-docker. Left .entire/.gitignore and .clawpatch/ alone — those look like unrelated local-tooling state.