fix(attribution): stop cross-repo Cursor session borrowing, backfill known model - #2298
Closed
qqzhangyanhua wants to merge 4 commits into
Closed
Conversation
…known model Two related mis-attribution bugs in session-event recovery: 1. select_latest_commit_metadata_metric_session had no time window and treated candidates with no repo_url as usable. On multi-repo machines this let a Co-authored-by: Cursor trailer fall back to whatever Cursor session last ran anywhere on the machine, including sessions from a completely unrelated repo. Now the latest-tool fallback only accepts an exact repo_url match; when no same-repo session exists it synthesizes a session instead of borrowing cross-repo. 2. Cursor session_event rows never carry a model. Recovery previously always wrote "unknown" into the recovered checkpoint metric even when the same session already had a real model on record from other checkpointed lines in the same commit. recovery_model_for_session now prefers that already-known model before falling back to the raw candidate's (usually absent) model. Also teaches CursorAgent::infer_cwd to recover the git worktree root from transcript tool-use file paths (path/file_path/target_directory), so future Cursor session_event rows are more likely to carry repo_url and avoid the unscoped fallback path entirely. Only affects attribution recovery for future commits.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…2298 1. latest_session_event_candidates_for_tools applied a global LIMIT 100 (ordered by event_ts DESC) before repo_url filtering happened in Rust. On a busy machine, 100+ newer session events for other/unscoped repos for the same tool would hide a genuinely matching older same-repo session, causing recovery to synthesize a session instead of finding the real one. Replaced with latest_session_event_candidate_for_repo, which pushes the exact repo_url check into the row scan itself (still ordered by event_ts DESC, id DESC) and returns on the first match, with no LIMIT truncating the search first. 2. CursorAgent::infer_cwd_from_transcript returned the first worktree root found among tool-use paths, silently mislabeling the whole session (and therefore every other event in it) if a conversation touched more than one repository. It now fails closed (returns None) when scanned tool paths resolve to more than one distinct worktree root. Co-authored-by: Cursor <cursoragent@cursor.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin flagged two follow-up issues from the previous fix: - latest_session_event_candidate_for_repo removed LIMIT entirely, so a non-matching lookup would scan every retained session event for a tool (up to a year of history) while holding the metrics DB lock, blocking telemetry and concurrent recovery. Re-added a LIMIT, now much larger (1000 vs the old 100) to keep the original fix's benefit while bounding worst-case lock hold time. - infer_cwd_from_transcript only scanned the first 40 lines, so a repository switch after that prefix was invisible to the fail-closed multi-repo check and would still mislabel the session. Widened the scan window to 1000 lines. Both bounds are documented trade-offs (predictable cost vs a rare, accepted miss), with regression tests covering the boundary. Co-authored-by: Cursor <cursoragent@cursor.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin correctly pointed out that any row LIMIT (100, then 1000) can always be defeated by enough newer foreign/unscoped events for the same tool, hiding a genuinely matching older same-repo session. Following the same precedent already used for session_id/tool/trace_id etc. (schema migration + backfill_event_metadata for legacy rows): - Added a migration (schema v5 -> v6) that adds a repo_url column and a partial index on (repo_url, tool, event_kind, event_ts DESC, id DESC). - repo_url is now extracted and cached at insert time (extract_metric_event_metadata) and backfilled for legacy rows (backfill_event_metadata_batch_after), mirroring the existing columns. - latest_session_event_candidate_for_repo now filters with 'repo_url = ?' directly in SQL and takes LIMIT 1, so the match is found via an index seek regardless of how much unrelated history exists, instead of scanning rows in Rust under the metrics DB lock. Replaced the scan-bound test with test_latest_session_event_candidate_for_repo_finds_match_past_a_thousand, which asserts the older same-repo session IS found past 1,500 foreign events (previously an accepted miss). Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
Comment on lines
+384
to
+390
| if from_version == 5 { | ||
| self.add_column_if_missing( | ||
| "metrics", | ||
| "repo_url", | ||
| "ALTER TABLE metrics ADD COLUMN repo_url TEXT DEFAULT NULL", | ||
| )?; | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 Completed backfills skip repository data
For databases already backfilled, migration 6 adds repo_url without resetting the completion marker. Existing events remain unsearchable by repository.
Prompt for agents
Migration 5 -> 6 adds metrics.repo_url, but existing installations can already have event_metadata_backfill_completed=1 from the earlier metadata backfill. The daemon then skips backfill_event_metadata_batch_once entirely, while the new latest_session_event_candidate_for_repo query requires the cached repo_url column. Introduce a distinct completion state for the repo_url backfill, or reset/version the existing marker during migration 5 -> 6. Ensure the backfill scans legacy session-event rows even when event_ts and event_kind are already populated, without repeatedly scanning rows whose source event genuinely has no repository URL.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+77
to
+80
| for line in reader | ||
| .lines() | ||
| .take(CURSOR_INFER_CWD_LINE_LIMIT) | ||
| .map_while(Result::ok) |
Contributor
There was a problem hiding this comment.
🔴 Later repository switches inherit stale identity
When processing precedes a later repository switch, infer_cwd_from_transcript caches only the first repository. Subsequent events receive its identity.
Prompt for agents
Cursor transcript inference is used in StreamWorker::process_session_blocking only when task and stored repo_work_dir are absent. The first inferred root is persisted by update_repo_work_dir, so future processing prefers that cached root and never re-runs inference after the transcript grows. A Cursor conversation that later switches repositories therefore labels subsequent session events with the original repository URL. Rework inferred repository state so transcript growth revalidates it before assigning batch attributes. A multi-repository transcript must fail closed rather than retain an earlier cached root. Add a test that processes a one-repository prefix, appends an event from a second repository, and processes the stream again.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two related mis-attribution bugs found via real repo data:
Co-authored-by: Cursortrailer and no session event within the 3s window fell back toselect_latest_commit_metadata_metric_session, which had no time window and treated candidates with a missingrepo_urlas usable. On a multi-repo machine this attached whatever Cursor session last ran anywhere on the machine — in the observed case, a session from reviewing an unrelated PR in a different repo the night before got attached to an IMS frontend commit.+2lines) session-event-recovered hunk had the correct session attached, but the recovered checkpoint metric recordedmodel: "unknown"because Cursor'ssession_eventrows never carry model info, and recovery only looked at that one event instead of reusing the model already on record for that same session from other checkpointed lines in the same commit.Root cause: Cursor doesn't push a working directory, so its
session_eventrows are frequently missingrepo_url, making the unscoped cross-repo fallback path close to default behavior on multi-repo machines.Fix
select_latest_commit_metadata_metric_sessionnow only accepts an exactrepo_urlmatch. If no same-repo session exists, it falls through to the existing synthesized-session fallback instead of borrowing a cross-repo/unscoped session. (The time-windowedselect_nearest_...path is untouched — its 3s window already makes an unscoped candidate low-risk.)recovery_model_for_sessionbackfills the recovered checkpoint metric'smodelfrom the session's already-known model (if any) before falling back to the session-event candidate's own (usually absent) model.CursorAgent::infer_cwdnow infers the git worktree root from transcript tool-use file paths (path/file_path/target_directory), so future Cursorsession_eventrows are more likely to carryrepo_urland avoid the unscoped fallback path entirely.Only affects attribution recovery for future commits; already-written notes are not rewritten.
Tests
recovery_model_for_session_reuses_known_model,recovery_model_for_session_falls_back_when_model_is_unknown_or_missing(attribution_recovery.rs)test_infer_cwd_from_transcript_tool_path,test_infer_cwd_none_without_repo_paths(cursor.rs)test_commit_metadata_recovery_does_not_attach_unscoped_latest_tool_session,test_commit_metadata_recovery_latest_ignores_newer_unscoped_session,test_commit_metadata_recovery_does_not_use_unscoped_session_from_another_project,test_cursor_infer_cwd_from_tool_path(covers same-machine stale unscoped session, same-machine newer unscoped session, and cross-project unscoped session)Verification
cargo fmt/cargo clippy --all-targets -- -D warnings: cleancargo test --lib(attribution_recovery + cursor agent): 102 passedcargo test --test integration session_event: 50 passedattribution/recovery/cursorfilters): 565 passed, 0 failedMade with Cursor