feat(#6551): support both project-scoped and group-scoped runners in VM scripts - #6894
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🤖 Finished Review · ✅ Success · Started 1:43 AM UTC · Completed 2:04 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.22 |
|
Risk Assessment: low (1/5) DetailsTiny hack-script change (4 files, 208 lines) by a bot in a non-protected, non-security-sensitive path with no dependency or CI changes. High recent churn and fix commits are expected for newly added scripts under active stabilization. Medium blast radius is the only elevated signal. Score unchanged from prior assessment. Previous runRisk Assessment: low (1/5) DetailsTiny hack-script change (4 files, 201 lines) by a bot in a non-protected, non-security-sensitive path with no dependency or CI changes. create-gcp-vm.sh shows high recent churn but expected for a newly added script under active stabilization. Medium blast radius is the only elevated signal. Score unchanged from prior assessment. Previous run (2)Risk Assessment: low (1/5) DetailsTiny hack-script change (4 files, 201 lines) by a bot in a non-protected, non-security-sensitive path with no dependency or CI changes. create-gcp-vm.sh shows high recent churn (7 commits, 6 fixes in 30d) but this is expected for a newly added script under active stabilization. Medium blast radius is the only elevated signal. Linked issue is a medium-priority chore with good scope alignment. Easily reversible. Score unchanged from prior assessment. Previous run (3)Risk Assessment: low (1/5) DetailsTiny hack-script change (3 files, 209 lines) by a bot in a non-protected, non-security-sensitive path with no dependency or CI changes; moderate churn on create-gcp-vm.sh and a medium blast radius are the only notable signals, but the limited blast radius and easy reversibility keep risk low. Score unchanged from prior assessment. Previous run (4)Risk Assessment: low (1/5) DetailsTiny hack-script change (3 files, 209 lines) by a bot in a non-protected, non-security-sensitive path with no dependency or CI changes; moderate churn on create-gcp-vm.sh and a medium blast radius are the only notable signals, but the limited blast radius and easy reversibility keep risk low. Score unchanged from prior assessment. Previous run (5)Risk Assessment: low (1/5) DetailsTiny hack-script change (3 files, 82 lines) by a bot in a non-protected, non-security-sensitive path with no dependency or CI changes; the only notable signal is that the PR converts from project-scoped to group-scoped runners instead of supporting both as the issue requests, but the overall risk remains low given the limited blast radius and easy reversibility. |
ReviewFindingsMedium
Next steps:
Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewFindingsMedium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsLow
Previous run (5)ReviewFindingsHigh
Medium
Low
Labels: PR modifies GitLab runner VM provisioning scripts under hack/gitlab-runner-vm/ Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
component/runner
|
🤖 Finished Fix · ✅ Success · Started 2:05 AM UTC · Completed 2:11 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.61 |
|
🤖 Review · Commit: |
🔧 Fix agent — iteration 1 (bot-triggered)Addressed 6 review findings: fixed 2 (added trust-boundary widening note and ref_protected recommendation to README security section), disagreed with 4 (scope-mismatch claim is incorrect — dual-mode already implemented; both stale-reference claims are incorrect — headers already updated; fail-open is per-spec visibility note). Fixed (2):
Disagreed (4):
Tests: passed Decision points
Next steps:
|
Add explicit security note in README that group-scoped runners widen credential access compared to project-scoped runners, and recommend ref_protected as a compensating control. Addresses #6894
|
🤖 Finished Review · ✅ Success · Started 2:13 AM UTC · Completed 2:33 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.14 |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
ready-for-merge
…scoped Replace PROJECT_ID env var with GROUP_ID in both create-openshift-vm.sh and create-gcp-vm.sh. Change the GitLab runner registration API call from runner_type=project_type to runner_type=group_type, project_id to group_id, and locked=true to locked=false. Group runners are shared across the group tree via invitations rather than locked to a single project. Update README.md environment variables table (PROJECT_ID to GROUP_ID), GL_TOKEN description (Owner role on the group), RUNNER_ACCESS_LEVEL trade-off note (group-tree scoping), quick-start examples, and security notes to document the new group-level scoping model via run_untagged=false and tag-based access control. Delete and setup scripts are unchanged — they find runners by description/tag and use registration-type-agnostic glrt-* tokens. Closes #6551
The previous commit converted entirely from project-scoped to group-scoped runners, but the issue requires dual-mode support. This restores PROJECT_ID as a valid option alongside GROUP_ID, with the two being mutually exclusive. When PROJECT_ID is set, runners register with runner_type=project_type, project_id, and locked=true (original behavior). When GROUP_ID is set, runners register with runner_type=group_type, group_id, and locked=false (new group-scoped behavior). Setting both or neither produces a clear error message. Changes: - create-openshift-vm.sh: accept either PROJECT_ID or GROUP_ID, branch the API call via scope_args array, update usage/error messages - create-gcp-vm.sh: same dual-mode validation and API branching - README.md: document both env vars as mutually exclusive with footnote, show group mode as recommended in quick-start examples, update security notes to explain both scoping models - Fix --help line numbers in both scripts to include full header Closes #6551
Add explicit security note in README that group-scoped runners widen credential access compared to project-scoped runners, and recommend ref_protected as a compensating control. Addresses #6894
ggallen
changed the title
dc3b9fd to
3ea9acc
Compare
|
🤖 Finished Review · ✅ Success · Started 2:41 AM UTC · Completed 3:01 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.83 |
fullsend-ai-review
Bot
removed
the
ready-for-merge
|
🤖 Finished Fix · ✅ Success · Started 3:03 AM UTC · Completed 3:12 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.34 |
…ope validation - Fix off-by-one in --help output for both create scripts (exclude trailing blank comment line from help text) - Extract PROJECT_ID/GROUP_ID mutual-exclusion validation and scope_args construction into lib.sh (validate_runner_scope, build_scope_args) to eliminate duplication across create scripts - Add shellcheck SC2034/SC2154 directives for cross-function variables Addresses #6894
🔧 Fix agent — iteration 2 (bot-triggered)Fixed off-by-one in --help output for both create scripts and extracted duplicated PROJECT_ID/GROUP_ID validation and scope_args construction into shared lib.sh helpers. PR title change from feat to chore could not be applied from sandbox — needs human or post-script action. Fixed (4):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 3:14 AM UTC · Completed 3:52 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.06 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 3:53 AM UTC · Completed 4:00 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.53 |
…branch The SC2034 directive on line 64 of lib.sh covers only the initial `scope_args=()` assignment (line 65), not the re-assignment inside the project branch (line 67). The group branch already had its own directive (line 73). Add the matching directive for consistency and correctness. Addresses #6894
🔧 Fix agent — iteration 3 (bot-triggered)Addressed both review findings. (1) PR title change from feat to chore was attempted but blocked by sandbox permissions — needs manual update or post-script handling. (2) Added missing shellcheck SC2034 disable directive in lib.sh build_scope_args() for the project-branch scope_args assignment, matching the existing pattern on the group branch. Fixed (2):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 4:02 AM UTC · Completed 4:22 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.62 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 4:23 AM UTC · Completed 4:30 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.88 |
…extraction The --help handlers in both create scripts used head/tail with hardcoded line numbers to extract the header comment block. Any edit to the header shifted the window silently. Replace with awk that extracts all comment lines after the shebang until the first non-comment line — immune to header edits. Addresses #6894
🔧 Fix agent — iteration 4 (bot-triggered)Fixed the logic-error finding by replacing brittle hardcoded Fixed (2):
Disagreed (1):
Tests: passed
Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 4:32 AM UTC · Completed 4:49 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.22 |
|
🤖 Finished Fix · ✅ Success · Started 4:51 AM UTC · Completed 4:55 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.11 |
🔧 Fix agent — iteration 5 (bot-triggered)The only remaining review finding is a PR title change from feat to chore. This requires a PR metadata mutation that is blocked by the sandbox network policy (both GraphQL and REST API return 403 policy_denied). All code-level findings from prior reviews have been addressed in iterations 1-4. A human must change the PR title to: chore(#6551): support both project-scoped and group-scoped runners in VM scripts Disagreed (1):
Tests: passed
Decision points
Next steps:
|
|
🤖 Finished Retro · ✅ Success · Started 11:47 AM UTC · Completed 12:06 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.23 |
Retro: PR #6894 — support both project-scoped and group-scoped runners in VM scriptsAgents repo: Timeline: Issue #6551 filed Aug 24 → first Total agent cost: ~$44.56 (6 reviews: $35.09, 5 fix runs: $9.47) for a 4-file, 208-line change to internal operator scripts under Improvement opportunities (all covered by existing issues)Review-fix loop driven by unfixable finding (~$30 wasted). After the human approved at 02:50, the review agent posted CHANGES_REQUESTED at 03:01 on the same commit, triggering 4 more cycles. The primary driver was a MEDIUM Code agent used wrong conventional commit type. Used Over-investment in review for trivially-scoped PR. $35 in review costs (6 runs at claude-opus-4-6 high effort) for internal Review agent off-by-one false positive. Incorrectly flagged Autonomy-readiness observationThe human reviewer incorrectly dismissed a valid review finding: they claimed What went well
|
1 participant
Summary
create-openshift-vm.sh,create-gcp-vm.sh) from project-scoped to group-scoped runner registration per GitLab admin team guidancePROJECT_IDenv var withGROUP_ID, change API params (runner_type=group_type,group_id,locked=false), and update all validation, usage text, error messages, and examplesREADME.mdenv vars table, security notes (group-level scoping viarun_untagged=false+ tags instead oflocked=true+project_type), and quick-start examplesContext
The GitLab admin team recommended group runners over project runners for platform services — they enable self-service token management, group-tree sharing via invitations, and tag-based access control.
GCP_PROJECT(the GCP project ID for the VM) is unrelated to the renamedPROJECT_ID(the GitLab project ID) and is unchanged.Delete scripts (
delete-openshift-vm.sh,delete-gcp-vm.sh) andsetup.share confirmed unchanged — they find runners by description/tag and use registration-type-agnosticglrt-*tokens.Testing
PROJECT_IDreferences (excludingGCP_PROJECT) in changed filesproject_typeorlocked=truereferencesCloses #6551
Post-script verification
agent/6551-group-scoped-runners)f594dd04dee5d22a76ce30613e7f95b8a3753acd..HEAD)