Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions bin/hive-open-issue.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#
# Usage (drop-in for the common gh shapes):
# hive-open-issue --repo <owner/repo> --title "<t>" [--body "<b>"|--body-file f] [--label a,b] [--parent <n>] [--blocked-by <n[,n]>] [--close-on-merge] [--needs-decision] [--no-consolidate <reason>]
# hive-open-issue comment --repo <owner/repo> <number|url> --body "<b>" [--needs-decision|--withdraw-decision] [--blocked-by <n[,n]>] [--blocked-on-pr <n>]
# hive-open-issue comment --repo <owner/repo> <number|url> --body "<b>" [--needs-decision|--withdraw-decision] [--blocked-by <n[,n]>] [--blocked-on-pr <n|owner/repo#n>]
# hive-open-issue claim --repo <owner/repo> <number|url>
# hive-open-issue close --repo <owner/repo> <number|url> [--override-reason "..."]
# hive-open-issue label --repo <owner/repo> <number|url> [--label a,b] [--remove-label c]
Expand Down Expand Up @@ -73,9 +73,10 @@
# request is unrelated work that merely shares a file or component with one.
# The reason is required, and the watcher records it in the result.
#
# --blocked-on-pr <n> (comment only) parks the issue on open PR <n> in the
# same repo (hivecommons/hive#13093): use it when the code a finding names
# exists only in an unmerged PR, so there is nothing to ship until it lands.
# --blocked-on-pr <n|owner/repo#n> (comment only) parks the issue on open PR
# <n> in the same repo, or on owner/repo#n in another one
# (hivecommons/hive#13093): use it when the code a finding names exists only
# in an unmerged PR, so there is nothing to ship until it lands.
# The watcher prefixes the note with the blocked-on marker, strips any
# @-mentions (the PR's author is not asked to do the hive's work), adds the
# `blocked` label and removes `help wanted`; the hive lifts the label itself
Expand Down Expand Up @@ -152,6 +153,7 @@ NEEDS_DECISION=0
WITHDRAW_DECISION=0
STEP_KIND=""; RELEASE_TAG=""; RELEASE_TARGET=""; STEP_REQUESTED=0
BLOCKED_ON_PR=""
BLOCKED_ON_PR_REPO=""
NO_CONSOLIDATE=""
NO_CONSOLIDATE_SET=0
DRY_RUN=0
Expand Down Expand Up @@ -280,8 +282,14 @@ if [ -n "${BLOCKED_ON_PR:-}" ] && [ "$KIND" != "comment" ]; then
echo "hive-open-issue: --blocked-on-pr only applies to a comment; nothing was written." >&2
exit 2
fi
# owner/repo#n names a PR in another repository: split it into the number
# and the repo the watcher reads it from.
if [ -n "${BLOCKED_ON_PR:-}" ] && [[ "$BLOCKED_ON_PR" =~ ^([A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)#([0-9]+)$ ]]; then
BLOCKED_ON_PR_REPO="${BASH_REMATCH[1]}"
BLOCKED_ON_PR="${BASH_REMATCH[2]}"
fi
if [ -n "${BLOCKED_ON_PR:-}" ] && ! [[ "$BLOCKED_ON_PR" =~ ^[0-9]+$ && "$BLOCKED_ON_PR" -gt 0 ]]; then
echo "hive-open-issue: --blocked-on-pr needs a positive PR number; nothing was written." >&2
echo "hive-open-issue: --blocked-on-pr needs a positive PR number or owner/repo#n; nothing was written." >&2
exit 2
fi

Expand Down Expand Up @@ -399,11 +407,12 @@ REMOVE_LABELS_JSON="$(printf '%s\n' "${REMOVE_LABELS[@]:-}" | python3 -c 'import
REVIEWERS_JSON="$(printf '%s\n' "${REVIEWERS[@]:-}" | python3 -c 'import json,sys; print(json.dumps([x.rstrip("\n") for x in sys.stdin if x.rstrip("\n")]))')"
TEAM_REVIEWERS_JSON="$(printf '%s\n' "${TEAM_REVIEWERS[@]:-}" | python3 -c 'import json,sys; print(json.dumps([x.rstrip("\n") for x in sys.stdin if x.rstrip("\n")]))')"
BLOCKED_BY_JSON="$(printf '%s\n' "${BLOCKED_BY[@]:-}" | python3 -c 'import json,sys; print(json.dumps([x.rstrip("\n") for x in sys.stdin if x.rstrip("\n")]))')"
python3 - "$TEMP_FILE" "$REQ_FILE" "$KIND" "$REPO" "$TITLE" "${OVERRIDE_REASON:-$BODY}" "$AGENT" "$LABELS_JSON" "${NUMBER:-0}" "$DRY_RUN" "${PARENT:-0}" "$REMOVE_LABELS_JSON" "$REVIEWERS_JSON" "$TEAM_REVIEWERS_JSON" "$BLOCKED_BY_JSON" "$NEEDS_DECISION" "${BLOCKED_ON_PR:-0}" "$STEP_KIND" "$RELEASE_TAG" "$RELEASE_TARGET" "$STEP_REQUESTED" "$NO_CONSOLIDATE" "$WITHDRAW_DECISION" <<'PY'
python3 - "$TEMP_FILE" "$REQ_FILE" "$KIND" "$REPO" "$TITLE" "${OVERRIDE_REASON:-$BODY}" "$AGENT" "$LABELS_JSON" "${NUMBER:-0}" "$DRY_RUN" "${PARENT:-0}" "$REMOVE_LABELS_JSON" "$REVIEWERS_JSON" "$TEAM_REVIEWERS_JSON" "$BLOCKED_BY_JSON" "$NEEDS_DECISION" "${BLOCKED_ON_PR:-0}" "$STEP_KIND" "$RELEASE_TAG" "$RELEASE_TARGET" "$STEP_REQUESTED" "$NO_CONSOLIDATE" "$WITHDRAW_DECISION" "$BLOCKED_ON_PR_REPO" <<'PY'
import json, os, re, sys
temporary, path, kind, repo, title, body, agent, labels, number, dry_run, parent, remove_labels, reviewers, team_reviewers, blocked_by, needs_decision, blocked_on_pr = sys.argv[1:18]
no_consolidate = sys.argv[22]
withdraw_decision = sys.argv[23]
blocked_on_pr_repo = sys.argv[24].strip()
labels = [part.strip() for value in json.loads(labels)
for part in value.split(",") if part.strip()]
remove_labels = [part.strip() for value in json.loads(remove_labels)
Expand Down Expand Up @@ -518,6 +527,9 @@ if kind == "comment" and int(blocked_on_pr) > 0:
# --blocked-on-pr: the watcher adds the marker and the `blocked` label
# itself (hivecommons/hive#13093).
req["blocked_on_pr"] = int(blocked_on_pr)
if blocked_on_pr_repo:
# owner/repo#n: the PR lives in another repository.
req["blocked_on_pr_repo"] = blocked_on_pr_repo
if dry_run == "1":
# Honour --dry-run: show exactly what would be queued, write nothing.
print("hive-open-issue: DRY RUN — no request written, nothing will be created. Would write %s:" % path)
Expand All @@ -541,7 +553,7 @@ if [ "$KIND" = "comment" ]; then
echo "hive-open-issue: will withdraw your decision park on $REPO#$NUMBER (refused if the park is not yours)"
fi
if [ -n "${BLOCKED_ON_PR:-}" ]; then
echo "hive-open-issue: will park the issue on $REPO#$BLOCKED_ON_PR (blocked label; help wanted removed; no one is @-mentioned)"
echo "hive-open-issue: will park the issue on ${BLOCKED_ON_PR_REPO:-$REPO}#$BLOCKED_ON_PR (blocked label; help wanted removed; no one is @-mentioned)"
fi
elif [ "$KIND" = "claim" ]; then
echo "hive-open-issue: requested claim of $REPO#$NUMBER (hive/claimed-by-$AGENT label)"
Expand Down
39 changes: 39 additions & 0 deletions bin/test_hive_open_issue.sh
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,45 @@ check "comment --blocked-by refusal writes no request" "" "$(find_req badblocked
rm -rf "$REQ_DIR"
mkdir -p "$REQ_DIR"

# --blocked-on-pr takes a same-repo number or owner/repo#n (a PR in another
# repository): the request carries the number and, for the second, its repo.
run_script "blockedonprbot" comment --repo "hiveorg/widgets" 7 --body "Lands after the upstream change." \
--blocked-on-pr "hiveorg/gadgets#42" >/dev/null
REQ_FILE="$(find_req blockedonprbot)"
if [ -n "$REQ_FILE" ]; then
GOT_BLOCKED="$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); print(d.get('blocked_on_pr'), d.get('blocked_on_pr_repo'))" "$REQ_FILE")"
check "comment --blocked-on-pr owner/repo#n carries the number and the repo" "42 hiveorg/gadgets" "$GOT_BLOCKED"
else
echo " FAIL: request file not created for --blocked-on-pr owner/repo#n test"
FAIL=$((FAIL + 1))
fi

rm -rf "$REQ_DIR"
mkdir -p "$REQ_DIR"

run_script "blockedonprsamebot" comment --repo "hiveorg/widgets" 7 --body "Lands after #41." --blocked-on-pr 41 >/dev/null
REQ_FILE="$(find_req blockedonprsamebot)"
if [ -n "$REQ_FILE" ]; then
GOT_BLOCKED="$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); print(d.get('blocked_on_pr'), d.get('blocked_on_pr_repo'))" "$REQ_FILE")"
check "comment --blocked-on-pr <n> stays same-repo (no blocked_on_pr_repo)" "41 None" "$GOT_BLOCKED"
else
echo " FAIL: request file not created for --blocked-on-pr <n> test"
FAIL=$((FAIL + 1))
fi

rm -rf "$REQ_DIR"
mkdir -p "$REQ_DIR"

set +e
run_script "badblockedonprbot" comment --repo "hiveorg/widgets" 7 --body "x" --blocked-on-pr "hiveorg/gadgets#" >/dev/null 2>&1
BAD_RC=$?
set -e
check "comment --blocked-on-pr refuses a malformed owner/repo#n (exit 2)" "2" "$BAD_RC"
check "comment --blocked-on-pr refusal writes no request" "" "$(find_req badblockedonprbot)"

rm -rf "$REQ_DIR"
mkdir -p "$REQ_DIR"

# --- Section 3d: --close-on-merge flag (filing-time opt-in, #10304) ---
run_script "closeonmergebot" --repo "hiveorg/widgets" --title "Sweep finding" --body "found by reading the code" --close-on-merge

Expand Down
1 change: 1 addition & 0 deletions changelog.d/added-blocked-pr-depends-on.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **A PR now waits for the pull requests its `Depends-on:` lines name, in the same repository or another one.** Before, only the dashboard's critical path read `Depends-on: #N` / `owner/repo#N`, and nothing stopped a merge lane from landing the PR first, so a person had to hold it and remember to release it. The blocked sweep (every 5 minutes, configured repositories) now puts the existing `blocked` label on an open, non-draft PR whose `Depends-on:` names an open pull request, with one `<!-- hive:blocked-on pr=owner/repo#N -->` comment per open target (the marker a relay's blocked verdict already writes), and lifts it with one `<!-- hive:unblocked -->` note once every marked pull request has merged or closed (the note says when one closed without merging) or is no longer listed. Every merge lane now refuses a `blocked` PR (`label:blocked`): self-authored and trusted-bot, trusted-author, contributor (whatever `exclude_labels` says), the queued merge-request lane, the merge train, the native merge queue and the dashboard's merge verdict and human-merge shortcuts. Like `hold` it is a gate, not a stage: the route and the activity monitor row name the target (`blocked on owner/repo#N`, `Blocked on PR #N — lifts when it merges`). People win: a person's `blocked` is never placed over or lifted, and a person who removes the hive's `blocked` is not overridden for the same dependencies (a newly listed open one re-arms it). Writes go through the write-back gate (`gate_comment`). An issue named in `Depends-on:` stays display-only. `hive-open-issue comment --blocked-on-pr` now also takes `owner/repo#n`.
13 changes: 10 additions & 3 deletions src/cmd/hive/main_sweeps.go
Original file line number Diff line number Diff line change
Expand Up @@ -1281,11 +1281,18 @@ func runBlockedIssuesSweepIfDue(ctx context.Context, ghClient *github.Client, cf
res, err := ghClient.SweepBlockedIssues(ctx, github.BlockedSweepOptions{Repos: repos})
if err != nil {
logger.Warn("blocked issues reconciliation sweep failed", "error", err)
return
}
if res.Unblocked+res.Retargeted+res.Stripped+res.Errors > 0 {
} else if res.Unblocked+res.Retargeted+res.Stripped+res.Errors > 0 {
logger.Info("blocked issues reconciliation sweep complete", "items", res.Items, "unblocked", res.Unblocked, "retargeted", res.Retargeted, "help_wanted_stripped", res.Stripped, "errors", res.Errors)
}
// The same sweep holds PRs on the open pull requests their Depends-on
// lines name, and lifts that hold once they land.
prRes, err := ghClient.SweepBlockedPRDependencies(ctx, github.BlockedSweepOptions{Repos: repos})
if err != nil {
logger.Warn("blocked PR dependency sweep: a repository could not be listed", "error", err)
}
if prRes.Placed+prRes.Marked+prRes.Unblocked+prRes.Suppressed+prRes.Errors > 0 {
logger.Info("blocked PR dependency sweep complete", "prs", prRes.PRs, "placed", prRes.Placed, "marked", prRes.Marked, "released_by_person", prRes.Released, "unblocked", prRes.Unblocked, "suppressed", prRes.Suppressed, "errors", prRes.Errors)
}
}

// runDuplicateSweepIfDue clusters open PRs by changed-file set and suggests
Expand Down
7 changes: 7 additions & 0 deletions src/cmd/hive/merge_eligibility.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,13 @@ func automergeBlockLabelReason(labels []string, trusted bool) string {
return "label:hold"
}
}
// Last, as in the lanes' labelBlockReason: a PR held on its Depends-on
// targets (or a person's `blocked`) is never merge-eligible.
for _, label := range labels {
if strings.EqualFold(strings.TrimSpace(label), github.BlockedLabel) {
return "label:" + github.BlockedLabel
}
}
return ""
}

Expand Down
24 changes: 24 additions & 0 deletions src/cmd/hive/merge_eligibility_blocked_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package main

import "testing"

// The dashboard's merge verdict refuses a `blocked` PR like the lanes do, so
// a PR held on its Depends-on targets never reads merge-eligible. A hold on
// the same PR still reads as the hold.
func TestAutomergeBlockLabelReasonBlocked(t *testing.T) {
for _, tc := range []struct {
labels []string
want string
}{
{[]string{"blocked"}, "label:blocked"},
{[]string{"kind/bug", "Blocked"}, "label:blocked"},
{[]string{"blocked", "hold"}, "label:hold"},
{[]string{"kind/bug"}, ""},
} {
for _, trusted := range []bool{true, false} {
if got := automergeBlockLabelReason(tc.labels, trusted); got != tc.want {
t.Fatalf("automergeBlockLabelReason(%v, %v) = %q, want %q", tc.labels, trusted, got, tc.want)
}
}
}
}
5 changes: 5 additions & 0 deletions src/docs/dashboard.md
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,11 @@ are untouched.
(`owner/repo#N` names another repository). Agents write it when they know
the order. A prerequisite in a tracked repository that is no longer open is
done and drops out; one in an untracked repository stays as `external`.
The line also holds the merge: while a pull request it names is open, the
blocked sweep puts `blocked` on the PR and no merge lane merges it, and
the row reads `Blocked on PR #N — lifts when it merges`. See
[`blocked`](labels-and-control-signals/reference.md#blocked). A
prerequisite that is an issue stays display-only here.
- An issue waits on the open PR whose closing link (`Fixes`, `Closes`,
`Resolves #N`, or GitHub's own closing reference) names it, and on its open
GitHub "blocked by" issues.
Expand Down
9 changes: 7 additions & 2 deletions src/docs/hive-open-issue.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Shapes are selected by an optional leading positional keyword (`comment`,

```sh
hive-open-issue --repo <owner/repo> --title "<t>" [--body "<b>"|--body-file f] [--label a,b] [--parent <n>] [--blocked-by <n[,n]>] [--needs-decision] [--no-consolidate <reason>] [--step-kind release --release-tag <tag> --release-target <sha>] [--claim]
hive-open-issue comment --repo <owner/repo> <number|url> --body "<b>" [--needs-decision|--withdraw-decision] [--blocked-by <n[,n]>] [--blocked-on-pr <n>]
hive-open-issue comment --repo <owner/repo> <number|url> --body "<b>" [--needs-decision|--withdraw-decision] [--blocked-by <n[,n]>] [--blocked-on-pr <n|owner/repo#n>]
hive-open-issue claim --repo <owner/repo> <number|url>
hive-open-issue label --repo <owner/repo> <number|url> [--label a,b] [--remove-label c]
hive-open-issue request-review --repo <owner/repo> <number|url> [--reviewer a,b] [--team-reviewer t]
Expand All @@ -68,7 +68,7 @@ hive-open-issue request-review --repo <owner/repo> <number|url> [--reviewer a,b]
| `--withdraw-decision` | — | comment | take back a decision park you placed yourself; see [Withdrawing your own park](#withdrawing-your-own-park). Cannot be combined with `--needs-decision` |
| `--claim` | — | issue | file the new issue **and** claim it for this agent in the same request (see [Filing your own next step](#filing-your-own-next-step)). The request carries `"claim": true`; the result reports `claimed`, or `claim_error` when the claim did not land |
| `--no-consolidate` | — | issue | requires a non-empty reason; the watcher skips open-issue consolidation (same-subject, file-set and component folds) and files a new issue, echoing the reason as `consolidation_skipped_reason` in the result and logging it. Use it for unrelated work that only shares a file or component with an open issue ([#14360](https://github.com/hivecommons/hive/issues/14360)). The request carries `"no_consolidate_reason"`. A request that declares `--parent` or `--blocked-by` needs no reason: it is a distinct DAG node and never takes the file-set, component or canonical-subject folds, and a genuine create reports `consolidation_skipped_reason` as `parent` or `blocked_by` ([#16065](https://github.com/hivecommons/hive/issues/16065)) |
| `--blocked-on-pr` | — | comment | park the issue on open PR `<n>` in the same repo: the watcher prefixes the note with `<!-- hive:blocked-on pr=owner/repo#n -->`, strips `@`-mentions from it, adds the `blocked` label and removes `help wanted` (see [Blocked on an unmerged PR](#blocked-on-an-unmerged-pr)) |
| `--blocked-on-pr` | — | comment | park the issue on open PR `<n>` in the same repo, or on `owner/repo#n` in another repository (the request then also carries `"blocked_on_pr_repo"`): the watcher prefixes the note with `<!-- hive:blocked-on pr=owner/repo#n -->`, strips `@`-mentions from it, adds the `blocked` label and removes `help wanted` (see [Blocked on an unmerged PR](#blocked-on-an-unmerged-pr)) |
| `--number` | — | comment, claim, close, label, request-review | the issue/PR number; a bare positional number or a `.../issues/N` or `.../pull/N` URL is also accepted |
| `--dry-run` | `-n` | all | validate the arguments and print the exact request that would be written, then exit `0` **without writing it** — nothing is created, commented, claimed, or closed. **This is how you test the tool.** A live call creates a real GitHub issue/comment/label change under the hive's App identity |

Expand Down Expand Up @@ -355,6 +355,11 @@ hive-open-issue comment --repo o/r 13093 --blocked-on-pr 13092 \
--body "Blocked on #13092: the flagged file exists only on that PR; nothing to ship until it lands."
```

A PR in another repository is named `owner/repo#n`
(`--blocked-on-pr acme/gadget#42`); the marker then reads
`<!-- hive:blocked-on pr=acme/gadget#42 -->` and the sweep lifts the block
when that PR merges or closes.

The watcher prefixes the body with the marker `<!-- hive:blocked-on
pr=o/r#13092 -->`, strips any `@login` mention from it (the PR's author is not
asked to do the hive's work; address them only in a review comment on their
Expand Down
Loading
Loading