Enforce injected environment access in generated Rust projects - #79
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Summary
WalkthroughThe template now enforces strict Rust and Rustdoc linting, injected environment access, warning-denied Makefile workflows, updated testing guidance, and contract tests for generated projects and documentation examples. ChangesTemplate policy enforcement
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (17 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideThe PR tightens lint and documentation enforcement for generated Rust projects, formalizes environment injection/testing policy in contributor guidance, updates Makefile contracts to treat Rustdoc warnings as errors, and adds tests that assert these policies are correctly rendered and enforced in generated projects. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 779e9bf1d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Makefile.jinja`:
- Line 41: Update the doctest command in the Makefile template to export
RUSTDOCFLAGS from RUSTDOC_FLAGS alongside the existing RUSTFLAGS assignment,
then update the related Makefile contract assertion and snapshot to require the
new command form.
- Line 16: Update the RUSTDOC_FLAGS assignment in the Makefile template to
always include --cfg docsrs and -D warnings while preserving any additional
inherited flags, rather than allowing an inherited value to replace the policy
flags.
🪄 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: 8d8b6f56-d01e-47cb-a467-196c9e6f6af5
📒 Files selected for processing (8)
template/AGENTS.md.jinjatemplate/Cargo.toml.jinjatemplate/Makefile.jinjatemplate/clippy.tomltemplate/tests/stub.rstests/helpers/tooling_contracts/makefile.pytests/test_template/__snapshots__/test_snapshots.ambrtests/test_template/test_lint_targets.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Deny assertion messages without context and direct process-environment operations in generated projects. Extend Rust and Rustdoc lint coverage and run doctests and documentation builds with warnings denied. Document environment injection as the required design, retain subprocess isolation as the sole testing exemption, and protect the generated contracts with focused rendering and lint-gate regressions.
Set `RUSTFLAGS` at target scope so the standard test runner and doctest command inherit one shared value without affecting unrelated targets.
Use the `mockable` 0.3 `DefaultEnv`, `Env::string`, and expectation APIs consistently across generated instructions and examples. Compile the actual documented dependency, production, and test snippets to prevent API drift. Describe subprocess isolation in terms of the generated application binary instead of a project-specific executable name.
Avoid an unused API-key binding in the production snippet and make the documentation regression enforce the same warning policy as CI.
Ensure inherited Rustdoc additions cannot replace the generated docsrs and warning-denial policy. Pass the resulting flags to doctests as well as documentation builds.
Require the generated Rustdoc flag composition to preserve optional Polonius support alongside inherited flags. Format the replayed Python tests so the rebased series passes the parent quality gates.
04bbec0 to
cb301ae
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@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, 2 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Prove the rendered contributor guidance and every practical Rust and Rustdoc rejection case through focused generated-project tests. Document warning-fatal validation, environment injection, migration steps, and the composition-root decision for maintainers and generated-project users.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/adr-005-injected-environment-boundary.md`:
- Around line 9-19: Rewrite the decision paragraph in the ADR for grammatical
clarity: separate the context, decision, and consequence into clear clauses; use
“decided to inject” and “reject direct” with parallel verb structure; preserve
the existing technical decisions and outcomes; and include the required
“Triage:” annotation for this Markdown grammar finding.
In `@docs/developers-guide.md`:
- Around line 28-37: Update the generated AGENTS.md command guidance in
template/AGENTS.md.jinja so make lint includes the warning-denied documentation
build before Clippy and Whitaker, matching the contract described in the
generated lint documentation. Preserve the existing validation commands and add
a rendered-output assertion if the command list is covered by tests.
🪄 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: 61d3a255-9016-448e-996b-3d2a6a186ae2
📒 Files selected for processing (7)
docs/adr-005-injected-environment-boundary.mddocs/developers-guide.mddocs/users-guide.mdtemplate/AGENTS.md.jinjatemplate/docs/users-guide.md.jinjatests/test_template/test_lint_targets.pytests/test_template/test_policy_guidance.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
Separate the environment boundary decision into parallel clauses with its review triage. Document the complete generated lint command sequence and prove the rendered guidance stays aligned with the Makefile contract.
Assert each new testing and maintenance rule in the rendered `AGENTS.md`. Render the disposable Rust test stub with the selected package name so its metadata check proves the generated project contract.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/users-guide.md`:
- Around line 91-93: Complete the environment migration guidance in
docs/users-guide.md lines 91-93 by adding actions for declaring the mockable
dependency, wiring the Env adapter at the composition root while retaining
DefaultEnv for production, and documenting contributor guidance; mirror the same
complete guidance in template/docs/users-guide.md.jinja lines 61-63 so generated
guides stay consistent.
In `@template/AGENTS.md.jinja`:
- Around line 171-179: Update the following `make test` guidance in
`template/AGENTS.md.jinja` to match the generated Makefile: document the
`$(CARGO) $(TEST_CMD)` command with its `cargo-nextest` fallback, warning-denial
`RUSTFLAGS`, and separate all-feature workspace doctests using `RUSTDOCFLAGS`,
replacing the stale `cargo test --workspace` command and description.
In `@tests/test_template/test_policy_guidance.py`:
- Around line 47-51: Parenthesize the adjacent multi-line string literals in the
test command list to satisfy ISC004, preserving the exact asserted Makefile
command text and the surrounding test structure.
🪄 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: 801cf0d8-4b6a-4c36-8b4c-d0c47d85929f
📒 Files selected for processing (8)
docs/adr-005-injected-environment-boundary.mddocs/developers-guide.mddocs/users-guide.mdtemplate/AGENTS.md.jinjatemplate/docs/users-guide.md.jinjatemplate/tests/stub.rs.jinjatests/test_template/test_lint_targets.pytests/test_template/test_policy_guidance.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
@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 check (1 error)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="leynos/agent-template-rust"
pr="79"
echo "=== PR checks ==="
gh pr view "$pr" --repo "$repo" --json url,headRefOid,statusCheckRollup \
--jq '{url, headRefOid, checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}]}'
echo
echo "=== PR files relevant to the finding ==="
gh pr view "$pr" --repo "$repo" --json files \
--jq '.files[].path' | rg '(^template/AGENTS\.md\.jinja$|^template/tests/stub\.rs\.jinja$|test_policy_guidance\.py$|test_compilation\.py$|test_lint_targets\.py$)' || true
echo
echo "=== Test and template outlines ==="
for path in \
tests/test_template/test_policy_guidance.py \
tests/test_template/test_compilation.py \
template/AGENTS.md.jinja \
template/tests/stub.rs.jinja
do
if [ -f "$path" ]; then
echo "--- $path ---"
ast-grep outline "$path" --items all 2>/dev/null || true
fi
done
echo
echo "=== Rendered-guidance and package-metadata assertions ==="
rg -n -i -C 4 \
'AGENTS\.md|execplan|historical record|Box|Arc|insta|trybuild|CARGO_PKG_NAME|package_name|non.empty|nonempty' \
tests/test_template/test_policy_guidance.py \
tests/test_template/test_compilation.py \
tests/test_template/test_lint_targets.py \
template/AGENTS.md.jinja \
template/tests/stub.rs.jinja 2>/dev/null || true
echo
echo "=== Relevant PR diff ==="
git diff --unified=12 "$(git merge-base origin/main HEAD)" HEAD -- \
tests/test_template/test_policy_guidance.py \
tests/test_template/test_compilation.py \
template/AGENTS.md.jinja \
template/tests/stub.rs.jinjaLength of output: 23856 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="leynos/agent-template-rust"
pr="79"
echo "=== Complete rendered-guidance test ==="
sed -n '1,140p' tests/test_template/test_policy_guidance.py
echo
echo "=== AGENTS patch from the GitHub PR API ==="
gh api --paginate "repos/${repo}/pulls/${pr}/files?per_page=100" \
--jq '.[] | select(.filename == "template/AGENTS.md.jinja") | .patch'
echo
echo "=== Focused-test patch from the GitHub PR API ==="
gh api --paginate "repos/${repo}/pulls/${pr}/files?per_page=100" \
--jq '.[] | select(.filename == "tests/test_template/test_policy_guidance.py") | .patch'
echo
echo "=== Stub patch from the GitHub PR API ==="
gh api --paginate "repos/${repo}/pulls/${pr}/files?per_page=100" \
--jq '.[] | select(.filename == "template/tests/stub.rs.jinja") | .patch'Length of output: 19293
The current PR head is
The same file also renders a project with assert_eq!(
env!("CARGO_PKG_NAME"),
"metadata_stub_example",This prevents the stub from passing with an unrelated non-empty constant. Do not create a follow-up issue. No further work is required for this finding. You are interacting with an AI system. |
Document the complete environment-injection migration and the generated test runner contract. Guard the rendered commands and migration steps while making adjacent command literals explicit for ISC004.
|
@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). Testing (Compile-Time / Ui) |
Summary
This branch makes environment injection and assertion diagnostics enforceable
defaults in generated Rust projects. It denies direct process-environment
operations, strengthens Rust and Rustdoc lint policy, and ensures documentation
and doctest warnings fail the public gates.
The generated contributor guidance now requires injected environment readers,
rejects process-wide locking as a testing workaround, and reserves environment
configuration for isolated child processes.
Review walkthrough
and Clippy configuration
to review the lint levels and reasoned forbidden-method policy.
and generated contributor guidance
for the Rustdoc, doctest, environment-injection, error-shape, and testing
contracts.
to see the ambient runtime environment read replaced by compile-time Cargo
metadata.
Makefile contract assertions,
and reviewed generated snapshot.
Validation
make check-fmt: passedmake lint: passedmake typecheck: passed; 29 source files checkedmake test: passed; 87 passed and 1 skippedmake spelling: passedrejection cases
git diff --check origin/main...HEAD: passedNotes
The template currently renders one package and no additional workspace member
manifests. Its package-local lint tables therefore cover the complete generated
workspace; future member manifests will still need to opt into workspace lints
or declare equivalent package-local levels.
References