Skip to content

chore: strip posthog for sessions/models/daily only #7079

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
michaelneale merged 1 commit into from
Feb 9, 2026
Merged

chore: strip posthog for sessions/models/daily only #7079

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
14 changes: 14 additions & 0 deletions crates/goose/src/posthog.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,11 @@ pub fn emit_error_with_context(error_type: &str, context: ErrorContext) {
return;
}

// Temporarily disabled - only session_started events are sent
let _ = (&error_type, &context);
return;
Comment on lines +247 to +249
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The PR title/description says “daily active user ping only”, but this change still sends session_started on every session start (not once per day); either update the PR wording or add a daily throttle (e.g., persist last-sent date) to match the intended behavior.

Copilot uses AI. Check for mistakes.

#[allow(unreachable_code)]
Comment on lines +247 to +251
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

This early return leaves a permanently unreachable block (suppressed via #[allow(unreachable_code)]), which is easy to forget/rot; prefer removing the dead code or gating it behind an explicit runtime/feature flag (so the implementation is either exercised or not compiled) instead of relying on unreachable code + dummy _ uses.

Copilot uses AI. Check for mistakes.
let installation = load_or_create_installation();
let error_type = error_type.to_string();

Expand All @@ -257,6 +262,10 @@ pub fn emit_custom_slash_command_used() {
return;
}

// Temporarily disabled - only session_started events are sent
return;

#[allow(unreachable_code)]
let installation = load_or_create_installation();
Comment on lines +265 to 269
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Same issue here: returning early and then keeping the old implementation behind #[allow(unreachable_code)] makes the codebase harder to maintain; consider deleting the unreachable block or guarding the send behavior via a feature/env/config flag instead.

Copilot uses AI. Check for mistakes.

tokio::spawn(async move {
Expand Down Expand Up @@ -533,6 +542,11 @@ pub async fn emit_event(
return Ok(());
}

// Temporarily disabled - only session_started events are sent
let _ = (event_name, &mut properties);
return Ok(());

#[allow(unreachable_code)]
Comment on lines +545 to +549
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

This no-op implementation relies on an early return Ok(()) plus an unreachable block; prefer a single explicit gate (feature/env/config) that either skips sending cleanly or compiles out the rest, and avoid borrowing &mut properties solely to satisfy unused_mut/unused-variable warnings.

Suggested change
// Temporarily disabled - only session_started events are sent
let _ = (event_name, &mut properties);
return Ok(());
#[allow(unreachable_code)]

Copilot uses AI. Check for mistakes.
let installation = load_or_create_installation();
let client = posthog_rs::client(POSTHOG_API_KEY).await;
let mut event = posthog_rs::Event::new(event_name, &installation.installation_id);
Expand Down
Loading