feature: Server Manager by warkanum · Pull Request #5 · bitechdev/ResolveSpec

warkanum · 2025-12-30T10:13:03Z

feature: Server Manager

Copilot

Pull request overview

This PR introduces a comprehensive Server Manager feature that replaces the existing graceful shutdown implementation with a more robust, production-ready HTTP/HTTPS server management system. The new architecture supports managing multiple server instances concurrently with enhanced TLS/HTTPS capabilities.

Key Changes:

Replaced standalone graceful shutdown module with a centralized server manager that can handle multiple server instances
Added comprehensive TLS/HTTPS support including certificate files, self-signed certificates, and Let's Encrypt AutoTLS
Integrated panic recovery middleware with metrics tracking
Enhanced logger with context extraction for better error tracking

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
`pkg/server/manager.go`	Core server manager implementation with graceful shutdown, instance management, and lifecycle control
`pkg/server/interfaces.go`	Defines Manager and Instance interfaces plus Config struct for server configuration
`pkg/server/tls.go`	TLS configuration utilities supporting certificate files, self-signed certs, and Let's Encrypt AutoTLS
`pkg/server/manager_test.go`	Comprehensive tests for server lifecycle, graceful shutdown, health checks, and callbacks
`pkg/server/example_test.go`	Example code demonstrating various usage patterns for the server manager
`pkg/server/README.md`	Updated documentation with new features, configuration options, and deployment examples
`pkg/server/shutdown.go` (deleted)	Removed in favor of integrated manager approach
`pkg/server/shutdown_test.go` (deleted)	Removed in favor of manager tests
`pkg/middleware/panic.go`	New panic recovery middleware with metrics and error tracking integration
`pkg/middleware/panic_test.go`	Tests for panic recovery middleware
`pkg/logger/logger.go`	Enhanced with context extraction for better error tracking with panics
`pkg/metrics/interfaces.go`	Added RecordPanic method to Provider interface
`pkg/metrics/prometheus.go`	Implemented panic tracking metric

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

Copilot · 2025-12-30T10:17:24Z

pkg/server/manager.go

+type serverInstance struct {
+	cfg            Config
+	gracefulServer *gracefulServer
+	certFile       string // Path to certificate file (may be temporary for self-signed)
+	keyFile        string // Path to key file (may be temporary for self-signed)
+	mu             sync.RWMutex
+	running        bool
+	serverErr      chan error


The serverInstance struct stores paths to potentially temporary certificate files (for self-signed SSL), but these files are never cleaned up when the server stops. This creates a resource leak. Implement cleanup in the Stop method to remove temporary certificate directories.

@copilot open a new pull request to apply changes based on this feedback

pkg/server/manager.go

pkg/server/tls.go

Copilot · 2025-12-30T10:17:25Z

pkg/server/tls.go

+
+		// Return basic TLS config - cert/key will be loaded by ListenAndServeTLS
+		tlsConfig := &tls.Config{
+			MinVersion: tls.VersionTLS12,


The TLS minimum version should be set to TLS 1.3 instead of TLS 1.2. TLS 1.2 has known vulnerabilities and TLS 1.3 is widely supported. This is a security best practice for new code in 2025.

pkg/server/manager.go

Copilot · 2025-12-30T10:17:26Z

pkg/server/tls.go

+		certFile, keyFile, err := saveCertToTempFiles(certPEM, keyPEM)
+		if err != nil {
+			return nil, "", "", fmt.Errorf("failed to save self-signed certificate: %w", err)
+		}
+
+		tlsConfig := &tls.Config{
+			MinVersion: tls.VersionTLS12,
+		}
+		return tlsConfig, certFile, keyFile, nil


The temporary certificate files created for self-signed SSL are never cleaned up. This will accumulate temporary directories in the system temp folder over time, leading to a resource leak. Consider implementing cleanup when the server stops, or use an in-memory certificate instead of writing to disk.

Suggested change

certFile, keyFile, err := saveCertToTempFiles(certPEM, keyPEM)

if err != nil {

return nil, "", "", fmt.Errorf("failed to save self-signed certificate: %w", err)

}

tlsConfig := &tls.Config{

MinVersion: tls.VersionTLS12,

}

return tlsConfig, certFile, keyFile, nil

cert, err := tls.X509KeyPair(certPEM, keyPEM)

if err != nil {

return nil, "", "", fmt.Errorf("failed to parse self-signed certificate: %w", err)

}

tlsConfig := &tls.Config{

MinVersion: tls.VersionTLS12,

Certificates: []tls.Certificate{cert},

}

// Certificate and key are kept in memory; no temporary files are created.

return tlsConfig, "", "", nil

Copilot · 2025-12-30T10:17:27Z

pkg/server/tls.go

+		}
+
+		tlsConfig := &tls.Config{
+			MinVersion: tls.VersionTLS12,


The TLS minimum version should be set to TLS 1.3 instead of TLS 1.2. TLS 1.2 has known vulnerabilities and TLS 1.3 is widely supported. This is a security best practice for new code in 2025.

pkg/logger/logger.go

Copilot · 2025-12-30T10:17:27Z

pkg/server/manager.go

+			select {
+			case s.serverErr <- err:
+			default:
+			}


The error handling for the serverErr channel is problematic. If Start() is called multiple times rapidly, the channel could be closed or have multiple errors written to it, potentially causing a panic or missing errors. Consider using sync.Once for initialization or better synchronization of the error channel lifecycle.

Copilot · 2025-12-30T10:17:27Z

pkg/logger/logger.go

-func CatchPanicCallback(location string, cb func(err any)) {
+func CatchPanicCallback(location string, cb func(err any), args ...interface{}) {
+	ctx, _ := extractContext(args...)
 	if err := recover(); err != nil {


This call to 'recover' has no effect because the enclosing function is never called using a defer statement.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-12-30T10:56:46Z

@warkanum I've opened a new pull request, #6, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2025-12-30T11:02:07Z

@warkanum I've opened a new pull request, #7, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

…ction Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Implement persistent certificate storage with reuse for self-signed SSL

Fix recover() not working in CatchPanic functions

warkanum

Looks good

warkanum · 2025-12-30T11:38:26Z

@copilot The go vet is failing:
Run go vet ./...

github.com/bitechdev/ResolveSpec/pkg/server

Error: pkg/server/manager.go:223:15: impossible type assertion: instance.(*gracefulServer)
*gracefulServer does not implement Instance (missing method Addr)

github.com/bitechdev/ResolveSpec/pkg/server [github.com/bitechdev/ResolveSpec/pkg/server.test]

Error: pkg/server/manager.go:223:15: impossible type assertion: instance.(*gracefulServer)
*gracefulServer does not implement Instance (missing method Addr)
Error: Process completed with exit code 1.

The lint as well:
pkg/server/interfaces.go:1: : # github.com/bitechdev/ResolveSpec/pkg/server [github.com/bitechdev/ResolveSpec/pkg/server.test]
Error: pkg/server/manager.go:223:15: impossible type assertion: instance.(*gracefulServer)
*gracefulServer does not implement Instance (missing method Addr) (typecheck)
package server
1 issues:

typecheck: 1

Copilot · 2025-12-30T11:38:33Z

@warkanum I've opened a new pull request, #8, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Fix impossible type assertion in Remove method

warkanum and others added 3 commits December 20, 2025 10:42

Work on server

4d1b8b6

Merge branch 'main' of github.com:bitechdev/ResolveSpec into server

8f83e8f

Better server manager

d4a6f9c

warkanum requested a review from Copilot December 30, 2025 10:13

Copilot started reviewing on behalf of warkanum December 30, 2025 10:13 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

Initial plan

c226dc3

Copilot AI mentioned this pull request Dec 30, 2025

Implement persistent certificate storage with reuse for self-signed SSL #6

Merged

warkanum and others added 6 commits December 30, 2025 12:57

Update pkg/server/manager.go

de50141

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update pkg/server/tls.go

feb023e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update pkg/server/manager.go

8adc386

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Implement cleanup for temporary certificate directories

267e84f

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Update pkg/logger/logger.go

bc2fdc1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Initial plan

5e6032c

Copilot AI mentioned this pull request Dec 30, 2025

Fix recover() not working in CatchPanic functions #7

Merged

Copilot AI and others added 8 commits December 30, 2025 11:03

Final verification and cleanup

64f5632

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Fix recover() usage in CatchPanic functions by returning deferred fun…

e10e2e1

…ction Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Improve CatchPanicCallback: extract context early and clarify example

4cb35a7

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Final update

92a83e9

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Refactor: Use persistent cert storage with reuse logic

ccf8522

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Add security improvements and race condition protection

4f8edd6

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Merge pull request #6 from bitechdev/copilot/sub-pr-5

16d4160

Implement persistent certificate storage with reuse for self-signed SSL

Merge pull request #7 from bitechdev/copilot/sub-pr-5-again

7e1718e

Fix recover() not working in CatchPanic functions

warkanum commented Dec 30, 2025

View reviewed changes

Initial plan

e0d21b1

Copilot AI mentioned this pull request Dec 30, 2025

Fix impossible type assertion in Remove method #8

Merged

Copilot AI and others added 2 commits December 30, 2025 11:44

Fix type assertion error in Remove method

eaecef6

Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com>

Merge pull request #8 from bitechdev/copilot/sub-pr-5

3d5d7b7

Fix impossible type assertion in Remove method

warkanum merged commit 645907d into main Dec 30, 2025
1 of 6 checks passed

warkanum deleted the server branch December 30, 2025 12:13

Conversation

warkanum commented Dec 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

warkanum Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

warkanum Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Dec 30, 2025

Uh oh!

Copilot AI commented Dec 30, 2025

Uh oh!

warkanum left a comment

Choose a reason for hiding this comment

Uh oh!

warkanum commented Dec 30, 2025

github.com/bitechdev/ResolveSpec/pkg/server

github.com/bitechdev/ResolveSpec/pkg/server [github.com/bitechdev/ResolveSpec/pkg/server.test]

Uh oh!

Copilot AI commented Dec 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants