mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-23 22:14:18 +02:00
Clarify PR-reopen flow and fix label-race that broke auto-reopen (#4518)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
d779414f8a
commit
a3ecd1edb1
4 changed files with 104 additions and 20 deletions
73
.github/scripts/triage-label.sh
vendored
Executable file
73
.github/scripts/triage-label.sh
vendored
Executable file
|
|
@ -0,0 +1,73 @@
|
|||
#!/usr/bin/env bash
|
||||
# Locked-down label helper for the Marvin triage workflow.
|
||||
#
|
||||
# Marvin runs on untrusted issue/PR bodies from non-write users, so it must
|
||||
# NOT be handed raw `gh api` (that would expose every endpoint the app token
|
||||
# can reach). This helper is the ONLY GitHub write it is allowed to perform:
|
||||
# it adds or removes repository labels on the one issue/PR being triaged.
|
||||
#
|
||||
# The target repo and number come from the environment set by the workflow —
|
||||
# never from the model — and the operation is fixed to the additive labels
|
||||
# endpoint (POST/DELETE /repos/{repo}/issues/{n}/labels), which works for both
|
||||
# issues and PRs and cannot clobber labels applied by other workflows.
|
||||
set -euo pipefail
|
||||
|
||||
repo="${TRIAGE_REPO:?TRIAGE_REPO not set}"
|
||||
number="${TRIAGE_NUMBER:?TRIAGE_NUMBER not set}"
|
||||
|
||||
if [[ ! "$number" =~ ^[0-9]+$ ]]; then
|
||||
echo "TRIAGE_NUMBER must be numeric, got: $number" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
op="${1:-}"
|
||||
shift || true
|
||||
case "$op" in
|
||||
add) method=POST ;;
|
||||
remove) method=DELETE ;;
|
||||
*)
|
||||
echo "usage: triage-label.sh <add|remove> <label>..." >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
|
||||
if [[ "$#" -eq 0 ]]; then
|
||||
echo "no labels given" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Reject anything that isn't a plausible label name. Notably blocks '/' so a
|
||||
# crafted value can't turn the DELETE path into a different endpoint.
|
||||
label_re="^[A-Za-z0-9 ._'-]+$"
|
||||
for label in "$@"; do
|
||||
if [[ ! "$label" =~ $label_re ]]; then
|
||||
echo "refusing suspicious label name: $label" >&2
|
||||
exit 1
|
||||
fi
|
||||
done
|
||||
|
||||
# Never let triage add or remove the Require Issue Link control labels. Those
|
||||
# govern PR enforcement (bypass-issue-check / trusted-contributor are sticky
|
||||
# exemptions) and reopening (missing-issue-link is how closed PRs are found),
|
||||
# so a prompt-injected triage run must not be able to grant an exemption or
|
||||
# break recovery. Enforced here — in code — not merely in the prompt.
|
||||
protected=" missing-issue-link bypass-issue-check trusted-contributor "
|
||||
for label in "$@"; do
|
||||
lower="${label,,}"
|
||||
if [[ "$protected" == *" $lower "* ]]; then
|
||||
echo "refusing to touch protected control label: $label" >&2
|
||||
exit 1
|
||||
fi
|
||||
done
|
||||
|
||||
if [[ "$method" == POST ]]; then
|
||||
args=()
|
||||
for label in "$@"; do
|
||||
args+=(-f "labels[]=$label")
|
||||
done
|
||||
gh api --method POST "/repos/${repo}/issues/${number}/labels" "${args[@]}"
|
||||
else
|
||||
for label in "$@"; do
|
||||
gh api --method DELETE "/repos/${repo}/issues/${number}/labels/${label}"
|
||||
done
|
||||
fi
|
||||
19
.github/workflows/marvin-label-triage.yml
vendored
19
.github/workflows/marvin-label-triage.yml
vendored
|
|
@ -49,13 +49,16 @@ jobs:
|
|||
PROMPT<<PROMPT_END
|
||||
You're an issue triage assistant for FastMCP, a Python framework for building Model Context Protocol servers and clients. Your task is to analyze issues/PRs and apply appropriate labels.
|
||||
|
||||
IMPORTANT: Your primary action should be to apply labels using mcp__github__update_issue. DO NOT post comments EXCEPT when applying the too-long label (see below).
|
||||
IMPORTANT: Your primary action should be to apply labels using the locked-down helper `.github/scripts/triage-label.sh`. DO NOT post comments EXCEPT when applying the too-long label (see below).
|
||||
|
||||
CRITICAL — LABEL MECHANICS:
|
||||
- `mcp__github__update_issue` REPLACES all labels on the issue — it does not add to them.
|
||||
- Before applying labels, read the issue's current labels with `mcp__github__get_issue`.
|
||||
- Always include any existing labels you want to keep alongside the new ones.
|
||||
- Apply labels ONLY through the helper, which adds or removes repository labels on THIS issue/PR. It already knows the target repo and number (from the workflow environment) — you never pass them:
|
||||
add: `bash .github/scripts/triage-label.sh add "label1" "label2"`
|
||||
remove: `bash .github/scripts/triage-label.sh remove "label1"`
|
||||
- The helper uses the additive REST labels endpoint, so it works for both issues and PRs and never clobbers labels applied by other workflows — notably the Require Issue Link workflow's `missing-issue-link` control label, which must survive or an auto-closed PR won't reopen when its author is assigned.
|
||||
- The helper is your ONLY GitHub write access. Do NOT use raw `gh api`, `gh issue edit`, `gh pr edit`, or any other mutation — they are not available to you.
|
||||
- Only apply labels that exist in the repository (from `gh label list` in step 1). Never invent labels.
|
||||
- Use `remove` only to correct a label you believe is wrong, and never remove the control labels `missing-issue-link`, `bypass-issue-check`, or `trusted-contributor`.
|
||||
|
||||
Issue/PR Information:
|
||||
- REPO: ${{ github.repository }}
|
||||
|
|
@ -131,7 +134,7 @@ jobs:
|
|||
- DON'T MERGE: Only if PR author explicitly states it's not ready
|
||||
|
||||
4. Apply selected labels:
|
||||
Use mcp__github__update_issue to apply your selected labels
|
||||
Add them with `bash .github/scripts/triage-label.sh add "label1" "label2"`.
|
||||
DO NOT post any comments unless applying too-long (see above)
|
||||
PROMPT_END
|
||||
EOF
|
||||
|
|
@ -149,11 +152,13 @@ jobs:
|
|||
allowed_non_write_users: "*"
|
||||
allowed_bots: "marvin-context-protocol"
|
||||
claude_args: |
|
||||
--allowedTools Bash(gh label list),mcp__github__get_issue,mcp__github__get_issue_comments,mcp__github__update_issue,mcp__github__add_issue_comment,mcp__github__get_pull_request_files
|
||||
--allowedTools Bash(gh label list),Bash(bash .github/scripts/triage-label.sh:*),mcp__github__get_issue,mcp__github__get_issue_comments,mcp__github__add_issue_comment,mcp__github__get_pull_request_files
|
||||
settings: |
|
||||
{
|
||||
"model": "claude-sonnet-4-6",
|
||||
"env": {
|
||||
"GH_TOKEN": "${{ steps.marvin-token.outputs.token }}"
|
||||
"GH_TOKEN": "${{ steps.marvin-token.outputs.token }}",
|
||||
"TRIAGE_REPO": "${{ github.repository }}",
|
||||
"TRIAGE_NUMBER": "${{ github.event.issue.number || github.event.pull_request.number || inputs.issue_number }}"
|
||||
}
|
||||
}
|
||||
|
|
|
|||
23
.github/workflows/require-issue-link.yml
vendored
23
.github/workflows/require-issue-link.yml
vendored
|
|
@ -354,29 +354,30 @@ jobs:
|
|||
async function enforceFailure(kind) {
|
||||
await addLabel();
|
||||
|
||||
const intro = kind === 'no-link'
|
||||
? '**This PR has been automatically closed** because its description does not reference a tracked issue.'
|
||||
: '**This PR has been automatically closed** because you are not assigned to the issue it references.';
|
||||
const reason = kind === 'no-link'
|
||||
? "it doesn't reference a tracked issue assigned to you"
|
||||
: "you aren't assigned to the issue it references";
|
||||
const steps = kind === 'no-link'
|
||||
? [
|
||||
`1. Find or [open an issue](https://github.com/${owner}/${repo}/issues/new/choose) describing the change.`,
|
||||
'2. Comment on the issue to ask a maintainer to assign it to you.',
|
||||
'3. Add `Fixes #<issue>`, `Closes #<issue>`, or `Resolves #<issue>` to the PR description.',
|
||||
'4. Once you are assigned and the link is present, the PR reopens automatically.',
|
||||
`1. Find or [open an issue](https://github.com/${owner}/${repo}/issues/new/choose) describing the change — if you open it, you have first claim on it.`,
|
||||
"2. Add `Fixes #<issue>`, `Closes #<issue>`, or `Resolves #<issue>` to **this** PR's description — edit it in place, don't open a new PR.",
|
||||
]
|
||||
: [
|
||||
'1. Comment on the linked issue to ask a maintainer to assign it to you.',
|
||||
'2. Once a maintainer assigns you, the PR reopens automatically.',
|
||||
"1. If you opened the linked issue, a maintainer will assign you when they pick it up and this PR reopens automatically. If someone else opened it, the PR reopens only if a maintainer chooses to assign it to you — please don't comment to ask.",
|
||||
];
|
||||
|
||||
const commentBody = [
|
||||
MARKER,
|
||||
intro,
|
||||
"**Don't open a new pull request — this one reopens on its own.** It's closed for " +
|
||||
`now because ${reason}, but the moment that's fixed it reopens automatically. Keep this ` +
|
||||
'PR and edit it; opening a fresh duplicate just starts you over and creates more to triage.',
|
||||
'',
|
||||
`Per [CONTRIBUTING.md](https://github.com/${owner}/${repo}/blob/main/CONTRIBUTING.md), an external PR must reference an issue that is assigned to its author. To proceed:`,
|
||||
`Per [CONTRIBUTING.md](https://github.com/${owner}/${repo}/blob/main/CONTRIBUTING.md), an external PR must reference an issue that's assigned to its author. To get there:`,
|
||||
'',
|
||||
...steps,
|
||||
'',
|
||||
"Once you're assigned and the link is present, this PR reopens automatically — no further action needed.",
|
||||
'',
|
||||
`*Maintainers: reopen this PR or remove the \`${LABEL}\` label to bypass this check.*`,
|
||||
].join('\n');
|
||||
|
||||
|
|
|
|||
|
|
@ -18,9 +18,13 @@ That's it. No need to diagnose root causes, propose API designs, or suggest impl
|
|||
|
||||
We encourage you to use LLMs to help identify bugs, write MREs, and prepare contributions. But if you do, your LLM must take into account the conventions and contributing guidelines of this repo — including how we want issues formatted and when it's appropriate to open a PR. Generic LLM output that ignores these guidelines tells us the contribution wasn't made thoughtfully, and we will close it. A good AI-assisted contribution is indistinguishable from a good human one. A bad one is obvious.
|
||||
|
||||
If you're driving an agent: do **not** have it post comments asking to be assigned to an issue or announcing that it intends to work on one. Those comments are ignored. If the agent intends to contribute, open a PR instead — it will be gated on assignment (see below). Comment on an issue only to propose a genuinely novel, differentiated solution, never to claim a task that's already described.
|
||||
|
||||
## When to open a pull request
|
||||
|
||||
An open issue is not an invitation to submit a PR. Issues track problems; whether and how to solve them is a separate decision. If you want to work on something, propose your approach in the issue first and ask a maintainer to assign it to you — especially for anything beyond a trivial fix. External PRs that reference an issue not assigned to their author are closed automatically (see [PR guidelines](#pr-guidelines)).
|
||||
An open issue is not an invitation to submit a PR, and it is not a queue you join by commenting. Issues track problems; who implements them and how is a separate decision maintainers make, and whoever opened the issue has first claim on it.
|
||||
|
||||
**Don't post drive-by comments claiming an issue** — "can I work on this?", "please assign me", "I'll take this." They don't affect who gets assigned, they're the most common form of noise we get, and automated versions are ignored. Whoever opens the issue has first claim on it; if that's you, a maintainer will assign you. If you want to implement something someone else reported, just open a PR — you don't need permission to try, and competing PRs are fine — but it's reviewed only if a maintainer assigns you to the issue, which usually won't happen if the reporter intends to handle it. The one comment worth posting is a genuinely different approach worth discussing; a substantive design proposal is welcome, a bare claim on the task is not.
|
||||
|
||||
**Bug fixes** — PRs are welcome for simple, well-scoped bug fixes where the problem and solution are both straightforward. "The function raises `TypeError` when passed `None` because of a missing guard" is a good candidate. If the fix requires design decisions or touches multiple subsystems, open an issue with a design proposal instead.
|
||||
|
||||
|
|
@ -34,7 +38,8 @@ An open issue is not an invitation to submit a PR. Issues track problems; whethe
|
|||
|
||||
If you do open a PR:
|
||||
|
||||
- **Reference an issue you're assigned to.** Every PR must reference a tracked issue using an auto-close keyword (`Fixes #123`, `Closes #123`, or `Resolves #123`), and the referenced issue must be assigned to you. If there isn't an issue, open one; then comment to ask a maintainer to assign it to you. This lets us deconflict effort and steer the approach before you invest time in code. External PRs that don't meet both conditions are automatically labeled `missing-issue-link` and closed; they reopen automatically once the link is present and you're assigned.
|
||||
- **Reference an issue you're assigned to.** Every PR must reference a tracked issue using an auto-close keyword (`Fixes #123`, `Closes #123`, or `Resolves #123`), and the referenced issue must be assigned to you. If there isn't an issue, open one. This lets us deconflict effort and steer the approach before you invest time in code. External PRs that don't meet both conditions are automatically labeled `missing-issue-link` and closed; they reopen automatically once the link is present and you're assigned.
|
||||
- **If your PR was auto-closed, don't open a new one.** Edit the *existing* PR to add the issue link, get assigned to that issue, and it reopens on its own — the branch and history are preserved. A duplicate PR just starts you over and adds to the triage pile.
|
||||
- **Keep it focused.** One logical change per PR. Don't bundle unrelated fixes or refactors.
|
||||
- **Match existing patterns.** Follow the code style, type annotation conventions, and test patterns you see in the codebase. Run `uv run prek run --all-files` before submitting.
|
||||
- **Write tests.** Bug fixes should include a test that fails without the fix. Enhancements should include tests for the new behavior.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue