Skip templated dependency audit on Dependabot PRs - #50
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
WalkthroughAdd a scheduled and manually triggered dependency audit, skip audit-only CI steps for Dependabot, add Makefile quality gates, update documentation and snapshots, and extend generated-template contract tests. ChangesAudit and tooling integration
Sequence Diagram(s)sequenceDiagram
participant Trigger
participant CIWorkflow
participant AuditWorkflow
participant CargoAudit
participant Makefile
Trigger->>CIWorkflow: Start pull-request CI
CIWorkflow->>CargoAudit: Install audit tooling for non-Dependabot actors
CIWorkflow->>Makefile: Run make audit for non-Dependabot actors
Trigger->>AuditWorkflow: Start weekly or manual audit
AuditWorkflow->>CargoAudit: Install cargo-audit
AuditWorkflow->>Makefile: Run make audit
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
leynos
marked this pull request as ready for review
July 3, 2026 08:26
![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
🤖 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 `@template/.github/workflows/audit.yml`:
- Around line 16-36: The audit workflow job is missing an explicit execution
bound, so add a timeout guard to the `audit` job itself in the GitHub Actions
workflow. Update the `jobs.audit` definition to include a `timeout-minutes`
limit so the unattended `cargo binstall` and `make audit` steps cannot hang
until the default runner timeout. Locate the change in the `audit` job block by
its `jobs.audit` key and keep the rest of the steps 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: ASSERTIVE
Plan: Pro Plus
Run ID: 48eb5045-df77-432b-961d-b3ab6bd9192f
📒 Files selected for processing (3)
template/.github/workflows/audit.ymltemplate/.github/workflows/ci.ymltests/test_template/__snapshots__/test_snapshots.ambr
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
lodyai
Bot
force-pushed
the
fix/skip-audit-for-dependabot
branch
from
July 19, 2026 17:42
1ccfb25 to
a111b8e
Compare
Owner
Author
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (1 error, 4 warnings)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR metadata and changed files ==='
gh pr view 50 --repo leynos/agent-template-rust --json number,headRefName,baseRefName,commits,files \
--jq '{number, headRefName, baseRefName, commits: [.commits[].oid], files: [.files[].path]}'
echo
echo '=== Relevant repository paths ==='
fd -t f -a . | rg '(^|/)(audit\.yml|ci\.yml|test_.*template|.*snapshot.*|users-guide.*|developers-guide.*|.*adr.*|.*execplan.*)$' || true
echo
echo '=== Workflow outlines ==='
for f in template/.github/workflows/ci.yml template/.github/workflows/audit.yml; do
if [ -f "$f" ]; then
echo "--- $f ---"
nl -ba "$f"
fi
done
echo
echo '=== Template test/snapshot references to workflow rendering and audit workflow ==='
rg -n -i -C 4 \
'audit\.yml|ci\.yml|workflow|dependabot\[bot\]|cargo-audit|make audit|schedule|workflow_dispatch' \
tests template docs README.md 2>/dev/null || true
echo
echo '=== Candidate documentation excerpts ==='
for f in $(fd -t f -a . docs | rg '(users-guide|developers-guide|adr|execplan)'); do
echo "--- $f ---"
rg -n -i -C 3 'dependabot|audit|workflow|security|ci' "$f" || true
done
echo
echo '=== Existing related GitHub issues ==='
gh issue list --repo leynos/agent-template-rust --state all --limit 100 \
--search 'Dependabot audit workflow scheduled timeout documentation snapshot' \
--json number,title,state,url \
--jq '.[] | "\(.state) #\(.number) \(.title) \(.url)"' || trueLength of output: 3531 The first inspection confirms that the PR now includes workflow, documentation, helper, behavioural-test, and snapshot changes, but the sandbox lacked |
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
buzzybee-df12
force-pushed
the
fix/skip-audit-for-dependabot
branch
from
July 21, 2026 11:47
3c1793b to
674c4df
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/helpers/tooling_contracts/workflows.py`:
- Around line 31-37: Reduce the _is_pinned_action docstring in
tests/helpers/tooling_contracts/workflows.py:31-37 to a concise single-line
summary, removing the extended rationale. Also reduce the _redact_pinned_shas
docstring in tests/test_template/test_snapshots.py:20-27 to one concise summary
line, preserving only its purpose.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 3705d3dc-64b0-4e3f-9626-100499bdc26f
📒 Files selected for processing (3)
tests/helpers/tooling_contracts/workflows.pytests/test_template/__snapshots__/test_snapshots.ambrtests/test_template/test_snapshots.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
2 tasks
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
`make audit` reports advisories against the whole lockfile, so in generated projects a newly published advisory fails every open Dependabot PR regardless of its content and deadlocks auto-merge. Condition the audit step and its audit-only prerequisites (cargo-audit install and the Python setup for manifest extraction) on the actor not being Dependabot, and add a templated weekly scheduled audit workflow as the compensating control so anything that slips through surfaces within days. Human pull requests keep the full audit gate.
The rendered ci.yml snapshot now includes the Dependabot conditions on the audit steps.
Add an explicit timeout to the generated scheduled audit workflow so unattended audit runs cannot hang until the GitHub runner default. Expose parent repository format, lint, and typecheck Makefile gates so the review-requested checks run as first-class commands. Keep those gates green by formatting the existing Python tests and tightening the small helper typing issues they surfaced.
Apply review feedback on the Dependabot audit-skip change:
- makefile.py: use the repository's string-annotation cast form
(`cast("str", ...)`) for the regex match group and drop the
constant-name `getattr` in the lazy make-parser import in favour of
direct `.make_load` attribute access.
- workflows.py: extend the scheduled audit workflow contract to require
least-privilege `contents: read` permissions and a checkout step that
disables credential persistence.
- test_helpers.py: replace the untyped recursive JSON strategy lambda
with a typed `_json_collections` helper, and add a failure message to
the resolved-socket assertion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Break the compound `assert checkout_steps and all(...)` in the scheduled audit workflow contract into two assertions: a presence check for the checkout step and the existing credential-persistence check. This satisfies Ruff PT018 while preserving both validations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the audit workflow tests resilient to Dependabot SHA bumps and
tighten the audit contract:
- test_snapshots.py: normalize every 40-hex SHA-pinned `uses:` ref to a
`@<sha>` placeholder before snapshot comparison, so routine dependency
bumps no longer break the structured-file snapshot. Regenerate the
affected snapshot.
- workflows.py: match the audit workflow's checkout and setup-python
steps by full 40-hex SHA pattern rather than exact SHA value, mirroring
the existing setup-rust check, and require the audit workflow triggers
to be exactly {schedule, workflow_dispatch} and its permissions to be
exactly {contents: read}.
The contract helpers still validate pinning by full 40-hex SHA on the raw
rendered text, so immutability is enforced without lockstep failures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Reduce the _is_pinned_action and _redact_pinned_shas docstrings to concise single-line summaries, dropping the extended rationale. - Document the public `make fmt` and `make typecheck` targets in both the parent and templated users guides, alongside the other public targets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the isinstance-based dict/list traversal guards in the workflow and snapshot test helpers with structural pattern matching via small typed helpers, preserving mapping and sequence semantics: - test_snapshots.py: _redact_pinned_shas now matches dict/list/other and delegates per-entry redaction to a typed _redact_mapping_entry helper. - workflows.py: add _step_mappings (match-based dict filter) and _disables_credential_persistence (match-based with-block check), and route the audit-workflow step filters through them, dropping the per-comprehension isinstance guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a FIXME on _redact_pinned_shas referencing issue #72, which proposes replacing the hand-rolled traversal with a syrupy `matcher=` as part of the planned snapshot-testing overhaul (once AST-aware Markdown snapshots are ready). Behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prior inverse filter collected steps that already had the Dependabot guard and compared their names, which proved the three audit steps were guarded but accepted an additional unguarded "Setup Python for audit manifest extraction" or "Audit dependencies" step, letting audit work run on Dependabot PRs. Filter by the three audit step names first, assert their expected order, then require every selected step to carry the Dependabot guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the guard-first audit-step assertion with a name-first check that collects every step matching the three audit step names as (name, if) tuples and asserts the ordered result exactly equals the three expected (name, guard) pairs. This fails if an audit step is duplicated without the guard, duplicated with a different guard, omitted, reordered, or left unguarded. Add a focused regression test that renders a valid CI workflow, appends an unguarded duplicate "Audit dependencies" step to build-test.steps, and asserts the contract raises AssertionError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The audit workflow checkout contract filtered checkout steps by _is_pinned_action before checking credential persistence, so an additional unpinned actions/checkout@<branch> step could persist credentials while the required pinned checkout still satisfied the contract. Collect every actions/checkout@ step, require exactly one, then assert full-SHA pinning and disabled credential persistence for it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the None: checkout_path entry from expected_steps. The dedicated checkout checks already require exactly one full-SHA-pinned checkout with disabled credential persistence regardless of name, so the unnamed constraint was redundant and would reject a compliant `name: Checkout source` step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the developer-documentation and property-coverage review warnings. Docs: - docs/developers-guide.md: document the parent make check-fmt/fmt/lint/ typecheck gates and their Ruff/mypy backing. - template/docs/developers-guide.md.jinja: document the generated project's make check-fmt/fmt/typecheck targets (rustfmt, mdformat, and cargo check). Property tests (Hypothesis): - test_helpers.py: pinned-SHA invariant for _is_pinned_action (accepts a full 40-hex SHA for the exact path; rejects short/long/uppercased/branch refs), the step-sequence invariant for _step_mappings (keeps only mappings, in order), and the credential-persistence predicate spec. - test_snapshots.py: _redact_pinned_shas strips every pinned SHA, is idempotent, preserves structure shape, and keeps the action path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add descriptive failure messages to the workflow-helper and snapshot redaction property assertions (exact/mismatched pin, non-SHA rejection, mapping-only ordering, credential verdict, pinned-SHA removal, idempotency, shape preservation, and path-preserving redaction). - Replace the isinstance branching in _iter_uses_values and _same_shape with structural match statements, preserving recursion into non-uses mapping entries and the key, length, container-type, and leaf-type comparisons. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the anonymous strategy lambdas with named, privately scoped, typed helper factories, preserving the generated values: - test_snapshots.py: _pinned_uses_ref, _unpinned_uses_ref, and the recursive _workflow_children child-strategy factory. - test_helpers.py: _with_persist_credentials for the persist-credentials with-block mapping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lodyai
Bot
force-pushed
the
fix/skip-audit-for-dependabot
branch
from
July 24, 2026 01:46
7d40c17 to
93ef049
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Makefile: add a shared REQUIRE_UVX prerequisite (same actionable installation message as the test target) to the check-fmt, fmt, lint, and typecheck recipes, so a missing uvx fails with a clear message rather than an obscure one. The test target keeps its existing guard. - test_helpers.py: dispatch _non_sha_refs on kind with an explicit match/case (branch is now its own case, not the fallback), and derive the "uppercased" ref by uppercasing the existing hex character at the chosen position (falling back to a hex letter for digits) instead of splicing in a literal "G". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_helpers.py (1)
1-567: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit
tests/test_helpers.py— it has blown well past the 400-logical-line cap.The visible span alone reaches line 567, with several sizeable unshown "unchanged" gaps between hunks, so the real file is considerably larger. This PR's own additions (the 163-289 property-test block is ~127 lines by itself) push it further over budget.
Extract the new
_is_pinned_action/_step_mappings/_disables_credential_persistenceHypothesis strategies and property tests (lines 163-289) into a dedicated module undertests/helpers/(e.g. alongsidetests/helpers/tooling_contracts/workflows.py, which they test directly), and consider moving the Docker-socket/actenvironment tests and the parent-Makefile contract test into their own files too.♻️ Suggested decomposition
tests/ test_helpers.py # keep the smaller, cohesive remainder test_workflow_contract_properties.py # move lines 163-289 here test_docker_environment.py # move Docker/act socket tests hereAs per path instructions, "Files must not exceed 400 logical lines: Decompose large modules into subpackages."
🤖 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 `@tests/test_helpers.py` around lines 1 - 567, Split the oversized tests/test_helpers.py into cohesive test modules, first moving the Hypothesis strategies and property tests for _is_pinned_action, _step_mappings, and _disables_credential_persistence into a dedicated workflow-contract properties module under tests/. Also extract the Docker/act environment tests and parent Makefile contract test into focused modules where appropriate, leaving test_helpers.py below the 400-logical-line limit and preserving all existing coverage.Source: Path instructions
🤖 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.
Outside diff comments:
In `@tests/test_helpers.py`:
- Around line 1-567: Split the oversized tests/test_helpers.py into cohesive
test modules, first moving the Hypothesis strategies and property tests for
_is_pinned_action, _step_mappings, and _disables_credential_persistence into a
dedicated workflow-contract properties module under tests/. Also extract the
Docker/act environment tests and parent Makefile contract test into focused
modules where appropriate, leaving test_helpers.py below the 400-logical-line
limit and preserving all existing coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ed000f18-b594-4125-b5ac-ea03d7e83ee6
📒 Files selected for processing (20)
Makefiledocs/developers-guide.mddocs/users-guide.mdtemplate/.github/workflows/audit.ymltemplate/.github/workflows/ci.yml.jinjatemplate/docs/developers-guide.md.jinjatemplate/docs/users-guide.md.jinjatests/__init__.pytests/helpers/__init__.pytests/helpers/tooling_contracts/cargo.pytests/helpers/tooling_contracts/makefile.pytests/helpers/tooling_contracts/orchestration.pytests/helpers/tooling_contracts/workflows.pytests/test_helpers.pytests/test_template/__snapshots__/test_snapshots.ambrtests/test_template/test_audit_targets.pytests/test_template/test_compilation.pytests/test_template/test_snapshots.pytests/test_template/test_tooling_contracts.pytests/utilities.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
💤 Files with no reviewable changes (2)
- tests/init.py
- tests/helpers/init.py
This was referenced Jul 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
make auditstep — and its audit-only prerequisites (cargo-auditinstall and the Python setup for manifest extraction) — ongithub.actor != 'dependabot[bot]'.audit.yml) as the compensating control, reusing the template's existing pinned action refs.Motivation
make auditreports advisories against the whole lockfile, so in generated projects a newly published advisory fails every open Dependabot PR regardless of its content, deadlocking Dependabot auto-merge (observed on leynos/corbusier#120, where even Dependabot's own fix for a vite advisory was blocked by unrelated advisories). The same change has been applied directly to the twelve existing estate repositories withmake auditgates; this PR brings the template in line so newly generated projects inherit it. Human pull requests keep the full audit gate.Test plan
actionlintclean on both templated workflow filesact-validationtemplate render passes in CIReferences