Repository navigation
Record accepted auto route and scoped Noema publication - #2632
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 49 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Noema LLM review
The ADR update is consistent, but the new regression test does not enforce the security-critical ordering it claims to cover: the read-only metadata refresh must occur before the repository-scoped write-token refresh. A workflow reordered to grant the write token first would still pass this test.
Reviewed changed lines
docs/adr/2026-10-11-public-personal-auto-authorization.md:5 (RIGHT): The status accurately records acceptance and preserves the separation between runtime App review and approval evidence.tests/test_noema_premerge_production_caller.py:36 (RIGHT): The test identifies the read-only metadata refresh block, but does not record its position for comparison with publication.tests/test_noema_premerge_production_caller.py:37 (RIGHT): The test identifies the repository-scoped publication block.tests/test_noema_premerge_production_caller.py:40 (RIGHT): The only ordering assertion compares model preparation with publication; it omits the required metadata-before-publication ordering.
Adversarial validation
tests/test_noema_premerge_production_caller.py:40 (RIGHT)confirmed: The new test rejects a workflow that performs the read-only metadata refresh after the write-scoped publication refresh. — The assertions only require read permission in the metadata block, write permission in the publication block, and Prepare Noema model verdict before publication. No assertion compares the metadata step index with the publication step index, so the reordered workflow still satisfies all checks.tests/test_noema_premerge_production_caller.py:38 (RIGHT)confirmed: The permission assertions alone guarantee that scope is read before write access is granted. — The blocks are independently extracted by name and checked only for their contained permission strings; their relative order is never validated.- Residual risk: A future workflow change could grant a repository-scoped write token before reading the pull request scope, while this regression test continues to pass.
Findings
- [medium] tests/test_noema_premerge_production_caller.py:40 (RIGHT): Add an explicit assertion that the read-only metadata refresh occurs before the repository-scoped publication refresh. As written, moving metadata inspection after write-token issuance would not fail the test, leaving the intended least-privilege ordering unprotected.
- Result: REQUEST_CHANGES
- Head SHA:
3e0709caa57fa76e31c2dfee01f49980e68ab4bc - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
There was a problem hiding this comment.
Noema LLM review
Approve the bounded documentation and test changes. The ADR explicitly separates accepted routing authorization from runtime App review and approval. Static adversarial checks show that the new test rejects a metadata permission upgrade and publication reordered before the metadata refresh. No blocking defect is established by the supplied diff.
Reviewed changed lines
docs/adr/2026-10-11-public-personal-auto-authorization.md:5 (RIGHT): The accepted status is explicitly qualified: runtime App review and approval remain separate evidence. This preserves the distinction between authorizing the route and approving a pull request.tests/test_noema_premerge_production_caller.py:36 (RIGHT): The permission check is limited to the named metadata step rather than searching the entire workflow. Removing that named step causes extraction to fail instead of allowing an unrelated read-permission declaration to satisfy the check.tests/test_noema_premerge_production_caller.py:38 (RIGHT): The assertion rejects replacing the metadata step's sole read-permission declaration with write permission. It remains a textual check, not validation of parsed YAML or effective token permissions.tests/test_noema_premerge_production_caller.py:40 (RIGHT): The relative-position assertion requires verdict preparation to precede the publication-token refresh.tests/test_noema_premerge_production_caller.py:41 (RIGHT): The assertion independently requires the metadata refresh to precede the publication-token refresh. Together with line 40, it checks both prerequisites without asserting that preparation precedes metadata refresh.
Adversarial validation
tests/test_noema_premerge_production_caller.py:38 (RIGHT)falsified: The added test would silently accept changing the metadata step's sole pull-request permission from read to write. — Static evaluation of the added membership assertion yields false because the extracted metadata body no longer contains permission-pull-requests: read. The test therefore fails on this mutation.tests/test_noema_premerge_production_caller.py:41 (RIGHT)falsified: The added test would silently accept minting the publication token before refreshing the metadata token. — The permission assertions can still succeed, but static evaluation makes the metadata step's source index greater than the publication step's index. The less-than assertion at this line is false, independently rejecting the ordering regression.- Residual risk: Validation was static; no tests were executed, and the referenced workflow was not supplied. These assertions do not establish effective repository scope, token freshness, or runtime App behavior. The ADR's stated merge date was not independently verified.
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
fcebff624b0d8ed6091353fc88ea8db1785b33fd - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
Record the accepted public
autoreview route after #2607 merged. Add a regression that binds ordinary Noema review to read-only PR metadata, then a fresh repository-scoped App write token only for publication. This exercises the restored normal review path without the unprovisioned premerge custody inputs.Validation: 6 focused tests pass with fatal warnings; git diff --check passes. Central Noema repair #2631 passed 672 related tests (2 skipped). This PR's live current-head App review and approval remain to be observed, not inferred.