Skip to content

Split Git-aware daemon cursor from its baseline#80

Open
maxkle1nz wants to merge 1 commit into
mainfrom
codex/m1nd-daemon-git-cursor-v2
Open

Split Git-aware daemon cursor from its baseline#80
maxkle1nz wants to merge 1 commit into
mainfrom
codex/m1nd-daemon-git-cursor-v2

Conversation

@maxkle1nz
Copy link
Copy Markdown
Owner

Summary

  • split the Git-aware daemon state into startup baseline, moving cursor, current HEAD, and last clean ref
  • keep git_baseline_ref stable while git_since_ref advances after successful reconciliation
  • expose git_head_ref and git_last_clean_ref explicitly for status/debugging

Validation

  • cargo fmt --check
  • cargo test -p m1nd-mcp daemon_start_detects_git_root_and_head -- --nocapture
  • cargo test -p m1nd-mcp daemon_start_prefers_merge_base_when_upstream_exists -- --nocapture
  • cargo test -p m1nd-mcp daemon_tick_uses_git_changed_set_when_available -- --nocapture
  • MCP smoke for git_baseline_ref / git_since_ref / git_head_ref / git_last_clean_ref

Why this matters

This makes the SCM-aware daemon state explicit and future-proof. The daemon now has a stable baseline and a moving cursor, which is the right substrate for later since/clock and Watchman-style semantics.

The SCM-aware daemon state now separates immutable startup baseline, moving reconciliation cursor, current HEAD, and last clean ref. This makes the adapter state explicit and gives us the right substrate for future since/clock semantics without changing the public MCP tool surface.

Constraint: Preserve the current daemon contract while making SCM state explicit enough for later Watchman-style evolution
Rejected: Reusing a single git_since_ref for baseline, cursor, and head | too ambiguous for future SCM-aware behavior
Rejected: Adding a dedicated cursor-management tool | premature public surface growth for adapter-internal state
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep baseline immutable, cursor movable, and head observational; later SCM backends should map onto these roles instead of collapsing them again
Tested: cargo fmt --check; cargo test -p m1nd-mcp daemon_start_detects_git_root_and_head -- --nocapture; cargo test -p m1nd-mcp daemon_start_prefers_merge_base_when_upstream_exists -- --nocapture; cargo test -p m1nd-mcp daemon_tick_uses_git_changed_set_when_available -- --nocapture; MCP smoke for baseline/head/cursor fields
Not-tested: Long-running daemon sessions with upstream movement after startup
Copilot AI review requested due to automatic review settings April 5, 2026 22:39
@maxkle1nz maxkle1nz enabled auto-merge (squash) April 5, 2026 22:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes the daemon’s Git-aware runtime state more explicit by splitting the stable startup baseline from the advancing cursor, and by exposing additional refs for status/debugging.

Changes:

  • Adds git_head_ref and git_last_clean_ref to the persisted DaemonRuntimeState.
  • Updates daemon start/status JSON outputs to include the new Git refs.
  • Updates tick logic to record current HEAD and advance the “since” cursor after successful Git diff scanning.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
m1nd-mcp/src/session.rs Extends persisted daemon runtime state with git_head_ref / git_last_clean_ref.
m1nd-mcp/src/daemon_handlers.rs Populates/exposes the new refs during daemon start/status, and updates tick behavior to track HEAD and advance refs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +523 to +527
state.daemon_state.git_head_ref = current_head.clone();
state.daemon_state.git_since_ref = current_head
.clone()
.or(state.daemon_state.git_since_ref.clone());
state.daemon_state.git_last_clean_ref = current_head;
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

git_since_ref/git_last_clean_ref are advanced immediately after the git diff succeeds, before any reconciliation/ingest work runs. If handle_ingest(...) later returns an error (it’s ? inside the loop), the tick exits early but the in-memory cursor has already moved, so the next tick may diff from the new ref and miss changes that were never ingested. Consider only updating these refs after the tick has successfully completed reconciliation (e.g., after the ingest loop and before persisting state), or staging the new refs in locals and committing them at the end on success.

Suggested change
state.daemon_state.git_head_ref = current_head.clone();
state.daemon_state.git_since_ref = current_head
.clone()
.or(state.daemon_state.git_since_ref.clone());
state.daemon_state.git_last_clean_ref = current_head;
state.daemon_state.git_head_ref = current_head;

Copilot uses AI. Check for mistakes.
Comment on lines +332 to +336
let git_head_ref = state
.daemon_state
.git_root
.as_deref()
.and_then(|root| git_head_ref(Path::new(root)));
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

In this scope, let git_head_ref = ... git_head_ref(...) shadows the git_head_ref(...) function name, which makes the code harder to read and prevents calling the function again later in the same scope. Consider renaming the local to something like head_ref/current_head_ref to avoid the identifier collision.

Copilot uses AI. Check for mistakes.
Comment on lines +523 to +527
state.daemon_state.git_head_ref = current_head.clone();
state.daemon_state.git_since_ref = current_head
.clone()
.or(state.daemon_state.git_since_ref.clone());
state.daemon_state.git_last_clean_ref = current_head;
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

git_last_clean_ref (and git_head_ref) are overwritten with current_head, which can be None if git rev-parse HEAD fails transiently. That would erase previously known values and make status/debug output less stable. Consider only updating these fields when current_head.is_some(), otherwise preserving the prior value (and possibly recording the failure in git_backend_error).

Suggested change
state.daemon_state.git_head_ref = current_head.clone();
state.daemon_state.git_since_ref = current_head
.clone()
.or(state.daemon_state.git_since_ref.clone());
state.daemon_state.git_last_clean_ref = current_head;
state.daemon_state.git_head_ref = current_head
.clone()
.or(state.daemon_state.git_head_ref.clone());
state.daemon_state.git_since_ref = current_head
.clone()
.or(state.daemon_state.git_since_ref.clone());
state.daemon_state.git_last_clean_ref = current_head
.or(state.daemon_state.git_last_clean_ref.clone());

Copilot uses AI. Check for mistakes.
Comment on lines +523 to +527
state.daemon_state.git_head_ref = current_head.clone();
state.daemon_state.git_since_ref = current_head
.clone()
.or(state.daemon_state.git_since_ref.clone());
state.daemon_state.git_last_clean_ref = current_head;
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

There are tests for successful start/tick paths, but there isn’t coverage for the failure case where reconciliation/ingest errors out. Given the new cursor semantics (git_since_ref/git_last_clean_ref), it would be valuable to add a test asserting these refs do not advance when handle_daemon_tick returns an error during ingest, to prevent silently skipping changes on the next tick.

Suggested change
state.daemon_state.git_head_ref = current_head.clone();
state.daemon_state.git_since_ref = current_head
.clone()
.or(state.daemon_state.git_since_ref.clone());
state.daemon_state.git_last_clean_ref = current_head;
state.daemon_state.git_head_ref = current_head;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants