Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 8 additions & 14 deletions cmd/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import (
"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/config"
"github.com/devsy-org/devsy/pkg/envfile"
"github.com/devsy-org/log"
"github.com/sirupsen/logrus"
"github.com/devsy-org/devsy/pkg/log"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -52,18 +51,13 @@ func AgentPersistentPreRunE(
}
parent.Annotations[AgentExecutedAnnotation] = "true"

if globalFlags.LogOutput == "json" {
log.Default.SetFormat(log.JSONFormat)
} else {
log.Default.MakeRaw()
}

switch {
case globalFlags.Quiet:
log.Default.SetLevel(logrus.FatalLevel)
case globalFlags.Debug:
log.Default.SetLevel(logrus.DebugLevel)
}
// Initialise the zap logger for the agent subprocess.
// stdout is the binary protocol channel, so all log output goes to stderr.
log.Init(log.Config{
Quiet: globalFlags.Quiet,
Debug: globalFlags.Debug,
Format: "json", // Agent must use JSON: single-line output is captured by TunnelLogStreamer.lastLines
})
Comment on lines +56 to +60

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Verbosity flag is not propagated to the agent logger.

log.Config supports a Verbosity field (0=error, 1=info+warn, 2=debug, 3=trace), but AgentPersistentPreRunE only forwards Quiet and Debug. If the parent process is invoked with -v/-vv (or DEVSY_VERBOSITY), the agent subprocess will silently ignore it and log at the default level, making it harder to debug agent-side issues.

Proposed fix
 	log.Init(log.Config{
+		Verbosity: globalFlags.Verbosity,
 		Quiet:  globalFlags.Quiet,
 		Debug:  globalFlags.Debug,
 		Format: "json", // Agent must always use JSON: stdout is binary protocol, host reads stderr via ReadJSONStream
 	})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
log.Init(log.Config{
Quiet: globalFlags.Quiet,
Debug: globalFlags.Debug,
Format: "json", // Agent must always use JSON: stdout is binary protocol, host reads stderr via ReadJSONStream
})
log.Init(log.Config{
Verbosity: globalFlags.Verbosity,
Quiet: globalFlags.Quiet,
Debug: globalFlags.Debug,
Format: "json", // Agent must always use JSON: stdout is binary protocol, host reads stderr via ReadJSONStream
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/agent/agent.go` around lines 56 - 60, AgentPersistentPreRunE currently
calls log.Init with only Quiet and Debug set, so the log.Config.Verbosity field
is not propagated; update AgentPersistentPreRunE so when calling log.Init it
also sets Verbosity from the parsed verbosity flag/environment (the same source
as globalFlags.Debug/Quiet — e.g. globalFlags.Verbosity or the
DEVSY_VERBOSITY-backed value) so log.Init(log.Config{Quiet: globalFlags.Quiet,
Debug: globalFlags.Debug, Verbosity: globalFlags.Verbosity, Format: "json"})
ensures the agent respects -v/-vv and DEVSY_VERBOSITY; keep the existing JSON
format behavior and tests.


if globalFlags.DevsyHome != "" {
_ = os.Setenv(config.EnvHome, globalFlags.DevsyHome)
Expand Down
24 changes: 4 additions & 20 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (
"github.com/devsy-org/devsy/pkg/config"
"github.com/devsy-org/devsy/pkg/log"
"github.com/devsy-org/devsy/pkg/telemetry"
log2 "github.com/devsy-org/log"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
flag "github.com/spf13/pflag"
"golang.org/x/crypto/ssh"
Expand All @@ -42,20 +40,6 @@ func NewRootCmd() *cobra.Command {
Format: globalFlags.LogOutput,
})

// Configure the old logger too, until the migration is complete.
if globalFlags.LogOutput == "json" {
log2.Default.SetFormat(log2.JSONFormat)
} else if globalFlags.LogOutput == "raw" {
log2.Default.SetFormat(log2.RawFormat)
}

switch {
case globalFlags.Quiet:
log2.Default.SetLevel(logrus.FatalLevel)
case globalFlags.Debug || os.Getenv(config.EnvDebug) == config.BoolTrue:
log2.Default.SetLevel(logrus.DebugLevel)
}

if globalFlags.DevsyHome != "" {
_ = os.Setenv(config.EnvHome, globalFlags.DevsyHome)
}
Expand Down Expand Up @@ -99,13 +83,13 @@ func Execute() {
}

if globalFlags.Debug {
log2.Default.Fatalf("%+v", err)
log.Fatalf("%+v", err)
} else {
if rootCmd.Annotations == nil ||
rootCmd.Annotations[agent.AgentExecutedAnnotation] != config.BoolTrue {
log2.Default.Error("Try using -v or --debug flag to see more verbose output")
log.Error("Try using -v or --debug flag to see more verbose output")
}
log2.Default.Fatal(err)
log.Fatal(err)
}
}
}
Expand Down Expand Up @@ -174,7 +158,7 @@ func inheritFlagsFromEnvironment(flags *flag.FlagSet) {
// set the variable holding the flag's value to the default supplied by the environment
err := flag.Value.Set(value)
if err != nil {
log2.Default.Fatalf(
log.Fatalf(
"failed to set flag %s from the environment variable %s with value %s: %+v",
flag.Name,
environmentVariable,
Expand Down
33 changes: 27 additions & 6 deletions e2e/tests/up/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
// Only the fields we need for test assertions are included.
type logLine struct {
Message string `json:"message,omitempty"`
Msg string `json:"msg,omitempty"`
}

type baseTestContext struct {
Expand Down Expand Up @@ -70,12 +71,32 @@ func findMessage(reader io.Reader, message string) error {
for scan.Scan() {
if line := scan.Bytes(); len(line) > 0 {
lineObject := &logLine{}
if err := json.Unmarshal(
line,
lineObject,
); err == nil &&
strings.Contains(lineObject.Message, message) {
return nil
if err := json.Unmarshal(line, lineObject); err == nil {
msg := lineObject.Message
if msg == "" {
msg = lineObject.Msg
}
if strings.Contains(msg, message) {
return nil
}
// Agent JSON may be embedded in the parent's error chain.
// Parse any nested JSON lines within the msg to resolve
// double-escaped quotes.
for part := range strings.SplitSeq(msg, "\n") {
part = strings.TrimSpace(part)
if len(part) > 0 && part[0] == '{' {
inner := &logLine{}
if json.Unmarshal([]byte(part), inner) == nil {
innerMsg := inner.Message
if innerMsg == "" {
innerMsg = inner.Msg
}
if strings.Contains(innerMsg, message) {
return nil
}
}
}
}
}
}
}
Expand Down
6 changes: 1 addition & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ require (
github.com/devsy-org/agentapi v1.0.1
github.com/devsy-org/api v1.1.0
github.com/devsy-org/apiserver v1.5.0
github.com/devsy-org/log v1.1.0
github.com/devsy-org/ssh v1.1.0
github.com/distribution/reference v0.6.0
github.com/docker/cli v29.4.0+incompatible
Expand All @@ -45,7 +44,6 @@ require (
github.com/onsi/ginkgo/v2 v2.28.1
github.com/onsi/gomega v1.39.1
github.com/pkg/sftp v1.13.10
github.com/sirupsen/logrus v1.9.4
github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966
github.com/spf13/cobra v1.10.2
github.com/spf13/pflag v1.0.10
Expand Down Expand Up @@ -92,7 +90,6 @@ require (
github.com/BurntSushi/toml v1.5.0 // indirect
github.com/Masterminds/semver/v3 v3.4.0 // indirect
github.com/NYTimes/gziphandler v1.1.1 // indirect
github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect
github.com/agext/levenshtein v1.2.3 // indirect
github.com/akutz/memconn v0.1.0 // indirect
github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa // indirect
Expand Down Expand Up @@ -196,7 +193,6 @@ require (
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/jsimonetti/rtnetlink v1.4.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/k0kubun/go-ansi v0.0.0-20180517002512-3bf9e2903213 // indirect
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
github.com/klauspost/compress v1.18.5 // indirect
github.com/kr/fs v0.1.0 // indirect
Expand Down Expand Up @@ -245,6 +241,7 @@ require (
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect
github.com/secure-systems-lab/go-securesystemslib v0.10.0 // indirect
github.com/shibumi/go-pathspec v1.3.0 // indirect
github.com/sirupsen/logrus v1.9.4 // indirect
github.com/stoewer/go-strcase v1.3.1 // indirect
github.com/tailscale/certstore v0.1.1-0.20231202035212-d3fa0460f47e // indirect
github.com/tailscale/go-winio v0.0.0-20231025203758-c4f33415bf55 // indirect
Expand Down Expand Up @@ -296,7 +293,6 @@ require (
google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 // indirect
gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
gvisor.dev/gvisor v0.0.0-20250205023644-9414b50a5633 // indirect
k8s.io/apiextensions-apiserver v0.35.0 // indirect
Expand Down
6 changes: 0 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63n
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w=
github.com/ProtonMail/go-crypto v1.3.0 h1:ILq8+Sf5If5DCpHQp4PbZdS1J7HDFRXz/+xKBiRGFrw=
github.com/ProtonMail/go-crypto v1.3.0/go.mod h1:9whxjD8Rbs29b4XWbB8irEcE8KHMqaR2e7GWU1R+/PE=
github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpHMqeKTCYkitsPqHNxTmd4SNR5r94FGM8=
github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d/go.mod h1:asat636LX7Bqt5lYEZ27JNDcqxfjdBQuJ/MM4CN/Lzo=
github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo=
github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558=
github.com/akutz/memconn v0.1.0 h1:NawI0TORU4hcOMsMr11g7vwlCdkYeLKXBcxWu2W/P8A=
Expand Down Expand Up @@ -235,8 +233,6 @@ github.com/devsy-org/api v1.1.0 h1:l7T9k7RVwatwN4lxeDTF3iN6EYmfGZgR3ZTJMDGha1M=
github.com/devsy-org/api v1.1.0/go.mod h1:mAZklKdnywJYiXDReBLte/H+3m69z6G7RHB3n1lI53Q=
github.com/devsy-org/apiserver v1.5.0 h1:tAr2tH9QwtNEu8BTbBgHkJD6ae43S5LDdHGphLsELB8=
github.com/devsy-org/apiserver v1.5.0/go.mod h1:2jKuqy8XdH/g4bIjmkj3efQgshN/NjMzm17SkvvSR88=
github.com/devsy-org/log v1.1.0 h1:LktC2pLnGwaduspO+euTzzj3Fbt4G1S1bDcb4f3xTSE=
github.com/devsy-org/log v1.1.0/go.mod h1:B/D8SOjJ34lQS5O1BupXwXvWsJRK+ul3JvYYahTkCE8=
github.com/devsy-org/ssh v1.1.0 h1:LKwUPuyijX+x8L4zzEsObBptRjr8OTKugVCl3UPMTOs=
github.com/devsy-org/ssh v1.1.0/go.mod h1:+/FJAVr49LxNS0E4IPOXQZOnwrvNqc6cNLT7unu3MD4=
github.com/devsy-org/tailscale v1.92.2 h1:8Ew/cGVUaQa9N6Acab9SgXDWJp/Kzn3T0UejLyWPTfo=
Expand Down Expand Up @@ -444,8 +440,6 @@ github.com/jsimonetti/rtnetlink v1.4.0 h1:Z1BF0fRgcETPEa0Kt0MRk3yV5+kF1FWTni6KUF
github.com/jsimonetti/rtnetlink v1.4.0/go.mod h1:5W1jDvWdnthFJ7fxYX1GMK07BUpI4oskfOqvPteYS6E=
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
github.com/k0kubun/go-ansi v0.0.0-20180517002512-3bf9e2903213 h1:qGQQKEcAR99REcMpsXCp3lJ03zYT1PkRd3kQGPn9GVg=
github.com/k0kubun/go-ansi v0.0.0-20180517002512-3bf9e2903213/go.mod h1:vNUNkEQ1e29fT/6vq2aBdFsgNPmy8qMdSay1npru+Sw=
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU=
Expand Down
64 changes: 39 additions & 25 deletions pkg/log/jsonstream.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ package log
import (
"encoding/json"
"io"
"strings"

"github.com/devsy-org/devsy/pkg/scanner"
"github.com/devsy-org/log"
"github.com/sirupsen/logrus"
)

func PipeJSONStream() (io.WriteCloser, chan struct{}) {
Expand All @@ -20,34 +19,49 @@ func PipeJSONStream() (io.WriteCloser, chan struct{}) {
return writer, done
}

var levelFuncs = map[logrus.Level]func(...any){
logrus.TraceLevel: Debug,
logrus.DebugLevel: Debug,
logrus.InfoLevel: Info,
logrus.WarnLevel: Warn,
logrus.ErrorLevel: Error,
logrus.PanicLevel: Error,
logrus.FatalLevel: Error,
// jsonLine is a self-contained representation of a JSON log line,
// replacing the old github.com/devsy-org/log.Line type.
type jsonLine struct {
Message string `json:"message,omitempty"`
Msg string `json:"msg,omitempty"`
Level string `json:"level,omitempty"`
}

func (l *jsonLine) text() string {
if l.Message != "" {
return l.Message
}
return l.Msg
}

var levelFuncs = map[string]func(...any){
"trace": Debug,
"debug": Debug,
"info": Info,
"warning": Warn,
"warn": Warn,
"error": Error,
"panic": Error,
"fatal": Error,
}

func ReadJSONStream(reader io.Reader) {
scan := scanner.NewScanner(reader)
for scan.Scan() {
lineObject, err := Unmarshal(scan.Bytes())
if err == nil && lineObject.Message != "" {
if fn, ok := levelFuncs[lineObject.Level]; ok {
fn(lineObject.Message)
}
line := scan.Bytes()
if len(line) == 0 {
continue
}
obj := &jsonLine{}
if err := json.Unmarshal(line, obj); err != nil {
continue
}
msg := obj.text()
if msg == "" {
continue
}
if fn, ok := levelFuncs[strings.ToLower(obj.Level)]; ok {
fn(msg)
}
}
Comment on lines 48 to 66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

git ls-files | head -20

Repository: devsy-org/devsy

Length of output: 585


🏁 Script executed:

fd -type f -name "jsonstream.go"

Repository: devsy-org/devsy

Length of output: 229


🏁 Script executed:

find . -name "*.go" -path "*log*" | head -20

Repository: devsy-org/devsy

Length of output: 542


🏁 Script executed:

cat -n pkg/log/jsonstream.go

Repository: devsy-org/devsy

Length of output: 1766


🏁 Script executed:

cat -n pkg/log/levels.go

Repository: devsy-org/devsy

Length of output: 1644


🏁 Script executed:

rg -i "zapcore.Encoder|NewJSONEncoder|EncodeLevel" -A 3

Repository: devsy-org/devsy

Length of output: 1618


🏁 Script executed:

rg -i "zap.Config|zap.NewProduction|zap.NewDevelopment" -A 5

Repository: devsy-org/devsy

Length of output: 812


🏁 Script executed:

rg "ReadJSONStream|PipeJSONStream" -B 3 -A 3

Repository: devsy-org/devsy

Length of output: 2934


🏁 Script executed:

rg "DPanic" -i

Repository: devsy-org/devsy

Length of output: 41


🏁 Script executed:

cat -n pkg/log/logger.go | head -40

Repository: devsy-org/devsy

Length of output: 1468


🏁 Script executed:

rg "func Info|func Warn|func Error|func Debug" -A 1

Repository: devsy-org/devsy

Length of output: 1394


Lines without a recognized level are silently dropped, creating a reliability regression for agent subprocess output.

The jsonLine.Level field has an omitempty tag, so when a log line lacks a level or contains an unknown level string, the map lookup in levelFuncs[strings.ToLower(obj.Level)] fails, and the message is never logged with no fallback.

While the agent is configured to use zap's JSON encoder (which outputs lowercase level strings—debug, info, warn, error, panic, fatal—all of which are mapped), this code lacks defensive handling for:

  • Missing level fields (empty string)
  • Unknown levels from custom encoders, future zap versions, or non-zap sources
Suggested fix: default to Info when level is missing/unknown
-		if fn, ok := levelFuncs[strings.ToLower(obj.Level)]; ok {
-			fn(msg)
-		}
+		fn, ok := levelFuncs[strings.ToLower(obj.Level)]
+		if !ok {
+			fn = Info
+		}
+		fn(msg)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/log/jsonstream.go` around lines 48 - 66, ReadJSONStream currently drops
messages when jsonLine.Level is empty or unknown; update the handling after
unmarshalling (in ReadJSONStream using jsonLine and levelFuncs) to normalize
obj.Level (strings.ToLower), attempt the lookup, and if lookup fails or
obj.Level == "" default to the info-level handler (e.g. use levelFuncs["info"]
or a safe info fallback) and call it with obj.text(); ensure you still skip
empty messages from obj.text() and preserve existing behavior for recognized
levels.

}

func Unmarshal(line []byte) (*log.Line, error) {
lineObject := &log.Line{}
err := json.Unmarshal(line, lineObject)
if err != nil {
return nil, err
}

return lineObject, nil
}
Loading
Loading