ci: build agentwatch-landing, and run the Docker build on pull requests(Closes #608)#621
Conversation
|
@SakethSumanBathini is attempting to deploy a commit to the sreerevanth's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe CI workflow adds a dedicated landing application build, makes Docker builds run for pull requests after both frontend builds, and simplifies dependency auditing by installing the crypto extra before running pip-audit. ChangesCI workflow changes
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. 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: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
154-154: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueSet
persist-credentials: falseon the checkout step.The
actions/checkout@v4step persists the GitHub token in local Git config by default. Sincelanding-builddoesn't need to push or perform authenticated Git operations after checkout, disabling credential persistence reduces exposure if the runner environment is compromised.🔒️ Proposed fix
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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/ci.yml at line 154, Update the actions/checkout@v4 step in the landing-build job to set persist-credentials to false, leaving the rest of the checkout configuration unchanged.Source: Linters/SAST tools
147-168: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a
permissions:block tolanding-buildfor least-privilege.The new
landing-buildjob inherits the default (or broad) token permissions because nopermissions:block is declared. Since this job only builds and doesn't publish artifacts or call authenticated APIs, it should use read-only permissions.🔒️ Proposed fix
landing-build: name: Landing Build runs-on: ubuntu-latest + permissions: + contents: read defaults: run: working-directory: agentwatch-landing🤖 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/ci.yml around lines 147 - 168, Add a job-level permissions block to the landing-build job, granting only read-only repository contents access needed by actions/checkout and leaving all other token permissions disabled. Keep the existing build steps and environment unchanged.Source: Linters/SAST tools
🤖 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/ci.yml:
- Line 174: Update the docker-build job’s needs configuration to include
landing-build alongside python-tests and frontend-build, ensuring docker-build
waits for and is blocked by a failing landing-build.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 154: Update the actions/checkout@v4 step in the landing-build job to set
persist-credentials to false, leaving the rest of the checkout configuration
unchanged.
- Around line 147-168: Add a job-level permissions block to the landing-build
job, granting only read-only repository contents access needed by
actions/checkout and leaving all other token permissions disabled. Keep the
existing build steps and environment unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a65dc68-28df-4ffe-b7e5-00cf9d69909d
📒 Files selected for processing (1)
.github/workflows/ci.yml
| docker-build: | ||
| name: Docker Build | ||
| runs-on: ubuntu-latest | ||
| needs: [python-tests, frontend-build] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
docker-build does not wait for landing-build.
The PR summary and stack context state that docker-build should wait for landing-build, but the needs array only lists python-tests and frontend-build. This means a failing landing-build won't block docker-build, and the new landing CI coverage isn't acting as a gate for the Docker build step.
🔧 Proposed fix
- needs: [python-tests, frontend-build]
+ needs: [python-tests, frontend-build, landing-build]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| needs: [python-tests, frontend-build] | |
| needs: [python-tests, frontend-build, landing-build] |
🤖 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/ci.yml at line 174, Update the docker-build job’s needs
configuration to include landing-build alongside python-tests and
frontend-build, ensuring docker-build waits for and is blocked by a failing
landing-build.
🧪 PR Test Results
Python 3.12 · commit 614e4a4 |
1 participant
(Closes #608)
Two gaps in CI coverage, same underlying shape: things that can break silently because nothing
builds them.
agentwatch-landing was never built
CI runs
python-lint,python-tests,integration-tests,frontend-build,docker-buildandsecurity. None of them touchagentwatch-landing/.It's a full Next.js application with its own
package.jsonand lockfile, and it was neverinstalled, never built and never type-checked by any workflow. A dependency break or a type error
landed silently, and the first person to find out was whoever deployed.
frontend/has had a buildjob all along, so the asymmetry looked accidental rather than deliberate.
The new
landing-buildjob mirrorsfrontend-build, with one difference: no separatetype-checkstep. Unlikefrontend, the landing app declares notype-checkscript inpackage.json— and doesn't need a standalone one, becausenext buildruns its own TypeScriptphase as part of the build:
Confirmed locally:
npm cisucceeds, and the build above is the first successful build this apphas had, as far as CI history shows. It's healthy — it's simply been running unprotected.
The Docker build never ran on a pull request
So the images were only built after a change had already merged. A pull request touching
Dockerfile.apigot no Docker build at all, which means a broken Dockerfile could only bediscovered on
main— at which point it's everyone's problem rather than the author's.Now it runs on pull requests too, so it's a gate rather than a post-mortem. The cost is modest: both
builds already use the Actions layer cache (
cache-from: type=gha), so unchanged layers are pulledrather than rebuilt.
This one matters immediately: I have a follow-up PR hardening
Dockerfile.api(non-root user,multi-stage build, no build tooling in the runtime image). Without this change, that PR would be
completely unverifiable by CI — it would land unchecked. With it, CI builds the image on the PR
itself and proves it works before anyone merges it.
Summary by CodeRabbit