Fix: Pin vendored fork content and take the upstream fetch off the PR gate #894
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -828,15 +828,18 @@ jobs: | |
| - name: Check .gitmodules file for Git-over-SSH URLs | ||
| run: "! grep 'git@' .gitmodules" | ||
|
|
||
| # Structural checks only: manifest shape and path confinement. It reaches no | ||
| # network, so it runs on every PR. It does not compare file contents against | ||
| # anything; each manifest's upstream commit is the provenance record, and | ||
| # verify-upstream-snapshots below is what checks the files against it. | ||
| validate-workspace-dependencies: | ||
| name: Validate workspace dependencies | ||
| runs-on: ubuntu-22.04 | ||
| timeout-minutes: 10 | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 | ||
| with: | ||
| python-version: "3.12" | ||
|
|
@@ -845,10 +848,121 @@ jobs: | |
| - name: Test dependency policy validator | ||
| run: python3 -m pytest bin/tests/test_validate_workspace_dependencies.py -v | ||
| - name: Validate dependency policy | ||
| run: python3 bin/validate_workspace_dependencies.py | ||
|
|
||
| # Fetches all eight pinned upstream repositories and compares every retained | ||
| # file. This stays off the PR gate because it would make merging depend on | ||
| # third-party hosts being reachable, with no retry, for changes that cannot | ||
| # affect the result. A failure here means real drift or an unreachable | ||
| # upstream, which someone should act on rather than have block unrelated work. | ||
| # Keyed to the same weekly cron as integration-test-weekly, not the 6-hourly | ||
| # one: drift moves slowly and each run fetches eight external repositories. | ||
| # Deliberately not run on push, which would turn an unreachable upstream into | ||
| # a red main. | ||
| verify-upstream-snapshots: | ||
| name: Verify vendored upstream snapshots | ||
| if: >- | ||
| github.event_name == 'workflow_dispatch' || | ||
| (github.event_name == 'schedule' && github.event.schedule == '0 6 * * 0') | ||
| runs-on: ubuntu-22.04 | ||
| timeout-minutes: 30 | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| lfs: true | ||
| - name: Verify Git LFS object integrity | ||
| run: git lfs fsck --objects | ||
| - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 | ||
| with: | ||
| python-version: "3.12" | ||
| - name: Validate dependency policy against pinned upstreams | ||
| env: | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| run: python3 bin/validate_workspace_dependencies.py --verify-upstream | ||
|
|
||
| # Without this, a scheduled failure reaches nobody, and taking --verify-upstream | ||
| # off the PR gate only makes sense if someone learns when it fails. Mirrors | ||
| # weekly-failure-issue, with its own title so the two dedupe separately. | ||
| upstream-drift-issue: | ||
| # The App token below does the issue writes, so this job needs nothing from | ||
| # the workflow's own GITHUB_TOKEN. | ||
| permissions: {} | ||
| needs: verify-upstream-snapshots | ||
| if: >- | ||
| always() && needs.verify-upstream-snapshots.result != 'success' && | ||
| needs.verify-upstream-snapshots.result != 'skipped' | ||
| runs-on: ubuntu-22.04 | ||
| steps: | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| # example_ws has issues disabled, so this has to be filed on moveit_pro, | ||
| # which the workflow's own GITHUB_TOKEN cannot write to. Same reason | ||
| # weekly-failure-issue mints a cross-repo App token. | ||
| - name: Generate cross-repo App token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 | ||
|
There was a problem hiding this comment. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow context ---'
sed -n '860,935p' .github/workflows/ci.yaml
printf '%s\n' '--- token action usage and permissions ---'
rg -n -C 4 'create-github-app-token|permission-issues|permissions:' .github/workflows/ci.yamlRepository: PickNikRobotics/moveit_pro_example_ws Length of output: 9477 🤖 get_repo_knowledge executed:
Length of output: 10656 🏁 Script executed: #!/bin/bash
set -eu
sed -n '860,935p' .github/workflows/ci.yaml
printf '%s\n' '--- token action usage and permissions ---'
rg -n -C 4 'create-github-app-token|permission-issues|permissions:' .github/workflows/ci.yamlRepository: PickNikRobotics/moveit_pro_example_ws Length of output: 9452 🌐 Web query:
💡 Result: In the Citations:
🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- issue API calls in upstream-drift-issue ---'
sed -n '928,975p' .github/workflows/ci.yaml
printf '%s\n' '--- pinned action contract ---'
curl -fsSL https://raw.githubusercontent.com/actions/create-github-app-token/bcd2ba49218906704ab6c1aa796996da409d3eb1/action.yml | sed -n '1,180p'
curl -fsSL https://raw.githubusercontent.com/actions/create-github-app-token/bcd2ba49218906704ab6c1aa796996da409d3eb1/README.md | rg -n -C 5 'permission-issues|permissions are specified|all permissions|limit.*permissions'Repository: PickNikRobotics/moveit_pro_example_ws Length of output: 14998 Limit the generated App token to issue access.
🧰 Tools🪛 zizmor (1.29.0)[warning] 1-1019: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block (excessive-permissions) [error] 903-903: dangerous use of GitHub App tokens (github-app): app token inherits blanket installation permissions (github-app) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
| with: | ||
| client-id: ${{ secrets.SISTER_REPOS_APP_CLIENT_ID }} | ||
| private-key: ${{ secrets.SISTER_REPOS_APP_PRIVATE_KEY }} | ||
| owner: ${{ github.repository_owner }} | ||
| repositories: | | ||
| moveit_pro_example_ws | ||
| moveit_pro | ||
| - name: Open or update the drift issue | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 | ||
| with: | ||
| github-token: ${{ steps.app-token.outputs.token }} | ||
| script: | | ||
| const issueOwner = 'PickNikRobotics'; | ||
|
JWhitleyWork marked this conversation as resolved.
|
||
| const issueRepo = 'moveit_pro'; | ||
| const title = 'Vendored upstream snapshots no longer match their pinned commits'; | ||
| // An unassigned issue in a shared tracker goes unread. Change this | ||
| // when the vendored-dependency owner changes. | ||
| const assignees = ['JWhitleyWork']; | ||
| const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; | ||
| const body = [ | ||
| `\`validate_workspace_dependencies.py --verify-upstream\` failed on ${new Date().toISOString()}.`, | ||
| '', | ||
| 'Either a vendored tree in `example_ws` drifted from the commit its', | ||
| '`UPSTREAM.yaml` pins, or one of the eight pinned upstream repositories', | ||
| 'was unreachable. The run output names which manifest and which path.', | ||
| '', | ||
| `- [Workflow run](${runUrl})`, | ||
| ].join('\n'); | ||
| // Dedupe on an exact open-issue title; search title matching is fuzzy. | ||
| // On a search error, create anyway: a duplicate beats a dropped signal. | ||
| let existing; | ||
| try { | ||
| const found = await github.rest.search.issuesAndPullRequests({ | ||
| q: `repo:${issueOwner}/${issueRepo} is:issue is:open in:title "${title}"`, | ||
| }); | ||
| existing = found.data.items.find((i) => i.title === title); | ||
| } catch (e) { | ||
| core.warning(`Issue dedupe search failed (${e.message}); creating a new issue.`); | ||
| existing = undefined; | ||
| } | ||
| if (existing) { | ||
| await github.rest.issues.createComment({ | ||
| owner: issueOwner, repo: issueRepo, issue_number: existing.number, body, | ||
| }); | ||
| core.info(`Commented on existing issue #${existing.number}.`); | ||
| } else { | ||
| // Assignment is best-effort: if a login is no longer valid the | ||
| // issue must still land rather than throw. | ||
| let created; | ||
| try { | ||
| created = await github.rest.issues.create({ | ||
| owner: issueOwner, repo: issueRepo, title, body, assignees, | ||
| }); | ||
| } catch (e) { | ||
| core.warning(`Create with assignees failed (${e.message}); retrying unassigned.`); | ||
| created = await github.rest.issues.create({ | ||
| owner: issueOwner, repo: issueRepo, title, body, | ||
| }); | ||
| } | ||
| core.info(`Opened issue #${created.data.number}.`); | ||
| } | ||
|
|
||
| validate_objectives: | ||
| runs-on: ubuntu-22.04 | ||
| steps: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: PickNikRobotics/moveit_pro_example_ws
Length of output: 13234
🌐 Web query:
GitHub Actions expressions needs.<job_id>.result cancelled always() success skipped documentation💡 Result:
In GitHub Actions, status check functions like success, failure, cancelled, and always are primarily used within if conditionals to control the execution flow of jobs and steps [1][2]. Key Concepts and Usage: Status Check Functions (in 'if' conditionals): - success: Returns true when all previous steps have succeeded [1]. This is the default condition applied if no other status function is specified in an if conditional [1][2]. - failure: Returns true when any previous step of a job fails [1]. For dependent jobs, it returns true if any ancestor job fails [1]. - cancelled: Returns true if the workflow was cancelled [1]. - always: Causes the step or job to always execute, regardless of whether previous steps or dependent jobs succeeded, failed, or were cancelled [1][3]. It is often used to ensure cleanup tasks run [1]. Job Dependencies (needs.<job_id>.result): - In workflows, you can use needs.<job_id>.result to check the status of a dependent job [4]. The possible values for this property are success, failure, cancelled, or skipped [4]. - By default, if a job fails or is skipped, all downstream jobs that depend on it are also skipped [3]. To override this and allow a job to run even if a dependency failed or was skipped, you must use the always function in the job-level if conditional [3]. Important Considerations: - Avoid using always for tasks that could suffer from critical failures (e.g., fetching sources), as the workflow might hang until it times out [1]. The recommended alternative for running regardless of outcome is if: ${{!cancelled }} [1]. - When referencing a job's result for conditional logic, remember that a job skipped due to path or branch filtering will report a status of success [5]. - For debugging, GitHub Actions provides job condition expression logs (available for job-level if conditions) which show the evaluation of the expression (e.g., Evaluating, Expanded, Result) [6].
Citations:
Do not create a drift issue for a canceled verification.
When
verify-upstream-snapshotsis canceled, its result iscancelled, so the current condition can runupstream-drift-issueand open a false drift issue. Requireneeds.verify-upstream-snapshots.result == 'failure'instead.🧰 Tools
🪛 zizmor (1.29.0)
[warning] 1-1019: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents