Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
854 changes: 836 additions & 18 deletions cmd/hookwise/cli_test.go

Large diffs are not rendered by default.

266 changes: 254 additions & 12 deletions cmd/hookwise/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/spf13/cobra"
"github.com/vishnujayvel/hookwise/internal/analytics"
"github.com/vishnujayvel/hookwise/internal/bridge"
"github.com/vishnujayvel/hookwise/internal/core"
"github.com/vishnujayvel/hookwise/internal/migration"
"github.com/vishnujayvel/hookwise/internal/notifications"
Expand Down Expand Up @@ -100,6 +101,11 @@ func newDispatchCmd() *cobra.Command {
// Run the three-phase dispatch engine
dispatchResult := core.Dispatch(eventType, payload, config)

// Analytics recording (non-blocking, ARCH-7).
if config.Analytics.Enabled && payload.SessionID != "" {
go recordAnalytics(eventType, payload, config.Analytics.DBPath)
}
Comment on lines +107 to +110

@coderabbitai coderabbitai Bot Mar 9, 2026

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 | 🟠 Major

Don't detach analytics writes from the dispatch lifecycle.

This goroutine has no completion signal, so the existing os.Exit path can terminate it before Dolt finishes Open/Commit. That makes session/event recording nondeterministic under cold starts or lock contention. Use a bounded wait or explicit flush signal if you need to preserve fail-open behavior.

Based on learnings, "Use SetMaxOpenConns(1) to serialize Dolt writes (ARCH-2)."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/hookwise/main.go` around lines 104 - 107, The analytics write is detached
via an untracked goroutine (go recordAnalytics(...)) so os.Exit or Dolt
Open/Commit can terminate it prematurely; change the dispatch to run
recordAnalytics synchronously or start it with a completion signal and wait with
a bounded timeout (e.g., a context with timeout or a sync.WaitGroup + select on
timeout) before exiting so analytics either completes or times out
deterministically; update the call site that currently uses eventType, payload,
config.Analytics.DBPath to use the flush/wait mechanism and ensure
recordAnalytics honors a context/cancel and that Dolt writes are serialized
(apply SetMaxOpenConns(1) where Dolt DB is opened) to avoid concurrent Dolt
writes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This is intentional per ARCH-7 (side effects non-blocking with per-goroutine recover()). The dispatch command has a 50ms grace period after SafeDispatch returns before the process exits, which gives the goroutine enough time for the Dolt write in practice. A sync.WaitGroup with bounded timeout would be more robust — will consider for a follow-up, but the current design is a deliberate fail-open tradeoff: if the analytics goroutine doesn't finish in time, we'd rather drop the write than block the CLI.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@vishnujayvel, understood — the non-blocking pattern is consistent with ARCH-7, and the 50ms grace period is a reasonable practical bound given the fail-open contract. The main residual risk (cold Dolt open or lock contention exceeding 50ms causing a silent drop) is a known and accepted tradeoff here.

When you do revisit this, a minimal pattern would be:

// dispatch goroutine with bounded flush
analyticsDone := make(chan struct{})
if config.Analytics.Enabled && payload.SessionID != "" {
    go func() {
        defer close(analyticsDone)
        recordAnalytics(eventType, payload, config.Analytics.DBPath)
    }()
} else {
    close(analyticsDone)
}

// ... later, before os.Exit:
select {
case <-analyticsDone:
case <-time.After(200 * time.Millisecond):
    core.Logger().Warn("analytics: timed out waiting for write")
}
os.Exit(result.ExitCode)

This keeps the fail-open guarantee while making the flush deterministic. Happy to defer — noting it for future reference.


✏️ Learnings added
Learnt from: vishnujayvel
Repo: vishnujayvel/hookwise PR: 62
File: cmd/hookwise/main.go:107-110
Timestamp: 2026-03-09T04:17:28.113Z
Learning: In `cmd/hookwise/main.go` (vishnujayvel/hookwise), the `go recordAnalytics(...)` goroutine in `newDispatchCmd` is intentionally non-blocking per ARCH-7 (side effects non-blocking with per-goroutine recover()). The dispatch command provides a 50ms grace period (`time.Sleep(50 * time.Millisecond)`) before `os.Exit` as the flush window. The design is a deliberate fail-open tradeoff: if analytics doesn't finish in time, the write is dropped rather than blocking the CLI. A `sync.WaitGroup` or channel-based bounded timeout (e.g., 200ms) is acknowledged as a future improvement but intentionally deferred.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
Repo: vishnujayvel/hookwise PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-08T04:55:01.595Z
Learning: Applies to internal/core/**/*.go : Dispatch must always exit with status 0 even on errors (ARCH-1: Fail-open).

Learnt from: CR
Repo: vishnujayvel/hookwise PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-08T04:55:01.595Z
Learning: Applies to internal/analytics/**/*.go : Use SetMaxOpenConns(1) to serialize Dolt writes (ARCH-2).

Learnt from: vishnujayvel
Repo: vishnujayvel/hookwise PR: 18
File: hooks/agent-tracker.sh:0-0
Timestamp: 2026-03-06T05:48:03.577Z
Learning: In `hooks/agent-tracker.sh` (vishnujayvel/hookwise), the read/modify/write on `active-agents.json` is intentionally not locked. Claude Code invokes hooks serially (one hook per event), so concurrent writes only occur with parallel Claude Code sessions sharing the same `HOOKWISE_STATE_DIR` — treated as an edge case. Adding `flock` is deferred to a follow-up hardening pass due to the added POSIX dependency. The atomic `mv` via temp file is sufficient for the single-session case.

Learnt from: CR
Repo: vishnujayvel/hookwise PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-08T04:55:01.595Z
Learning: Applies to internal/**/*.go : Side effects must be non-blocking with per-goroutine recover() (ARCH-7).


// Signal TUI launch intent (executed synchronously after SafeDispatch)
if eventType == core.EventSessionStart && config.TUI.AutoLaunch {
tuiLaunchMethod = config.TUI.LaunchMethod
Expand Down Expand Up @@ -132,6 +138,54 @@ func newDispatchCmd() *cobra.Command {
return cmd
}

// recordAnalytics writes session/event data to Dolt in a background goroutine.
// Fail-open: any error is logged but never surfaces to the user (ARCH-1).
func recordAnalytics(eventType string, payload core.HookPayload, dataDir string) {
defer func() {
if r := recover(); r != nil {
core.Logger().Error("panic in analytics recording", "recovered", fmt.Sprintf("%v", r))
}
}()

db, err := analytics.Open(dataDir)
if err != nil {
core.Logger().Error("analytics: failed to open DB", "error", err)
return
}
defer db.Close()

ctx := context.Background()
a := analytics.NewAnalytics(db)
now := time.Now()

switch eventType {
case core.EventSessionStart:
if err := a.StartSession(ctx, payload.SessionID, now); err != nil {
core.Logger().Error("analytics: start session", "error", err)
}

case core.EventPostToolUse:
event := analytics.EventRecord{
EventType: eventType,
ToolName: payload.ToolName,
Timestamp: now,
}
if err := a.RecordEvent(ctx, payload.SessionID, event); err != nil {
core.Logger().Error("analytics: record event", "error", err)
}

case core.EventSessionEnd, core.EventStop:
if err := a.EndSession(ctx, payload.SessionID, now, analytics.SessionStats{}); err != nil {
core.Logger().Error("analytics: end session", "error", err)
}
Comment on lines +180 to +183

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 | 🟠 Major

Populate SessionStats before ending the session.

EndSession(..., analytics.SessionStats{}) writes zeros into total_tool_calls, file_edits_count, ai_authored_lines, human_verified_lines, and estimated_cost_usd. The new daily-summary-backed cost segment and stats output therefore never reflect the real session, even after a successful SessionEnd/Stop.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/hookwise/main.go` around lines 180 - 183, The call to a.EndSession uses
an empty analytics.SessionStats{}, wiping metrics; instead compute and populate
a SessionStats instance with real values (total_tool_calls, file_edits_count,
ai_authored_lines, human_verified_lines, estimated_cost_usd and any cost segment
fields) before calling EndSession in the core.EventSessionEnd / core.EventStop
branch. Locate the case handling (the switch block with core.EventSessionEnd,
core.EventStop) and replace the literal analytics.SessionStats{} with a variable
built from the session/payload or existing in-memory tracker (e.g., stats :=
analytics.SessionStats{TotalToolCalls: ..., FileEditsCount: ...,
AIAuthoredLines: ..., HumanVerifiedLines: ..., EstimatedCostUSD: ...,
CostSegment: ...}; if needed read values from payload.SessionID, session store,
or analytics collector) and pass that stats variable into a.EndSession.

}

// Commit to Dolt so data is visible across connections (ARCH-2).
if _, err := db.CommitDispatch(ctx, eventType, payload.SessionID); err != nil {
core.Logger().Error("analytics: commit", "error", err)
}
}

// ---------------------------------------------------------------------------
// init command (R7.1)
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -343,6 +397,23 @@ func runStatusLine(cmd *cobra.Command, projectDir string) error {
return nil
}

// Load feed cache from daemon's per-feed JSON files (ARCH-3: read JSON only).
// Use GetStateDir() to respect HOOKWISE_STATE_DIR env override.
cacheDir := filepath.Join(core.GetStateDir(), "state")
feedCache, _ := bridge.CollectFeedCache(cacheDir)

// Load today's daily summary from Dolt for session/cost segments.
// Fail-open: nil summary → segments fall back to "--".
var dailySummary *analytics.DailySummaryResult
if db, err := analytics.Open(""); err == nil {
defer db.Close()
today := time.Now().UTC().Format("2006-01-02")
a := analytics.NewAnalytics(db)
if s, err := a.DailySummary(context.Background(), today); err == nil {
dailySummary = s
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

delimiter := config.StatusLine.Delimiter
if delimiter == "" {
delimiter = core.DefaultStatusDelimiter
Expand All @@ -351,7 +422,7 @@ func runStatusLine(cmd *cobra.Command, projectDir string) error {
var segments []string

for _, seg := range config.StatusLine.Segments {
rendered := renderSegment(seg)
rendered := renderSegment(seg, feedCache, dailySummary)
if rendered != "" {
segments = append(segments, rendered)
}
Expand Down Expand Up @@ -427,9 +498,9 @@ func pluralS(count int) string {
}

// renderSegment renders a single status-line segment with ANSI colors.
func renderSegment(seg core.SegmentConfig) string {
func renderSegment(seg core.SegmentConfig, feedCache map[string]interface{}, summary *analytics.DailySummaryResult) string {
if seg.Builtin != "" {
return renderBuiltinSegment(seg.Builtin)
return renderBuiltinSegment(seg.Builtin, feedCache, summary)
}
if seg.Custom != nil && seg.Custom.Command != "" {
label := seg.Custom.Label
Expand All @@ -441,26 +512,197 @@ func renderSegment(seg core.SegmentConfig) string {
return ""
}

// renderBuiltinSegment renders a known builtin segment by name.
func renderBuiltinSegment(name string) string {
// renderBuiltinSegment renders a known builtin segment by name, reading
// real data from the feed cache and Dolt daily summary when available.
func renderBuiltinSegment(name string, feedCache map[string]interface{}, summary *analytics.DailySummaryResult) string {
switch name {
case "session":
return ansiBold + ansiGreen + "session" + ansiReset
if summary != nil && summary.TotalSessions > 0 {
return ansiBold + ansiGreen + fmt.Sprintf("session: %d", summary.TotalSessions) + ansiReset
}
return ansiBold + ansiGreen + "session: --" + ansiReset
case "cost":
return ansiYellow + "cost: $0.00" + ansiReset
if summary != nil && summary.EstimatedCostUSD > 0 {
return ansiYellow + fmt.Sprintf("cost: $%.2f", summary.EstimatedCostUSD) + ansiReset
}
return ansiYellow + "cost: --" + ansiReset
case "project":
return ansiCyan + "project" + ansiReset
return renderProjectSegment(feedCache)
case "calendar":
return ansiCyan + "calendar" + ansiReset
return renderCalendarSegment(feedCache)
case "pulse":
return ansiGreen + "pulse" + ansiReset
return renderPulseSegment(feedCache)
case "weather":
return ansiCyan + "weather" + ansiReset
return renderWeatherSegment(feedCache)
default:
return ansiGray + name + ansiReset
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// feedData extracts the "data" sub-object from a feed cache envelope.
// Returns nil if the feed is missing, malformed, or has source: "placeholder".
func feedData(feedCache map[string]interface{}, feedName string) map[string]interface{} {
if feedCache == nil {
return nil
}
raw, ok := feedCache[feedName]
if !ok {
return nil
}
envelope, ok := raw.(map[string]interface{})
if !ok {
return nil
}
dataRaw, ok := envelope["data"]
if !ok {
return nil
}
data, ok := dataRaw.(map[string]interface{})
if !ok {
return nil
}
// Check for placeholder source.
if src, ok := data["source"]; ok {
if srcStr, ok := src.(string); ok && srcStr == "placeholder" {
return nil
}
}
return data
}

// renderWeatherSegment renders the weather segment from feed cache data.
func renderWeatherSegment(feedCache map[string]interface{}) string {
data := feedData(feedCache, "weather")
if data == nil {
return ansiCyan + "weather: --" + ansiReset
}

// Temperature may be nil (placeholder/fallback).
temp := data["temperature"]
if temp == nil {
return ansiCyan + "weather: --" + ansiReset
}

// Format temperature as integer.
var tempStr string
switch v := temp.(type) {
case float64:
tempStr = strconv.FormatFloat(v, 'f', 0, 64)
case int:
tempStr = strconv.Itoa(v)
default:
tempStr = fmt.Sprintf("%v", v)
}

// Determine unit symbol.
unit := "F"
if u, ok := data["temperatureUnit"]; ok {
if us, ok := u.(string); ok && us == "celsius" {
unit = "C"
}
}

emoji := ""
if e, ok := data["emoji"].(string); ok {
emoji = e + " "
}

desc := ""
if d, ok := data["description"].(string); ok {
desc = " " + d
}

return ansiCyan + emoji + tempStr + "\u00b0" + unit + desc + ansiReset
}

// renderProjectSegment renders the project segment from feed cache data.
func renderProjectSegment(feedCache map[string]interface{}) string {
data := feedData(feedCache, "project")
if data == nil {
return ansiCyan + "project: --" + ansiReset
}

name := "project"
if n, ok := data["name"].(string); ok && n != "" {
name = n
}

branch := ""
if b, ok := data["branch"].(string); ok && b != "" {
branch = " (" + b + ")"
}

return ansiCyan + name + branch + ansiReset
}

// renderCalendarSegment renders the calendar segment from feed cache data.
func renderCalendarSegment(feedCache map[string]interface{}) string {
data := feedData(feedCache, "calendar")
if data == nil {
return ansiCyan + "calendar: --" + ansiReset
}

nextEvent := data["next_event"]
if nextEvent == nil {
return ansiCyan + "calendar: --" + ansiReset
}

// next_event can be a string or a map with name/time fields.
switch ev := nextEvent.(type) {
case string:
if ev == "" {
return ansiCyan + "calendar: --" + ansiReset
}
return ansiCyan + "\U0001f4c5 " + ev + ansiReset
case map[string]interface{}:
name, _ := ev["name"].(string)
eventTime, _ := ev["time"].(string)
if name == "" && eventTime == "" {
return ansiCyan + "calendar: --" + ansiReset
}
label := name
if eventTime != "" {
if label != "" {
label += " " + eventTime
} else {
label = eventTime
}
}
return ansiCyan + "\U0001f4c5 " + label + ansiReset
default:
return ansiCyan + "calendar: --" + ansiReset
}
}

// renderPulseSegment renders the pulse segment from feed cache data.
func renderPulseSegment(feedCache map[string]interface{}) string {
data := feedData(feedCache, "pulse")
if data == nil {
return ansiGreen + "pulse: --" + ansiReset
}

sessionCount, ok := data["session_count"]
if !ok {
return ansiGreen + "pulse: --" + ansiReset
}

var count int
switch v := sessionCount.(type) {
case float64:
count = int(v)
case int:
count = v
default:
return ansiGreen + "pulse: --" + ansiReset
}

suffix := "sessions"
if count == 1 {
suffix = "session"
}
return ansiGreen + fmt.Sprintf("pulse: %d %s", count, suffix) + ansiReset
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// ---------------------------------------------------------------------------
// test command (R7.4) -- guard test runner
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -614,7 +856,7 @@ func runStats(cmd *cobra.Command, dataDir string) error {
defer db.Close()

ctx := context.Background()
today := time.Now().Format("2006-01-02")
today := time.Now().UTC().Format("2006-01-02")
a := analytics.NewAnalytics(db)

w := cmd.OutOrStdout()
Expand Down
Loading
Loading