feat: add make notices for third-party license aggregation#234
Merged
Conversation
lockwobr
requested review from
ayuskauskas,
mskalka and
rice-riley
as code owners
May 13, 2026 23:44
4 tasks
Contributor
📝 WalkthroughWalkthroughAdds a Merge Gate GitHub Actions workflow that conditionally verifies operator Go dependency license compliance when Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent/THIRD_PARTY_NOTICES.md (1)
41-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix multiple consecutive blank lines.
The markdownlint pipeline failure indicates multiple consecutive blank lines at the end of the file (Expected: 1; Actual: 2). Remove the extra blank line.
📝 Proposed fix
Remove one blank line at the end of the file. The file should end with exactly one newline after the closing triple backticks.
🤖 Prompt for 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. In `@agent/THIRD_PARTY_NOTICES.md` at line 41, Remove the extra blank line at the end of THIRD_PARTY_NOTICES.md so there is exactly one trailing newline after the final closing triple backticks; open the file, delete the extra empty line following the closing ``` fence so the file ends with a single newline.
🤖 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 `@scripts/generate-notices.py`:
- Line 59: Rename the ambiguous loop variable `l` in the set comprehension used
to build `stdlib_ignore` to a descriptive name (e.g., `line` or `pkg_path`) to
improve readability; specifically update the comprehension expression in the
assignment to `stdlib_ignore = ",".join(sorted({l.split("/")[0] for l in
stdlib.splitlines() if l}))` by replacing `l` with the chosen name in every
occurrence (`{... for l in stdlib.splitlines() if l}`, and `l.split("/")[0]`) so
the logic remains unchanged but the variable is clearer.
- Around line 135-137: The helper function src is missing a return type
annotation; update its signature from def src(e): to def src(e) -> str: so the
function explicitly declares it returns a string (keeping the existing logic
that returns u or "n/a"), ensuring type consistency for callers and static type
checkers when using src.
- Around line 63-64: The current code calls shutil.rmtree(LICENSES_CACHE)
whenever LICENSES_CACHE.exists(), which will raise if LICENSES_CACHE is a file;
update the removal logic to either (a) check that LICENSES_CACHE.is_dir() before
calling shutil.rmtree(LICENSES_CACHE) or (b) wrap shutil.rmtree in a try/except
and handle OSError by logging and continuing, or call
shutil.rmtree(LICENSES_CACHE, ignore_errors=True); reference LICENSES_CACHE and
shutil.rmtree when making the change so the check/exception handling is applied
in the same location.
---
Outside diff comments:
In `@agent/THIRD_PARTY_NOTICES.md`:
- Line 41: Remove the extra blank line at the end of THIRD_PARTY_NOTICES.md so
there is exactly one trailing newline after the final closing triple backticks;
open the file, delete the extra empty line following the closing ``` fence so
the file ends with a single newline.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: b3f1a023-43cb-4401-b05c-cb8256f31b93
📒 Files selected for processing (12)
.github/workflows/merge-gate.yaml.github/workflows/release.yml.gitignoreCONTRIBUTING.mdMakefileNOTICETHIRD_PARTY_NOTICES.mdagent/THIRD_PARTY_NOTICES.mddocs/release-process.mdoperator/Makefileoperator/THIRD_PARTY_NOTICES.mdscripts/generate-notices.py
💤 Files with no reviewable changes (1)
- NOTICE
lockwobr
force-pushed
the
worktree-3rd-party
branch
from
May 13, 2026 23:55
f63af78 to
1cdc651
Compare
Contributor
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/release.yml:
- Around line 78-82: The workflow uses actions/setup-python@v6 with
python-version: '3.x', which can drift; update the setup step in the release.yml
(the actions/setup-python block) to pin a specific minor version (e.g., '3.13'
or the project-supported '3.10') instead of '3.x' so releases are
deterministic—modify the python-version field in that setup step to the chosen
explicit minor version.
- Around line 89-111: The release creation step should be made idempotent:
instead of unconditionally running gh release create in the "Create GitHub
Release" step, first check whether the release for "${{ github.ref_name }}"
already exists (e.g. call gh release view "${{ github.ref_name }}" and test its
exit code) and if it exists use gh release edit --title --notes to update it,
otherwise run gh release create --title --notes; keep the "Upload third-party
notices to release" step as-is but ensure it runs after the guarded create/edit
so uploads don’t fail when a release already exists (use the same "${{
github.ref_name }}" variable and NOTICES_FILE mapping in the case block).
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: b18a747c-30c3-4485-a6a6-52dedec2abe8
📒 Files selected for processing (13)
.github/workflows/lint-ci.yaml.github/workflows/merge-gate.yaml.github/workflows/release.yml.gitignoreCONTRIBUTING.mdMakefileNOTICETHIRD_PARTY_NOTICES.mdagent/THIRD_PARTY_NOTICES.mddocs/release-process.mdoperator/Makefileoperator/THIRD_PARTY_NOTICES.mdscripts/generate-notices.py
💤 Files with no reviewable changes (1)
- NOTICE
lockwobr
force-pushed
the
worktree-3rd-party
branch
from
May 14, 2026 00:35
1cdc651 to
18f8449
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
97-101:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the release step fully idempotent with create-or-edit logic.
Line 97 performs a check-then-create flow; on concurrent reruns this can still fail with
already_exists, and existing releases never get refreshed title/notes. Prefer explicit create-or-edit handling.Suggested minimal change
- if ! gh release view "${{ github.ref_name }}" >/dev/null 2>&1; then - gh release create "${{ github.ref_name }}" \ - --title "${{ github.ref_name }}" \ - --notes "${{ steps.release_notes.outputs.notes }}" - fi + if gh release view "${{ github.ref_name }}" >/dev/null 2>&1; then + gh release edit "${{ github.ref_name }}" \ + --title "${{ github.ref_name }}" \ + --notes "${{ steps.release_notes.outputs.notes }}" + else + gh release create "${{ github.ref_name }}" \ + --title "${{ github.ref_name }}" \ + --notes "${{ steps.release_notes.outputs.notes }}" \ + || gh release edit "${{ github.ref_name }}" \ + --title "${{ github.ref_name }}" \ + --notes "${{ steps.release_notes.outputs.notes }}" + fiDoes `gh release create <tag>` fail when a release for that tag already exists, and is `gh release edit <tag>` the recommended way to make rerun-safe/idempotent workflows?🤖 Prompt for 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. In @.github/workflows/release.yml around lines 97 - 101, Replace the current check-then-create flow that uses `gh release view` then `gh release create` for `${{ github.ref_name }}` with an idempotent create-or-edit pattern: attempt `gh release create "${{ github.ref_name }}" --title "${{ github.ref_name }}" --notes "${{ steps.release_notes.outputs.notes }}"` and if it fails due to an existing release (already_exists) call `gh release edit "${{ github.ref_name }}" --title "${{ github.ref_name }}" --notes "${{ steps.release_notes.outputs.notes }}"` to refresh title/notes; alternatively you can unconditionally call `gh release edit` if the release exists (detectable via `gh release view`) and otherwise create it—ensure you handle exit codes and stderr redirection the same way as the original script.
🤖 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/merge-gate.yaml:
- Around line 32-36: Update the license verification filter to include agent
dependency paths so changes to agent deps trigger verification: add
'agent/go.mod', 'agent/go.sum', and 'agent/vendor/**' to the filters block (in
the same style as 'operator/go.mod', etc.), and update the verify-licenses job
to also run the agent license check (e.g., invoke make notices-agent or the
equivalent Python dependency validation command) so both operator and agent
license checks run when their files change.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 97-101: Replace the current check-then-create flow that uses `gh
release view` then `gh release create` for `${{ github.ref_name }}` with an
idempotent create-or-edit pattern: attempt `gh release create "${{
github.ref_name }}" --title "${{ github.ref_name }}" --notes "${{
steps.release_notes.outputs.notes }}"` and if it fails due to an existing
release (already_exists) call `gh release edit "${{ github.ref_name }}" --title
"${{ github.ref_name }}" --notes "${{ steps.release_notes.outputs.notes }}"` to
refresh title/notes; alternatively you can unconditionally call `gh release
edit` if the release exists (detectable via `gh release view`) and otherwise
create it—ensure you handle exit codes and stderr redirection the same way as
the original script.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 16eef9f6-7fc4-4a5c-9bf6-f1004d5d5fcb
📒 Files selected for processing (13)
.github/workflows/lint-ci.yaml.github/workflows/merge-gate.yaml.github/workflows/release.yml.gitignoreCONTRIBUTING.mdMakefileNOTICETHIRD_PARTY_NOTICES.mdagent/THIRD_PARTY_NOTICES.mddocs/release-process.mdoperator/Makefileoperator/THIRD_PARTY_NOTICES.mdscripts/generate-notices.py
💤 Files with no reviewable changes (1)
- NOTICE
ayuskauskas
previously approved these changes
May 14, 2026
lockwobr
force-pushed
the
worktree-3rd-party
branch
from
May 14, 2026 16:49
18f8449 to
5bc51ac
Compare
ayuskauskas
approved these changes
May 14, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/merge-gate.yaml (1)
32-36: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider adding agent dependency paths to the license verification.
The current filter only checks
operator/Go dependencies. Since the PR adds license generation for both operator and agent, changes toagent/vendor/**should also trigger verification.📝 Suggested enhancement
filters: | deps: - 'operator/go.mod' - 'operator/go.sum' - 'operator/vendor/**' + - 'agent/vendor/**'And update the
verify-licensesjob to also regenerate/verify agent notices:- name: Check licenses env: GOFLAGS: -mod=vendor run: make -C operator license-check + - name: Verify agent licenses + run: make notices-agent🤖 Prompt for 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. In @.github/workflows/merge-gate.yaml around lines 32 - 36, The workflow filter currently only covers operator dependency files ('operator/go.mod', 'operator/go.sum', 'operator/vendor/**') so changes to agent dependencies won’t trigger license verification; update the filters:deps list to also include the agent paths (e.g., 'agent/go.mod', 'agent/go.sum', 'agent/vendor/**') and update the verify-licenses job (the verify-licenses job block) to run the agent notice generation/verification steps in addition to the operator ones so agent license changes are regenerated and verified.
🤖 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.
Duplicate comments:
In @.github/workflows/merge-gate.yaml:
- Around line 32-36: The workflow filter currently only covers operator
dependency files ('operator/go.mod', 'operator/go.sum', 'operator/vendor/**') so
changes to agent dependencies won’t trigger license verification; update the
filters:deps list to also include the agent paths (e.g., 'agent/go.mod',
'agent/go.sum', 'agent/vendor/**') and update the verify-licenses job (the
verify-licenses job block) to run the agent notice generation/verification steps
in addition to the operator ones so agent license changes are regenerated and
verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2fda2885-fd14-4d18-bbd7-2be68c152590
📒 Files selected for processing (13)
.github/workflows/lint-ci.yaml.github/workflows/merge-gate.yaml.github/workflows/release.yml.gitignoreCONTRIBUTING.mdMakefileNOTICETHIRD_PARTY_NOTICES.mdagent/THIRD_PARTY_NOTICES.mddocs/release-process.mdoperator/Makefileoperator/THIRD_PARTY_NOTICES.mdscripts/generate-notices.py
💤 Files with no reviewable changes (1)
- NOTICE
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
2 participants
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.
Summary
Adds an aicr-inspired third-party notices workflow. Generates per-component
THIRD_PARTY_NOTICES.mdfiles (operator/CLI viago-licenses, agent viapip-licenses) plus a combined rollup at the repo root, regenerates them at release time, and attaches the appropriate file to every component GitHubrelease. Adds a path-gated merge-gate workflow for
go-licenses checkand drops that check frommake lint's prerequisites. Deletes the stalehand-curated
NOTICEfile.Why
NOTICEat the repo root had drifted from reality (mixed Go + Python, no clear regen path, no CI gate).THIRD_PARTY_NOTICES.mdper release; skyhook should have the same attribution story.make lintwas runninglicense-checkon every developer's local lint cycle, even when deps hadn't changed. Moving it to a path-filtered CI jobkeeps the gate but takes it off the hot path.
What changed
New: notices generator
scripts/generate-notices.pygo-licensesfor the operator pass andpip-licensesagent/.notices-venv) for the agent pass, then composes a root rollup by inlining both component files with their#operator/THIRD_PARTY_NOTICES.mdagent/THIRD_PARTY_NOTICES.mdjsonschema).THIRD_PARTY_NOTICES.md(root)New: Makefile targets
The agent pass caches a venv at
agent/.notices-venv. First run ~30s (installs pip-licenses + the agent's pinned deps); subsequent runs ~2s.New: merge-gate CI workflow
.github/workflows/merge-gate.yamlmirrors aicr'sverify-licensespattern:check-pathsusesdorny/paths-filter@v3to detect changes tooperator/go.mod,operator/go.sum, oroperator/vendor/**.verify-licensesrunsmake -C operator license-checkonly when those paths change.verify-licenses-skipis a paired skip job — keeps required-check parity when deps don't change, so branch protection requiringverify-licensesdoesn't block PRs that don't touch deps.
Modified: release workflow
.github/workflows/release.ymlnow regenerates the notices files in CI and attaches the appropriate one to the GitHub release based on the tagprefix:
operator/v*operator/THIRD_PARTY_NOTICES.mdagent/v*agent/THIRD_PARTY_NOTICES.mdchart/v*THIRD_PARTY_NOTICES.md(rollup — chart packages both images)Upload uses
gh release upload --clobberso the step is idempotent on workflow re-runs.Modified:
operator/Makefilelint: golangci-lint license-check→lint: golangci-lint. Thelicense-checktarget itself is unchanged; CI invokes it via the merge-gateworkflow.
Removed:
NOTICEStale hand-curated file at the repo root. Apache-2.0 attribution is satisfied by the repo
LICENSEplus the generatedTHIRD_PARTY_NOTICES.mdfiles.No remaining references in source.
Docs
docs/release-process.md— new "Third-Party Notices" section explaining the three files, the fourmaketargets, prerequisites, when toregenerate, and both CI workflows.
CONTRIBUTING.md— new bullet under "Pull Requests" reminding contributors to runmake noticeswhen bumping Go or Python deps.New patterns introduced (per
.claude/CLAUDE.md)Upload uses
gh release upload --clobberso the step is idempotent on workflow re-runs.Modified:
operator/Makefilelint: golangci-lint license-check→lint: golangci-lint. Thelicense-checktarget itself is unchanged; CI invokes it via the merge-gateworkflow.
Removed:
NOTICEStale hand-curated file at the repo root. Apache-2.0 attribution is satisfied by the repo
LICENSEplus the generatedTHIRD_PARTY_NOTICES.mdfiles.No remaining references in source.
Docs
docs/release-process.md— new "Third-Party Notices" section explaining the three files, the fourmaketargets, prerequisites, when toregenerate, and both CI workflows.
CONTRIBUTING.md— new bullet under "Pull Requests" reminding contributors to runmake noticeswhen bumping Go or Python deps.New patterns introduced (per
.claude/CLAUDE.md)Calling these out explicitly since none exist elsewhere in the repo today:
dorny/paths-filteris new to skyhook's workflows. Justification: aicr uses it, and it's the cleanest way to satisfy the "required-check alwayspasses" property via a paired skip job. Alternative
paths:filter at trigger level doesn't give that property.##@ LicensesMakefile section — new section in the rootMakefile. Trivially symmetric with existing##@ Build,##@ Test, etc.pip-licensesas a build-time Python dependency, installed into a cached venv atagent/.notices-venv(gitignored). Replaces a hand-rolledagent/vendor/walk. Standard tool; same role asgo-licenseson the Go side.CI behavior changes
operator/go.{mod,sum}oroperator/vendor/**now hit a new required check (verify-licenses). PRs that don't touch those pathssee
verify-licenses-skipcomplete in ~5 seconds — no impact.make lintno longer runslicense-check;make lintis now strictlygolangci-lint. Runmake -C operator license-checkdirectly (orpush to a PR) when you want to verify dep licenses locally.
Test plan
make noticesregenerates all three files locally without error.make notices-operatoronly touchesoperator/THIRD_PARTY_NOTICES.md(verify via mtime).make notices-agentonly touchesagent/THIRD_PARTY_NOTICES.md.make notices-rolluponly touches the root file; errors with an actionable message if either component file is missing.agent/THIRD_PARTY_NOTICES.mdcorrectly listsjsonschema 4.23.0with the MIT license text inlined.operator/THIRD_PARTY_NOTICES.mdlists all 62 Go deps.#-prefixed lines are not mangled).make lint(underoperator/) no longer invokeslicense-check.make -C operator license-checkstill works standalone.operator/go.mod;verify-licenses-skipruns on a PR that doesn't.release.ymlworkflow uploads the correct file per component tag (verify on the next operator/agent/chart release, or via a release-dry-run).