[TRTLLM-12502][infra] Add GitHub Action to sync LFS objects from fork…#13826
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughA new GitHub Actions workflow is added that triggers when a pull request from a fork is merged. It detects LFS-tracked files changed in the PR, fetches their objects from the fork, pushes them to the upstream repository's LFS storage, verifies the sync succeeded, and posts a PR comment with the result. ChangesLFS Sync Workflow
Sequence DiagramsequenceDiagram
participant GH as GitHub
participant Runner as Actions Runner
participant Fork as Fork Remote
participant Origin as Upstream Repo
participant LFS as Git LFS Storage
GH->>Runner: Trigger workflow (merged fork PR)
Runner->>Runner: Checkout merge commit (lfs:false) & install git-lfs
Runner->>Runner: Detect LFS-tracked changed files
Runner->>Fork: Add temporary remote & git lfs fetch (PR head SHA)
Fork->>Runner: Serve LFS objects
Runner->>Origin: git lfs push --all origin
Runner->>LFS: Upload objects to upstream storage
Runner->>Runner: Clone upstream, git lfs pull, verify files
alt verification success
Runner->>GH: Post success PR comment (files synced)
else verification failure
Runner->>GH: Post failure PR comment (manual commands)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Microsoft Presidio Analyzer (2.2.362).github/workflows/lfs-sync.ymlMicrosoft Presidio Analyzer failed to scan this file Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/lfs-sync.yml:
- Around line 173-193: The script currently injects fork-controlled values via
`${{ ... }}` into bash command strings; replace those interpolations by
extracting values from context.payload (e.g., const forkRepo =
context.payload.pull_request.head.repo.full_name and const forkRef/commit =
context.payload.pull_request.head.ref/sha) before calling
github.rest.issues.createComment, and when composing the manual sync commands
ensure these extracted variables are safely shell-escaped (or passed as
environment variables to a separate run step) instead of concatenating raw
values into the command strings (references: the script block building the issue
body, github.rest.issues.createComment call, and the git remote add/fetch
lines).
- Around line 85-170: The fetch/push steps exit the job on failure so the "sync
failed" comment never runs; add id: fetch_fork and id: push_lfs to the "Fetch
LFS objects from contributor fork" and "Push LFS objects to this repository"
steps and set continue-on-error: true on both so the workflow continues on
failure, then change the "Verify with a fresh clone" and the "Comment on PR —
sync failed" steps to run unconditionally using if: always(); in the failure
comment step use a conditional that checks the outcomes and verify output (e.g.
if any of steps.fetch_fork.outcome != 'success' || steps.push_lfs.outcome !=
'success' || steps.verify.outputs.sync_ok == 'false') to decide whether to post
the failure notification. Ensure references use the ids fetch_fork, push_lfs,
and verify to locate the steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 069003f5-51ff-40c7-93d4-eb6d975e0329
📒 Files selected for processing (1)
.github/workflows/lfs-sync.yml
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/lfs-sync.yml:
- Around line 173-174: The failure condition for the "Comment on PR — sync
failed" step doesn't account for the verify step crashing (its outcome
undefined), so update the if expression to also check that steps.verify.outcome
!= 'success' in addition to steps.verify.outputs.sync_ok == 'false' and the
existing fetch-lfs/push-lfs checks; this ensures the failure comment runs when
the verify step fails or crashes (refer to steps.verify,
steps.verify.outputs.sync_ok, steps.verify.outcome, steps.fetch-lfs,
steps.push-lfs, and steps.detect-lfs.outputs.has_lfs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f48fc621-9ff5-43eb-8100-f96126790c00
📒 Files selected for processing (1)
.github/workflows/lfs-sync.yml
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
/bot skip --comment "github action" |
|
PR_Github #47123 [ skip ] triggered by Bot. Commit: |
|
PR_Github #47123 [ skip ] completed with state |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
/bot skip --comment "test on niukuo#18 (comment)" |
|
PR_Github #47220 [ skip ] triggered by Bot. Commit: |
|
PR_Github #47220 [ skip ] completed with state |
|
/bot skip --comment "github action tested in niukuo#20 (comment)" |
|
PR_Github #47272 [ skip ] triggered by Bot. Commit: |
|
PR_Github #47272 [ skip ] completed with state |
… after PR merge Signed-off-by: Yiteng Niu <6831097+niukuo@users.noreply.github.com>
|
✅ LFS objects already in storage (2864 files) — no sync needed. These LFS-tracked files are already present in this repository's LFS storage:
|
NVIDIA#13826) Signed-off-by: Yiteng Niu <6831097+niukuo@users.noreply.github.com>
2 participants
… after PR merge
Summary by CodeRabbit
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.