ci(asf.yaml): stop blocking merge on unresolved review threads - #1155
Merged
Conversation
`required_conversation_resolution: true` made *any* open review thread a hard merge gate. With the approval requirement currently lifted (see the note above `protected_branches`), it was the one gate a reviewer could trip by accident. The failure mode is specific: a reviewer leaves a nit they explicitly mark non-blocking, the PR is then unmergeable until someone resolves it, and the reviewer ends up resolving their own advisory comment purely to unblock the merge. That destroys the signal the comment was meant to carry — an unresolved thread should mean "the author has not looked at this yet", not "the merge button is stuck". Unresolved threads stay visible in the PR UI; they are simply no longer a hard gate. The `zizmor` / `prek` / `tests-ok` status checks and linear history are untouched, so nothing merges without green CI. Restore alongside `required_pull_request_reviews` if the project later wants threads to gate merge again. Generated-by: Claude Code (Opus 5)
4 tasks
potiuk
added a commit
that referenced
this pull request
Sep 8, 2026
The marker sat at a1cff44, 17 commits behind main. Bumping it alone would claim those commits are described by the specs, so the drift is closed first. Specs updated for what actually shipped: - meta-and-quality-tooling: skill-evals errors, rather than passing, when a case's CLI produced no gradeable output (#1161). - security-reporting: the tracker dashboard projects the current partial bucket to its end-of-bucket value, splitting RATE series (accumulate from zero) from LEVEL series (carry over), and deliberately not projecting mean-based signals (#1158). - project-agnosticism: <PROJECT> and <project> are two placeholders holding different values, and the lint carries both spellings plus spaced variants (#1154). - adapters: the forwarder relay's contact_handle defaults to an org-level shared inbox rather than a named individual (#1135). The multi-hop coordinator case is designed in RFC-AI-0008 and unimplemented. - issue-management-family: the family's eval suites, and the note that --cli runs belong outside a credential-denying sandbox (#1145). Commits needing no spec change: #1152, #1143 and #1156 updated their own specs in-commit; #1149, #1147, #1151 are behaviour-preserving bug fixes; #1155 and #1141 are CI and dependency chores; #1159's spec edits landed with it; #1144 removes hardcoded literals that no spec asserted. One genuine gap recorded rather than papered over: no spec covers marketplace distribution or the dev-version stamping rule from #1160, which is load-bearing because `claude plugin update` compares version strings, not commit SHAs. Logged in adoption-and-setup as wanting its own spec. Generated-by: Claude Code (Opus 5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
required_conversation_resolution: truemade any open review thread a hard merge gate. With the approval requirement currently lifted, it was the one gate a reviewer could trip by accident.zizmor/prek/tests-okandrequired_linear_historyare untouched, so nothing merges without green CI.Not included: Actions workflow approval
The other half of the request — stop requiring maintainer approval for contributors' workflow runs — cannot be expressed in
.asf.yaml. I checked the asfyaml implementation rather than guessing:asfyaml/feature/github/has modules for branch protection, rulesets, merge buttons, collaborators, pull requests, protected tags, pages, environments and Copilot review — but no Actions module, and noactions/fork/approvalkeys anywhere in the feature set.That setting lives in GitHub's Settings → Actions → General → Fork pull request workflows from outside collaborators. It needs either a repo admin or an ASF Infra (JIRA) request. Worth noting
potiukcurrently haspush/triagebut notadminormaintainon this repo, so it needs Infra.Empirically the repo is on "Require approval for all outside collaborators" rather than the first-time-contributor default:
AmirF194is a returningCONTRIBUTORwith three open PRs and every run is still gated. Moving it to "Require approval for first-time contributors" would unblock the recurring contributors while keeping the guard for brand-new accounts.Type of change
prek, workflows, validators)Test plan
prekpasses on the changed file.required_conversation_resolutionis a supportedprotected_brancheskey (asfyaml/feature/github/branch_protection.py:140), so this lands rather than being silently ignored.protected_brancheswithin a few minutes of merge tomain.Linked issues
Follow-up to the review pass on #1141, #1144, #1145, #1143, #1142.