Resolve each session's repository before uploading it for backfill - #260
Resolve each session's repository before uploading it for backfill#260gowshik450526511 wants to merge 1 commit into
Conversation
A replayed session lands with no repository, so backfilled work cannot be attributed to a repo at all. The companion change in ai-gateway-data recovers the working directory from the transcript, but a project is not in the transcript: it comes from git's origin remote, and by the time a session is replayed the backend is nowhere near the checkout. So the resolution has to happen here, while the code is still standing on the machine where the repos exist. Each session payload gains a `projects` map of cwd -> "org/repo", which coding_tools_backfill_service stamps onto the parsed records. _backfill_remote_path mirrors unbound.py::_github_remote_path rather than matching on github.com, so a self-hosted Enterprise remote resolves the same way. Output is lowercased "<org>/<repo>", the shape _get_project already emits, so a replayed row is comparable to a live one rather than merely similar. One git call per distinct checkout per run, cached across every transcript -- including the ones that fail to resolve, so a deleted directory is not retried once per session. A 5,000-session backfill costs a handful of git invocations. Applied to all five uploaders. The cwd lookup checks the payload / data nesting Codex and Copilot use, matching the server's _entry_cwd. Nothing is required of the server: a backend that does not read `projects` ignores it, and a session where nothing resolves omits the key entirely, so it is indistinguishable from an older client. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_016P7LvdubpsPtgR5vSenSSi
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| path = _backfill_remote_path(result.stdout.strip()) | ||
| if not path: | ||
| return None | ||
| parts = path.split('/') | ||
| if len(parts) < 2: | ||
| return None | ||
| org = parts[0][:-4] if parts[0].endswith('.git') else parts[0] | ||
| repo = parts[1][:-4] if parts[1].endswith('.git') else parts[1] |
There was a problem hiding this comment.
If a checkout uses a hostless origin such as file:///srv/git/x, this parser treats the local path components as srv/git, causing replayed work to be attributed to a fabricated project instead of leaving the project unset as the live resolver does.
| except Exception: | ||
| return None |
There was a problem hiding this comment.
The broad exception handler silently discards Git availability, timeout, invalid-directory, and decoding errors, leaving operators unable to distinguish these failures from an intentionally unresolvable checkout when project attribution is missing.
Context Used: P0 — Critical (must block merge)
Django / Backend ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
1 finding — 0 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
Findings
🟡 Hostless file:// remotes fabricate org/repo attribution
claude-code/hooks/setup.py:842 (same logic in claude-code/hooks/mdm/setup.py, codex/hooks/setup.py, codex/hooks/mdm/setup.py, copilot/hooks/setup.py)
Impact: For origins like file:///srv/git/x, _backfill_remote_path strips the scheme and treats path segments as org/repo (e.g. srv/git), uploading a fabricated project map—contradicting the PR’s “no guess / wrong repo is worse than missing” rule and misattributing replayed sessions.
Fix: Reject hostless scheme URLs (file://, etc.) before path parsing, matching live unbound.py::_get_project behavior; add a test case for file:///… expecting no project.
Flagged by: Lead, Greptile
Notes (not raised as findings): Gitleaks — no secrets. Claude — no issues. Semgrep pickle/file-permission hits are on unchanged lines outside this diff. subprocess.run(['git', '-C', cwd, ...]) uses a list argv (no shell injection); cwd is transcript-sourced on the local machine during backfill (expected trust boundary).
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head f6b9dd1b · 2026-08-21T21:41Z
2 participants
Companion to ai-gateway-data#2681. Ship that one first — it plumbs the
cwdthis map is keyed on.Why this has to happen client-side
A replayed session lands with no repository, so backfilled work can't be attributed to a repo at all. #2681 recovers the working directory from the transcript, but a project isn't in the transcript — it comes from git's
originremote, and by the time a session is replayed the backend is nowhere near the checkout.unbound.py::_get_projectresolves it by runninggit -C <cwd> remote get-url origin. That only works on the machine holding the checkout. So the resolution has to happen here, in the uploader, while we're still standing on it.What changed
Each session payload gains a
projectsmap ofcwd -> "org/repo", whichcoding_tools_backfill_service.resolve_record_projects()stamps onto the parsed records._backfill_remote_pathmirrorsunbound.py::_github_remote_pathrather than matching ongithub.com, so a self-hosted Enterprise remote resolves the same way. Handles both the SSH scp form and the scheme form."<org>/<repo>"— the exact shape_get_projectemits, so a replayed row is comparable to a live one rather than merely similar.gitcall per distinct checkout per run, cached across every transcript. The cache stores failures too, so a deleted directory isn't retried once per session. A 5,000-session backfill costs a handful of git invocations, not thousands.payload/datanesting Codex and Copilot use, matching the server's_entry_cwd.Applied to all five uploaders:
claude-code,codex,copilot, and bothmdmvariants. The server-side resolver handles every tool type, so fixing one tree would have left the others blind.Nothing is required of the server
A backend that doesn't read
projectsignores it. A session where nothing resolves omits the key entirely, so it's indistinguishable from an older client. Either order of deployment is safe — though reader-first means data starts landing the moment devices update.Deliberately not inferred
A
cwdthat doesn't resolve gets no project rather than a guessed one. A wrong repository is worse than a missing one: it attributes someone's work to another team, and nothing downstream could tell it was a guess. The server-side half enforces the same rule.Tests
claude-code/hooks/test_setup.py— 40 passed, 8 new covering: SSH/HTTPS/Enterprise URL forms, unparseable remotes, the lowercasedorg/repooutput, a plain folder yielding nothing, git failure never raising, the per-directory cache (asserting exactly one call per distinct cwd), failures being cached too, and malformed entries.Full repo:
augment/hooks133,codex/hooks33,copilot/hooks25,binary/tests887 — all pass. Three failures remain and are identical onstaging, verified by stashing:test_long_fields_are_capped_so_the_body_fits_the_pipe,test_helper_is_byte_identical_across_trees,test_keys_limited_to_identity_fields.Also verified live: resolving this repo's own directory returns
websentry-ai/setup.Reviewer note
test_setup.pyis CRLF in this repo. I rewrote it with LF on the first pass, which showed as a 1,102-line diff; the endings are restored, and the diff is now the 84 added lines only. Worth a glance at the raw diff to confirm.🤖 Generated with Claude Code
https://claude.ai/code/session_016P7LvdubpsPtgR5vSenSSi
Greptile Summary
The PR resolves each historical session’s working directories to lowercase repository identifiers before upload, caching Git lookups across the run and applying the behavior to Claude Code, Codex, Copilot, and managed variants.
git remote get-url originresolution.Confidence Score: 4/5
The PR should not merge until hostless Git remotes are rejected consistently with live repository resolution; silent failure handling is also worth improving.
A valid
file://origin is currently converted into a fabricated organization/repository and uploaded as authoritative attribution, while repository-resolution exceptions provide no actionable diagnostics.Files Needing Attention: claude-code/hooks/setup.py, claude-code/hooks/mdm/setup.py, codex/hooks/setup.py, codex/hooks/mdm/setup.py, copilot/hooks/setup.py
Important Files Changed
file://origins or diagnostic behavior.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR T[Transcript entries] --> C[Extract distinct cwd values] C --> K{Cached?} K -->|Yes| M[Reuse cached project] K -->|No| G[git remote get-url origin] G --> P[Parse and lowercase org/repo] P --> S[Store result including failure] M --> U[Add projects map to session] S --> U U --> B[Upload backfill payload]Reviews (1): Last reviewed commit: "Resolve each session's repository before..." | Re-trigger Greptile
Context used (6)
Django / Backend ... (source)