Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ linters:
- pattern: '^.*\.Checkout$'
msg: "go-git Checkout deletes .gitignored dirs - use CheckoutBranch() from git_operations.go"
pkg: 'github\.com/go-git/go-git'
- pattern: 'logging\.(Debug|Info|Warn|Error|LogDuration)\(\s*context\.(Background|TODO)\(\)'
msg: "Don't pass context.Background()/TODO() to logging.X — thread the request ctx so the ctx-carried logger is used"
Comment thread
gtrrz-victor marked this conversation as resolved.
Outdated
govet:
enable-all: true
disable:
Expand Down
6 changes: 4 additions & 2 deletions cmd/entire/cli/agent/copilotcli/transcript.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,10 @@ func extractTokenUsageFromEvents(events []copilotEvent, preferSessionShutdown bo

var data sessionShutdownData
if err := json.Unmarshal(events[i].Data, &data); err != nil {
logging.Debug(context.Background(), "copilot-cli: session.shutdown data unmarshal failed",
"err", err)
// Malformed session.shutdown event; skip and rely on per-message
// totals. CalculateTokenUsage on the agent.Agent interface has no
// context.Context, so this path cannot log to the request-scoped
// logger; drop the trace rather than emit through a Background ctx.
continue
}

Expand Down
16 changes: 6 additions & 10 deletions cmd/entire/cli/agent/opencode/opencode.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,12 @@ func (a *OpenCodeAgent) ReadSession(input *agent.HookInput) (*agent.AgentSession
return nil, fmt.Errorf("failed to read session: %w", err)
}

// Parse to extract computed fields
modifiedFiles, err := ExtractModifiedFiles(data)
if err != nil {
// Non-fatal: we can still return the session without modified files
logging.Warn(context.Background(), "failed to extract modified files from opencode session",
slog.String("session_ref", input.SessionRef),
slog.String("error", err.Error()),
)
modifiedFiles = nil
}
// Parse to extract computed fields. ReadSession has no context.Context, so
// any non-fatal error here cannot reach the request-scoped logger; instead
// of swallowing silently, return modifiedFiles=nil and let the caller (which
// has a ctx) decide whether to surface the issue. The session itself is
// still usable without modified-file metadata.
modifiedFiles, _ := ExtractModifiedFiles(data) //nolint:errcheck // intentional best-effort; see comment
Comment thread
gtrrz-victor marked this conversation as resolved.
Outdated

return &agent.AgentSession{
AgentName: a.Name(),
Expand Down
9 changes: 1 addition & 8 deletions cmd/entire/cli/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,7 @@ external_agents in settings. Run 'entire configure' to see the full list.`,
}

func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName types.AgentName, force bool) error {
// Initialize structured logger so logging.Warn/Info write to .entire/logs/ not stderr.
if err := logging.Init(ctx, sessionID); err != nil {
// Init failed — logging will use stderr fallback, non-fatal.
_ = err
}
defer logging.Close()

logCtx := logging.WithComponent(ctx, "attach")
logCtx := logging.WithSession(logging.WithComponent(ctx, "attach"), sessionID)

// Open repository once — shared across all operations.
repo, err := openRepository(ctx)
Expand Down
7 changes: 2 additions & 5 deletions cmd/entire/cli/checkpoint/committed_tripwire_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/entireio/cli/cmd/entire/cli/checkpoint/id"
"github.com/entireio/cli/cmd/entire/cli/jsonutil"
"github.com/entireio/cli/cmd/entire/cli/logging"
"github.com/entireio/cli/cmd/entire/cli/paths"
"github.com/entireio/cli/cmd/entire/cli/versioninfo"
"github.com/entireio/cli/redact"
Expand All @@ -27,10 +26,8 @@ func TestWriteStandardCheckpointEntries_RefusesUnexpectedSessionZeroOverwrite(t
}
store := NewGitStore(repo)

if err := logging.Init(context.Background(), ""); err != nil {
t.Fatalf("logging.Init() error = %v", err)
}
defer logging.Close()
// Logger lifecycle is owned by main.go in production; tests just need the
// no-op fallback (slog.Default) for any logging.X calls inside the tripwire.

checkpointID, err := id.Generate()
if err != nil {
Expand Down
7 changes: 2 additions & 5 deletions cmd/entire/cli/checkpoint/v2_committed_tripwire_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/entireio/cli/cmd/entire/cli/checkpoint/id"
"github.com/entireio/cli/cmd/entire/cli/jsonutil"
"github.com/entireio/cli/cmd/entire/cli/logging"
"github.com/entireio/cli/cmd/entire/cli/paths"
"github.com/entireio/cli/cmd/entire/cli/versioninfo"

Expand All @@ -22,10 +21,8 @@ func TestV2WriteMainCheckpointEntries_RefusesUnexpectedSessionZeroOverwrite(t *t
repo := initTestRepo(t)
store := NewV2GitStore(repo, "origin")

if err := logging.Init(context.Background(), ""); err != nil {
t.Fatalf("logging.Init() error = %v", err)
}
defer logging.Close()
// Logger lifecycle is owned by main.go in production; tests fall back to
// slog.Default for any logging.X calls inside the tripwire.

checkpointID, err := id.Generate()
if err != nil {
Expand Down
10 changes: 1 addition & 9 deletions cmd/entire/cli/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"github.com/charmbracelet/huh"
"github.com/entireio/cli/cmd/entire/cli/checkpoint"
"github.com/entireio/cli/cmd/entire/cli/logging"
"github.com/entireio/cli/cmd/entire/cli/paths"
"github.com/entireio/cli/cmd/entire/cli/session"
"github.com/entireio/cli/cmd/entire/cli/settings"
Expand Down Expand Up @@ -66,18 +65,11 @@ func newCleanCmd() *cobra.Command {
return errors.New("--all and --session cannot be used together")
}

// Check if in git repository before initializing logging,
// to avoid creating .entire/logs in arbitrary directories.
// Require a git worktree.
if _, err := paths.WorktreeRoot(ctx); err != nil {
return errors.New("not a git repository")
}

// Initialize logging
logging.SetLogLevelGetter(GetLogLevel)
if err := logging.Init(ctx, ""); err == nil {
defer logging.Close()
}

if allFlag {
return runCleanAll(ctx, cmd, forceFlag, dryRunFlag)
}
Expand Down
9 changes: 0 additions & 9 deletions cmd/entire/cli/explain.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,6 @@ Note: --session filters the list view; the positional arg, --commit, and --check
return nil
}

// Only initialize logging when inside a git worktree to avoid
// creating .entire/logs/ in arbitrary directories.
if _, err := paths.WorktreeRoot(cmd.Context()); err == nil {
logging.SetLogLevelGetter(GetLogLevel)
if err := logging.Init(cmd.Context(), ""); err == nil {
defer logging.Close()
}
}

// Positional arg is mutually exclusive with --checkpoint, --commit, --session
var positional string
if len(args) > 0 {
Expand Down
26 changes: 8 additions & 18 deletions cmd/entire/cli/hook_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ import (
"github.com/spf13/cobra"
)

// agentHookLogCleanup stores the cleanup function for agent hook logging.
// Set by PersistentPreRunE, called by PersistentPostRunE.
var agentHookLogCleanup func()

// currentHookAgentName stores the agent name for the currently executing hook.
// Set by newAgentHookVerbCmdWithLogging before calling the handler.
// This allows handlers to know which agent invoked the hook without guessing.
Expand Down Expand Up @@ -53,13 +49,7 @@ func newAgentHooksCmd(agentName types.AgentName, handler agent.HookSupport) *cob
Short: handler.Description() + " hook handlers",
Hidden: true,
PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
agentHookLogCleanup = initHookLogging(cmd.Context())
return nil
},
PersistentPostRunE: func(_ *cobra.Command, _ []string) error {
if agentHookLogCleanup != nil {
agentHookLogCleanup()
}
cmd.SetContext(enrichHookContext(cmd.Context()))
return nil
},
}
Expand Down Expand Up @@ -89,10 +79,11 @@ func getHookType(hookName string) string {
// executeAgentHook runs the core hook execution logic for a given agent and hook name.
// It handles git repo checks, enabled checks, logging, event parsing, and lifecycle dispatch.
// Used by both the registered subcommand path and the RunE fallback for external agents.
// When initLogging is true, it initializes and cleans up hook logging (used by the RunE fallback
// since it doesn't go through PersistentPreRunE). Built-in agent subcommands pass false since
// their parent command's PersistentPreRunE already handles logging.
func executeAgentHook(cmd *cobra.Command, agentName types.AgentName, hookName string, initLogging bool) error {
// When enrichCtx is true, it enriches cmd.Context() with the discovered session ID
// (used by the RunE fallback since it doesn't go through PersistentPreRunE).
// Built-in agent subcommands pass false since their parent command's PersistentPreRunE
// already handles enrichment.
func executeAgentHook(cmd *cobra.Command, agentName types.AgentName, hookName string, enrichCtx bool) error {
// Skip silently if not in a git repository - hooks shouldn't prevent the agent from working
if _, err := paths.WorktreeRoot(cmd.Context()); err != nil {
return nil
Expand All @@ -104,9 +95,8 @@ func executeAgentHook(cmd *cobra.Command, agentName types.AgentName, hookName st
return nil
}

if initLogging {
cleanup := initHookLogging(cmd.Context())
defer cleanup()
if enrichCtx {
cmd.SetContext(enrichHookContext(cmd.Context()))
}

// Initialize logging context with agent name
Expand Down
79 changes: 25 additions & 54 deletions cmd/entire/cli/hook_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/entireio/cli/cmd/entire/cli/agent"
"github.com/entireio/cli/cmd/entire/cli/agent/claudecode"
"github.com/entireio/cli/cmd/entire/cli/logging"
"github.com/entireio/cli/cmd/entire/cli/logging/loggingtest"
"github.com/entireio/cli/cmd/entire/cli/paths"
"github.com/entireio/cli/cmd/entire/cli/session"
"github.com/entireio/cli/cmd/entire/cli/testutil"
Expand Down Expand Up @@ -82,9 +83,9 @@ func TestNewAgentHookVerbCmd_LogsInvocation(t *testing.T) {
// Enable debug logging
t.Setenv(logging.LogLevelEnvVar, "DEBUG")

// Initialize logging (normally done by PersistentPreRunE)
cleanup := initHookLogging(context.Background())
defer cleanup()
// Inject a buffer-backed logger via loggingtest to capture the hook's
// log output. In production main.go installs the logger via WithLogger.
ctx, buf := loggingtest.New(t)

// Create a transcript file for the hook input
transcriptPath := filepath.Join(tmpDir, "transcript.jsonl")
Expand All @@ -93,7 +94,7 @@ func TestNewAgentHookVerbCmd_LogsInvocation(t *testing.T) {
}

// Create stdin with session-start hook input
hookInput := map[string]interface{}{
hookInput := map[string]any{
"session_id": sessionID,
"transcript_path": transcriptPath,
}
Expand All @@ -102,7 +103,8 @@ func TestNewAgentHookVerbCmd_LogsInvocation(t *testing.T) {
// Create the command with logging - use session-start hook which is a pass-through
cmd := newAgentHookVerbCmdWithLogging(agent.AgentNameClaudeCode, claudecode.HookNameSessionStart)

// Set stdin
// Set stdin and the test ctx
cmd.SetContext(ctx)
cmd.SetIn(bytes.NewReader(inputJSON))
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
Expand All @@ -113,50 +115,25 @@ func TestNewAgentHookVerbCmd_LogsInvocation(t *testing.T) {
t.Fatalf("command execution failed: %v", err)
}

// Close logging to flush
cleanup()

// Verify log file was created and contains expected content
logFile := filepath.Join(logsDir, "entire.log")
content, err := os.ReadFile(logFile)
if err != nil {
t.Fatalf("failed to read log file: %v", err)
records := loggingtest.Records(t, buf)
if len(records) == 0 {
t.Fatal("expected at least one log record")
}

logContent := string(content)
t.Logf("log content: %s", logContent)

// Parse each log line as JSON
lines := strings.Split(strings.TrimSpace(logContent), "\n")
if len(lines) == 0 {
t.Fatal("expected at least one log line")
}

// Check for hook invocation log and perf span log
foundInvocation := false
foundPerfSpan := false
for _, line := range lines {
var entry map[string]interface{}
if err := json.Unmarshal([]byte(line), &entry); err != nil {
t.Errorf("failed to parse log line as JSON: %v", err)
continue
}

msg, msgOK := entry["msg"].(string)

// Hook invocation log: msg="hook invoked", hook="session-start"
if msgOK && entry["hook"] == claudecode.HookNameSessionStart && strings.Contains(msg, "invoked") {
for _, rec := range records {
// Hook invocation log: msg contains "invoked", hook="session-start"
if rec.Attrs["hook"] == claudecode.HookNameSessionStart && strings.Contains(rec.Msg, "invoked") {
foundInvocation = true
// Verify component is set
if entry["component"] != "hooks" {
t.Errorf("expected component='hooks', got %v", entry["component"])
if rec.Attrs["component"] != "hooks" {
t.Errorf("expected component='hooks', got %v", rec.Attrs["component"])
}
}

// Perf span log: msg="perf", op="session-start", duration_ms present
if msgOK && msg == "perf" && entry["op"] == claudecode.HookNameSessionStart {
if rec.Msg == "perf" && rec.Attrs["op"] == claudecode.HookNameSessionStart {
foundPerfSpan = true
if _, ok := entry["duration_ms"]; !ok {
if _, ok := rec.Attrs["duration_ms"]; !ok {
t.Error("expected duration_ms in perf span log")
}
}
Expand Down Expand Up @@ -188,14 +165,11 @@ func TestClaudeCodeHooksCmd_HasLoggingHooks(t *testing.T) {

require.NotNil(t, claudeCodeCmd, "expected to find claude-code subcommand under hooks")

// Verify PersistentPreRunE is set
// PersistentPreRunE enriches the cobra context with the discovered
// session ID. The logger and log file are owned by main.go, so no
// PersistentPostRunE cleanup is needed.
if claudeCodeCmd.PersistentPreRunE == nil {
t.Error("expected PersistentPreRunE to be set for logging initialization")
}

// Verify PersistentPostRunE is set
if claudeCodeCmd.PersistentPostRunE == nil {
t.Error("expected PersistentPostRunE to be set for logging cleanup")
t.Error("expected PersistentPreRunE to be set for ctx enrichment")
}
}

Expand All @@ -217,14 +191,11 @@ func TestGeminiCLIHooksCmd_HasLoggingHooks(t *testing.T) {

require.NotNil(t, geminiCmd, "expected to find gemini subcommand under hooks")

// Verify PersistentPreRunE is set
// PersistentPreRunE enriches the cobra context with the discovered
// session ID. The logger and log file are owned by main.go, so no
// PersistentPostRunE cleanup is needed.
if geminiCmd.PersistentPreRunE == nil {
t.Error("expected PersistentPreRunE to be set for logging initialization")
}

// Verify PersistentPostRunE is set
if geminiCmd.PersistentPostRunE == nil {
t.Error("expected PersistentPostRunE to be set for logging cleanup")
t.Error("expected PersistentPreRunE to be set for ctx enrichment")
}
}

Expand Down
Loading
Loading