Add release blessing surface registry#9201
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eee01eefcd
ℹ️ 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".
| *, | ||
| allow_legacy_desktop: bool = False, | ||
| ) -> SurfaceBlessing: | ||
| if allow_legacy_desktop and surface_id == "desktop-macos" and "blessed" in metadata: |
There was a problem hiding this comment.
Prefer namespaced desktop blessing before legacy fallback
When a release body keeps the legacy blessed keys for current promotion compatibility while also adding blessed.desktop-macos.*, this branch always ignores the namespaced desktop metadata. During the migration that can make a valid namespaced desktop blessing fail because stale legacy fields are missing/false, or let a stale legacy blessed: true pass even when the namespaced desktop blessing was cleared; prefer namespaced desktop metadata when present and only fall back to legacy fields otherwise.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
3 issues found across 3 files
Confidence score: 3/5
- In
.github/scripts/release_blessing.py, desktop promotion may prefer stale legacy blessing fields even whenblessed.desktop-macosmetadata exists, which can promote the wrong release state for desktop as-is — update resolution order to prefer namespaced metadata and only fall back to legacy keys. - In
.github/scripts/release_blessing.py, dependency closure checks can accept blessing data for the wrong surface if a mismatched dictionary key is supplied, so incorrect cross-surface blessing could be treated as valid — enforce thatSurfaceBlessing.surface_idmatchesrequired_surfacebefore accepting it. - In
.github/scripts/test_release_blessing.py, the legacy desktop test does not assertblessing.blessed_at, leaving a parsing regression undetected even ifblessedAtinput is broken — add a direct assertion onblessing.blessed_atbefore merging to lock in legacy compatibility.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
eee01ee to
3ee5ac4
Compare
|
Monitor review pass for this PR — no code change needed. Thread reviewed: This is already implemented correctly. The legacy fallback in This is covered by No change pushed; thread left unresolved (not addressed by code, since the behavior is already correct). |
kodjima33
left a comment
There was a problem hiding this comment.
Release blessing surface registry — approve-only; .github/ CI/CD surface, needs Nik.
Summary
Stack
Verification