Repository navigation
🌱 refactor(proxy): split github_proxy.go by concern (move-only) - #14733
Conversation
Move-only split of src/crux/proxy/github_proxy.go into per-concern files inside package proxy. No signature, behaviour or exported-name changes. Source-scanning tests now read all non-test github_proxy*.go files. Refs hivecommons/crux#23 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Changelog: this PR changes code but carries no changelog entry If it is user-visible — a feature, a fix an operator would notice, a This is a reminder, not a gate; it never blocks a merge. |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
Generated by changelog-fragment-guard from the PR title (#5675). Edit the wording freely, or replace this file with the no-changelog label if the change is not user-visible. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: hivecommons-hive[bot] <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: CI pending — sweep will merge when green (govulncheck is shared incident #14726, fix in #14731). Move-only refactor: needs the no-changelog label if changelog-fragment-guard asks for one. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Move-only claim verified; one PR-caused CI failure to fix in docs.
Verified the "move-only" claim mechanically: the sorted multiset of non-blank, non-package/import lines of v5:src/crux/proxy/github_proxy.go (3369 lines) is byte-identical to the union of the eleven github_proxy*.go non-test files on head dde0cf2. No code line was added, dropped or edited — so the proxy policy/CA/MITM surfaces this touches carry no behavioural change. The three source-invariant tests (TestProxyHTTPHasNoInlineCopyPairs, TestAllTLSUpstreamDialsAreBound, TestAllClientTLSHandshakesAreBound, TestHTTPRelayDeadlinesAreBoundAndObservable) now read via readGitHubProxySource (upstream_timeout_test.go:28), which globs github_proxy*.go minus _test.go, so their string counts are unchanged by the split. fleet-report.md file references were updated to the right new files (proxy: CONNECT dial failed → github_proxy_connect.go, inference upstream … → github_proxy_inference.go, confirmed by grep).
Finding (medium, blocks CI): the red relative links and anchors resolve check is this PR's. python3 src/scripts/check-docs-citations.py src/docs on the head reports exactly three DRIFTs, all src/crux/proxy/github_proxy.go:<line> citations that are now out of range in the 442-line stub:
src/docs/design/README.md:171→github_proxy.go:2662(the per-request usage record; now ingithub_proxy_copilot.go,forwardCopilotResponseWithUsageat:255)src/docs/design/copilot-cost-capture.md:45→github_proxy.go:877(explicit-CONNECT sniff seam; now ingithub_proxy_connect.go)src/docs/design/copilot-cost-capture.md:47→github_proxy.go:2559(proxyCopilotHTTP; nowgithub_proxy_copilot.go:129)
--fix only handles api-reference.md, so these need hand edits. While there, copilot-cost-capture.md:54,58,63,81,114,129,143 cite bare github_proxy.go:NNNN without the src/crux/proxy/ prefix — the checker doesn't see them, but they are equally stale after the split.
The red govulncheck is the base-branch GO-2026-6860 finding that #14731 bumps, not this diff.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: re-homed the three src/docs citations into the split files; govulncheck red is the base-branch GO-2026-6860 incident (#14726, fixed by #14731). CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Move-only claim verified; looks correct to me. I did not take the body's word for it: I diffed the sorted non-import, non-blank lines of src/crux/proxy/github_proxy.go at the merge-base (7a506f7) against the concatenation of the eleven github_proxy*.go files at 7b0d6a1. 3379 lines each, zero differences — every declaration and doc comment moved intact, nothing added or dropped.
- Test plumbing is right:
readGitHubProxySource(src/crux/proxy/upstream_timeout_test.go) globsgithub_proxy*.goand skips_test.go, so the two existing test files that match the glob (github_proxy_coverage_test.go,github_proxy_util_test.go) do not inflate therelayTunnel(/deadline counts the source-invariant tests assert on. Thewant 3/want 1constants are unchanged, consistent with no code having moved out of the package. - The two doc citations rewritten in
src/docs/design/copilot-cost-capture.mdresolve:github_proxy_copilot.go:236is therecordCopilotUsageWithCachecall,github_proxy_connect.go:469and:160are the twoIsCopilotAPIHost(host) && p.tokenSink != nil && agentName != ""guards. src/Dockerfile:673andsrc/deploy/entrypoint.sh:2590still citegithub_proxy.go warnSockMarkOnce; that function stays ingithub_proxy.go:123, so those are still accurate.
One low-severity docs gap, since the PR already started on this file: src/docs/design/copilot-cost-capture.md keeps 16 further github_proxy.go:NNNN citations (:2589, :2642, :2662 ×2, :2331, :2245, :152-207, :461, :1053, :1421, …) that now point past the end of a 442-line file. They were approximate before; after this split they are dangling. Worth a follow-up sweep of that one doc (no CI gate covers design docs, so nothing is red because of it).
The only red check is govulncheck — baseline GO-2026-6860 in golang.org/x/net@v0.61.0, which #14731 bumps; not this diff.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Verified the move-only claim independently against v5; it holds.
How I checked (head 7b0d6a1 vs v5 src/crux/proxy/github_proxy.go, 3,726 lines)
- Every one of the 143 top-level chunks in the eleven new
github_proxy*.gofiles (declaration plus its preceding doc comment, split on blank lines at column 0) appears as a contiguous verbatim substring of the old file. - The multiset of non-blank code lines (comments and
package/importlines stripped, leading whitespace trimmed) is identical in both directions: 2,237 lines, nothing only-in-old, nothing only-in-new. Same for comment lines: 1,132 each, no difference. So nothing was dropped and nothing was added beyond per-file imports. - The test edits are the necessary follow-through:
upstream_timeout_test.gogainsreadGitHubProxySource, which globsgithub_proxy*.goand skips_test.go, so the three source-scanning invariants (TestAllTLSUpstreamDialsAreBoundand friends) andclose_wait_leak_test.gonow cover the split files instead of silently passing on a 442-line stub. That is the one place a split like this usually breaks a guard, and it is handled. docs/design/copilot-cost-capture.md,fleet-report.md,design/README.mdre-anchorpath:linecitations to the new files; I spot-checkedgithub_proxy_copilot.go:236andgithub_proxy_connect.go:469and they point at the right lines.
Not from this diff: the red govulncheck is the pre-existing golang.org/x/net GO-2026-6860 finding that #14731 bumps.
Looks correct to me; the only thing left for a maintainer is the rebase, since the branch is now a little behind v5.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Looks correct to me, and the "move-only" claim checks out mechanically.
Verified at 0159fb9: I concatenated every non-test src/crux/proxy/github_proxy*.go at the base (v5) and at the PR head, dropped package/import lines and blank lines, and sorted the remaining lines. Both sides are 3383 lines and the sorted multisets are byte-identical — no statement, comment or declaration was added, dropped or edited; only files and import blocks changed.
The two source-scanning tests were adjusted the right way: readGitHubProxySource (upstream_timeout_test.go:28-50) globs github_proxy*.go, skips _test.go, and fails if nothing matched, so close_wait_leak_test.go and the TLS-dial-bound test keep covering the whole proxy rather than silently shrinking to the one remaining github_proxy.go. Docs that cite the file (src/docs/design/copilot-cost-capture.md, src/docs/fleet-report.md, src/docs/design/README.md) were repointed.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Move-only claim verified independently; looks correct to me.
- Took
src/crux/proxy/github_proxy.goat currentorigin/v5(3,726 lines), stripped blank/package/importlines, sorted, and diffed against the same filtering ofgithub_proxy.go+ the ten newgithub_proxy_*.gofiles at 0159fb9: 3,379 lines on each side, zero differences. No declaration was changed, added or dropped. - Before this PR the only non-test
github_proxy*.goin the package wasgithub_proxy.go, so the newreadGitHubProxySourceglob (upstream_timeout_test.go:28-55) concatenates exactly the moved content and the hard-coded invariant counts (wantRelayTunnelCalls = 3inclose_wait_leak_test.go, the single "copilot sniff" string) still refer to the same text. gofmt -l src/crux/proxy/is clean andgo build ./crux/proxy/passes at the PR head; the CI test shards are green (the only non-success rollups are supersededSign-off survives the squashruns marked CANCELLED).
Nothing else to flag.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Summary
Fixes hivecommons/crux#23.
src/crux/proxy/github_proxy.gowas a 3,726-line god file. Review diffs and merge conflicts piled up in it because unrelated changes (CONNECT handling, write-surface policy, Copilot, inference routing) all landed in the same file. The sync guard on hivecommons/crux refuses Go edits there, so this change is opened here againstsrc/crux.This is a move-only split within
package proxy. Every moved declaration and its doc comment is byte-identical to the original. I checked this by diffing the sorted non-import lines of the old file against all the new files: they match exactly. There are no signature, behaviour or exported-name changes. Each file imports only the packages it uses.github_proxy.goGitHubProxytype, constructors,ListenAddr, small helpersgithub_proxy_setters.goSet*wiring (SetAgentTokenSource,SetRepoPausedFunc,SetWriteSurfaceEnforceFunc, …)github_proxy_connect.goStart/handleConn, transparent TLS, ClientHello/SNI,prefixConn,handleConnectDirect,tunnelDirect, plain forwardgithub_proxy_mitm.goproxyHTTP/proxyHTTPHostgithub_proxy_identity.gogithub_proxy_policy.gogithub_proxy_violations.goViolations/AgentViolations, violation counting, issue-shape (githubIssueShapeViolation), origin labelgithub_proxy_copilot.gogithub_proxy_inference.goSetInferenceRoute, entitlements,GatewayHealth, translator, reroutegithub_proxy_tunnel.gostallBoundedBody/stallBoundedWriter,relayTunnelgithub_proxy_ca.gocachedCert,forgeCert, CA generation and loadingTwo other changes come with the split:
TestProxyHTTPHasNoInlineCopyPairsand three tests inupstream_timeout_test.goused to check invariants by readinggithub_proxy.gosource. They now read all non-testgithub_proxy*.gofiles through a newreadGitHubProxySourcehelper. The expected counts don't change because the code is the same, just spread across files.src/docs/fleet-report.mdnow point to the new files. Older design docs that cite historical line numbers were left as they are.This is a refactor that users won't notice, so it needs the
no-changeloglabel.Related issues
Closes hivecommons/crux#23
Testing
gofmt -lis clean. I checked by script that every import is used in each file, and that the sorted non-import lines of the old file match the new files exactly.— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88