fix: address copilot review comments
- Fix mockReader to return io.EOF instead of os.ErrNotExist - Check flusher support before WriteHeader in SSE handlers - Add bounds check for ContainerID truncation in deploy - Update configPath comment to document auto-discovery behavior
- commit
- f12ab34fe38b223ec723af720376b8f53d9d15ac
- author
- brice · · 2026-01-17 07:05 UTC
- parent
- f76706f
- signature
- unsigned
4 files · +23 -15
M internal/adapters/in/cli/deploy.go +5 -1
| @@ -37,7 +37,11 @@ Examples: | |||
| 37 | 37 | if err != nil { | |
| 38 | 38 | return fmt.Errorf("failed to deploy: %w", err) | |
| 39 | 39 | } | |
| 40 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, result.ContainerID[:12]))) | |
| 40 | + | containerID := result.ContainerID | |
| 41 | + | if len(containerID) > 12 { | |
| 42 | + | containerID = containerID[:12] | |
| 43 | + | } | |
| 44 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | |
| 41 | 45 | return nil | |
| 42 | 46 | } | |
| 43 | 47 | ||
| @@ -37,7 +37,11 @@ Examples: | |||||
| 37 | if err != nil { | 37 | if err != nil { | ||
| 38 | return fmt.Errorf("failed to deploy: %w", err) | 38 | return fmt.Errorf("failed to deploy: %w", err) | ||
| 39 | } | 39 | } | ||
| 40 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, result.ContainerID[:12]))) | 40 | + | containerID := result.ContainerID |
| 41 | + | if len(containerID) > 12 { | |||
| 42 | + | containerID = containerID[:12] | |||
| 43 | + | } | |||
| 44 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | |||
| 41 | return nil | 45 | return nil | ||
| 42 | } | 46 | } | ||
| 43 | 47 | ||||
M internal/adapters/in/cli/routes.go +2 -1
| @@ -14,7 +14,8 @@ import ( | |||
| 14 | 14 | "github.com/spf13/cobra" | |
| 15 | 15 | ) | |
| 16 | 16 | ||
| 17 | - | // configPath is set by the root command | |
| 17 | + | // configPath for local operations. If empty, config is auto-discovered | |
| 18 | + | // from standard locations (/etc/gordon/gordon.toml, ~/.config/gordon/gordon.toml, ./gordon.toml). | |
| 18 | 19 | var configPath string | |
| 19 | 20 | ||
| 20 | 21 | // truncateImage shortens long image references for display. | |
| @@ -14,7 +14,8 @@ import ( | |||||
| 14 | "github.com/spf13/cobra" | 14 | "github.com/spf13/cobra" | ||
| 15 | ) | 15 | ) | ||
| 16 | 16 | ||||
| 17 | - | // configPath is set by the root command | 17 | + | // configPath for local operations. If empty, config is auto-discovered |
| 18 | + | // from standard locations (/etc/gordon/gordon.toml, ~/.config/gordon/gordon.toml, ./gordon.toml). | |||
| 18 | var configPath string | 19 | var configPath string | ||
| 19 | 20 | ||||
| 20 | // truncateImage shortens long image references for display. | 21 | // truncateImage shortens long image references for display. | ||
M internal/adapters/in/http/admin/handler.go +14 -12
| @@ -646,18 +646,19 @@ func (h *Handler) streamProcessLogs(w http.ResponseWriter, r *http.Request, line | |||
| 646 | 646 | ctx := r.Context() | |
| 647 | 647 | log := zerowrap.FromCtx(ctx) | |
| 648 | 648 | ||
| 649 | - | // Set up SSE | |
| 650 | - | w.Header().Set("Content-Type", "text/event-stream") | |
| 651 | - | w.Header().Set("Cache-Control", "no-cache") | |
| 652 | - | w.Header().Set("Connection", "keep-alive") | |
| 653 | - | w.WriteHeader(http.StatusOK) | |
| 654 | - | ||
| 649 | + | // Check for flusher support before setting up SSE | |
| 655 | 650 | flusher, ok := w.(http.Flusher) | |
| 656 | 651 | if !ok { | |
| 657 | 652 | h.sendError(w, http.StatusInternalServerError, "streaming not supported") | |
| 658 | 653 | return | |
| 659 | 654 | } | |
| 660 | 655 | ||
| 656 | + | // Set up SSE | |
| 657 | + | w.Header().Set("Content-Type", "text/event-stream") | |
| 658 | + | w.Header().Set("Cache-Control", "no-cache") | |
| 659 | + | w.Header().Set("Connection", "keep-alive") | |
| 660 | + | w.WriteHeader(http.StatusOK) | |
| 661 | + | ||
| 661 | 662 | ch, err := h.logSvc.FollowProcessLogs(ctx, lines) | |
| 662 | 663 | if err != nil { | |
| 663 | 664 | log.Warn().Err(err).Msg("failed to follow process logs") | |
| @@ -685,18 +686,19 @@ func (h *Handler) streamContainerLogs(w http.ResponseWriter, r *http.Request, lo | |||
| 685 | 686 | ctx := r.Context() | |
| 686 | 687 | log := zerowrap.FromCtx(ctx) | |
| 687 | 688 | ||
| 688 | - | // Set up SSE | |
| 689 | - | w.Header().Set("Content-Type", "text/event-stream") | |
| 690 | - | w.Header().Set("Cache-Control", "no-cache") | |
| 691 | - | w.Header().Set("Connection", "keep-alive") | |
| 692 | - | w.WriteHeader(http.StatusOK) | |
| 693 | - | ||
| 689 | + | // Check for flusher support before setting up SSE | |
| 694 | 690 | flusher, ok := w.(http.Flusher) | |
| 695 | 691 | if !ok { | |
| 696 | 692 | h.sendError(w, http.StatusInternalServerError, "streaming not supported") | |
| 697 | 693 | return | |
| 698 | 694 | } | |
| 699 | 695 | ||
| 696 | + | // Set up SSE | |
| 697 | + | w.Header().Set("Content-Type", "text/event-stream") | |
| 698 | + | w.Header().Set("Cache-Control", "no-cache") | |
| 699 | + | w.Header().Set("Connection", "keep-alive") | |
| 700 | + | w.WriteHeader(http.StatusOK) | |
| 701 | + | ||
| 700 | 702 | ch, err := h.logSvc.FollowContainerLogs(ctx, logDomain, lines) | |
| 701 | 703 | if err != nil { | |
| 702 | 704 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to follow container logs") | |
| @@ -646,18 +646,19 @@ func (h *Handler) streamProcessLogs(w http.ResponseWriter, r *http.Request, line | |||||
| 646 | ctx := r.Context() | 646 | ctx := r.Context() | ||
| 647 | log := zerowrap.FromCtx(ctx) | 647 | log := zerowrap.FromCtx(ctx) | ||
| 648 | 648 | ||||
| 649 | - | // Set up SSE | 649 | + | // Check for flusher support before setting up SSE |
| 650 | - | w.Header().Set("Content-Type", "text/event-stream") | |||
| 651 | - | w.Header().Set("Cache-Control", "no-cache") | |||
| 652 | - | w.Header().Set("Connection", "keep-alive") | |||
| 653 | - | w.WriteHeader(http.StatusOK) | |||
| 654 | - | ||||
| 655 | flusher, ok := w.(http.Flusher) | 650 | flusher, ok := w.(http.Flusher) | ||
| 656 | if !ok { | 651 | if !ok { | ||
| 657 | h.sendError(w, http.StatusInternalServerError, "streaming not supported") | 652 | h.sendError(w, http.StatusInternalServerError, "streaming not supported") | ||
| 658 | return | 653 | return | ||
| 659 | } | 654 | } | ||
| 660 | 655 | ||||
| 656 | + | // Set up SSE | |||
| 657 | + | w.Header().Set("Content-Type", "text/event-stream") | |||
| 658 | + | w.Header().Set("Cache-Control", "no-cache") | |||
| 659 | + | w.Header().Set("Connection", "keep-alive") | |||
| 660 | + | w.WriteHeader(http.StatusOK) | |||
| 661 | + | ||||
| 661 | ch, err := h.logSvc.FollowProcessLogs(ctx, lines) | 662 | ch, err := h.logSvc.FollowProcessLogs(ctx, lines) | ||
| 662 | if err != nil { | 663 | if err != nil { | ||
| 663 | log.Warn().Err(err).Msg("failed to follow process logs") | 664 | log.Warn().Err(err).Msg("failed to follow process logs") | ||
| @@ -685,18 +686,19 @@ func (h *Handler) streamContainerLogs(w http.ResponseWriter, r *http.Request, lo | |||||
| 685 | ctx := r.Context() | 686 | ctx := r.Context() | ||
| 686 | log := zerowrap.FromCtx(ctx) | 687 | log := zerowrap.FromCtx(ctx) | ||
| 687 | 688 | ||||
| 688 | - | // Set up SSE | 689 | + | // Check for flusher support before setting up SSE |
| 689 | - | w.Header().Set("Content-Type", "text/event-stream") | |||
| 690 | - | w.Header().Set("Cache-Control", "no-cache") | |||
| 691 | - | w.Header().Set("Connection", "keep-alive") | |||
| 692 | - | w.WriteHeader(http.StatusOK) | |||
| 693 | - | ||||
| 694 | flusher, ok := w.(http.Flusher) | 690 | flusher, ok := w.(http.Flusher) | ||
| 695 | if !ok { | 691 | if !ok { | ||
| 696 | h.sendError(w, http.StatusInternalServerError, "streaming not supported") | 692 | h.sendError(w, http.StatusInternalServerError, "streaming not supported") | ||
| 697 | return | 693 | return | ||
| 698 | } | 694 | } | ||
| 699 | 695 | ||||
| 696 | + | // Set up SSE | |||
| 697 | + | w.Header().Set("Content-Type", "text/event-stream") | |||
| 698 | + | w.Header().Set("Cache-Control", "no-cache") | |||
| 699 | + | w.Header().Set("Connection", "keep-alive") | |||
| 700 | + | w.WriteHeader(http.StatusOK) | |||
| 701 | + | ||||
| 700 | ch, err := h.logSvc.FollowContainerLogs(ctx, logDomain, lines) | 702 | ch, err := h.logSvc.FollowContainerLogs(ctx, logDomain, lines) | ||
| 701 | if err != nil { | 703 | if err != nil { | ||
| 702 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to follow container logs") | 704 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to follow container logs") | ||
M internal/usecase/logs/service_test.go +2 -1
| @@ -2,6 +2,7 @@ package logs | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "io" | |
| 5 | 6 | "os" | |
| 6 | 7 | "path/filepath" | |
| 7 | 8 | "testing" | |
| @@ -249,7 +250,7 @@ func TestTailLines(t *testing.T) { | |||
| 249 | 250 | type mockReader struct{} | |
| 250 | 251 | ||
| 251 | 252 | func (m *mockReader) Read(p []byte) (n int, err error) { | |
| 252 | - | return 0, os.ErrNotExist // Return EOF immediately | |
| 253 | + | return 0, io.EOF | |
| 253 | 254 | } | |
| 254 | 255 | ||
| 255 | 256 | func (m *mockReader) Close() error { | |
| @@ -2,6 +2,7 @@ package logs | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "io" | |||
| 5 | "os" | 6 | "os" | ||
| 6 | "path/filepath" | 7 | "path/filepath" | ||
| 7 | "testing" | 8 | "testing" | ||
| @@ -249,7 +250,7 @@ func TestTailLines(t *testing.T) { | |||||
| 249 | type mockReader struct{} | 250 | type mockReader struct{} | ||
| 250 | 251 | ||||
| 251 | func (m *mockReader) Read(p []byte) (n int, err error) { | 252 | func (m *mockReader) Read(p []byte) (n int, err error) { | ||
| 252 | - | return 0, os.ErrNotExist // Return EOF immediately | 253 | + | return 0, io.EOF |
| 253 | } | 254 | } | ||
| 254 | 255 | ||||
| 255 | func (m *mockReader) Close() error { | 256 | func (m *mockReader) Close() error { | ||