Add deterministic Foundry broadcast manifest ingestion#110
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds deterministic ingestion of sanitized Foundry broadcast JSON: a generator CLI produces broadcast-derived deployment-manifest inputs (with a --check drift mode), a unittest suite validates generator behavior, CI/Makefile/scripts are wired to run the checks, broadcast-derived artifacts are committed and included in release-manifest/checksum outputs, and documentation/ops entries are updated. ChangesFoundry Broadcast Manifest Ingestion
Estimated code review effort 🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (1)
scripts/test_broadcast_manifest_input.py (1)
297-313: ⚡ Quick winAdd a regression test for secret-key casing/format variants.
Please extend this test to include at least one variant key (for example
PrivateKey/RPCURL) so sanitization hardening is CI-protected.Proposed test addition
def test_generator_rejects_secret_like_keys(self) -> None: @@ with self.assertRaisesRegex(generator.BroadcastManifestError, "forbidden"): generator.build_manifest_input( template, broadcast, root / "out.json", root / "manifest.json", ) + + def test_generator_rejects_secret_like_key_variants(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + template = template_config(root) + broadcast = broadcast_file(root) + data = generator.load_json(broadcast) + data["PrivateKey"] = "not-for-commit" + write_json(broadcast, data) + + with self.assertRaisesRegex(generator.BroadcastManifestError, "forbidden"): + generator.build_manifest_input( + template, + broadcast, + root / "out.json", + root / "manifest.json", + )🤖 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 `@scripts/test_broadcast_manifest_input.py` around lines 297 - 313, Extend test_generator_rejects_secret_like_keys to add additional key-casing/format variants (e.g., "PrivateKey", "RPCURL") to the broadcast JSON and assert generator.build_manifest_input raises generator.BroadcastManifestError for each variant; use generator.load_json and write_json as already used to mutate the broadcast, and either loop over a list of forbidden variants or add separate assertions to ensure sanitization rejects mixed-case and alternate-format secret-like keys.
🤖 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 `@scripts/generate_broadcast_manifest_input.py`:
- Around line 119-124: The current exact-string check in assert_no_secret_keys
allows casing/format variants to bypass forbidden-key detection; normalize keys
before comparison by creating a normalized forbidden set from
FORBIDDEN_SECRET_KEYS (e.g., lowercase and strip non-alphanumeric characters)
and compare against a similarly normalized form of each inspected key inside
assert_no_secret_keys (refer to the function name and FORBIDDEN_SECRET_KEYS
symbol). Ensure normalization is applied consistently (module-level normalized
set) and use that when raising BroadcastManifestError so keys like "PrivateKey",
"private_key", or "RPC-URL" are caught.
---
Nitpick comments:
In `@scripts/test_broadcast_manifest_input.py`:
- Around line 297-313: Extend test_generator_rejects_secret_like_keys to add
additional key-casing/format variants (e.g., "PrivateKey", "RPCURL") to the
broadcast JSON and assert generator.build_manifest_input raises
generator.BroadcastManifestError for each variant; use generator.load_json and
write_json as already used to mutate the broadcast, and either loop over a list
of forbidden variants or add separate assertions to ensure sanitization rejects
mixed-case and alternate-format secret-like keys.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e5bb0b5-9914-4848-8b45-8deec66febcf
📒 Files selected for processing (25)
.github/workflows/ci.ymlCHANGELOG.mdMakefiledeployments/README.mddeployments/address-books/anvil-6529stream-v0.1.0-001-broadcast.jsondeployments/broadcasts/anvil-6529stream-v0.1.0-001-run-latest.jsondeployments/config/anvil-6529stream-v0.1.0-001-broadcast.jsondeployments/examples/anvil-6529stream-v0.1.0-001-broadcast.jsondocs/deployment.mddocs/release-policy.mddocs/status.mddocs/tooling.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.mdrelease-artifacts/latest/SHA256SUMSrelease-artifacts/latest/release-checksums.jsonrelease-artifacts/latest/release-manifest.jsonscripts/check.ps1scripts/check.shscripts/generate_address_books.pyscripts/generate_broadcast_manifest_input.pyscripts/generate_release_checksums.pyscripts/generate_release_manifest.pyscripts/test_broadcast_manifest_input.pyscripts/test_release_manifest.py
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 `@ops/AUTONOMOUS_RUN.md`:
- Line 5071: Replace the non-deterministic placeholder line "Head SHA: latest
branch head after CodeRabbit-response push." with the concrete 40-character
commit SHA for the referenced head (or include both pre/post-change SHAs if
relevant) so the AUTONOMOUS_RUN entry is reproducible; locate the exact string
in AUTONOMOUS_RUN.md and substitute the placeholder with the actual commit
SHA(s) (e.g., "Head SHA: <40-char SHA>" or "Head SHA: <oldSHA> -> <newSHA>").
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36219eb1-6236-4c81-a118-e859fcfd338e
📒 Files selected for processing (3)
ops/AUTONOMOUS_RUN.mdscripts/generate_broadcast_manifest_input.pyscripts/test_broadcast_manifest_input.py
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/test_broadcast_manifest_input.py
- scripts/generate_broadcast_manifest_input.py
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
1 participant
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
Implements P1-DEPLOY-004 by adding an offline, deterministic bridge from sanitized Foundry
run-latest.jsonbroadcast output into deployment-manifest evidence.scripts/generate_broadcast_manifest_input.pyand focused tests for chain ID, deployment contract set, receipt success, tx/address matching, duplicate/missing/unexpected deployments, secret-like key rejection, boolean receipt status rejection, and check-mode drift.Closes #109.
Validation
python scripts\test_broadcast_manifest_input.pypython scripts\generate_broadcast_manifest_input.py --checkpython scripts\test_deployment_manifest.pypython scripts\generate_deployment_manifest.py --checkpython scripts\generate_deployment_manifest.py --config deployments\config\anvil-6529stream-v0.1.0-001-broadcast.json --checkpython scripts\test_address_books.pypython scripts\generate_address_books.py --checkpython scripts\test_release_manifest.pypython scripts\generate_release_manifest.py --checkpython scripts\test_release_checksums.pypython scripts\generate_release_checksums.py --checkpython scripts\test_changelog_check.pypython scripts\check_changelog.pypython -m py_compile ...for release/deployment helper scriptsbash -n scripts/check.sh scripts/bootstrap-ec2.shscripts/check.ps1andscripts/bootstrap-windows.ps1git diff --check(line-ending warnings only)make checkpowershell -ExecutionPolicy Bypass -File scripts\check.ps1Notes
This PR intentionally does not add live fork/testnet broadcasting, explorer submission, production contract verification, detached checksum signatures, or signed tags. Those remain Gate E/G release-ceremony work.
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests