Skip to content

fix: resolve import cycle and launcher API changes #564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lpcox merged 1 commit into from
Feb 1, 2026
+39 −63
Merged

fix: resolve import cycle and launcher API changes #564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 36 additions & 50 deletions internal/server/handlers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"strings"
"testing"

"github.com/githubnext/gh-aw-mcpg/internal/config"
"github.com/githubnext/gh-aw-mcpg/internal/launcher"
"github.com/githubnext/gh-aw-mcpg/internal/sys"
"github.com/stretchr/testify/assert"
Expand All @@ -17,7 +18,7 @@
// TestHandleOAuthDiscovery tests the OAuth discovery endpoint
func TestHandleOAuthDiscovery(t *testing.T) {
assert := assert.New(t)

handler := handleOAuthDiscovery()

tests := []struct {
Expand Down Expand Up @@ -62,12 +63,10 @@
// TestHandleClose_MethodValidation tests that only POST requests are accepted
func TestHandleClose_MethodValidation(t *testing.T) {
assert := assert.New(t)
require := require.New(t)


ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand Down Expand Up @@ -126,11 +125,10 @@
func TestHandleClose_SuccessfulShutdown(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand All @@ -151,7 +149,7 @@

// Parse response body
var response map[string]interface{}
err = json.NewDecoder(rec.Body).Decode(&response)
err := json.NewDecoder(rec.Body).Decode(&response)
require.NoError(err)

// Verify response structure
Expand All @@ -167,11 +165,10 @@
func TestHandleClose_Idempotency(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand Down Expand Up @@ -200,7 +197,7 @@

// Parse response body
var response map[string]interface{}
err = json.NewDecoder(rec2.Body).Decode(&response)
err := json.NewDecoder(rec2.Body).Decode(&response)
require.NoError(err)

assert.Equal("Gateway has already been closed", response["error"])
Expand All @@ -209,12 +206,10 @@
// TestHandleClose_MultipleRequests tests behavior with multiple simultaneous close requests
func TestHandleClose_MultipleRequests(t *testing.T) {
assert := assert.New(t)
require := require.New(t)


ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand Down Expand Up @@ -249,7 +244,7 @@
goneCount := 0

for _, rec := range responses {
if rec.Code == http.StatusOK {

Check failure on line 247 in internal/server/handlers_test.go

View workflow job for this annotation

GitHub Actions / lint 

QF1003: could use tagged switch on rec.Code (staticcheck)
okCount++
} else if rec.Code == http.StatusGone {
goneCount++
Expand All @@ -268,11 +263,10 @@
func TestHandleClose_ResponseFormat(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand All @@ -289,7 +283,7 @@

// Parse response
var response map[string]interface{}
err = json.NewDecoder(rec.Body).Decode(&response)
err := json.NewDecoder(rec.Body).Decode(&response)
require.NoError(err)

// Verify all required fields are present
Expand All @@ -306,12 +300,10 @@
// TestHandleClose_RemoteAddress tests that remote address is logged
func TestHandleClose_RemoteAddress(t *testing.T) {
assert := assert.New(t)
require := require.New(t)


ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand All @@ -334,12 +326,10 @@
// TestHandleClose_EmptyBody tests that close endpoint works without request body
func TestHandleClose_EmptyBody(t *testing.T) {
assert := assert.New(t)
require := require.New(t)


ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand All @@ -361,12 +351,10 @@
// TestHandleClose_WithRequestBody tests that close endpoint ignores request body
func TestHandleClose_WithRequestBody(t *testing.T) {
assert := assert.New(t)
require := require.New(t)


ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)

mockLauncher := launcher.New(ctx, &config.Config{})

unifiedServer := &UnifiedServer{
launcher: mockLauncher,
sysServer: sys.NewSysServer([]string{}),
Expand All @@ -390,29 +378,27 @@
// TestShutdownErrorJSON tests the shutdown error constant format
func TestShutdownErrorJSON(t *testing.T) {
assert := assert.New(t)

// Verify it's valid JSON
var parsed map[string]interface{}
err := json.Unmarshal([]byte(shutdownErrorJSON), &parsed)
assert.NoError(err)

// Verify structure
assert.Equal("Gateway is shutting down", parsed["error"])
}

// TestHandleClose_ShouldExitFlag tests that ShouldExit controls process termination
func TestHandleClose_ShouldExitFlag(t *testing.T) {
assert := assert.New(t)
require := require.New(t)


ctx := context.Background()
mockLauncher, err := launcher.New(ctx, map[string]*launcher.BackendConfig{})
require.NoError(err)
mockLauncher := launcher.New(ctx, &config.Config{})

tests := []struct {
name string
testMode bool
shouldExit bool
name string
testMode bool
shouldExit bool
}{
{
name: "Test mode prevents exit",
Expand Down
16 changes: 3 additions & 13 deletions internal/tty/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,14 @@ package tty
import (
"os"
"strings"

"github.com/githubnext/gh-aw-mcpg/internal/logger"
)

var log = logger.New("tty:container")

// IsRunningInContainer detects if the current process is running inside a container
// IsRunningInContainer detects if the current process is running inside a container.
// Note: This package cannot use the logger package to avoid import cycles,
// since logger imports tty for terminal detection.
func IsRunningInContainer() bool {
log.Print("Detecting container environment")

// Method 1: Check for /.dockerenv file (Docker-specific)
if _, err := os.Stat("/.dockerenv"); err == nil {
log.Print("Container detected: /.dockerenv file exists")
return true
}

Expand All @@ -27,19 +22,14 @@ func IsRunningInContainer() bool {
strings.Contains(content, "containerd") ||
strings.Contains(content, "kubepods") ||
strings.Contains(content, "lxc") {
log.Print("Container detected: /proc/1/cgroup contains container indicators")
return true
}
} else {
log.Printf("Failed to read /proc/1/cgroup: %v", err)
}

// Method 3: Check environment variable (set by Dockerfile)
if os.Getenv("RUNNING_IN_CONTAINER") == "true" {
log.Print("Container detected: RUNNING_IN_CONTAINER=true")
return true
}

log.Print("Not running in container")
return false
}
Loading