Repository navigation
ci: port engineer bot mention workflow from Go driver - #566
Conversation
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
There was a problem hiding this comment.
Verdict: 1 High
The workflow's authorization, credential scrubbing and diff-reference handling look sound, with one high issue. The PR checkout only receives node_modules from the trusted install, so the generated, gitignored lib/version.ts is missing there. The agent's first unit-test or type-check run will fail until it builds.
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Medium
Mostly solid: one medium concern. The authorization gating, credential cleanup before the PR checkout, and the trusted-dependency handoff all look correct. The mention workflow and the follow-up workflow don't share a concurrency group, so both can push to the same PR branch at once.
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Medium · 1 Low
The port looks mostly sound: the default-branch dependency install, the credential scrub before the PR checkout, and the diff reference are wired correctly. One medium concern: the scrub may not actually remove the OIDC token request variables while the job still has id-token: write. One low note: dropped @bot requests get no feedback on the PR.
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good. There's one low-severity robustness note on how the diff reference file is created. Authorization runs in layers (author association, then a live permission check, then a same-repo/open check repeated after the queue wait), dependencies install from the default branch before any PR code is checked out, and credential scrubbing matches the existing follow-up workflow. All npm scripts the prompt references (build, test, type-check, lint, prettier) exist in package.json.
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good overall, with one low-severity concern: the bash allowlist still permits npm ci and npm run e2e, which the mention prompt forbids. Authorization, the PR-eligibility recheck, the trusted-dependency handoff and the guards on the diff reference file all hold up. Nit: the new workflow has no explanatory header comment, unlike its sibling workflows.
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
There was a problem hiding this comment.
Verdict: 3 Low
Overall the workflow looks sound: authorization is checked before any secrets are requested, dependencies are installed from the trusted default branch and credentials are cleared before the PR checkout, and the diff-reference file is handled safely. There are three low-severity concerns: the concurrency.queue key may not be supported (which would also break the existing follow-up workflow), only the prompt stops the destructive npm ci, and rejected @bot requests get no feedback.
Port the Go driver's engineer-bot mention workflow so maintainers with write access can start a review, ask a question, or request changes with a top-level
@botcomment on an open, same-repository PR.Load bot setup and dependencies from the trusted default branch, clear registry credentials before the PR checkout, and reuse dependencies with npm offline. Require a PR build before verification, clear runner-injected OIDC variables inside each engineer agent shell, and serialize mention/follow-up jobs through the existing per-PR queue. Preserve authorization and the checked-out SHA/diff reference.
Validation: formatting, YAML/shell/Python checks, authorization and dependency-transfer checks, and credential-inheritance checks passed. A clean checkout built successfully and passed 119 affected unit tests. Live mention execution requires merging to the default branch.
This PR was created with GitHub MCP.