ci: use docker/cagent-action/.github/workflows/review-pr.yml#24132
ci: use docker/cagent-action/.github/workflows/review-pr.yml#24132dvdksn wants to merge 1 commit intodocker:mainfrom
Conversation
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Review Summary
Workflow refactoring looks good! ✅
This PR successfully migrates from an inline agent configuration to using the reusable workflow from docker/cagent-action. The configuration is clear and well-structured with no documentation quality issues found.
The additional-prompt parameter appropriately configures the review agent with clear instructions and guidelines. The content is technically accurate and suitable for its purpose as agent configuration.
derekmisler
left a comment
There was a problem hiding this comment.
the only reason i haven't approved this yet is because we need this one to land first, or the prompt files will be ignored
|
@dvdksn could we make it aware of pages being removed, and for those to check if a correct redirect is added? i.e., to prevent things like #23949 (comment) |
|
I think we were also looking at (or already have?) helping with triage; for 404 reports, it could also check wayback-machine to verify if it was a page that previously existed to exclude reports for made-up links 🤔 (if I'm not sure, I usually do a quick check to see if there was anything at that URL in the first place #23949 (comment)) |
|
@thaJeztah yes that would be a good addition. Let me update the instruction 👍 |
|
@thaJeztah for 404 - yes, for the triage agent (todo) we could probably just add a tool to check the wayback api, eg: https://archive.org/wayback/available?url=docs.docker.com/engine/reference/builder |
Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
f3b26ff to
44692cc
Compare
| anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| review: | ||
| uses: docker/cagent-action/.github/workflows/review-pr.yml@latest | ||
| secrets: inherit |
There was a problem hiding this comment.
Silly question; I see secrets: inherit what permissions does that grant, and are all permissions needed for this workflow?
There was a problem hiding this comment.
Oh; nevermind; GitHub hid those lines, but I'm guessing it's these;
permissions:
contents: read
pull-requests: write
issues: write(Was mostly concerned it could inherit write access on content 😅)
There was a problem hiding this comment.
Still wondering; would it be good to set the top-level defaults to not be permissive, and explicitly set more permissions here? I recall that's what we do in most places; e.g. https://github.com/moby/moby/blob/3068dc540f79ea43d312918857d1465f25ad128f/.github/workflows/ci.yml#L3-L10
# Default to 'contents: read', which grants actions to read commits.
#
# If any permission is set, any permission not included in the list is
# implicitly set to "none".
#
# see https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions
permissions:
contents: readAnd then in steps that must have more permissions, we set it locally;
https://github.com/moby/moby/blob/3068dc540f79ea43d312918857d1465f25ad128f/.github/workflows/ci.yml#L130-L138
govulncheck:
runs-on: ubuntu-24.04
timeout-minutes: 120 # guardrails timeout for the whole job
# Always run security checks, even with 'ci/validate-only' label
permissions:
# required to write sarif report
security-events: write
# required to check out the repository
contents: read
Uh oh!
There was an error while loading. Please reload this page.