Skip to content
Open
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
43 changes: 42 additions & 1 deletion src/commands/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,17 @@ use crate::authorship::ignore::{
use crate::authorship::stats::{CommitStats, stats_from_authorship_log, write_stats_to_terminal};
use crate::authorship::virtual_attribution::VirtualAttributions;
use crate::authorship::working_log::CheckpointKind;
use crate::daemon::control_api::ControlRequest;
use crate::error::GitAiError;
use crate::git::find_repository;
use crate::git::repo_storage::InitialAttributions;
use crate::git::repository::{InternalGitProfile, Repository, exec_git_with_profile};
use crate::git::status::MAX_PATHSPEC_ARGS;
use serde::Serialize;
use std::collections::{BTreeMap, HashSet};
use std::time::{SystemTime, UNIX_EPOCH};
use std::time::{Duration, SystemTime, UNIX_EPOCH};

const DAEMON_STATUS_TIMEOUT: Duration = Duration::from_millis(250);

#[derive(Serialize)]
struct CheckpointInfo {
Expand All @@ -25,6 +28,7 @@ struct CheckpointInfo {
#[derive(Serialize)]
struct StatusOutput {
stats: CommitStats,
checkpoint_processing_pending: bool,
/// Per-checkpoint session breakdown. Omitted entirely when `--diff-only`
/// is requested, so consumers that only care about the current diff scope
/// get just the diff-scoped `stats`.
Expand Down Expand Up @@ -54,6 +58,7 @@ pub fn handle_status(args: &[String]) {

fn run_status(json: bool, diff_only: bool) -> Result<(), GitAiError> {
let repo = find_repository(&[])?;
let checkpoint_processing_pending = checkpoint_processing_pending(&repo);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Pending-work warning can be missed for work that arrives while status is being computed

Whether background processing is still pending is checked (checkpoint_processing_pending(&repo) at src/commands/status.rs:61) before the attribution data is read rather than after, so work that starts during the read is displayed without any warning.
Impact: Users can occasionally see incomplete attribution numbers with no indication that more processing was still happening.

Ordering of the pending probe relative to the working-log read

The probe happens first at src/commands/status.rs:61, while the working log and checkpoints are read afterwards at src/commands/status.rs:70-72 and the diff stats later still. If a checkpoint is accepted by the daemon between the probe and the reads, the printed status reflects a partially-processed state but checkpoint_processing_pending is false. Probing after all reads closes that window: the worst case then becomes a harmless false-positive warning (work that finished during the read) instead of a silently incomplete report.

Prompt for agents
In run_status (src/commands/status.rs), checkpoint_processing_pending(&repo) is evaluated before the working log, checkpoints and diff stats are read. A checkpoint admitted by the daemon during those reads yields incomplete output with no warning. Move the pending probe so it runs after the data used for the output has been gathered (both in the early-return empty branch and in the main path), so the flag conservatively reports pending work that overlapped the read.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

let ignore_patterns = effective_ignore_patterns(&repo, &[], &[]);
let ignore_matcher = build_ignore_matcher(&ignore_patterns);

Expand All @@ -73,10 +78,13 @@ fn run_status(json: bool, diff_only: bool) -> Result<(), GitAiError> {
if json {
let output = StatusOutput {
stats: CommitStats::default(),
checkpoint_processing_pending,
checkpoints: if diff_only { None } else { Some(vec![]) },
};
let json_str = serde_json::to_string(&output)?;
println!("{}", json_str);
} else if checkpoint_processing_pending {
print_checkpoint_processing_pending();
} else {
eprintln!(
"No checkpoints recorded since last commit ({})",
Expand Down Expand Up @@ -167,6 +175,7 @@ fn run_status(json: bool, diff_only: bool) -> Result<(), GitAiError> {
if json {
let output = StatusOutput {
stats,
checkpoint_processing_pending,
checkpoints: if diff_only {
None
} else {
Expand All @@ -178,6 +187,9 @@ fn run_status(json: bool, diff_only: bool) -> Result<(), GitAiError> {
return Ok(());
}

if checkpoint_processing_pending {
print_checkpoint_processing_pending();
}
write_stats_to_terminal(&stats, true);

if diff_only {
Expand Down Expand Up @@ -212,6 +224,35 @@ fn run_status(json: bool, diff_only: bool) -> Result<(), GitAiError> {
Ok(())
}

fn checkpoint_processing_pending(repo: &Repository) -> bool {
let Ok(config) = crate::daemon::DaemonConfig::from_env_or_default_paths() else {
return false;
};
let request = ControlRequest::StatusFamily {
repo_working_dir: repo.path().to_string_lossy().to_string(),
};
let Ok(response) = crate::daemon::send_control_request_with_timeout(
&config.control_socket_path,
&request,
DAEMON_STATUS_TIMEOUT,
) else {
return false;
};

response.ok
&& response
.data
.as_ref()
.and_then(|data| data.get("pending_checkpoints"))
.and_then(serde_json::Value::as_u64)
.is_some_and(|count| count > 0)
}

fn print_checkpoint_processing_pending() {
eprintln!("Checkpoint processing is still in progress. Status may be incomplete.");
eprintln!();
}

fn format_time_ago(timestamp: u64) -> String {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
Expand Down
1 change: 1 addition & 0 deletions src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6544,6 +6544,7 @@ impl ActorDaemonCoordinator {
last_error: status
.last_error
.or_else(|| self.latest_side_effect_error(&family_key).ok().flatten()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Status warns about pending work even when the pending work belongs to a different repository

The pending-work count reported for a specific repository is actually the background service's global count of unfinished checkpoints across every repository (self.outstanding_checkpoint_state().0 at src/daemon.rs:6546), so a repository with nothing in flight can still be reported as busy.
Impact: Users can see a "processing still in progress, status may be incomplete" warning in a repository that has no pending work, simply because another repository on the machine is being processed.

Global ingress quota vs. family-scoped response field

FamilyStatus is a per-family (per-repo) response (src/daemon/control_api.rs:139-146), but outstanding_checkpoint_state() returns self.checkpoint_ingress_quota.outstanding() (src/daemon.rs:6788-6790), which is a single process-wide counter incremented in the checkpoint receive loop (src/daemon.rs:7404) and released on reservation drop (src/daemon.rs:225). It has no family/repo dimension. The production daemon is shared system-wide, so git-ai status in repo A reports checkpoint_processing_pending = true while repo B's checkpoints are still queued (src/commands/status.rs:242-249).

A family-scoped count would need to be derived from per-family sequencer state (e.g. counting FamilySequencerEntry::Checkpoint entries for the resolved family) rather than the global quota.

Prompt for agents
status_for_family in src/daemon.rs returns pending_checkpoints from self.outstanding_checkpoint_state(), which delegates to the process-wide checkpoint_ingress_quota (src/daemon.rs:6788). Because the daemon is shared across all repositories, a StatusFamily response for repo A reports checkpoints that are queued for repo B, causing git-ai status (src/commands/status.rs) to print a spurious 'processing still in progress' warning. Consider deriving a family-scoped pending count, e.g. by counting outstanding FamilySequencerEntry::Checkpoint entries (and in-flight checkpoint side effects) for the resolved family key, and use that for the family status response.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

pending_checkpoints: self.outstanding_checkpoint_state().0,
})
}

Expand Down
2 changes: 2 additions & 0 deletions src/daemon/control_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ pub struct FamilyStatus {
pub family_key: String,
pub latest_seq: u64,
pub last_error: Option<String>,
#[serde(default)]
pub pending_checkpoints: usize,
}

/// A telemetry envelope sent from client to daemon.
Expand Down
84 changes: 84 additions & 0 deletions tests/integration/status_unit.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
use crate::repos::test_repo::{DaemonTestScope, TestRepo};
use git_ai::authorship::stats::CommitStats;
use git_ai::authorship::working_log::{AgentId, CheckpointKind};
use git_ai::commands::checkpoint_agent::orchestrator::{
BaseCommit, CheckpointFile, CheckpointRequest,
};
use git_ai::daemon::checkpoint::PreparedPathRole;
use git_ai::daemon::send_checkpoint_request_with_timeout;
use serde::Deserialize;
use std::fs;
use std::path::PathBuf;
use std::time::{Duration, Instant};

#[derive(Debug, Deserialize)]
#[allow(dead_code)]
struct StatusOutput {
stats: CommitStats,
checkpoints: Vec<serde_json::Value>,
checkpoint_processing_pending: bool,
}

fn extract_json_object(output: &str) -> String {
Expand Down Expand Up @@ -61,6 +70,81 @@ fn test_status_remains_available_when_daemon_is_not_running() {
status.checkpoints.is_empty(),
"offline status should preserve the empty-checkpoint result"
);
assert!(
!status.checkpoint_processing_pending,
"an unavailable daemon should not break status or report a pending checkpoint"
);
}

#[test]
fn test_status_reports_pending_checkpoint_without_waiting_for_daemon_sync() {
let repo = TestRepo::new_with_daemon_env(&[(
"GIT_AI_TEST_DELAY_CHECKPOINT_SIDE_EFFECT",
"status-pending-checkpoint=2000",
)]);
write_file(&repo, "pending.txt", "committed\n");
repo.git_og(&["add", "pending.txt"]).unwrap();
repo.git_og(&["commit", "-m", "initial"]).unwrap();
let base_commit = repo
.git_og(&["rev-parse", "HEAD"])
.unwrap()
.trim()
.to_string();

write_file(&repo, "pending.txt", "committed\nAI edit\n");
let request = CheckpointRequest {
trace_id: "status-pending-checkpoint".to_string(),
checkpoint_kind: CheckpointKind::AiAgent,
agent_id: Some(AgentId {
tool: "mock_ai".to_string(),
id: "status-pending-session".to_string(),
model: "test".to_string(),
}),
files: vec![CheckpointFile {
path: PathBuf::from("pending.txt"),
content: Some("committed\nAI edit\n".to_string()),
repo_work_dir: repo.path().to_path_buf(),
base_commit: BaseCommit::Sha(base_commit),
}],
path_role: PreparedPathRole::Edited,
stream_source: None,
metadata: Default::default(),
};
let response = send_checkpoint_request_with_timeout(
&repo.daemon_control_socket_path(),
&request,
Duration::from_millis(500),
)
.expect("checkpoint should receive an asynchronous receipt");
assert!(
response.ok,
"checkpoint receipt should succeed: {response:?}"
);

let text = repo
.git_ai_without_pre_sync_for_test(&["status"])
.expect("text status should succeed while checkpoint processing is pending");
assert!(
text.contains("Checkpoint processing is still in progress. Status may be incomplete."),
"text status should warn that its results may be incomplete: {text}"
);

let started = Instant::now();
let raw = repo
.git_ai_without_pre_sync_for_test(&["status", "--json"])
.expect("status should succeed while checkpoint processing is pending");
let elapsed = started.elapsed();
let status: StatusOutput =
serde_json::from_str(&extract_json_object(&raw)).expect("valid status JSON");

assert!(
status.checkpoint_processing_pending,
"status should report the accepted checkpoint as pending"
);
assert!(
elapsed < Duration::from_secs(1),
"status should not wait for the delayed checkpoint side effect: {elapsed:?}"
);
}

/// Migrated from src/commands/status.rs test_get_working_dir_diff_stats_post_filter_equivalence
Expand Down