Skip to content

refactor: move collaborator_permission to proxy, add strutil.RandomBytes#6687

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-semantic-function-clustering
May 29, 2026
Merged

refactor: move collaborator_permission to proxy, add strutil.RandomBytes#6687
lpcox merged 3 commits into
mainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

Addresses two of three refactoring opportunities identified by semantic function clustering analysis: a domain-specific file misplaced in a generic utility package, and a duplicated crypto/rand usage pattern that can share a common primitive.

Move httputil/collaborator_permission.goproxy/

ParseCollaboratorPermissionArgs, LogAndWrapCollaboratorPermission, and FetchCollaboratorPermission are GitHub REST API helpers for a specific MCP tool — not generic HTTP utilities. Moving them to internal/proxy/ aligns package responsibility with actual content.

  • internal/proxy/collaborator_permission.go — moved file, updated package/logger namespace (proxy:collaborator_permission)
  • internal/proxy/collaborator_permission_helpers_test.go — unit tests relocated from httputil
  • internal/proxy/proxy.go — callers now use unqualified names (same package)
  • internal/server/unified.go — updated to import proxy and call proxy.ParseCollaboratorPermissionArgs / proxy.FetchCollaboratorPermission
  • internal/httputil/collaborator_permission{,_test}.go — deleted

Add strutil.RandomBytes as shared crypto/rand primitive

internal/tracing/provider.go called crypto/rand.Read directly while internal/strutil already owns random generation. Added RandomBytes(n int) ([]byte, error) and its testable core randomBytesFromReader, then refactored randomHexFromReader to delegate to it:

// strutil/random_hex.go
func randomBytesFromReader(n int, r io.Reader) ([]byte, error) { ... }
func RandomBytes(n int) ([]byte, error) { return randomBytesFromReader(n, rand.Reader) }
func randomHexFromReader(n int, r io.Reader) (string, error) {
    b, err := randomBytesFromReader(n, r)
    ...
    return hex.EncodeToString(b), nil
}

// tracing/provider.go — crypto/rand import removed
func generateRandomSpanID() (trace.SpanID, error) {
    var id trace.SpanID
    b, err := strutil.RandomBytes(len(id))
    if err != nil {
        return id, fmt.Errorf("failed to generate random span ID: %w", err)
    }
    copy(id[:], b)
    return id, nil
}

Deferred

server/difc_log.go (Priority 3) is left in place — moving to internal/difc/ is only warranted if other packages need those DIFC log helpers, which is not currently the case.

Copilot AI added 2 commits May 29, 2026 12:58
…RandomBytes

- Move internal/httputil/collaborator_permission.go to internal/proxy/collaborator_permission.go
  - Updated package name from 'httputil' to 'proxy'
  - Updated logger namespace from 'httputil:collaborator_permission' to 'proxy:collaborator_permission'
- Move unit tests to internal/proxy/collaborator_permission_helpers_test.go
- Remove old httputil/collaborator_permission.go and its test file
- Update proxy/proxy.go to call ParseCollaboratorPermissionArgs and FetchCollaboratorPermission
  without httputil prefix (now in the same package)
- Update server/unified.go to import proxy and use proxy.ParseCollaboratorPermissionArgs
  and proxy.FetchCollaboratorPermission
- Add strutil.RandomBytes(n int) ([]byte, error) as a shared crypto/rand primitive
- Refactor strutil.randomHexFromReader to delegate to randomBytesFromReader
- Add tests for randomBytesFromReader and RandomBytes in strutil/random_hex_test.go
- Update tracing/provider.go to use strutil.RandomBytes instead of crypto/rand.Read directly,
  removing the direct crypto/rand import from the tracing package
Copilot AI changed the title [WIP] Refactor semantic function clustering analysis based on opportunities refactor: move collaborator_permission to proxy, add strutil.RandomBytes May 29, 2026
Copilot finished work on behalf of lpcox May 29, 2026 13:01
Copilot AI requested a review from lpcox May 29, 2026 13:01
@lpcox lpcox marked this pull request as ready for review May 29, 2026 13:09
Copilot AI review requested due to automatic review settings May 29, 2026 13:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors ownership boundaries by moving collaborator-permission REST helpers from generic HTTP utilities into the proxy package, and extracts reusable random byte generation into strutil.

Changes:

  • Relocated collaborator-permission helper code/tests into internal/proxy and updated call sites in proxy/server code.
  • Added strutil.RandomBytes and refactored RandomHex internals to share the byte-generation primitive.
  • Updated tracing span ID generation to use the shared strutil.RandomBytes helper.
Show a summary per file
File Description
internal/proxy/collaborator_permission.go Moves collaborator-permission helper implementation into the proxy package and updates logger namespace.
internal/proxy/collaborator_permission_helpers_test.go Relocates helper tests into the proxy package and avoids test-name collision.
internal/proxy/proxy.go Updates proxy caller to use same-package collaborator-permission helpers.
internal/server/unified.go Updates server caller to use exported proxy collaborator-permission helpers.
internal/strutil/random_hex.go Adds reusable random byte generation and delegates random hex generation through it.
internal/strutil/random_hex_test.go Adds coverage for RandomBytes and its testable reader-backed helper.
internal/tracing/provider.go Replaces direct crypto/rand span ID generation with strutil.RandomBytes.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 7/7 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 3648af7 into main May 29, 2026
29 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch May 29, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic Function Clustering Analysis: 3 Refactoring Opportunities

3 participants