Add metadata fixture UTF-8 safety tests#121
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.
|
@coderabbitai review |
|
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 (4)
✅ Files skipped from review due to trivial changes (4)
📝 WalkthroughWalkthroughAdds fixture-level negative tests for invalid UTF-8 in metadata/animation data URIs, enforces LF line endings for release artifacts/JS/Python via .gitattributes, updates docs to separate fixture-level checks from remaining production enforcement, and refreshes release manifest checksums and ops/roadmap entries. ChangesMetadata Fixture Safety & Release Artifact Consistency
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 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 |
✅ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
release-artifacts/latest/release-manifest.json (1)
232-257:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegenerate and recommit
release-manifest.jsonto clear manifest driftCI already shows this file is stale:
python3 scripts/generate_release_manifest.py --checkreportschanged release-artifacts/latest/release-manifest.json. Until that is fixed, Line 234–235 and Line 255–256 cannot be treated as authoritative for release integrity.🤖 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 `@release-artifacts/latest/release-manifest.json` around lines 232 - 257, Run the release manifest generator and commit the updated JSON to eliminate drift: execute the same command used by CI (python3 scripts/generate_release_manifest.py --check) or run python3 scripts/generate_release_manifest.py to regenerate release-artifacts/latest/release-manifest.json, verify the output, and recommit the updated release-manifest.json so the sha256/size entries are consistent with the current files.Source: Pipeline failures
scripts/test_metadata_fixtures.py (1)
63-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRespect explicit malformed URI inputs.
Using
orhere silently turns an explicit empty-string malformed URI into a valid generated fixture. Switch to an explicitis not Nonecheck so the helper can faithfully build intentionally bad fixtures.🔧 Proposed fix
- pending_uri = malformed_pending_uri or encode_data_uri( - checker.JSON_DATA_URI_PREFIX, json.dumps(pending_metadata, separators=(",", ":")) - ) + pending_uri = ( + malformed_pending_uri + if malformed_pending_uri is not None + else encode_data_uri( + checker.JSON_DATA_URI_PREFIX, json.dumps(pending_metadata, separators=(",", ":")) + ) + ) @@ - final_metadata["animation_url"] = malformed_animation_uri or encode_data_uri( - checker.HTML_DATA_URI_PREFIX, animation_html - ) + final_metadata["animation_url"] = ( + malformed_animation_uri + if malformed_animation_uri is not None + else encode_data_uri(checker.HTML_DATA_URI_PREFIX, animation_html) + )Also applies to: 78-80
🤖 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_metadata_fixtures.py` around lines 63 - 65, The current construction of pending_uri uses "malformed_pending_uri or encode_data_uri(...)" which treats an explicit empty-string malformed_pending_uri as falsy and overrides the intent; change this to an explicit None check: set pending_uri to malformed_pending_uri if malformed_pending_uri is not None, otherwise call encode_data_uri with checker.JSON_DATA_URI_PREFIX and json.dumps(pending_metadata, separators=(",", ":")); make the same change for the corresponding final_uri block (the similar code at the later section around lines 78-80) so an explicit "" malformed URI is preserved rather than replaced.
🤖 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/ROADMAP.md`:
- Around line 1635-1637: Update the remaining-work bullet that currently reads
"Complete the remaining render-safety work for full browser execution
sandboxing, production invalid UTF-8 enforcement, and production structured
attributes or semantic attribute schema validation" to explicitly call out
"production raw-attribute enforcement" (e.g., append or insert "production
raw-attribute enforcement" alongside the structured/semantic attribute
validation wording) so the roadmap/test matrix clearly distinguishes
fixture-level coverage from remaining production enforcement work.
---
Outside diff comments:
In `@release-artifacts/latest/release-manifest.json`:
- Around line 232-257: Run the release manifest generator and commit the updated
JSON to eliminate drift: execute the same command used by CI (python3
scripts/generate_release_manifest.py --check) or run python3
scripts/generate_release_manifest.py to regenerate
release-artifacts/latest/release-manifest.json, verify the output, and recommit
the updated release-manifest.json so the sha256/size entries are consistent with
the current files.
In `@scripts/test_metadata_fixtures.py`:
- Around line 63-65: The current construction of pending_uri uses
"malformed_pending_uri or encode_data_uri(...)" which treats an explicit
empty-string malformed_pending_uri as falsy and overrides the intent; change
this to an explicit None check: set pending_uri to malformed_pending_uri if
malformed_pending_uri is not None, otherwise call encode_data_uri with
checker.JSON_DATA_URI_PREFIX and json.dumps(pending_metadata, separators=(",",
":")); make the same change for the corresponding final_uri block (the similar
code at the later section around lines 78-80) so an explicit "" malformed URI is
preserved rather than replaced.
🪄 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: e46a2a01-1356-48fb-86b1-660dbd74f5ce
📒 Files selected for processing (12)
.gitattributesCHANGELOG.mddocs/metadata.mddocs/status.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.mdrelease-artifacts/README.mdrelease-artifacts/latest/SHA256SUMSrelease-artifacts/latest/release-checksums.jsonrelease-artifacts/latest/release-manifest.jsonscripts/test_metadata_fixtures.pytest/README.md
|
CI follow-up pushed in |
|
CodeRabbit follow-up pushed in
The prior release-manifest drift finding was fixed in |
|
@coderabbitai review Please review the latest head after the follow-up fixes for the prior comments. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get current state of the PR diff for the follow-up commits
gh pr view 121 --repo 6529-Collections/6529Stream --json headRefName,headRefOid,commits | jq '{branch: .headRefName, head: .headRefOid, lastCommits: .commits[-5:]}'
# Get the latest diff
gh pr diff 121 --repo 6529-Collections/6529Stream | head -500Length of output: 39372 🏁 Script executed: #!/bin/bash
# Read the key files changed in the follow-up
cat scripts/test_metadata_fixtures.pyLength of output: 8977 🏁 Script executed: #!/bin/bash
# Check roadmap changes
cat ops/ROADMAP.md
# Check autonomous run state
tail -80 ops/AUTONOMOUS_RUN.mdLength of output: 161873 🏁 Script executed: #!/bin/bash
# Verify release manifest consistency
cat release-artifacts/latest/release-manifest.json | python3 -c "import json,sys; data=json.load(sys.stdin); print(json.dumps(data, indent=2))" | head -100
# Check SHA256SUMS
cat release-artifacts/latest/SHA256SUMS
# Verify checksums file
cat release-artifacts/latest/release-checksums.json | python3 -c "import json,sys; data=json.load(sys.stdin); print(json.dumps(data, indent=2))"Length of output: 12441 🏁 Script executed: #!/bin/bash
# Check .gitattributes
cat .gitattributes
# Check docs changes
cat docs/metadata.md | head -100
# Check test README
cat test/README.mdLength of output: 20851
Follow-up fixes confirmed (
Release manifest chain verified (
Docs and roadmap
No actionable findings on the current head. The PR is good to merge. [approve] |
1 participant
Closes #119.
Closes #120.
Summary
Notes
StreamCoreremains at 24,135 runtime bytes with 441 bytes of EIP-170 headroom.Validation
python scripts\test_metadata_fixtures.pypython scripts\check_metadata_fixtures.pymake metadata-fixtures-checkpython scripts\generate_dependency_artifact_manifest.py --checkpython scripts\test_dependency_artifact_manifest.pypython -m py_compile scripts\check_metadata_fixtures.py scripts\test_metadata_fixtures.py scripts\generate_dependency_artifact_manifest.py scripts\test_dependency_artifact_manifest.pymake release-checksumsmake release-manifest-checkmake release-checksums-checkpython scripts\check_changelog.pymake checkpowershell -ExecutionPolicy Bypass -File scripts\check.ps1bash -n scripts/check.shgit diff --checkSummary by CodeRabbit
New Features
Documentation
Tests
Chores