Skip to content

ci: update workflows and extend detect-secrets exclude#10286

Merged
HimavarshaVS merged 2 commits into
mainfrom
better-index-ci
Oct 15, 2025
Merged

ci: update workflows and extend detect-secrets exclude#10286
HimavarshaVS merged 2 commits into
mainfrom
better-index-ci

Conversation

@ogabrielluiz
Copy link
Copy Markdown
Contributor

@ogabrielluiz ogabrielluiz commented Oct 15, 2025

Remove component index from detec-secrets to avoid false positives that are already evaluated on the python files and update the CI workflow to not try to commit to community PR branches.

Summary by CodeRabbit

  • CI

    • Updated PR automation to support forked PRs and manual runs.
    • Separated checkout logic for manual vs same-repo events to improve reliability.
    • Expanded conditions for setup steps and script checks across supported triggers.
    • Refined PR comments to distinguish fork vs same-repo updates and include change summaries.
  • Chores

    • Tuned secret-scanning configuration to exclude a generated asset, reducing false positives.

…from secrets detection

- Modified the exclude pattern for the detect-secrets hook to prevent false positives for the component_index.json file located in src/lfx/src/lfx/_assets.
…st handling

- Changed the event trigger from `pull_request` to `pull_request_target` to improve security when handling forked pull requests.
- Updated conditions for repository checkout and dependency installation to ensure safe execution only for same-repo pull requests.
- Adjusted PR comment conditions to reflect the new event trigger, maintaining functionality for both community forks and same-repo updates.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 15, 2025

Walkthrough

Updates the GitHub Actions workflow to use pull_request_target and workflow_dispatch with refined conditional steps for checkout, dependency installation, script checks, and PR commenting based on PR origin. Extends detect-secrets pre-commit excludes to ignore the generated component_index.json file.

Changes

Cohort / File(s) Summary
Workflow: update-component-index
.github/workflows/update-component-index.yml
Switched trigger to pull_request_target; added workflow_dispatch. Split checkout logic for same-repo vs forks and for manual runs. Broadened conditions for uv setup, installs, and script checks. Refined PR comment steps to distinguish same-repo vs community fork contexts and updated messages.
Pre-commit: detect-secrets config
.pre-commit-config.yaml
Extended detect-secrets exclude patterns to include src/lfx/src/lfx/_assets/component_index.json.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User as PR Author / Triggerer
  participant GH as GitHub Actions
  participant WF as update-component-index.yml
  participant Repo as Repo (same/fork)
  participant PR as Pull Request

  User->>GH: Create/Update PR (from same repo or fork)
  User->>GH: Or trigger workflow_dispatch
  GH->>WF: Start workflow with event context

  alt Event: workflow_dispatch
    WF->>Repo: Checkout default branch (workflow_dispatch path)
    WF->>WF: Install uv and dependencies
    WF->>WF: Check script existence
    WF->>PR: Post comment (manual run summary)
  else Event: pull_request_target
    alt Head repo == base repo (same-repo)
      WF->>Repo: Checkout PR head (same-repo path)
      WF->>WF: Install uv and dependencies
      WF->>WF: Check script existence
      WF->>PR: Comment as same-repo auto-update
    else Head repo != base repo (community fork)
      WF->>Repo: Do not checkout PR head
      WF->>PR: Comment with community-fork instructions/summary
    end
  end

  note over WF,PR: Conditions guard each step based on event type and repo origin.
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

size:S, ignore-for-release

Suggested reviewers

  • msmygit
  • jordanrfrazier

Pre-merge checks and finishing touches

❌ Failed checks (1 error, 1 warning)
Check name Status Explanation Resolution
Test Coverage For New Implementations ❌ Error
Test File Naming And Structure ⚠️ Warning
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Test Quality And Coverage ✅ Passed The pull request only updates CI workflow triggers and pre-commit configuration without introducing new application logic or endpoints that would require behavioral tests, so no additional test coverage is needed for functionality that does not exist.
Excessive Mock Usage Warning ✅ Passed No test files were altered by this pull request, so there are no changes to mocking usage to review.
Title Check ✅ Passed The title succinctly captures the two primary changes in the pull request by indicating updates to the CI workflows and the extension of the detect-secrets exclusion patterns, matching the detailed modifications to both the GitHub Actions workflow and the pre-commit configuration.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch better-index-ci

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot changed the title @coderabbitai ci: update workflows and extend detect-secrets exclude Oct 15, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (7)
.pre-commit-config.yaml (2)

33-33: Anchor the new exclude pattern for consistency and precision

Current alternative lacks a start anchor. Use ^ to mirror others and avoid accidental mid-path matches.

-        exclude: '(^docs/|^SECURITY\.md$|src/lfx/src/lfx/_assets/component_index\.json$)'
+        exclude: '(^docs/|^SECURITY\.md$|^src/lfx/src/lfx/_assets/component_index\.json$)'

36-41: Confirm Biome formatting on the generated index

biome check applies to json and can rewrite the generated component_index.json. If you want to avoid churn, consider excluding that file here as well; otherwise, confirm the generator already outputs Biome-compliant JSON.

.github/workflows/update-component-index.yml (5)

22-35: Add fetch-depth: 0 to ensure git history availability

Your diagnostics step uses git log/show for prior content. Default checkout depth=1 may hide history. Fetch full history for reliability in both checkout paths.

       - name: Checkout repository (PR from same repo only)
         if: github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name == github.repository
         uses: actions/checkout@v4
         with:
           repository: ${{ github.event.pull_request.head.repo.full_name }}
           ref: ${{ github.event.pull_request.head.ref }}
           token: ${{ secrets.GITHUB_TOKEN }}
+          fetch-depth: 0

       - name: Checkout repository (workflow_dispatch)
         if: github.event_name == 'workflow_dispatch'
         uses: actions/checkout@v4
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
+          fetch-depth: 0

71-109: Change detection is solid; minor robustness nit

Use git add -N + git diff on the target file; jq-based metrics are guarded. All good. If you want to ensure stable diffs on JSON, consider normalizing with jq -S before diffing.


110-131: Push loop is fine; consider job-level concurrency

The retry+rebase loop helps. Add a concurrency group to avoid parallel jobs racing on the same PR branch.

 jobs:
   update-index:
     runs-on: ubuntu-latest
+    concurrency:
+      group: component-index-${{ github.event.pull_request.number || github.ref }}
+      cancel-in-progress: true

133-143: Fork PR comment unconditionally posts on matching paths

This always prompts contributors, which is acceptable. If noise becomes an issue, you could add a label gate (e.g., needs-index) or a manual re-run check before commenting.


24-24: Pin GitHub Actions to commit SHAs

For supply-chain hardening, pin actions/checkout, astral-sh/setup-uv, and actions/github-script to specific commit SHAs instead of major tags.

Also applies to: 32-32, 38-38, 44-44, 134-134, 146-146

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c83ba92 and aed2e55.

📒 Files selected for processing (2)
  • .github/workflows/update-component-index.yml (5 hunks)
  • .pre-commit-config.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Validate PR
🔇 Additional comments (3)
.github/workflows/update-component-index.yml (3)

1-4: Good PR-target security banner

Clear guidance and protections against executing fork code. LGTM.


36-48: Conditioning looks correct for PR-target vs forks

Install and script checks only on same-repo PRs or dispatch. Safe and intentional. LGTM.

If you intend to allow same-repo maintainers to run workflow_dispatch on feature branches, consider adding an input for ref and passing it to checkout.


145-266: Nice, informative auto-update comment

Good diff summary and SHA display. Optional: guard optional chaining with Node 20 (default for github-script) is fine.

@github-actions github-actions Bot added the lgtm This PR has been approved by a maintainer label Oct 15, 2025
@HimavarshaVS HimavarshaVS added this pull request to the merge queue Oct 15, 2025
Merged via the queue into main with commit 424b620 Oct 15, 2025
21 of 23 checks passed
@HimavarshaVS HimavarshaVS deleted the better-index-ci branch October 15, 2025 14:53
Adam-Aghili pushed a commit that referenced this pull request Oct 15, 2025
* chore: update pre-commit configuration to exclude specific JSON file from secrets detection

- Modified the exclude pattern for the detect-secrets hook to prevent false positives for the component_index.json file located in src/lfx/src/lfx/_assets.

* ci: update GitHub Actions workflow to enhance security for pull request handling

- Changed the event trigger from `pull_request` to `pull_request_target` to improve security when handling forked pull requests.
- Updated conditions for repository checkout and dependency installation to ensure safe execution only for same-repo pull requests.
- Adjusted PR comment conditions to reflect the new event trigger, maintaining functionality for both community forks and same-repo updates.
korenLazar pushed a commit to kiran-kate/langflow that referenced this pull request Nov 13, 2025
…0286)

* chore: update pre-commit configuration to exclude specific JSON file from secrets detection

- Modified the exclude pattern for the detect-secrets hook to prevent false positives for the component_index.json file located in src/lfx/src/lfx/_assets.

* ci: update GitHub Actions workflow to enhance security for pull request handling

- Changed the event trigger from `pull_request` to `pull_request_target` to improve security when handling forked pull requests.
- Updated conditions for repository checkout and dependency installation to ensure safe execution only for same-repo pull requests.
- Adjusted PR comment conditions to reflect the new event trigger, maintaining functionality for both community forks and same-repo updates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants