Skip to content

feat: implement issue #602 — Compliance: stub-surface-drift-dependency-audit.yml-on - #610

Open
don-petry wants to merge 2 commits into
mainfrom
dev-lead/issue-602-20261002-1917
Open

don-petry wants to merge 2 commits into
mainfrom
dev-lead/issue-602-20261002-1917

Conversation

@don-petry

@don-petry don-petry commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Compliance: stub-surface-drift-dependency-audit.yml-on

From the issue: Category: ci-workflows Severity: error Check: stub-surface-drift-dependency-audit.yml-on

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

No test files were added or updated. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the existing CI (bats + lint) guards the change.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.

Closes #602

Review in cubic

Summary by CodeRabbit

  • Chores
    • Dependency-audit status checks now run for merge-queue updates, alongside pull requests and pushes.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Repository: petry-projects/google-app-scripts/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 4e9ae39c-d256-4e96-b57a-979aa2e33829

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: petry-projects/google-app-scripts/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 298633b4-8934-48ca-b304-33c1d0b83b99

📥 Commits

Reviewing files that changed from the base of the PR and between 8318036 and bea4ff8.

📒 Files selected for processing (1)
  • .github/workflows/dependency-audit.yml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The dependency-audit workflow now runs on merge_group events. Its comment documents that this trigger is required for the Detect ecosystems status check on merge-queue refs.

Changes

Dependency audit workflow

Layer / File(s) Summary
Merge-queue trigger and documentation
.github/workflows/dependency-audit.yml
The workflow adds the merge_group trigger and documents its requirement for the Detect ecosystems status check on merge-queue refs. Pull-request and push triggers remain unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: donpetry-bot

Merge Risk: ⚪ Minimal · up to bea4f

The dependency-audit check now runs on merge-queue refs while retaining its existing pull-request and push triggers. No actionable merge risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to bea4f

The changed surface does not map to a changed system, dependency edge, entrypoint, or external dependency.

Changed systems: None identified.

Architecture concerns
No architecture-level concerns identified.

Review details

Before / after behavior

  • observed — Modified behavior in .github/workflows/dependency-audit.yml: The comment now identifies merge_group as required for the Detect ecosystems status check on merge-queue refs and instructs readers not to remove or flag it as drift.
  • observed — Modified behavior in .github/workflows/dependency-audit.yml: The workflow now also runs on merge_group events; pull-request and push triggers remain unchanged.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the dependency-audit workflow trigger-surface drift and the related compliance issue. This matches the addition of the merge_group trigger.
Linked Issues check ✅ Passed Issue #602 requires the dependency-audit caller stub to re-sync its on: triggers with the canonical template. The PR adds the required merge_group trigger to `.github/workflows/dependency-audit.ym…
Out of Scope Changes check ✅ Passed The changes are limited to .github/workflows/dependency-audit.yml. The trigger addition and related comment directly implement issue #602. No unrelated production or configuration change is shown.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #610
No changes were committed, but the PR still can't be marked done: required check coverage is still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-10-02T19:56:26Z

@don-petry

Copy link
Copy Markdown
Collaborator Author

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required check coverage is still pending. I'll re-check automatically.
Next attempt after: 2026-10-02T19:56:26Z

@don-petry
don-petry enabled auto-merge (squash) October 2, 2026 19:26
@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@donpetry-bot

donpetry-bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 548d788a440cedf8cde7fac98581c8d7b22ec26e — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: bea4ff8d766b3597e9be12e32e3d3fb21b74fa7f
Review mode: triage-approved (single reviewer)

Summary

One-file sync of the dependency-audit.yml caller stub: it adds the merge_group: trigger and the matching header note so the stub's on: block matches the canonical standards/workflows/dependency-audit.yml. The change is correct and low-risk. It is escalated only because the non-required dependency-audit / npm audit check is red. That failure was already happening on main and this PR doesn't cause it.

Linked issue analysis

Closes #602 (stub-surface-drift-dependency-audit.yml-on). I compared the PR-head file with the canonical template from petry-projects/.github. The on: block and the header note now match exactly. The only remaining difference is cosmetic: one space instead of two before the # NOSONAR comment on the uses: line. That difference was already on main, is outside the on: surface, and doesn't affect behaviour. The issue is fully addressed.

Findings

  • [Blocking, not caused by this PR] dependency-audit / npm audit is failing. The repo's existing lockfile has known advisories: high brace-expansion (GHSA-q2hr-2g5m-vwhr, GHSA-qhr7-859c-m2p7, GHSA-6j4f-fj2g-mc7p) and moderate fast-uri (GHSA-hrr3-gc8f-f4qj). The latest main push run (8318036, 2026-10-01) fails the same way, and this PR changes no dependency files. The check is not a required status check. Fix: run npm audit fix / bump the lockfile in a separate PR on main, then re-run this check. This PR itself needs no code change.
  • [Nit, already on main] uses: line has one space before # NOSONAR(githubactions:S7637) where the canonical template has two. Cosmetic only; it may keep a byte-exact drift check flagging the file.
  • No security concerns. It only adds a trigger to a stub that forwards to a pinned first-party reusable (dependency-audit/v2-stable). Permissions stay contents: read, no secrets are used in run: steps, and no third-party reusable is added. Gitleaks, CodeQL, AgentShield and SonarCloud all pass. The run_secret_scanning MCP tool isn't available in this run; gitleaks passed.

CI status

All required checks pass: SonarCloud, CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems, build-and-test, coverage. Non-required: dependency-audit / npm audit FAILED (the lockfile advisories above, also failing on main). Other dev-lead/review runs show CANCELLED or SKIPPED because they were superseded.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@don-petry
don-petry disabled auto-merge October 2, 2026 20:03
@don-petry

Copy link
Copy Markdown
Collaborator Author

dependency-audit / npm audit verifies no known high/moderate advisories in the dependency tree; this diff makes that true by running npm audit fix to bump the vulnerable transitive brace-expansion and fast-uri in package-lock.json (not by relaxing the audit level). Local npm audit now reports 0 vulnerabilities; tests pass.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) October 2, 2026 20:04
@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

@donpetry-bot

donpetry-bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 548d788a440cedf8cde7fac98581c8d7b22ec26e — click to expand prior review.

Review — fix requested (cycle 2/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: 548d788a440cedf8cde7fac98581c8d7b22ec26e
Review mode: triage-approved (single reviewer)

Summary

This is a re-review after the dev-lead commit 548d788. That commit ran npm audit fix to fix the prior blocker, the failing dependency-audit / npm audit check. The code is correct and low-risk. It is held for two CI reasons. Playwright UI Tests is red on this commit, although it passed on the previous commit. And the npm audit check that this commit is meant to fix is still queued, so the fix isn't confirmed in CI yet.

Linked issue analysis

Closes #602 (stub-surface-drift-dependency-audit.yml-on). The workflow stub change is the same as in the prior review. It adds the merge_group: trigger and the header note, so the on: block matches the canonical template. The issue is fully addressed, and the new commit doesn't touch the workflow file.

Findings

Prior findings:

  • [Resolved in code, waiting on CI] dependency-audit / npm audit failing. Commit 548d788 bumps the vulnerable dev-only transitive packages in package-lock.json: brace-expansion 5.0.9 → 5.0.12 (high: GHSA-q2hr-2g5m-vwhr, GHSA-qhr7-859c-m2p7, GHSA-6j4f-fj2g-mc7p) and fast-uri 3.1.7 → 3.1.8 (moderate: GHSA-hrr3-gc8f-f4qj). Both are patch-level bumps that resolve from registry.npmjs.org. I checked both integrity hashes against npm view <pkg>@<ver> dist.integrity and they match. The audit level was not relaxed. The re-run of dependency-audit / npm audit (run 37058215516) is still queued, so CI hasn't confirmed the fix yet.
  • [Nit, already on main, unchanged] On the uses: line, the # NOSONAR comment has one space before it instead of two. This is cosmetic only.

New on this commit:

  • [Blocking: CI red] Playwright UI Tests failed (run 37058214960). 96 tests passed and 1 failed: deploy/tests/ui.spec.js:663 (calendar-to-sheets config form renders all fields and saves). It timed out after 15s waiting for #btn-deploy to become enabled. The same suite passed on the previous commit bea4ff8, on main (8318036), and on every other recent branch run. The only change since bea4ff8 is two dev-only transitive lockfile patch bumps, which are very unlikely to affect DOM button-enable logic. This is probably a flaky test, not a regression. Fix: re-run the Playwright job. If it fails again, investigate whether the lockfile bump is related.
  • No security concerns. The lockfile changes are integrity-verified patch bumps of dev dependencies. The workflow stub forwards only to the pinned first-party reusable, with contents: read. Gitleaks, CodeQL, AgentShield and SonarCloud pass. The run_secret_scanning MCP tool was not available in this run. There are no unresolved review threads.

CI status

All 6 required checks pass: SonarCloud, CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems, build-and-test, coverage. Node.js Tests and Secret scan (gitleaks) also pass. Non-required checks: Playwright UI Tests FAILED (one probably-flaky timeout; passed on the previous commit). dependency-audit / npm audit and autofix are QUEUED. The dev-lead runs show CANCELLED or SKIPPED because they were superseded.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-02T21:14:09Z.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Review — fix requested (cycle 3/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: 548d788a440cedf8cde7fac98581c8d7b22ec26e
Review mode: triage-approved (single reviewer)

Summary

This is a re-review at 548d788, the same commit the cycle-2 review looked at. Since that review, the dependency-audit / npm audit check finished and passed, so the lockfile fix is now confirmed in CI. The code is correct and low-risk. One blocker is left: Playwright UI Tests is still red. It has not been re-run (run 37058214960, attempt 1). The evidence points to a flaky test, not a regression, but CI is not green, so this PR can't be auto-approved yet.

Linked issue analysis

Closes #602 (stub-surface-drift-dependency-audit.yml-on). I compared the PR's dependency-audit.yml against the canonical petry-projects/.github/standards/workflows/dependency-audit.yml. The on: block now matches it, including the added merge_group: trigger. The only difference is the spacing before # NOSONAR on the uses: line. That spacing is already on main, and it isn't part of the on: drift check. The issue is fully addressed.

Findings

Prior findings:

  • [Resolved] dependency-audit / npm audit failing. It now passes on 548d788. The fix is two lockfile patch bumps for dev-only transitive packages: brace-expansion 5.0.9 → 5.0.12 and fast-uri 3.1.7 → 3.1.8. Both resolve from registry.npmjs.org, and the audit level was not relaxed.
  • [Still open, blocking: CI red] Playwright UI Tests failed. 1 test failed and 96 passed: deploy/tests/ui.spec.js:663 (calendar-to-sheets config form renders all fields and saves). It hit the 15s timeout in locator.click. This is still attempt 1; nobody has re-run it. The evidence says flaky: the suite passed on the previous commit bea4ff8 and on main (8318036). Two other branch runs (36627354185, 35813178576) failed the same way, with a 15s timeout in a different deploy/tests/ui.spec.js test each time. The only change in this commit is the two dev-only lockfile bumps. Fix: re-run the Playwright job. If it passes, this PR is ready to approve.
  • [Nit, already on main, unchanged] The # NOSONAR comment on the uses: line has one space before it instead of two. This is cosmetic only.

New since the prior review: No new commits and no new issues.

Security: No concerns. The workflow stub forwards only to the pinned first-party dependency-audit/v2-stable reusable, with contents: read. The lockfile changes are registry-resolved patch bumps. Gitleaks, CodeQL, AgentShield and SonarCloud all pass. The run_secret_scanning MCP tool was not available in this run. There are 0 unresolved review threads and no open questions from human reviewers.

CI status

All 6 required checks pass: SonarCloud, CodeQL, agent-shield / AgentShield, dependency-audit / Detect ecosystems, build-and-test, coverage. These also pass: dependency-audit / npm audit (the prior blocker), Node.js Tests, Secret scan (gitleaks), autofix, and the Analyze jobs. Playwright UI Tests (not required) FAILED with one probably-flaky timeout and has not been re-run. The dev-lead runs show CANCELLED because they were superseded. The audits for other ecosystems were SKIPPED because those ecosystems aren't used here.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compliance: stub-surface-drift-dependency-audit.yml-on

2 participants