Skip to content

UN-4522 [MISC] Release branching V2: port release-line workflows - #2381

Open
chandrasekharan-zipstack wants to merge 31 commits into
mainfrom
un-4522/release-v2-read-only
Open

chandrasekharan-zipstack wants to merge 31 commits into
mainfrom
un-4522/release-v2-read-only

Conversation

@chandrasekharan-zipstack

@chandrasekharan-zipstack chandrasekharan-zipstack commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What

Release branching for this repo. Each enterprise release has its own line here. A line is a release/X.Y branch that carries ee-vX.Y.Z(-rc.N) tags. Fixes merged to main are cherry-picked onto the lines that still need them. Community releases do not change: they are still cut from main with create-release.yaml as plain vX.Y.Z tags.

  • .github/release/: the release logic and its tests.

    • relmgr.py: commit classification, version allocation and the PR title rule.
    • relstate.py: release state, derived from git only.
    • backport.py and backport_merge.py: picks and conflict PRs.
    • convgate.py: the convergence gate.
    • oss.py: the line operations that the enterprise repo's release workflows run against this repo.

    This directory is byte-identical to the enterprise repo's copy. relstate.REPOS holds both repos' settings and picks one by GITHUB_REPOSITORY.

    The scripts refuse unsafe CLI arguments: a value that starts with - is refused, and a repo, state or output path must resolve under the working directory, GITHUB_WORKSPACE, RUNNER_TEMP or the temp dir (GITHUB_STEP_SUMMARY is trusted). To run a script by hand against a clone outside those roots, cd into the clone first.

  • state.yaml (dispatch only, read-only): prints every release/X.Y branch with its state.

  • lint-pr.yaml: checks every PR title into main or release/** against the rule that the release changelog depends on, [TYPE] text or TICKET [TYPE] text. PRs from forks are checked too, title and labels, including reruns on an edit or a label change. The run reads only the event payload and main's relmgr, with a read-only token.

  • backport.yaml:

    • Each push to main picks fixes onto the release lines. A clean pick is merged by the bot. A conflict opens a PR for a person to resolve.
    • When someone closes a conflict PR, the workflow records how it was closed.
  • release-branch-ci.yaml: every commit on a release line must be a pick from main, a decline, a release-branch-only change or a build-generated commit. A failed push run raises no alert here: this repo has no release alert channel or webhook secret.

  • release-branch-only.yaml (pull_request_target, when the release-branch-only label is added to a PR into release/**): adds one empty bot commit carrying Release-branch-only: <TICKET> (ticket from the title, else #<PR number>) to the PR head, unless a commit there already has the line. It runs only for an open PR, refuses a head named main, release/* or backport/*, fails with a message for a fork head, and mints the App token only when it has a commit to push.

  • release-tooling-tests.yaml: runs every .github/release/test_*.py on a pull request that changes .github/release/** or one of the release workflows.

  • run-platform.sh: latest now resolves from plain vX.Y.Z tags only, so an ee- line tag never becomes a community install.

This repo has no old release workflow to retire. create-release.yaml stays as it is for community releases from main.

Where release state comes from

All release state comes from git: a line's own ee-vX.Y.Z tags, plus a stabilized/X.Y tag. No registry is read, and no workflow here reads the enterprise repo's state.

A line takes fixes (stabilizing) until every enabled target of its paired enterprise line is GA, not just until its own first ee-vX.Y.0. The enterprise repo's Promote marks that point by writing stabilized/X.Y here, on the stable tag's commit, in the promote that leaves the last of those targets GA. That tag marks the promote, not any registry push after it. If the enterprise line later takes an RC for a target with no GA yet (for example on-prem enabled after the cloud GA), the enterprise RC build deletes stabilized/X.Y: the line is stabilizing again, and Promote writes the tag again at its last GA. Nobody deletes it by hand.

Tokens and secrets

These workflows name no new secret. They use the following:

  • GITHUB_TOKEN, for reads.

  • The GitHub App that create-release.yaml already uses (vars.PUSH_TO_MAIN_APP_CLIENT_ID, secrets.PUSH_TO_MAIN_APP_PRIVATE_KEY). Backport and Release branch only (release-branch-only.yaml) need it for the pushes and PRs that must start other workflows: GitHub does not start a workflow for a push or PR made with GITHUB_TOKEN.

    Both Backport jobs also pass the App's slug from the token step. record records a pick PR, and pick squash-merges a leftover pick PR, only when its author is that App, which shows as app/<slug>.

What this needs from settings (not in this PR)

  • The App's permissions: it needs contents write, pull requests write and issues write (labels and comments) on this repo.
  • The App as bypass actor: it must be a bypass actor on the new release/* ruleset, because it merges clean picks.
  • Labels: backport-conflict, backport-merged, backport-declined, release-branch-only, fix/security, fix/critical and fix/needs-backport. None of them exists today.
  • release/* ruleset: squash merge only, 1 approval with stale reviews dismissed on push, and required checks check and lint-pr.
  • Tag ruleset for ee-v* and stabilized/* (optional): the existing "Protect release tags" ruleset covers refs/tags/v* only, so nothing protects ee- or stabilized/ tags today. If a ruleset is added for them, the App needs to bypass it, because it creates both and deletes stabilized/ tags.
  • Merge settings: the squash commit message must stay COMMIT_MESSAGES (current setting). The Release-branch-only: trailer survives only in that format.

Rollout

Merging this PR switches the workflows on. On main they change nothing until a release line exists: today's state is no branches and no next minor, and a Backport rescan picks nothing. Both results come from running the workflows' scripts locally against a clone of this repo; they wrote nothing.

The enterprise repo cuts the first line. Its first Cut is run by hand with line=0.185, and numbering is automatic after that. The cut creates release/0.185 here and tags ee-v0.185.0-rc.1. Line tags (ee-vX.Y.Z and ee-vX.Y.Z-rc.N) are git tags only: they create no GitHub release, so no Docker Hub build runs for them. Community images still come only from create-release.yaml's plain vX.Y.Z releases and :latest.

PR title rule against recent history

  • Method: I ran relmgr.pr_fix_class(title, labels) over the most recent merged PRs from gh pr list --state merged.
  • Last 100 merged PRs (2026-08-07 to 2026-10-26, 91 of them into main): 2 fail: one uses [PERF] and one is fix(mcp): .... Leading spaces in a title are stripped before the check. None of the 100 came from a fork.
  • Fork PRs:
    • 6 of the last 1000 merged PRs came from forks, and 2 of those 6 fail.
    • Of the 12 fork PRs among the last 300 PRs in any state, 10 fail as submitted. Every failure is an open or closed-unmerged PR, and the two that pass were merged, so contributors would need to retitle.

The check blocks nothing until a ruleset requires it. Once one does, a fork PR titled like the failures above is blocked until it is retitled.

lint-pr on this PR skips with a notice: it reads the rule from main, and main has no .github/release/relmgr.py yet.

Existing tags

This repo has 616 tags. None of them has the ee- prefix, so the release tooling ignores every one of them: 615 are plain vX.Y.Z, and the other is v0.161.4_1.

Code style

.github/release/ruff.toml extends the repo's ruff settings. It ignores three rules that the release tooling relies on:

  • E731: short local lambdas.
  • E741: l as the name for a label.
  • E402 in tests: they set GITHUB_REPOSITORY before they import relstate.

Tests

The release test suites (python3 .github/release/test_*.py) all pass, locally and in the new Release tooling tests workflow on this PR: backport_merge 23, backport 39, convgate 39, links 2, oss 46, relstate 59. The 19 test_oss.py tests that check the enterprise repo's workflows are skipped here; they pass against that repo's workflow files.

The guards were mutation-checked: each one-line break was killed by the suites.

The flows were also run end to end in a private sandbox copy of this repo and the enterprise repo: cut, take RC, promote, backport, the stabilized/X.Y write and reopen, a failed build, a short-sha decline, and retire. They have not run against the real repos or App.

🤖 Generated with Claude Code

Adds the release tooling under .github/release and two workflows that write
nothing: state.yaml prints every release/X.Y branch with its state, and
lint-pr.yaml checks PR titles against the release rule. The OSS line reads
its own ee-vX.Y.Z git tags; no registry is read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
…un-platform.sh

Backport picks each fix merged to main onto the release lines and records
how people close backport conflict PRs. Release branch CI checks that every
commit on a release line is a pick, a decline, release-branch-only or
build-generated. Both use the repo's existing GitHub App token for the pushes
and PRs that must start other workflows.

run-platform.sh now resolves "latest" from plain vX.Y.Z tags only, so the
ee-vX.Y.Z(-rc.N) line tags never become a community install.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Comment thread .github/release/backport.py Fixed
Comment thread .github/release/convgate.py Fixed
Comment thread .github/release/convgate.py Fixed
Comment thread .github/release/oss.py Fixed
Comment thread .github/release/relmgr.py Fixed
Comment thread .github/release/relmgr.py Fixed
Comment thread .github/workflows/release-branch-ci.yaml Fixed
Comment thread .github/workflows/release-branch-ci.yaml Fixed
Comment thread .github/workflows/release-branch-ci.yaml Fixed
Comment thread .github/workflows/release-branch-ci.yaml Fixed
Registry reads move to Artifact Registry behind RELSTATE_AR_* variables that
the calling workflow sets, so no registry location lives in this directory.
OSS lines still read their own git tags. Includes that copy's review fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 1. Tip 4910d26f. Review: /unstract:review-deep (INITIAL), the binding review for every round of this PR.

Review — #2381 (round 1 of self-review, INITIAL)

Reviewed under unstract plugin 2.1.74 (/unstract:review-deep). Head 4910d26f28f5715bd555cb5c5246b04bf8bee40e, merge base ee81e18d6678a2adae3294efcfd6ecf83ce42258 (base fetch ok). 18 files, 8837 lines; count verified against GitHub. Mode: INITIAL (no prior review by this reviewer).

Signals: lenses 4, 8, 11, 13, 16, 17 raised (App token and pull_request_target in workflows, gh/git subprocess calls, four new CI workflows, ~4700 test lines, heavy comment blocks, a changed PR-title rule and "latest" tag rule).

Verdict — BLOCK

Summary — Critical: 0 · High: 3 · Medium: 12 · Low: 9 · Lenses run: 19/19 (12, 18, 19 N/A with reason)

Security deep-dive: done (find sub-task returned; no candidate at confidence >= 0.7, so no filter sub-tasks were needed; nothing reported at >= 8).
boundary-sweep: 9 sinks traced, clean (step summaries, ::error::/::notice:: lines, PR comments, commit messages; the App token reaches git only through a credential helper and is never echoed).

Note on ownership: everything under .github/release/ must stay byte-identical with the enterprise repo's copy, so findings there are fixed upstream and copied in, not edited here.

Findings

Each finding is an inline comment.

  • #1 [High] The record job can never match the bot, so every record run fails once the token is an App installation token (.github/release/backport_merge.py:281)
  • #2 [High] No test covers a forged cherry-pick trailer, which is the release-branch CI integrity check's main negative case (.github/release/relmgr.py:163)
  • #3 [High] The RM-confirmation hold for security/critical picks in dispatch mode is untested (.github/release/backport.py:1030)
  • #4 [Medium] The trailer job pushes with the App token to any same-repo head, including main or another release/*, and runs on closed PRs, contrary to its header (.github/workflows/release-branch-ci.yaml:110)
  • #5 [Medium] "Merge note" tells the author to squash and merge when the classify step failed (.github/workflows/release-branch-ci.yaml:81)
  • #6 [Medium] A labelled PR passes check whether or not the Release-branch-only: commit exists (.github/release/relmgr.py:241)
  • #7 [Medium] A multi-ticket title (UN-1 UN-2 [FIX] ...) is unclassifiable, so that fix is never backported (.github/release/relmgr.py:17)
  • #8 [Medium] backport_targets treats the OSS line and its paired enterprise line differently after the first promote (.github/release/relstate.py:433)
  • #9 [Medium] If gh pr create fails after push_head, the pick head is left with no PR (.github/release/backport.py:817)
  • #10 [Medium] After a failed decline record, finish_open advises deleting the head, which re-picks the declined fix (.github/release/backport.py:740)
  • #11 [Medium] Push-mode rescan() and main() are untested, including the missing count (.github/release/backport.py:577)
  • #12 [Medium] The inherited-commit path of is_converged is never exercised (.github/release/relmgr.py:442)
  • #13 [Medium] No workflow runs the .github/release suites, and Lint PR re-implements release_only_refusal inline (.github/workflows/lint-pr.yaml:53)
  • #14 [Medium] The Lint PR header says every PR carries a [TYPE] prefix, but a fix/ label waives the prefix (.github/workflows/lint-pr.yaml:3)
  • #15 [Medium] The rationale for exempting OSS in promote_blockers contradicts the tag rule (.github/release/relstate.py:541)
  • #16 [Low] PYTHONDONTWRITEBYTECODE cites a git add -A that does not exist (.github/workflows/backport.yaml:64)
  • #17 [Low] Comments name a "Backport PR" workflow, and Promote / Take RC build, which do not exist in this repo (.github/workflows/backport.yaml:25)
  • #18 [Low] oss.py cut --line is not validated (.github/release/oss.py:191)
  • #19 [Low] REPOS entries are positional 5-tuples, and oss.py reads [4] (.github/release/relstate.py:39)
  • #20 [Low] The ga/promoted fields have different shapes in the GIT and registry backends (.github/release/relstate.py:236)
  • #21 [Low] ls-remote errors are read as "no head" (.github/release/backport.py:805)
  • #22 [Low] A failed required check is reported as "merge pending check" (.github/release/backport_merge.py:158)
  • #23 [Low] A RuntimeError after the picks discards the step summary (.github/release/backport.py:908)
  • #24 [Low] A forced OSS promote can record an empty reason (.github/release/oss.py:294)

Unanchored findings

  • PR body, "PR title rule against recent history": the "4 fail" count is stale. pr_fix_class now strips leading whitespace, so the same 100 PRs give 2 failures (#2328 [PERF], #2231 fix(mcp):). The body needs correcting.
  • PR body: "Only the values in relstate's REPOS table differ per repo" is wrong. Every file in .github/release/ is byte-identical, and REPOS is keyed by GITHUB_REPOSITORY. The body needs correcting.
  • Repo prerequisites are not in place yet (already listed in the body's settings section): labels, a squash-only release/* ruleset (rebase merge is allowed at repo level), an ee-v* tag ruleset, and the App as bypass actor.
  • run-platform.sh:L167 now ignores suffixed tags such as v0.161.4_1. Today's latest is unchanged (v0.198.0); is a suffixed tag ever meant to be latest?
  • actionlint was not run: the auto-mode classifier denied it in this session.

Lens checklist

  1. Lens 1 — See findings #13 (PR-body items under Unanchored findings)
  2. Lens 2 — See findings #13
  3. Lens 3 — See findings #1, #5, #6, #9, #10, #21, #22, #23, #24
  4. Lens 4 — See findings #4
  5. Lens 5 — See findings #4
  6. Lens 6 — Clean
  7. Lens 7 — See findings #1, #7, #8, #18, #19, #20
  8. Lens 8 — See findings #1
  9. Lens 9 — Clean
  10. Lens 10 — Clean
  11. Lens 11 — See findings #13
  12. Lens 12 — N/A — no LLM, prompt or agent surface in the diff
  13. Lens 13 — See findings #2, #3, #11, #12, #13
  14. Lens 14 — Clean
  15. Lens 15 — See findings #13, #16
  16. Lens 16 — See findings #4, #14, #15, #16, #17
  17. Lens 17 — Clean
  18. Lens 18 — N/A — no chart, on-prem values, schema or on-prem build path; run-platform.sh is the community installer
  19. Lens 19 — N/A — no scaling or workload-lifecycle configuration

Assessed by the orchestrator: lenses 12, 17, 18 (d)/(e), 19, the lens 15 pinned gate (ruff 0.3.4 check and format clean), and the boundary sweep. Lens 17 notes: the merge-tree sweep covered 40 of 417 identifiers (no stale restatement among its hits). By hand, the title rule was compared with create-release.yaml's auto-bump classifier, and the "latest" rule with every tag resolver in the repo; neither contradicts.

Open questions for the author

  • Has the record path ever run under the App installation token rather than a PAT?
  • Should an OSS line stop taking plain fixes while the enterprise line that pins it still takes them (#8)?

Assumptions made

  • GitHub's documented installation-token restriction on GET /user holds. Finding #1 stands on the app/<slug> mismatch alone.
  • The squash commit message setting stays COMMIT_MESSAGES (checked read-only: it is today).

cost: agents=10 wall=577s lenses-examined=16 of 19 files=18 lines=8837

Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/relmgr.py Outdated
Comment thread .github/release/backport.py Outdated
Comment thread .github/workflows/release-branch-ci.yaml Outdated
Comment thread .github/workflows/release-branch-ci.yaml
Comment thread .github/release/relstate.py
Comment thread .github/release/backport.py Outdated
Comment thread .github/release/backport_merge.py
Comment thread .github/release/backport.py
Comment thread .github/release/oss.py
- release-branch-ci: the trailer job runs only on an open PR and refuses
  a head named main, release/* or backport/*. Merge note no longer
  suggests merging when the commit check did not finish.
- lint-pr: call relmgr.release_only_refusal instead of an inline copy;
  the header says a fix/ label waives the [TYPE] prefix.
- backport: drop a stale PYTHONDONTWRITEBYTECODE and a comment naming
  workflows this repo does not have.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
…ug to record

.github/release is copied unchanged from the enterprise repo's copy
(tree ceebe27). record() now identifies the bot's pick PRs by the
App's slug, which the record job passes from the token step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 2 of 10. Round tip b1b22fb2ca8d7691f99614382948cad3969124ff; round delta 4910d26f..b1b22fb2. Review: /unstract:review-deep (binding for every round of this PR's loop).

Review — #2381 (round 2 of self-review, FOLLOWUP)

Reviewed under unstract plugin 2.1.74 (/unstract:review-deep). Head b1b22fb2ca8d7691f99614382948cad3969124ff, merge base ee81e18d6678a2adae3294efcfd6ecf83ce42258 (base fetch ok). 18 files, 8975 lines; count verified against GitHub. Round delta 4910d26f..b1b22fb2: 13 files, +185/-47.

Verdict — BLOCK

Prior context fetched — followup-fetch.sh exited 0 (6 of 6). Since-boundary: 4910d26f28f5715bd555cb5c5246b04bf8bee40e (review 5663546818).

Scope change — YES — held() now walks main first-parent without merges and skips bot-authored commits, and ga now reads the first stable tag rather than the latest. Neither change is at a prior finding site. Re-scanned the whole round delta under all 19 lenses.

Summary — Critical: 0 · High: 1 · Medium: 4 · Low: 5 (open, including prior findings still partially resolved) · Lenses run: 19/19 (12, 18, 19 N/A with reason)

Signals: lenses 4, 8, 11, 13, 16, 17 raised (the App-slug author check, a changed git walk, workflow guards, new tests, many new comments, a changed support-window rule).

Security deep-dive: done (the find and filter sub-tasks both returned; nothing at confidence >= 8). One candidate at 7/10 that the filter confirmed is rated below as #N1 under this rubric. SonarCloud's 10 security alerts on this PR were triaged: none is exploitable.
boundary-sweep: 3 sinks traced, clean (the record() error messages carry only the public PR author login and the App slug; the merge note and the trailer refusal print branch names).

Prior findings status

  • #1 [High] record never matches the bot under the App token → RESOLVED. .github/release/backport_merge.py:L282-L293 checks author.is_bot and login == "app/" + RELEASE_BOT_APP_SLUG, and raises when the variable is unset. .github/workflows/backport.yaml:L136 passes the slug from steps.bot.outputs.app-slug; that output exists in create-github-app-token@v3. The finish_open half of the suggested fix was not taken: see #N1.
  • #2 [High] forged cherry-pick trailer untested → RESOLVED. test_relstate test_35b fails when the guard is replaced with if False: (mutation run).
  • #3 [High] dispatch RM-confirmation hold untested → RESOLVED. test_backport DispatchHold fails under all three mutations of .github/release/backport.py:L1050-L1062.
  • #4 [Medium] trailer job pushes to any same-repo head → RESOLVED. .github/workflows/release-branch-ci.yaml:L115 adds the open-state condition, and :L128-L130 refuses main, release/* and backport/*.
  • #5 [Medium] Merge note says squash after a failed classify → RESOLVED. .github/workflows/release-branch-ci.yaml:L86,L92-L93 reads steps.classify.outcome. The wording is refined in #N5.
  • #6 [Medium] label passes without an RBO commit → WAIVED. Accepted in code as a known limit at .github/release/relmgr.py:L253-L254; the user chose to fix only the High findings in .github/release.
  • #7 [Medium] multi-ticket titles unclassifiable → WAIVED. Known limit at .github/release/relmgr.py:L16-L17; the wording is refined in #N4.
  • #8 [Medium] OSS line stabilized while its cloud line is still stabilizing → PARTIALLY RESOLVED. Documented at .github/release/relstate.py:L266-L267. Whether this is intended is with the user.
  • #9 [Medium] orphan head after a failed gh pr create → WAIVED. Documented in the module docstring, .github/release/backport.py:L10-L11.
  • #10 [Medium] advice to delete the head re-picks a declined fix → WAIVED. Documented at .github/release/backport.py:L741-L742.
  • #11 [Medium] rescan/main untested → WAIVED. Known limit at .github/release/backport.py:L594.
  • #12 [Medium] is_converged inherited path untested → WAIVED. Known limit at .github/release/relmgr.py:L447.
  • #13 [Medium] inline duplicate of release_only_refusal; no workflow runs the suites → PARTIALLY RESOLVED. .github/workflows/lint-pr.yaml:L53 now calls relmgr.release_only_refusal. Still open: no workflow runs .github/release/test_*.py, so the heredoc glue is untested. Adding a CI job is a scope question for the release owner.
  • #14 [Medium] lint-pr header ignores the fix/ label waiver → RESOLVED. .github/workflows/lint-pr.yaml:L4.
  • #15 [Medium] misleading OSS :latest rationale → RESOLVED. .github/release/relstate.py:L542-L544.
  • #16 [Low] stale PYTHONDONTWRITEBYTECODE → RESOLVED. Removed from .github/workflows/backport.yaml.
  • #17 [Low] "Backport PR" workflow names → PARTIALLY RESOLVED. The .github/workflows/backport.yaml:L25 part is fixed. .github/release/backport.py:L29, .github/release/backport_merge.py:L4 and .github/release/relmgr.py:L30,L33,L196 are unchanged (below the floor).
  • #18 [Low] cut --line unvalidated → WAIVED. Known limit at .github/release/oss.py:L191.
  • #19 [Low] REPOS positional tuple → WAIVED. Below the floor; declined by the owner.
  • #20 [Low] ga/promoted shapes differ by backend → WAIVED. Below the floor; declined by the owner.
  • #21 [Low] ls-remote errors read as absent → WAIVED. Known limit at .github/release/backport.py:L830.
  • #22 [Low] a failed status check reads as pending → WAIVED. Known limit at .github/release/backport_merge.py:L158.
  • #23 [Low] RuntimeError discards the summary → WAIVED. Known limit at .github/release/backport.py:L933; its placement is refined in #N7.
  • #24 [Low] forced promote with an empty reason → WAIVED. Known limit at .github/release/oss.py:L246.

New findings

[High] N1 [Lens 4] — finish_open merges any single-commit PR on a backport/* head with the App token, without checking who opened it

  • Location: .github/release/backport.py:L739-L741
  • Failure mode: an existing backport/X.Y/<sha7> head with one open, unlabelled PR is squash-merged by the App, which is meant to bypass the release/* ruleset. The function checks neither the PR's author nor that its one commit is the pick of <sha7>. So a writer without review bypass can push such a head with any single commit and open a PR. The next rescan or dispatch then merges it onto the release line unreviewed. The easiest window is a security or critical fix that stays held for the RM. The docstring states the limit, but it is a review bypass, not a missing nicety.
  • Evidence: L786-L799 check one commit, no conflict markers and no backport-* label, then squash. record() (.github/release/backport_merge.py:L282-L293) does the author check this path lacks.
  • Suggested fix: apply record()'s check (author.is_bot and login == "app/" + RELEASE_BOT_APP_SLUG) in finish_open before squashing, and pass RELEASE_BOT_APP_SLUG to the pick job too. Protecting backport/* refs with a ruleset is an optional second layer.
  • Confidence: Medium. The path is certain from the code; the impact depends on the App being a bypass actor, which is planned but not yet configured.
  • Verification: UNVERIFIED — does a ruleset-bypassing App squash a PR that has no approving review?

[Medium] N2 [Lens 13] — No test fails if promoted reads the first stable tag instead of the latest

  • Location: .github/release/relstate.py:L401-L405
  • Failure mode: ga and promoted used to share one index; now each has its own. Changing ("promoted", builds, -1) to 0 leaves every suite green. convgate then scans the newer line at an older patch than the one that ships.
  • Evidence: test_relstate passes 50/50 under that mutation. No fixture puts two stable tags on one line's build artifact; test_5b asserts only ga.
  • Suggested fix: add two stable build tags to a line and assert that promoted is the newer one.
  • Confidence: High
  • Verification: VERIFIED — mutation run in a temp copy.

[Medium] N3 [Lens 16, 15] — The _next_minor docstring now promises that a cut never reuses a shipped minor, and the cutover check it used to ask for is gone

  • Location: .github/release/relstate.py:L300-L303
  • Failure mode: the code counts only tags merged into main (L308). An old-flow release tag that was never merged back is ignored, and its minor can be proposed again. The deleted sentence told the RM to check the proposal against git tag -l before the first cut.
  • Suggested fix: restore the previous wording ("a minor shipped by this flow", plus the git tag -l caveat).
  • Confidence: High
  • Verification: VERIFIED — diffed against 4910d26f. Today every stable tag is on main, so nothing is affected yet.

[Low] N4 [Lens 16] — The PR_TITLE_RE known limit says a two-ticket title always fails; a fix/ label classifies it

  • Location: .github/release/relmgr.py:L16-L17
  • Suggested fix: say "without a fix/ label", or drop the sentence.
  • Confidence: High
  • Verification: VERIFIED — called pr_fix_class with no label, fix/security and fix/needs-backport.

[Low] N5 [Lens 16] — The release-branch-ci header and merge note overstate or misname what happens

  • Location: .github/workflows/release-branch-ci.yaml:L16, .github/workflows/release-branch-ci.yaml:L92-L93
  • Failure mode: L16 says "the bot never pushes to" main, release/* or backport/*. Only job trailer is restricted; Backport's bot pushes backport/* heads. L93 says "did not finish" when the check finished and refused on conflict markers.
  • Suggested fix: "Job trailer never pushes…"; "The commit check failed; see its log."
  • Confidence: High
  • Verification: VERIFIED — read both push sites and the classify exit path.

[Low] N6 [Lens 16] — The finish_open known limit says a person's PR is merged "when it holds only a pick"; any single commit is enough

  • Location: .github/release/backport.py:L739-L740
  • Suggested fix: "when it holds one commit". This becomes moot if #N1 is fixed.
  • Confidence: High
  • Verification: VERIFIED — read L782-L799.

[Low] N7 [Lens 16] — The push_title known-limit comment sits inside the except ValueError block it does not describe

  • Location: .github/release/backport.py:L933
  • Suggested fix: move it above try:, and say "a gh failure that survives fix_class's retry".
  • Confidence: High
  • Verification: VERIFIED — read L141-L148 and L928-L936.

Unanchored findings

  • No workflow runs the .github/release suites (prior #13, still open). Scope question for the release owner.
  • SonarCloud's quality gate fails on this PR: Security C and Reliability D on new code. The 10 inline vulnerability alerts were triaged above as not exploitable. The one "bug" (relmgr.py, IndexError) flags commits[:1], and a slice cannot raise. The rest are code-smell ratings.
  • Not verified without a live run: the App's permissions, its bypass on rulesets, and a non-empty app-slug output in a real record run.

Lens checklist

  1. Lens 1 — Clean
  2. Lens 2 — Clean
  3. Lens 3 — Clean
  4. Lens 4 — See findings #N1
  5. Lens 5 — Clean
  6. Lens 6 — Clean
  7. Lens 7 — Clean
  8. Lens 8 — Clean
  9. Lens 9 — Clean
  10. Lens 10 — Clean
  11. Lens 11 — Clean
  12. Lens 12 — N/A — no LLM, prompt or agent surface in the diff
  13. Lens 13 — See findings #N2
  14. Lens 14 — Clean
  15. Lens 15 — See findings #N3
  16. Lens 16 — See findings #N3, #N4, #N5, #N6, #N7
  17. Lens 17 — Clean
  18. Lens 18 — N/A — no chart, on-prem values, schema or on-prem build path
  19. Lens 19 — N/A — no scaling or workload-lifecycle configuration

Assessed by the orchestrator: lenses 4 (with the security deep-dive), 5, 6, 7 (consumer search: ga readers checked at relstate L269-L283, L447, L553, L573; convgate reads promoted only), 12, 14, 17, 18, 19, the lens 15 pinned gate (ruff 0.3.4 check and format clean, run by me), and the boundary sweep. Lens 17: the support-window rule changed from latest to first GA, and git grep finds no other statement of the old rule. Bot-commit exclusion in held() matches relmgr.classify_commit's build-generated class.

Open questions for the author

  • Should finish_open check the pick PR's author before the App merges it (#N1)? The earlier decision to fix only the High findings predates this being rated as a review bypass.
  • Has any real record run seen a non-empty steps.bot.outputs.app-slug?

Assumptions made

  • The App will be a bypass actor on the release/* ruleset, as the PR body asks. If it is not, #N1 drops to Medium.

cost: agents=8 wall=349s lenses-examined=16 of 19 files=18 lines=8975

Comment thread .github/release/backport.py Outdated
Comment thread .github/release/relstate.py Outdated
Comment thread .github/release/relstate.py Outdated
Comment thread .github/release/relmgr.py Outdated
Comment thread .github/workflows/release-branch-ci.yaml Outdated
Comment thread .github/release/backport.py Outdated
Comment thread .github/release/backport.py Outdated
Self-review round 2, N5: the header said "the bot" never pushes to
main, release/* or backport/*, but Backport's bot pushes backport/*
heads; only job `trailer` has that guard. The merge note said the
commit check "did not finish" when it had failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
.github/release is copied from the enterprise repo's tree b2f9c04:
a leftover pick PR is squashed only when the release App opened it for
its pick, the promoted index is tested, and several comments are
corrected. The pick job now passes RELEASE_BOT_APP_SLUG, which
finish_open reads for that author check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 3 of 10. Round tip 72d0eb22e38bad3ae4da27555ac4bac95636d70a; round delta b1b22fb2..72d0eb22. Review: /unstract:review-deep (binding for every round of this PR's loop).

Review — #2381 (round 3 of self-review, FOLLOWUP)

Reviewed under unstract plugin 2.1.74 (/unstract:review-deep). Head 72d0eb22e38bad3ae4da27555ac4bac95636d70a, merge base ee81e18d6678a2adae3294efcfd6ecf83ce42258 (base fetch ok). 18 files, 9025 lines; count verified against GitHub. Round delta b1b22fb2..72d0eb22: 8 files, +88/-38.

Verdict — REQUEST CHANGES

Prior context fetched — followup-fetch.sh exited 0 (6 of 6). Since-boundary: b1b22fb2ca8d7691f99614382948cad3969124ff (review 5664505775).

Scope change — NO — the delta stays at prior finding sites: finish_open, record, the comments named in round 2, and the promoted test.

Summary — Critical: 0 · High: 0 · Medium: 4 · Low: 3 (open, including prior findings still partially resolved) · Lenses run: 19/19 (12, 18, 19 N/A with reason)

Signals: lenses 4, 13, 16, 17 raised (an authorization check before a ruleset-bypassing merge, new tests, changed docstrings, a rule now stated in two jobs).

Security deep-dive: done (find and filter sub-tasks both returned; nothing at confidence >= 8). The round-2 author-check gap is closed. One residual candidate was filtered out at 3/10; the cold read reproduced it, and it is rated below as R1.
boundary-sweep: 2 sinks traced, clean (the bot_refusal message carries only the PR number, the public author login and the App slug; the finish_open failure rows print the head name).

Prior findings status

  • N1 [High] finish_open squashes without checking who opened the PR → RESOLVED. .github/release/backport.py:L789-L791 refuses a PR not authored by app/<RELEASE_BOT_APP_SLUG> via the shared backport_merge.bot_refusal. .github/workflows/backport.yaml:L100 passes the slug to the pick job. Mutations of both checks are caught. The content half of the gap remains: see R1.
  • N2 [Medium] promoted index untested → RESOLVED. .github/release/test_relstate.py:L619-L623; mutating -1 to 0 now fails test_25_promoted_from_build_artifact.
  • N3 [Medium] _next_minor docstring overclaims → RESOLVED. .github/release/relstate.py:L300-L303 now says only stable tags merged into main count. A wording refinement is R4.
  • N4 [Low] PR_TITLE_RE known limit → RESOLVED. .github/release/relmgr.py:L16-L17.
  • N5 [Low] release-branch-ci header and merge note → RESOLVED. Fixed in 3385720: .github/workflows/release-branch-ci.yaml:L16, :L93.
  • N6 [Low] finish_open known limit wording → OBSOLETE. The known limit was removed with the N1 fix.
  • N7 [Low] push_title comment placement → RESOLVED. .github/release/backport.py:L935-L936.
  • #8 [Medium] OSS line stabilized while its cloud line is still stabilizing → PARTIALLY RESOLVED. Unchanged; still an open decision.
  • #13 [Medium] no workflow runs the suites → PARTIALLY RESOLVED. Unchanged; still a scope question.
  • #17 [Low] "Backport PR" workflow names → RESOLVED. Comments now name Backport's record job. .github/release/backport_merge.py:L343 keeps "Backport PR" as a step-summary heading, which is output, not a workflow name.

New findings

[Medium] R1 [Lens 4, 16] — finish_open checks the leftover pick PR's author and trailer, not that its commit is the pick

  • Location: .github/release/backport.py:L786-L800
  • Failure mode: a bot-opened clean pick PR stays open when its squash returned "merge pending check" or a run died after gh pr create. A repo writer can force-push one commit with any content to that head, with a message carrying (cherry picked from commit <sha7…>). Every guard then passes: the PR's author is still the App, there is one commit, the trailer string matches the head's sha7, and there are no conflict markers. The next run squashes it with the App token, past the release ruleset. The docstring (L736-L739) and the inline comment (L789) say a commit "that is not that pick" fails.
  • Evidence: the cold read reproduced it on a scratch copy; a commit adding a new file under a matching trailer came back merged. pick_trailer (.github/release/relmgr.py:L200-L204) is a string check, and finish_open never calls rm.on_main or compares content.
  • Why Medium, not High: the security filter rated it 3/10. A writer who can force-push that head can already push a branch whose workflow mints the same App token, because the App key is a repository secret. This check is defence in depth, and the docstring overstates it.
  • Suggested fix: pass sha from pick() into finish_open. Merge only when the head commit is the pick: compare git patch-id --stable of <sha> and <headRefOid>, or re-cherry-pick onto <headRefOid>^ and compare trees. Add a test that changes the tree under a valid trailer. Otherwise, narrow the docstring and comment to what is checked.
  • Confidence: High
  • Verification: VERIFIED — reproduced in a scratch copy.

[Medium] R2 [Lens 3, 16] — A person's PR on a pick head with two or more commits is reported as a pending conflict PR, not refused

  • Location: .github/release/backport.py:L786-L787
  • Failure mode: the commit-count check returns pending ("conflict PR pending") before bot_refusal runs. A person-opened, unlabelled PR with two or more commits is skipped on every run. In push mode, this_run drops skipped rows and conflict_prs lists only labelled PRs, so nothing reports it. The docstring says "a PR a person opened … fails unmerged".
  • Suggested fix: move the bm.bot_refusal(pr) check above the commit-count check (after the label check, so a labelled conflict PR still waits). Add a two-commit case to test_not_the_bots_pick_is_not_squashed.
  • Confidence: High
  • Verification: VERIFIED — read L782-L791. Nothing is merged; this is a silent stall against the documented behaviour.

[Low] R3 [Lens 7, 11] — The record job runs main's script with the release line's copy of its env

  • Location: .github/workflows/backport.yaml:L134-L141
  • Failure mode: pull_request_target uses the base line's copy of this job, which checks out and runs main's backport_merge.py. When a later change on main makes the script require a new env var or argument, every line cut earlier sends the old env. Its record runs then fail, and declines go unrecorded. Today both sides come from this PR and agree.
  • Suggested fix: add a comment at the env block saying main's backport_merge.py must accept the env of every live line's copy of this job.
  • Confidence: Medium
  • Verification: UNVERIFIED — a contract for future changes, not a present break.

[Low] R4 [Lens 16] — The new _next_minor sentence reads as covering retired lines too

  • Location: .github/release/relstate.py:L301-L302
  • Failure mode: "Only stable tags merged into main count" is true of the vX.Y.Z source (git tag --merged main). Retired lines are counted from git tag -l with no --merged filter (L314-L319), so an off-main GA tag on a retired line still raises the next minor.
  • Suggested fix: "Of the plain vX.Y.Z tags, only those merged into main count; …".
  • Confidence: High
  • Verification: VERIFIED — read _next_minor and relmgr._tags.

[Low] R5 [Lens 17, 16] — backport.py's run line omits RELEASE_BOT_APP_SLUG, which finish_open now needs

  • Location: .github/release/backport.py:L17
  • Failure mode: the header lists what the script needs to run (a clone pushing with the bot token, GH_TOKEN). backport_merge.py:L22 names RELEASE_BOT_APP_SLUG and backport.py does not. A caller following it leaves the slug unset; every leftover pick PR then fails unmerged.
  • Suggested fix: add "and RELEASE_BOT_APP_SLUG set to the release App's slug".
  • Confidence: High
  • Verification: VERIFIED — git grep RELEASE_BOT_APP_SLUG.

Unanchored findings

  • The App's private key is a repository secret, so any branch push by a writer can mint a token that bypasses the release/* ruleset. Settings question for the release owner: scope the key to an environment that only main and release/* workflows can use. This bounds R1 and the round-2 N1 class.
  • No workflow runs the .github/release suites (prior #13).
  • Not verified without a live run: the App's permissions, its bypass on rulesets, and a non-empty app-slug output in either job.

Lens checklist

  1. Lens 1 — Clean
  2. Lens 2 — Clean
  3. Lens 3 — See findings #R2
  4. Lens 4 — See findings #R1
  5. Lens 5 — Clean
  6. Lens 6 — Clean
  7. Lens 7 — See findings #R3
  8. Lens 8 — Clean
  9. Lens 9 — Clean
  10. Lens 10 — Clean
  11. Lens 11 — See findings #R3
  12. Lens 12 — N/A — no LLM, prompt or agent surface in the diff
  13. Lens 13 — Clean
  14. Lens 14 — Clean
  15. Lens 15 — Clean
  16. Lens 16 — See findings #R1, #R2, #R4, #R5
  17. Lens 17 — See findings #R5
  18. Lens 18 — N/A — no chart, on-prem values, schema or on-prem build path
  19. Lens 19 — N/A — no scaling or workload-lifecycle configuration

Assessed by the orchestrator: lenses 4 (with the security deep-dive), 5, 6, 7 (consumer search: bot_refusal has two callers, finish_open and record, and both jobs pass the slug), 8, 11, 12, 14, 17, 18, 19, the lens 15 pinned gate (ruff 0.3.4 check and format clean; all six suites pass), and the boundary sweep. Lens 13: the test analyzer mutation-verified every new guard (11 mutations, all caught by at least one suite). The type-design agent was not dispatched: no type, schema or wire format changed.

Open questions for the author

  • Should finish_open merge only a commit whose content is the pick (R1), or should the docstring narrow to the author and trailer checks?
  • Is a ruleset planned that restricts pushes to backport/* heads? It would conflict with the conflict PR's "amend the pick commit" instruction.

Assumptions made

  • gh pr list --json author returns an App author as {"login": "app/<slug>", "is_bot": true}; the test analyzer confirmed this shape on real PRs.

cost: agents=6 wall=335s lenses-examined=16 of 19 files=18 lines=9025

Comment thread .github/release/backport.py Outdated
Comment thread .github/release/backport.py
Comment thread .github/workflows/backport.yaml
Comment thread .github/release/relstate.py Outdated
Comment thread .github/release/backport.py Outdated
Self-review round 3, R3: pull_request_target runs each release line's
copy of the record job, which runs main's backport_merge.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
.github/release is copied from the enterprise repo's tree b00d22b:
finish_open refuses a PR the release App did not open before it counts
commits, its docstring and refusal say the check covers the pick
trailer, not the commit's content, and two comments are corrected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Lint PR now runs only for a PR whose head is in this repo; a maintainer
fixes a fork PR's title before merging. A new workflow runs every
.github/release/test_*.py on a pull request that changes the release
tooling or a workflow that runs it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
.github/release is copied from the enterprise repo's tree cd75cc4:
- an OSS line stays stabilizing until an on-prem-ga/X.Y tag exists,
  written by `oss.py promote --on-prem`;
- finish_open refuses a PR the release App did not open before it
  reads the labels;
- the bot's pick PR with more than one commit fails with the steps to
  take, instead of waiting where no summary shows it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 4 (review-deep, FOLLOWUP) — round tip da87df3d

Review: /unstract:review-deep, as strat instructed for every round. Round delta: 72d0eb22..da87df3d (3703de4, 50ce92c, 208616c, da87df3). Reviewed under unstract plugin v2.1.74. Base main fetched (base_fetch ok), merge base ee81e18d.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 3 · Low: 2 · Lenses run: 19/19 (lenses-examined 15; N/A on 5, 12, 18 and 19 with reasons below)

Prior context fetched — followup-fetch.sh exited 0. Previous-review SHA used as the "since" boundary: 72d0eb22.

Scope change — NO

Prior findings status

  • R1 [Medium] finish_open checks the leftover pick PR's author and trailer, not that its commit is the pick → WAIVED. Known limit written in code at .github/release/backport.py:L741-L743, and in the PR body's Tokens section. Residual: a writer who force-pushes the bot's open pick PR with one trailer-bearing commit gets it squashed. A writer can already mint the App token through a workflow.
  • R2 [Medium] A person's PR on a pick head with two or more commits is reported as pending → RESOLVED. bot_refusal now runs before the label and count checks (.github/release/backport.py:L791-L794, da87df3). A person's PR fails; a bot PR with more than one commit fails with a remedy (L796-L801).
  • R3 [Low] The record job runs main's script with the release line's copy of its env → RESOLVED. Comment at .github/workflows/backport.yaml:L135-L136 (3703de4).
  • R4 [Low] The new _next_minor sentence reads as covering retired lines too → RESOLVED. .github/release/relstate.py:L299-L303 (50ce92c).
  • R5 [Low] backport.py's run line omits RELEASE_BOT_APP_SLUG → RESOLVED. .github/release/backport.py:L17-L18 (50ce92c).
  • #8 [Medium] backport_targets treats the OSS line and its paired enterprise line differently → RESOLVED. The OSS line stays stabilizing until an on-prem-ga/X.Y tag exists (.github/release/relstate.py:L274, L409-L410, da87df3). Follow-on question filed as new finding #3.
  • #13 [Medium] No workflow runs the .github/release suites → RESOLVED. .github/workflows/release-tooling-tests.yaml (208616c) runs all six suites; run 40482725635 passed on da87df3. The inline-copy half was already resolved in round 2.

New findings — 5, posted inline below.

Lens checklist

  1. Correctness vs intent — Clean (code-reviewer)
  2. Edge cases — See findings #5
  3. Error handling — See findings #3, #5
  4. Security — Clean (security deep-dive done, 0 findings; boundary sweep below)
  5. Data integrity & migrations — N/A — no schema, data store or migration in the diff
  6. Concurrency — Clean (infra agent: two concurrent --on-prem promotes, loser's atomic push rejected and re-runnable)
  7. API compatibility — See findings #3 (orchestrator consumer search across both repos)
  8. Reliability — Clean (infra agent)
  9. Performance — Clean (code-reviewer: one rev-parse per KEPT line)
  10. Observability — Clean (code-reviewer)
  11. Operational safety — Clean (infra agent; fork skip is a documented trade-off, see #4)
  12. LLM/agent — N/A — no prompt, model or agent code
  13. Tests — See findings #2
  14. Dependencies & build — Clean (orchestrator: no new dependency; test files import only stdlib and each other; actions/checkout@v6 as in the repo's other workflows)
  15. Conventions — Clean (code-reviewer; ruff passed in pre-commit at da87df3; orchestrator ran no extra linter)
  16. Doc accuracy — See findings #1, #4
  17. Cross-document consistency — Clean (orchestrator: lens17-merge-sweep.sh exit 1, every hit for on-prem-ga, ON_PREM_GA, on_prem_ga, bot_refusal, release-tooling-tests, head.repo.full_name, stabilizing classified consistent; all under .github/)
  18. On-prem deployment experience — N/A — no chart, values, schema or on-prem build path in the diff
  19. Scaling & workload lifecycle — N/A — no workload, replica or worker setting

Security deep-dive: done — find sub-task returned no candidates at >= 0.7; no findings at confidence >= 8.
boundary-sweep: 3 sinks traced, clean (the finish_open failure messages carry a PR number and head name; the on-prem-ga tag message carries two version tags; Lint PR prints the title back to its own author).
Mutation check (orchestrator, scratch copy): 6 mutants of the round's guards (refusal order, commit count, the "once" guard, the on_prem condition, both KEPT states) all killed.

Open questions / assumptions
Finding #3 depends on the enterprise V2 port; this review cannot see it.
Assumes a job skipped by if: satisfies a required check (GitHub's documented behaviour); the PR body states it.

cost: agents=7 wall=08:30 lenses-examined=15 of 19 files=19 lines=9157

Comment thread .github/release/backport.py Outdated
Comment thread .github/release/oss.py Outdated
Comment thread .github/release/relstate.py Outdated
Comment thread .github/workflows/lint-pr.yaml Outdated
Comment thread .github/release/backport.py
.github/release tree 7ea98f2, matching the enterprise PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 5 (review-deep, FOLLOWUP): round tip ea3b9949

Review: /unstract:review-deep (FOLLOWUP), the same review as every round. Round tip ea3b9949beeaf247bba4839019d0df8047cea61a. Round delta da87df3d..ea3b9949, 4 files, +36/-6:

  • 3a552c48: the lint-pr fork comment.
  • The .github/release tree 7ea98f29: the finish_open docstring, the 0-commit message and test_cli_on_prem_flag.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 1 · Low: 1 (new). Prior findings still open: #3 and #5, both Medium. Lenses run: 15/19 (4 N/A, listed below).

Prior context fetched — followup-fetch.sh exited 0, 6 of 6 fetches. The "since" boundary is the previous review's SHA, da87df3dc527dc38fb18b53e91c0eb48bb507409 (review 5665145267).

Scope change — NO

Prior findings status

  • #1 [Medium] The finish_open docstring says a person's commits on the bot's PR wait for a person → RESOLVED. The docstring at .github/release/backport.py:738-739 now says a PR with more than one commit, or none, fails unmerged, which matches L797. Fix: ea3b9949.

  • #2 [Medium] The --on-prem CLI flag is never exercised → RESOLVED. test_cli_on_prem_flag (.github/release/test_oss.py:240-254) drives oss.main with --on-prem. A mutant that drops a.on_prem at oss.py:419 makes it error. Fix: ea3b9949.

  • #3 [Medium] An OSS line stays stabilizing until oss.py promote --on-prem runs, and no caller exists yet → NOT RESOLVED. The OSS-side code is unchanged. Which enterprise workflow calls promote --on-prem, and what end state a cloud-only line reaches, are still undecided. This is waiting on the release owner's decision, not on code here. The residual failure is unchanged: until a caller ships, every OSS line stays stabilizing and keeps taking picks.

  • #4 [Low] Lint PR's fork note names only the title → RESOLVED. .github/workflows/lint-pr.yaml:12 now names both title and labels. Fix: 3a552c48. The second sentence of that fix is new finding 1.

  • #5 [Low → Medium] A bot head with 0 commits ahead gets the wrong remedy → NOT RESOLVED. The remedy changed, but it is still wrong (.github/release/backport.py:797-806). finish_open is reached only after is_converged is False, so a 0-commit head means the head was moved onto a commit already on the line, and the fix is still owed. "close it, it adds nothing" sends the person into the record job:

    • backport_merge.first_pick sees an empty head_messages and returns ("", []) (backport_merge.py:91-98).
    • decline then comments "Decline it with a hand-made empty commit" and raises (backport_merge.py:208-227).
    • Following that comment declines a fix nobody chose to decline.
    • Not following it leaves the head, which reports "exists with no open PR: delete it or reopen". Reopening returns to n==0.
    • Only deleting the head recovers, and no message says so.

    Raised to Medium because the bot's own text points at an unwanted decline. Suggested fix: for n=="0", tell the person to delete the head; the next run re-picks the fix. The test at .github/release/test_backport.py:776-783 pins the message string only. Four of the seven agents flagged this independently. Sent to the owner of .github/release.

Lens checklist

  • Lens 1: Clean — the title UN-4522 [MISC] ... matches the convention. The description is checked against the diff after this round's fix.
  • Lens 2: Clean
  • Lens 3: See findings #5 (prior)
  • Lens 4: Clean — security deep-dive done, 0 findings. Boundary sweep: 1 sink traced (the finish_open failure message, which carries only a PR number and a count), clean.
  • Lens 5: N/A — no data, schema or migration in the delta
  • Lens 6: Clean — the new branch returns a string and writes nothing. The CLI test restores the working directory through addCleanup (assessed by the infra agent).
  • Lens 7: Clean — no wire, CLI or contract change. The only change is the text of an existing failure message. Assessed by me; no type agent was dispatched.
  • Lens 8: See findings #5 (prior)
  • Lens 9: Clean
  • Lens 10: Clean
  • Lens 11: Clean — the lint-pr change is a comment only. A job skipped by if still counts as passed under a ruleset.
  • Lens 12: N/A — no LLM or agent surface
  • Lens 13: Clean — both new tests are mutation-verified: dropping the n == "0" branch fails FinishOpen, and dropping a.on_prem errors test_cli_on_prem_flag. All six suites pass.
  • Lens 14: Clean — no dependency or build change
  • Lens 15: Clean — no ruff config change. The pinned gate is not run here because ruff is not installed in this environment; the suites pass.
  • Lens 16: See findings #1, #2
  • Lens 17: See findings #1 — merge-result sweep exit 1, every hit classified. relmgr.py:17 is consistent. lint-pr.yaml:13 is finding 1. backport.py:802 is prior #5.
  • Lens 18: N/A — no chart, on-prem values or on-prem build path in the delta
  • Lens 19: N/A — no scaling or lifecycle values in the delta

Security deep-dive: done (0 findings; the filtering sub-tasks completed).

Open questions / assumptions

Should a 0-commit pick head be deleted by the bot itself, since it owns both the PR and the head, rather than left to a person?

Assumed: the precondition for #5 (the bot's head force-pushed onto the release tip or an ancestor) is rare, which is why it is Medium and not High.

cost: agents=7 wall=04:39 lenses-examined=15 of 19 files=4 lines=42

Comment thread .github/workflows/lint-pr.yaml Outdated
Comment thread .github/release/backport.py Outdated
Backport warns only on push-to-main rescans of PRs merged into main, so a
fork PR merged into a release branch is never flagged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
.github/release tree 07e99c7, matching the enterprise PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 6 (review-deep, FOLLOWUP): round tip e8f6fc94

Review: /unstract:review-deep (FOLLOWUP), the same review as every round. Round tip e8f6fc94089a0012b774cfa8b09cac169d8d2945. Round delta ea3b9949..e8f6fc94, 5 files, +38/-7:

  • 3aedb82b: the lint-pr fork comment loses its second sentence.
  • The .github/release tree 07e99c72: the finish_open 0-commit message, the empty-head path in first_pick/decline/record/summary, the docstring label wording, and their tests.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 1 · Low: 4 (new). Prior findings still open: #3 (Medium). Lenses run: 15/19 (4 N/A, listed below).

Prior context fetched — followup-fetch.sh exited 0, 6 of 6 fetches. The "since" boundary is the previous review's SHA, ea3b9949beeaf247bba4839019d0df8047cea61a (review 5665273270).

Scope change — NO

Prior findings status

  • R5-1 [Medium] The Lint PR fork comment presents Backport as a backstop → RESOLVED. .github/workflows/lint-pr.yaml:12 now says only that a fork PR is not checked and the maintainer checks title and labels. Fix: 3aedb82b.
  • R5-2 [Low] "A labelled conflict PR waits" is narrower than the check → RESOLVED. .github/release/backport.py:738 now says any backport-* label waits, matching L793. Fix: e8f6fc94. The sentence's order against the person-PR rule is new finding 4.
  • #5 [Medium] A bot head with 0 commits ahead gets the wrong remedy → RESOLVED. .github/release/backport.py:802-803 now says delete the head and the next run re-picks; deleting it closes the PR and the record job returns skipped: no pick (backport_merge.py:325-326), so no decline is written. Fix: e8f6fc94. What happens when a person closes the PR instead is new finding 1.
  • #3 [Medium] An OSS line stays stabilizing until oss.py promote --on-prem runs, and no caller exists yet → NOT RESOLVED. The OSS-side code is unchanged. Still waiting on the release owner's decision.

Lens checklist

  • Lens 1: Clean — the title UN-4522 [MISC] ... matches the convention. The description matches the diff; the delta changes no behaviour it describes.
  • Lens 2: Clean
  • Lens 3: See findings #1
  • Lens 4: Clean — security deep-dive done, 0 findings. Boundary sweep: 2 sinks traced (the finish_open 0-commit message and the new summary line, which carry only a branch, a head name and a PR number), clean.
  • Lens 5: N/A — no data, schema or migration in the delta
  • Lens 6: Clean — a concurrent re-push to the head name does not affect the record job, which reads by PR number and old head oid (assessed by the infra agent).
  • Lens 7: Clean — no wire, CLI or contract change; record's result string is read only by summary and main in the same file. Assessed by me; no type agent was dispatched.
  • Lens 8: See findings #1
  • Lens 9: Clean
  • Lens 10: Clean
  • Lens 11: Clean — the lint-pr change is a comment only.
  • Lens 12: N/A — no LLM or agent surface
  • Lens 13: Clean — every new guard is mutation-verified: removing if not msgs, if msg is None, the record skip, the summary no-sha branch or the n == "0" branch each turns a test red (5 of 5 killed, run twice independently). The only edited expectation follows the changed message. All six suites pass.
  • Lens 14: Clean — no dependency or build change
  • Lens 15: Clean — no ruff config change. The pinned gate is not run here because ruff is not installed in this environment; the suites pass.
  • Lens 16: See findings #3, #4, #5
  • Lens 17: See findings #2 — merge-result sweep exit 1, every hit classified. backport.yaml:17 is finding 2; backport_merge.py:12 is qualified by L19-20; the "close it to decline" texts at backport.py:115,805,817 are covered by finding 1.
  • Lens 18: N/A — no chart, on-prem values or on-prem build path in the delta
  • Lens 19: N/A — no scaling or lifecycle values in the delta

Security deep-dive: done (0 findings; the identification pass returned no candidates, so the filters had nothing to run on).

Open questions / assumptions

Does GitHub report a PR as MERGED, rather than closed, when its head is force-pushed to a commit already on the base? If so, record takes the merged path for this case and labels it backport-merged with no pick landed. Not tested here; it needs a sandbox run.

Can a closed PR whose head has no commit past its base be reopened on GitHub? If not, the "reopen its PR" half of backport.py:783 is impossible for this case.

Assumed: the precondition (a pick head moved onto the base) is rare, which is why finding 1 is Medium and not High.

cost: agents=7 wall=04:47 lenses-examined=15 of 19 files=5 lines=45

Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/workflows/backport.yaml Outdated
Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/backport.py Outdated
Comment thread .github/release/backport.py Outdated
.github/release tree 811222f, matching the enterprise PR: a closed backport PR whose head has no
commit past the base gets its head deleted and one comment on how to decline by hand.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 7 (review-deep, FOLLOWUP): round tip 39646d31

Review: /unstract:review-deep (FOLLOWUP), the same review as every round. Round tip 39646d31b23fb6c62fe151bfff05c631ca6db3fc. Round delta e8f6fc94..39646d31, 5 files, +48/-13: the .github/release tree 811222fc (on a close with no pick, record deletes the head and comments once; the docstrings; the "treats the fix as not picked" wording; tests) and one backport.yaml header line.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 1 · Medium: 0 · Low: 3 (new). Prior findings still open: #3 (Medium). Lenses run: 15/19 (4 N/A, listed below).

Prior context fetched — followup-fetch.sh exited 0, 6 of 6 fetches. The "since" boundary is the previous review's SHA, e8f6fc94089a0012b774cfa8b09cac169d8d2945 (review 5665385507).

Scope change — NO

Prior findings status

  • R6-1 [Medium] Closing a 0-commit pick PR records nothing and tells the closer nothing → RESOLVED. record now deletes the head and posts one comment per close (backport_merge.py:329-346), and the summary gains a Next line. Fix: 39646d31. The way the head is deleted is new finding 1.
  • R6-2 [Low] backport.yaml header says every closed-unmerged PR becomes a decline → RESOLVED. backport.yaml:19 names the no-pick case. Fix: 39646d31.
  • R6-3 [Low] Module and decline docstrings → RESOLVED. backport_merge.py:10,20-21,215-217. Fix: 39646d31.
  • R6-4 [Low] finish_open docstring order → RESOLVED. backport.py:738-739 now follows the code's order. Fix: 39646d31.
  • R6-5 [Low] "re-picks the fix" is false on a stabilized line → RESOLVED. backport.py:802-803 now says "treats the fix as not picked". Fix: 39646d31.
  • #3 [Medium] An OSS line stays stabilizing until oss.py promote --on-prem runs, and no caller exists yet → NOT RESOLVED. The OSS-side code is unchanged. Still waiting on the release owner's decision.

New findings are posted as inline comments in this review.

Lens checklist

  • Lens 1: Clean — the title UN-4522 [MISC] ... matches the convention. The description's backport bullet ("records how it was closed") still holds; the delta changes no other behaviour it describes.
  • Lens 2: Clean
  • Lens 3: See findings #1, #3
  • Lens 4: Clean — security deep-dive done, 0 findings. Boundary sweep: 2 sinks traced (the NO_PICK PR comment and the summary Next line, which carry only a branch name, a head name and a PR number), clean.
  • Lens 5: N/A — no data, schema or migration in the delta
  • Lens 6: See findings #1 — record runs on one PR are serialised by backport-pr-N, but a re-run of PR N's record can act on a head PR M now owns (assessed by the infra agent).
  • Lens 7: Clean — no wire, CLI or contract change; record's result string is read only by summary and main in the same file. Assessed by me; no type agent was dispatched.
  • Lens 8: Clean — delete_head tolerates an already-gone ref, so a re-run after a failed comment reaches the comment (the over-broad 422 match is finding 3).
  • Lens 9: Clean
  • Lens 10: Clean
  • Lens 11: See findings #1
  • Lens 12: N/A — no LLM or agent surface
  • Lens 13: See findings #2 — mutation: removing the delete_head call, the dedupe, or the summary Next line each turns a test red (3 of 4 killed); the dedupe's closedAt bound survives. The two edited expectations follow the changed messages; no test is weakened. All six suites pass.
  • Lens 14: Clean — no dependency or build change
  • Lens 15: Clean — no ruff config change. The pinned gate is not run here because ruff is not installed in this environment; the suites pass.
  • Lens 16: See findings #3, #4
  • Lens 17: Clean — merge-result sweep exit 1, every hit classified: the "re-pick" text at backport.py:12,744 covers other cases (a declined fix; a failed decline record); backport.yaml:18 "head kept" is qualified by the new line 19; nothing still says "no label or comment".
  • Lens 18: N/A — no chart, on-prem values or on-prem build path in the delta
  • Lens 19: N/A — no scaling or lifecycle values in the delta

Security deep-dive: done (0 findings; the identification pass returned no candidates, so the filters had nothing to run on).

Open questions / assumptions

Assumed, not run live: a branch delete made with the App's installation token closes the open PR and fires pull_request_target: closed with the App as sender. Finding 1's cascade depends on it. A sandbox run would settle it.

Still open from round 6: does GitHub report MERGED when a head is force-pushed onto a base commit?

cost: agents=7 wall=04:13 lenses-examined=15 of 19 files=5 lines=61

Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/test_backport_merge.py
Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/backport_merge.py Outdated
Port .github/release tree b307043 from the enterprise copy: the no-pick
close checks the head ref's sha before deleting, so a re-run cannot close a
later re-pick PR on the same name. The comment no longer claims a delete or
a direct push to the release branch, and the dedupe's close-time bound is
tested. The Backport header says the delete is conditional.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 8 (review-deep, FOLLOWUP): round tip 360767f7

Review: /unstract:review-deep (FOLLOWUP), the same review as every round. Round tip 360767f7bdacf350a86d30bcbc2678903a4933d7. Round delta 39646d31..360767f7, 3 files, +44/-14: the .github/release tree b3070439 (new head_sha helper; the no-pick path deletes the head only while the ref is still at the PR's headRefOid; reworded NO_PICK comment, summary Next line and docstring; tests for a moved or gone ref and for the closedAt bound) and one backport.yaml header line.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 3 · Low: 1 (new). Prior findings still open: #3 (Medium). Lenses run: 15/19 (4 N/A, listed below).

Prior context fetched — followup-fetch.sh exited 0, 6 of 6 fetches. The "since" boundary is the previous review's SHA, 39646d31b23fb6c62fe151bfff05c631ca6db3fc (review 5665502088). Base fetch: ok.

Scope change — NO

Prior findings status

  • R7-1 [High] A re-run of a no-pick record deletes the head by name → RESOLVED. record deletes only when head_sha(slug, head) == pr["headRefOid"] (backport_merge.py:344); a re-run with the ref at another sha or gone writes nothing (test_backport_merge.py:283-287). Fix: 360767f7. What the kept-head branch reports is new finding 2.
  • R7-2 [Low] The closedAt bound on the NO_PICK dedupe is untested → RESOLVED. test_backport_merge.py:288-291; dropping the bound now fails the suite. Fix: 360767f7.
  • R7-3 [Low] The comment says "Deleted" when nothing was deleted → RESOLVED. The comment no longer claims a delete (backport_merge.py:357-360). Fix: 360767f7. The 422 match in delete_head was left as is, with reasons given in-thread.
  • R7-4 [Low] The hand-decline instruction says to push to the release branch → RESOLVED. It now says to ask the release manager (backport_merge.py:358-360). Fix: 360767f7.
  • #3 [Medium] An OSS line stays stabilizing until oss.py promote --on-prem runs, and no caller exists yet → NOT RESOLVED. The OSS-side code is unchanged. Still waiting on the release owner's decision.

New findings are posted as inline comments in this review.

Lens checklist

  • Lens 1: Clean — the title UN-4522 [MISC] ... matches the convention. The description's backport bullet ("records how it was closed") still holds; the delta changes no other behaviour it describes.
  • Lens 2: Clean — the guard compares the same commit the no-pick decision read (head_messages uses headRefOid); the singular git/ref endpoint is an exact match (code-reviewer agent).
  • Lens 3: See findings #1, #2
  • Lens 4: Clean — security deep-dive done, 0 findings at confidence ≥ 8; the delete is confined to the PR's own backport/X.Y/<sha7> by the existing head, base, cross-repo and author checks. Boundary sweep: 3 sinks traced (NO_PICK PR comment, summary Next line, the raised RuntimeError), which carry only branch, head, PR number and gh's stderr; clean.
  • Lens 5: N/A — no data, schema or migration in the delta
  • Lens 6: Clean — a pick pushes only when ls-remote finds no head (backport.py:847-853), so while the closed PR's head exists no pick can re-create it in the read-to-delete window; record runs on one PR are serialised by backport-pr-N (infra agent, self-authored dispatch).
  • Lens 7: Clean — no wire, CLI or contract change; record's result string is read only in the same file. Assessed by me; no type agent was dispatched.
  • Lens 8: Clean — a non-404 read error raises before any write, so the job fails and "Re-run jobs" recovers it (except under finding 1).
  • Lens 9: Clean
  • Lens 10: Clean
  • Lens 11: See findings #2
  • Lens 12: N/A — no LLM or agent surface
  • Lens 13: See findings #3 — mutation (scratch copy, git archive): guard → True killed; guard accepting a gone ref killed; 404 path re-raising killed; closedAt bound dropped killed; non-404 swallowed SURVIVED. The test agent's extra mutants (is not None, != "b"*40, always None) were all killed. The writes() filter excludes only git/ref/ reads (deletes use git/refs/), so no test is weakened. All six suites pass.
  • Lens 14: Clean — no dependency or build change
  • Lens 15: Clean — no ruff config change. The pinned gate is not run here because ruff is not installed in this environment; the suites pass.
  • Lens 16: See findings #2, #4
  • Lens 17: Clean — merge-result sweep exit 1, every hit classified: backport.py:803 and test_backport.py:782 describe a person deleting the head, where "not picked" holds; test_backport_merge.py:268 describes the default case, where the ref is at headRefOid; backport.yaml:19-20 matches the guard.
  • Lens 18: N/A — no chart, on-prem values or on-prem build path in the delta
  • Lens 19: N/A — no scaling or lifecycle values in the delta

Security deep-dive: done (0 findings; the finder returned no candidates, so the filters had nothing to run on).

Open questions / assumptions

Assumed, not run live: a closed PR's headRefOid stays at its close-time sha when the branch is later pushed, deleted or re-created. The guard and finding 2 both depend on this.

Below the floor, not filed: head_sha followed by delete_head is not atomic (the ref DELETE takes no expected sha). Lens 6 above explains why no pick can land in that window.

Still open from earlier rounds: does GitHub report MERGED when a head is force-pushed onto a base commit? Does a branch delete made with the App token fire pull_request_target: closed?

cost: agents=7 wall=03:58 lenses-examined=15 of 19 files=3 lines=58

Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/test_backport_merge.py Outdated
Comment thread .github/release/test_backport_merge.py Outdated
Port the release tooling at tree c38bc5f: on a close with no pick on the
head, record no longer deletes the head. It comments once that a person
deletes the head or re-picks onto it, and how to decline the fix by hand.
head_sha and its tests are removed. The backport.yaml header says the same.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 9 (review-deep, FOLLOWUP): round tip 06442fc3

Review: /unstract:review-deep (FOLLOWUP), the same review as every round. Round tip 06442fc37d5fe39022259e6203ee0bc2fe962178. Round delta 360767f7..06442fc3, 3 files, +22/-47: the .github/release tree c38bc5f5 (the no-pick close no longer deletes the head; head_sha and its tests are removed; the NO_PICK comment, summary Next line, docstring and code comment say removing the head is manual) and the backport.yaml header lines 19-20.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 3 · Low: 2 (new). Prior findings still open: #3 (Medium). Lenses run: 15/19 (4 N/A, listed below).

Prior context fetched — followup-fetch.sh exited 0, 6 of 6 fetches. The "since" boundary is the previous review's SHA, 360767f7bdacf350a86d30bcbc2678903a4933d7 (review 5665630875). Base fetch: ok.

Scope change — NO

Prior findings status

  • R8-1 [Medium] head_sha treats any error whose text contains "404" as "ref gone" → OBSOLETE. head_sha is removed; the no-pick path reads no ref. Change: 06442fc3.
  • R8-2 [Medium] A kept head goes unreported, and the texts still describe a delete → RESOLVED. Nothing is deleted on the no-pick path, and the comment, summary, docstring and code comment now say removing the head is manual (backport_merge.py:329-346, :395). Fix: 06442fc3. What those texts tell a person to do is new finding 1.
  • R8-3 [Medium] The non-404 re-raise in head_sha is untested → OBSOLETE. The code is removed. Change: 06442fc3.
  • R8-4 [Low] The test comment says "so it stays" for a deleted head → OBSOLETE. The test case is removed; the remaining comment says nothing is deleted (test_backport_merge.py:263-264). Change: 06442fc3.
  • #3 [Medium] An OSS line stays stabilizing until oss.py promote --on-prem runs, and no caller exists yet → NOT RESOLVED. The OSS-side code is unchanged. Still waiting on the release owner's decision.

New findings are posted as inline comments in this review.

Lens checklist

  • Lens 1: Clean — the title UN-4522 [MISC] ... matches the convention. The description's backport bullet ("records how it was closed") still holds; the delta changes no other behaviour it describes.
  • Lens 2: Clean — removing head_sha leaves no caller (grep head_sha .github/release/*.py is empty); delete_head is still used on the merged path only.
  • Lens 3: See findings #1, #2, #5
  • Lens 4: Clean — security deep-dive done (see the line below). The delta removes a delete and writes only a deduped PR comment that carries the base, head and fixed text.
  • Lens 5: N/A — no data, schema or migration in the delta
  • Lens 6: Clean — removing the compare-and-delete also removes the race it guarded; the no-pick path writes one deduped comment, and record runs on one PR are serialised by backport-pr-N (infra agent, self-authored dispatch).
  • Lens 7: Clean — no wire, CLI or contract change; record's result string is unchanged. Assessed by me; no type agent was dispatched.
  • Lens 8: Clean — a record re-run is idempotent: the NO_PICK comment is deduped against closedAt and a re-run writes nothing (test_backport_merge.py:276-277).
  • Lens 9: Clean
  • Lens 10: Clean
  • Lens 11: See findings #4
  • Lens 12: N/A — no LLM or agent surface
  • Lens 13: Clean — mutation (scratch copy, git archive; test agent): delete_head restored on the no-pick path killed; dedupe dropped killed; closedAt bound dropped killed; >= → > killed; comment text changed killed; Summary Next line changed killed. The removed ref= fixture, git/ref stub and writes() filter covered only the removed head_sha, so no test is weakened. All six suites pass.
  • Lens 14: Clean — no dependency or build change
  • Lens 15: Clean — no ruff config change. The pinned gate is not run here because ruff is not installed in this environment; the suites pass.
  • Lens 16: See findings #1, #3
  • Lens 17: See findings #4 — merge-result sweep exit 1, every hit classified: the "re-pick" hits at backport_merge.py:21, :329, :343, :395, test_backport_merge.py:416 and backport.yaml:20 are finding 1; backport.py:12 and :744 describe a declined fix and hold; delete_head at :316 is the merged path; the hits outside .github are unrelated.
  • Lens 18: N/A — no chart, on-prem values or on-prem build path in the delta
  • Lens 19: N/A — no scaling or lifecycle values in the delta

Security deep-dive: done (0 findings at confidence ≥ 8; 2 candidates, both filtered below the floor, filters completed).

Open questions / assumptions

Assumed, not run live: findings 1-3 come from reading pick and finish_open; no Backport run was made against a kept head.

Does GitHub allow reopening a closed PR whose head has no commit past its base? Finding 1's reopen half depends on it.

Still open from earlier rounds: does GitHub report MERGED when a head is force-pushed onto a base commit? Does a branch delete made with the App token fire pull_request_target: closed? Finding 3 assumes a person's delete does.

cost: agents=7 wall=06:00 lenses-examined=15 of 19 files=3 lines=69

Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/backport_merge.py Outdated
Comment thread .github/release/backport_merge.py
Comment thread .github/release/backport.py Outdated
Comment thread .github/release/backport.py Outdated
Delete the head to have the fix picked again, or keep it until a hand-made
decline is on the base; the delete line is posted only while the head still
holds the PR's commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Mirrors the shared release tooling: Publish reads the registry through
relstate._read, and the OSS workflow tests stub registry reads in-process.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Replaces the on-prem-ga/X.Y tag: cloud Promote writes stabilized/X.Y in the
promote that leaves every enabled cloud target GA (oss.py promote --stabilized).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
An enterprise RC that builds a target with no GA yet makes the OSS line
stabilizing again (oss.py reopen, relstate.reopens); Promote writes the tag
again at the line's last GA.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
@chandrasekharan-zipstack
chandrasekharan-zipstack marked this pull request as ready for review October 28, 2026 12:08
@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

via Greptile

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The changes since the previous review appear safe to merge.

Summary

This PR adds git-based enterprise release lines, backports, release checks, and tests. Community releases stay separate.

  • Enterprise release lines get their own versions and support state.
  • The convergence gate shows when supported release lines lack a fix.
  • Fixes from main are offered to release lines that still need them.
  • Release-line pull requests show whether their changes fit the line.
  • The installer’s latest choice only considers community release tags.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Fix merged to main] --> B[Backport selects release lines]
  B --> C{Pick succeeds?}
  C -->|Yes| D[Bot squash-merges pick]
  C -->|Conflict| E[Open conflict PR]
  E --> F[Person resolves and merges]
  E --> G[Person closes PR]
  G --> H[Bot records decline]
  D --> I[Release branch CI]
  F --> I
  H --> I
Loading

Reviews (7) · Last reviewed commit: "fix: pass workflow inputs to the release..." · Reviewed by Greptile

Comment thread .github/release/relmgr.py
picked_from resolves each -x trailer to its full sha, so the newer pick
replaces a hand-written decline that used a short sha.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Port of the release tooling change from the enterprise repo (tree e07ef33).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
- Release tooling matches the cloud copy: tidier comments, the newest RC kept on abandon, guard refusals reported
  as CLI errors, and tests for the runner roots and the topic-branch head check.
- release-branch-only.yaml adds the Release-branch-only commit on the label, as in the cloud repo.
- Backport runs on Python 3.12 without bytecode files and drops an unused token; Branch state drops one too.
- Comment fixes in the release workflows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
A dispatch input that starts with "-" now reaches the CLI as a value, so it is refused with the usual message
instead of an argparse usage error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbT4FqysueCepjZ1uCpPk1
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
⚪ e2e-agent-kv e2e 0 0 0 1 0.1
✅ e2e-api-deployment e2e 3 0 0 0 16.8
✅ e2e-coowners e2e 1 0 0 0 1.4
✅ e2e-etl e2e 1 0 0 0 20.4
✅ e2e-login e2e 2 0 0 0 1.1
✅ e2e-prompt-studio e2e 1 0 0 0 7.3
✅ e2e-smoke e2e 2 0 0 0 2.4
✅ e2e-workflow e2e 1 0 0 0 16.4
✅ frontend unit 629 0 0 0 20.3
✅ integration-backend integration 1448 0 0 26 57.3
✅ integration-connectors integration 1 0 0 7 6.9
✅ integration-workers integration 181 0 0 1 39.3
❌ ui e2e 0 1 0 0 0.0
✅ unit-agent-kv-schema unit 143 0 0 0 1.8
✅ unit-backend unit 2052 0 0 1 61.9
✅ unit-connectors unit 82 0 0 0 12.0
✅ unit-core unit 281 0 0 0 3.5
✅ unit-filesystem unit 1 0 0 0 4.6
✅ unit-platform-service unit 18 0 0 0 2.9
✅ unit-rig unit 120 0 0 0 4.7
✅ unit-runner unit 12 0 0 0 3.1
✅ unit-sdk1 unit 878 0 0 0 42.1
✅ unit-tool-sidecar unit 8 0 0 0 1.8
✅ unit-workers unit 1534 0 0 2 127.8
TOTAL 7399 1 0 38 455.8

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • mcp-server-auth — covered by integration-backend
  • mcp-platform-auth — covered by integration-backend
  • platform-key-whoami — covered by integration-backend
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants