diff --git a/.claude/skills/code-review/SKILL.md b/.claude/skills/code-review/SKILL.md index 2542ee68f..bcc2698dd 100644 --- a/.claude/skills/code-review/SKILL.md +++ b/.claude/skills/code-review/SKILL.md @@ -19,6 +19,13 @@ Be friendly and welcoming while maintaining high standards. Call out what works Even perfect code for unwanted features should be rejected. +### Dependency version compatibility + +When a PR adapts code to a new version of a dependency (e.g., removing a parameter that was dropped upstream, using a new API): +- **The version pin in `pyproject.toml` must match.** If the change breaks compatibility with the previously-pinned minimum version, the minimum version must be bumped. Otherwise users on the old version get a regression. +- **If backwards compatibility with the old version is desired**, the code must handle both versions (e.g., try/except, version check). Simply deleting the old API usage without bumping the pin is always wrong — it silently breaks users on the old version. +- **Lock file (`uv.lock`) changes should be scoped to the PR's purpose.** A PR fixing a ty compatibility issue should not also include unrelated dependency version bumps (anthropic, google-auth, etc.) from running `uv sync --upgrade`. These create noise and make the diff harder to review. + ### API design and naming Identify confusing patterns or non-idiomatic code: diff --git a/.claude/skills/review-issue/SKILL.md b/.claude/skills/review-issue/SKILL.md new file mode 100644 index 000000000..384aa137c --- /dev/null +++ b/.claude/skills/review-issue/SKILL.md @@ -0,0 +1,168 @@ +--- +name: review-issue +description: Review an incoming external issue (and any gated-closed PR behind it) and decide whether to assign the contributor or decline. Use when the maintainer says "look at this issue", "review issue #N", "should we take this", or asks whether to assign someone. Assigning the author auto-reopens their PR for normal review. This is the entry point for incoming-issue triage — distinct from review-pr, which responds to bot reviews on your own open PR. +--- + +# Triaging contributions under the issue-link gate + +FastMCP auto-closes external PRs unless the author is **assigned to a referenced issue** +(see [require-issue-link.yml](../../../.github/workflows/require-issue-link.yml)). The practical +effect: contributors open an issue, open a PR, get auto-closed, and ask to be assigned. The +maintainer almost never sees the PR directly — **the issue is the decision point**, and +**assigning the author is the single action that reopens their PR** and sends it into review. + +This skill turns "look at this issue" into one of two outcomes: +- **Assign** — the issue is valid, we want it fixed, an external PR is appropriate, and a sound + PR already exists → assign the author (auto-reopens the PR) and queue it for code review. +- **Decline** — leave the issue/PR closed and explain why on the issue. + +Be opinionated about declining. The gate moved spam from junk PRs to junk issues; this skill is +worthless if it just rubber-stamps assignment. Assignment is a commitment to review and likely +merge, not a courtesy. + +## How the gate works (the part that matters here) + +- External PR is closed unless its body has `Fixes/Closes/Resolves #N` **and** the author is + assigned to issue `#N`. +- **Assigning the author to the issue auto-reopens their closed PR** and re-runs the check — + this is the lever you pull. `gh issue edit N --add-assignee `. The assignment fires a + `require-issue-link` run; expect it to pass. If it fails, the gate itself misbehaved (not the + PR) — investigate the run, don't re-assign. +- Maintainer-authored PRs are exempt. A `trusted-contributor` label exempts a contributor up + front. Reopening the PR or removing the `missing-issue-link` label applies a sticky + `bypass-issue-check`. +- Sibling bots have usually already run on the issue: `marvin-triage-issue` (investigates + + recommends), `marvin-dedupe-issues` / `auto-close-duplicates` (dupes), `auto-close-needs-mre` + (missing MRE). Read their comments before re-deriving anything. + +## Step 1 — Orient + +Read the issue, its bot triage, and any PR behind it. Run these together: + +```bash +gh issue view N --repo PrefectHQ/fastmcp \ + --json number,title,state,author,body,labels,assignees,comments +# Find PRs the author opened that reference this issue (they're likely CLOSED): +gh pr list --repo PrefectHQ/fastmcp --state all --search "author: #N in:body" \ + --json number,title,state,url,labels +``` + +If a PR exists, pull its metadata and any review-bot comments (CodeRabbit, Codex). Treat the bot +comments as leads, not conclusions — they often don't run on closed PRs at all, and even when +they do you still owe the PR your own read: + +```bash +gh pr view --repo PrefectHQ/fastmcp --json number,title,body,labels,files,additions,deletions +gh pr view --repo PrefectHQ/fastmcp --comments +``` + +## Step 2 — Classify the issue (is it valid AND a real bug?) + +- Is there a real, reproducible problem? For bugs, demand an MRE that shows FastMCP misbehaving + — not user config error, not a question, not an upstream-SDK issue. +- Is it a duplicate or already fixed on `main`? Check the dedupe bot's comment and recent commits. +- If the issue itself is weak, **stop here and decline** — don't evaluate the PR. A good PR + attached to a bad issue is still declined. + +**A reproducible MRE is not the same as a bug.** This is the trap that produces wrong verdicts: +an MRE can demonstrate real, observable behavior that is nonetheless *not a bug*, because it +violates no contract the framework intends to hold. The decisive question is not "does this +reproduce?" but "does the demonstrated behavior violate the intended contract for this API?" A +shared-mutable-state MRE only matters if callers are *supposed* to mutate that state; an +ordering/timing MRE only matters if the framework promises an order; a "wrong" value only matters +relative to what the API guarantees. An MRE that has to reach past the supported surface to +trigger the behavior (mutating a field meant to be set only at construction, depending on an +internal that isn't part of the public contract) is showing you a property, not a defect. + +You usually cannot read the intended contract off the code — the code shows what it *does*, not +what it *promises*. **The maintainer is often the only authoritative source for the contract, so +stopping to ask is legitimate and expected here.** Ask "is X a supported pattern / does this API +promise Y?" before sinking time into investigating a fix. If the behavior is in-contract correct, +decline — no matter how cleanly the PR fixes it, and no matter how real the MRE looks. + +## Step 3 — Investigate the PR (mandatory; do NOT skip if a PR exists) + +The most common failure of this skill is judging a PR from the diff hunk and the PR description +alone. That is a cursory review and it produces wrong verdicts — a redundant-looking conditional +can be a real bug fix; a tidy-looking diff can patch the wrong layer. **You cannot assess a PR +without reading the code it changes in context.** Reading `gh pr diff` is necessary but never +sufficient. + +Do all of this before forming any opinion on quality: + +1. **Read the diff in full**, then **open every file it touches in the repo** (`Read`, not just + the patch). The hunk shows *what changed*; the file shows *what it changed into*. +2. **Trace the functions and values the change depends on.** Grep for the called functions, + the fields being set, and the defaults. If the PR overrides or replaces a value, find what + produced the original value and what consumes it downstream. +3. **Establish the actual root cause from the issue's MRE**, then check whether the change fixes + *that* — at the layer where the bug originates, not a compensating patch elsewhere. +4. **Check consistency with adjacent code.** Does the new value/behavior match how nearby code + already handles the same case? An inconsistency is a real finding; a match is evidence the fix + is correct. +5. **Run or read the tests** the PR adds/changes — do they actually exercise the bug, and would + they fail without the fix? + +Write down, for yourself, a one-line answer to: *what was broken, where, and does this change fix +it there?* If you can't answer from evidence you've actually read, you haven't investigated yet. + +Then separate findings by severity: a **cosmetic** nit (style, a redundant-but-harmless line) is a +review comment, not a blocker. A **substantive** defect (wrong layer, breaks an adjacent path, +doesn't actually fix the MRE) changes the verdict. Don't let a cosmetic nit read as a reason to +decline, and don't let a clean style read as evidence of correctness. + +## Step 4 — Decide if an external PR is appropriate (CONTRIBUTING.md) + +This is the gate CONTRIBUTING.md actually enforces. Map the change to a category: + +- **Simple, well-scoped bug fix** → external PR welcome. Assignable. +- **Docs / typo / example fix** → welcome. Assignable. +- **Auth provider** → assignable (auth is the one integration exception). +- **Enhancement / feature** → needs a maintainer-approved design proposal *in the issue first*. + Do **not** assign just because code exists. If the proposal is sound, the path is "approve the + approach in the issue, then assign" — not "assign because they were fast." +- **Third-party integration** (middleware, provider adapters, non-auth) → decline; belongs in a + separate package. +- **Sweeping / multi-subsystem change with no prior discussion** → decline. + +Combine the category with the Step 3 investigation: does it fix the cause or paper over a symptom? +Does it read like unedited LLM output (verbose body, speculative/shotgun changes)? CONTRIBUTING.md +says we close those — a closed PR that reads that way is staying closed. + +## Step 5 — Recommend, then act + +Present a short verdict to the maintainer before mutating anything: **assign** or **decline**, +one or two sentences of reasoning, and the exact command you'll run. Wait for confirmation on +borderline calls; for clear-cut ones you may proceed and report. + +**Assign** (valid issue + appropriate external contribution + sound PR exists): + +```bash +gh issue edit N --repo PrefectHQ/fastmcp --add-assignee +``` + +That reopens the PR automatically. Then hand off to code review — invoke the `code-review` / +`review-pr` skills on the reopened PR. Assignment is not approval; the code still gets the normal +pass. + +If a PR's head branch was deleted, assignment can't reopen it — the workflow comments asking the +author to open a fresh PR. Don't try to force it. + +**Decline** (invalid issue, wrong contribution type, or low-quality PR): leave it closed and +comment on the **issue** explaining the decision, pointing to the relevant CONTRIBUTING.md +section. Per repo rules, use `--body-file`, never inline `--body`, for any comment that could +contain `$`, backticks, or code: + +```bash +gh issue comment N --repo PrefectHQ/fastmcp --body-file /tmp/triage-reply.md +``` + +Keep the reply short and point to the relevant CONTRIBUTING.md section. (If a `github-reply` +skill is available for maintainer voice/tone, use it — but it isn't required.) + +## What this skill does NOT do + +- It doesn't bypass the gate via `trusted-contributor` / `bypass-issue-check` — that's a + deliberate maintainer escalation, not a triage outcome. +- It doesn't merge. Assignment → reopen → review → (maybe) merge are distinct steps. +- It doesn't re-run the first-pass triage the bots already did; read their output instead. diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md new file mode 100644 index 000000000..ae37ee33a --- /dev/null +++ b/.claude/skills/review-pr/SKILL.md @@ -0,0 +1,108 @@ +--- +name: review-pr +description: Monitor and respond to automated PR reviews (Codex bot). Use when pushing a PR, checking review status, or responding to bot feedback. Handles the full cycle of push -> wait for review -> evaluate comments -> fix -> re-push. +--- + +# PR Review Workflow + +This repo has `chatgpt-codex-connector[bot]` configured as an automated reviewer. After every push to a PR branch, Codex reviews the diff and either: +- Reacts with a thumbs-up on its review body (no suggestions — PR is clean) +- Posts inline comments with suggestions (each tagged with a priority badge) + +## Checking review status + +After pushing, check whether Codex has reviewed the latest commit: + +```bash +# Get the latest commit SHA on the branch +LATEST=$(git rev-parse HEAD) + +# Check if Codex has reviewed that specific commit +gh api repos/PrefectHQ/fastmcp/pulls/{PR_NUMBER}/reviews \ + | jq "[.[] | select(.user.login == \"chatgpt-codex-connector[bot]\" and .commit_id == \"$LATEST\")] | length" +``` + +If the count is 0, Codex hasn't reviewed the latest push yet. Wait and check again. + +If the count is > 0, check for inline comments on the latest review: + +```bash +# Get the review body to check for thumbs-up +gh api repos/PrefectHQ/fastmcp/pulls/{PR_NUMBER}/reviews \ + | jq '[.[] | select(.user.login == "chatgpt-codex-connector[bot]") | {state, body: .body[:300], commit_id: .commit_id}] | last' +``` + +A clean review from Codex looks like a review body that contains a thumbs-up reaction or says "no suggestions." If the body contains "Here are some automated review suggestions," there are inline comments to evaluate. + +## Evaluating Codex comments + +Fetch all inline comments from Codex: + +```bash +gh api repos/PrefectHQ/fastmcp/pulls/{PR_NUMBER}/comments \ + | jq '[.[] | select(.user.login == "chatgpt-codex-connector[bot]") | {body, path, line, created_at}]' +``` + +Codex comments include priority badges: +- `P0` (red) — Critical issue, likely a real bug +- `P1` (orange) — Important, worth fixing +- `P2` (yellow) — Moderate, evaluate on merit + +**How to evaluate Codex comments:** + +1. **Treat Codex as a competent but sometimes overzealous reviewer.** It catches real bugs (cache eviction ordering, silent data loss, missing validation) but also suggests scope expansions and hypothetical improvements. + +2. **Fix real bugs** — issues in code you actually changed where behavior is incorrect or data is silently lost. + +3. **Dismiss scope expansion** — if a comment points out a pre-existing limitation unrelated to your diff, note it as a potential follow-up but don't block the PR. + +4. **Dismiss speculative concerns** — if a comment describes a scenario that requires very specific conditions and the existing behavior is acceptable, dismiss it. + +5. **When fixing, be proactive** — if Codex found one instance of a pattern bug (e.g., missing role validation in one handler), check all similar code paths before pushing. Codex will find the next instance on the next review cycle, so get ahead of it. + +## Responding to every comment + +**Every Codex comment must get a visible response** — either a fix or a reply explaining why it was dismissed. The maintainer can't see your reasoning otherwise. + +- **If fixing**: The fix itself is the response. No reply needed unless the fix is non-obvious. +- **If dismissing**: Reply to the comment thread with a brief explanation of why. Keep it to 1-2 sentences. Examples: + - "This is pre-existing behavior unrelated to this diff — the scope lookup fallback existed before caching was added. Worth a follow-up issue but not blocking this PR." + - "The AsyncExitStack handles cleanup when the session exits, so the subprocess isn't leaked — just kept alive slightly longer than necessary in this edge case." + - "Gemini supports a much wider range of media types than OpenAI/Anthropic, so a restrictive allowlist would be inaccurate here." + +Use `gh api` to reply (note: use `in_reply_to`, not a `/replies` sub-path): + +```bash +# Reply to a specific review comment +gh api repos/PrefectHQ/fastmcp/pulls/{PR_NUMBER}/comments \ + -f body="Your reply here" \ + -F in_reply_to={COMMENT_ID} +``` + +## The fix-push-review cycle + +After evaluating comments: + +1. Fix all real issues in one batch +2. Reply to all dismissed comments with reasoning +3. Think about what patterns Codex might flag next — check similar code paths proactively +4. Commit and push +5. Check that Codex reviews the new commit +6. Repeat until Codex gives a clean review (thumbs-up) or only has dismissible comments + +## Responding to stale comments + +Codex sometimes re-posts old comments that reference code you've already fixed (they appear on the old commit's diff). These are stale — verify the fix is in the latest commit and reply noting the fix is already in place. + +## Labels — never apply or invent them + +**Do not apply labels to PRs or issues programmatically, and never create new ones.** Issues and PRs in this repo are auto-labeled by a bot based on title, body, and code changes — there's no fixed canonical list to match against, and GitHub's "add labels" API auto-creates any label name that doesn't already exist, so a typo or guessed name silently pollutes the repo's label list with a stray, uncolored duplicate. There is no MCP tool to delete a label, so a mistaken creation can only be cleaned up by hand in repo settings. + +Don't call out a "suggested" or "appropriate" label in the PR body either — the bot doesn't read it, and it just adds noise. + +## When a PR is ready + +A PR is ready for human review when: +- All Codex comments are either fixed or replied to with dismissal reasoning +- CI checks pass +- The diff is clean and focused on the stated purpose diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index 267df6812..1e6139cfb 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -3,33 +3,30 @@ description: Report a bug or unexpected behavior in FastMCP labels: [bug, pending] body: - - type: markdown - attributes: - value: Thanks for contributing to FastMCP! 🙏 - - type: markdown attributes: value: | + Thanks for reporting a bug! + + A good bug report is one of the most valuable contributions you can make — see [CONTRIBUTING.md](../../CONTRIBUTING.md). If the fix is straightforward, a PR is also welcome. + ### Before you submit - To help us help you, please: - - - 🔄 **Make sure you're testing on the latest version of FastMCP** - many issues are already fixed in newer versions - - 🔍 **Check if someone else has already reported this issue** or if it's been fixed on the main branch - - 📋 **You MUST include a copy/pasteable and properly formatted MRE** (minimal reproducible example) below or your issue may be closed without response - - 💡 **The ideal issue is a clear problem description and an MRE — that's it.** If you've done a genuine investigation and have a non-obvious insight into the root cause, include it. But please don't speculate or ask an LLM to generate a diagnosis or proposed fix. We have LLMs too, and an incorrect analysis is harder to work with than none at all. - - ✂️ **Keep it short.** A one-paragraph description and a working MRE is the ideal bug report. Issues that are difficult to parse — due to length, speculation, or generated content — may be closed without response. - - Thanks for helping to make FastMCP better! 🚀 + - Make sure you're testing on the **latest version** of FastMCP — many issues are already fixed in newer releases + - Check if someone else has **already reported this** or if it's been fixed on the main branch + - You **must** include a copy/pasteable, properly formatted MRE (minimal reproducible example) or your issue may be closed without response + - **The ideal issue is a clear problem description and an MRE — that's it.** If you've done genuine investigation and have a non-obvious insight into the root cause, include it. But please don't speculate or ask an LLM to generate a diagnosis. We have LLMs too, and an incorrect analysis is harder to work with than none at all. + - **Keep it short.** A clear description plus a concise MRE is ideal — aim to fit in a single screen. Issues that include unsolicited root cause analysis, proposed fixes, or multi-section diagnostic writeups will be labeled `too-long` and not triaged until condensed. + - **Using an LLM?** Great — but it must follow these guidelines. Generic LLM output that ignores our contributing conventions will be closed. See [CONTRIBUTING.md](../../CONTRIBUTING.md). - type: textarea id: description attributes: - label: Description + label: What happened? description: | - Please explain what you're experiencing and what you would expect to happen instead. + Describe the bug in a few sentences. What did you do, what happened, and what did you expect instead? - Provide as much detail as possible to help us understand and solve your problem quickly. + Do NOT include root cause analysis, proposed fixes, or diagnostic writeups — just describe the problem. validations: required: true diff --git a/.github/ISSUE_TEMPLATE/enhancement.yml b/.github/ISSUE_TEMPLATE/enhancement.yml index a803ec399..39c66647d 100644 --- a/.github/ISSUE_TEMPLATE/enhancement.yml +++ b/.github/ISSUE_TEMPLATE/enhancement.yml @@ -3,33 +3,27 @@ description: Suggest an idea or improvement for FastMCP labels: [enhancement, pending] body: - - type: markdown - attributes: - value: Thanks for contributing to FastMCP! 🙏 - - type: markdown attributes: value: | + Thanks for suggesting an improvement to FastMCP! + + Enhancement issues are the **primary way** features and improvements get into FastMCP. Maintainers use well-written issues to implement changes that fit the codebase's patterns and ship quickly. A clear issue here is more impactful than a PR — see [CONTRIBUTING.md](../../CONTRIBUTING.md) for why. + ### Before you submit - To help us evaluate your enhancement request: - - - 🔍 **Check if this has already been requested** - search existing issues first - - 💭 **Think about the broader impact** - how would this affect other users? - - 📋 **Consider implementation complexity** - is this a small change or a major feature? - - ✂️ **Keep it short.** Describe the problem you're trying to solve and why existing behavior falls short. Skip proposed implementations unless you have a specific, well-considered suggestion — we don't need LLM-generated API designs. Requests that are difficult to parse may be closed without response. - - Thanks for helping to make FastMCP better! 🚀 + - 🔍 **Check if this has already been requested** — search existing issues first + - 🎯 **Describe the problem you're trying to solve**, not the solution you want — we'll figure out the best implementation + - ✂️ **Keep it short.** A motivating description and a concrete use case is the ideal request — aim to fit in a single screen. Skip proposed implementations, API designs, or multi-option analyses — maintainers will figure out the approach. Requests that are difficult to parse will be labeled `too-long` and not triaged until condensed. + - 🤖 **Using an LLM?** Great — but it must follow these guidelines. Generic LLM output that ignores our contributing conventions will be closed. See [CONTRIBUTING.md](../../CONTRIBUTING.md). - type: textarea id: description attributes: label: Enhancement description: | - Please describe the enhancement: + What problem or use case does this solve? How does current behavior fall short? - - What problem or use case would it solve? - - How would it improve your workflow or experience with FastMCP? - - Are there any alternative solutions you've considered? + Focus on the *what* and *why* — the motivating scenario. You don't need to propose an API or implementation. validations: required: true diff --git a/.github/actions/run-claude/action.yml b/.github/actions/run-claude/action.yml index fff6788a6..b79131462 100644 --- a/.github/actions/run-claude/action.yml +++ b/.github/actions/run-claude/action.yml @@ -37,10 +37,15 @@ inputs: required: false default: "" + extra-allowed-tools: + description: "Additional comma-separated tools to append to allowed-tools" + required: false + default: "" + model: description: "Model to use for Claude" required: false - default: "claude-opus-4-6" + default: "claude-opus-4-8" allowed-bots: description: "Allowed bot usernames, or '*' for all bots" @@ -88,7 +93,7 @@ runs: track_progress: ${{ inputs.track-progress }} prompt: ${{ inputs.prompt }} claude_args: | - ${{ (inputs.allowed-tools != '' || inputs.extra-allowed-tools != '') && format('--allowedTools {0}{1}', inputs.allowed-tools, inputs.extra-allowed-tools != '' && format(',{0}', inputs.extra-allowed-tools) || '') || '' }} + ${{ (inputs.allowed-tools != '' || inputs.extra-allowed-tools != '') && format('--allowedTools ''{0}{1}''', inputs.allowed-tools, inputs.extra-allowed-tools != '' && format(',{0}', inputs.extra-allowed-tools) || '') || '' }} ${{ inputs.mcp-servers != '' && format('--mcp-config ''{0}''', inputs.mcp-servers) || '' }} --model ${{ inputs.model }} settings: | diff --git a/.github/actions/run-pytest/action.yml b/.github/actions/run-pytest/action.yml index ff429a4cc..c82e9c0bd 100644 --- a/.github/actions/run-pytest/action.yml +++ b/.github/actions/run-pytest/action.yml @@ -3,7 +3,7 @@ description: "Run pytest with appropriate flags for the test type and platform" inputs: test-type: - description: "Type of tests to run: unit, integration, or client_process" + description: "Type of tests to run: unit, integration, client_process, or conformance" required: false default: "unit" @@ -19,25 +19,47 @@ runs: MAX_PROCS="2" EXTRA_FLAGS="" elif [ "${{ inputs.test-type }}" == "client_process" ]; then - MARKER="client_process" + MARKER="client_process or subprocess_heavy" TIMEOUT="5" MAX_PROCS="0" EXTRA_FLAGS="-x" + elif [ "${{ inputs.test-type }}" == "conformance" ]; then + MARKER="conformance" + TIMEOUT="120" + MAX_PROCS="0" + EXTRA_FLAGS="-x" else - MARKER="not integration and not client_process" + MARKER="not integration and not client_process and not subprocess_heavy and not conformance" TIMEOUT="5" MAX_PROCS="4" EXTRA_FLAGS="" fi + # Windows previously ran serially: parallel workers crashed intermittently + # when many tests spawned stdio subprocesses (#2715, reverted in #2726). + # Most of those tests now run in-memory, but tests that spawn a fresh + # interpreter importing all of FastMCP still crash xdist workers on the + # 2-core Windows runners. They carry the subprocess_heavy marker and run + # in the serial client_process step instead. PARALLEL_FLAGS="" - if [ "$MAX_PROCS" != "0" ] && [ "${{ runner.os }}" != "Windows" ]; then + if [ "$MAX_PROCS" != "0" ]; then PARALLEL_FLAGS="--numprocesses auto --maxprocesses $MAX_PROCS --dist worksteal" fi + # pytest-timeout has no signal-based method on Windows, so it falls back + # to the thread method, which dumps stacks and os._exit()s the process. + # Under a contended runner that turns a single slow test into a dead + # xdist worker, failing whichever unrelated test that worker happened to + # be running. Give parallel Windows runs more headroom so ordinary + # scheduling jitter does not take a worker down. + if [ "$RUNNER_OS" == "Windows" ] && [ "$MAX_PROCS" != "0" ]; then + TIMEOUT=$((TIMEOUT * 4)) + fi + uv run --no-sync pytest \ --inline-snapshot=disable \ --timeout=$TIMEOUT \ + --durations=50 \ -m "$MARKER" \ $PARALLEL_FLAGS \ $EXTRA_FLAGS \ diff --git a/.github/dependabot.yml b/.github/dependabot.yml deleted file mode 100644 index 20d3ccecf..000000000 --- a/.github/dependabot.yml +++ /dev/null @@ -1,14 +0,0 @@ -version: 2 -updates: - - package-ecosystem: "pip" - directory: "/" - schedule: - interval: "daily" - labels: - - "dependencies" - - package-ecosystem: "github-actions" - directory: "/" - schedule: - interval: "weekly" - labels: - - "dependencies" diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 68212c72f..1b3333782 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,28 +1,22 @@ ## Description - + -**Contributors Checklist** - +## Contribution type -- [ ] My change closes #(issue number) -- [ ] I have followed the repository's development workflow -- [ ] I have tested my changes manually and by adding relevant tests -- [ ] I have performed all required documentation updates + -**Review Checklist** - +- [ ] Bug fix (simple, well-scoped fix for a clearly broken behavior) +- [ ] Documentation improvement +- [ ] Enhancement (maintainers typically implement enhancements — see [CONTRIBUTING.md](../CONTRIBUTING.md)) +## Checklist + +- [ ] This PR addresses an existing issue (or fixes a self-evident bug) +- [ ] I have read [CONTRIBUTING.md](../CONTRIBUTING.md) +- [ ] I have added tests that cover my changes +- [ ] I have run `uv run prek run --all-files` and all checks pass - [ ] I have self-reviewed my changes -- [ ] My Pull Request is ready for review - ---- +- [ ] If I used an LLM, it followed the repo's contributing conventions (not generic output) diff --git a/.github/release.yml b/.github/release.yml index 5ff95aace..5397d75e4 100644 --- a/.github/release.yml +++ b/.github/release.yml @@ -8,12 +8,25 @@ changelog: labels: - feature - - title: Enhancements 🔧 + - title: Breaking Changes ⚠️ + labels: + - breaking change + exclude: + labels: + - contrib + - security + + - title: Enhancements ✨ labels: - enhancement exclude: labels: - breaking change + - security + + - title: Security 🔒 + labels: + - security - title: Fixes 🐞 labels: @@ -21,13 +34,7 @@ changelog: exclude: labels: - contrib - - - title: Breaking Changes 🛫 - labels: - - breaking change - exclude: - labels: - - contrib + - security - title: Docs 📚 labels: @@ -41,6 +48,9 @@ changelog: - title: Dependencies 📦 labels: - dependencies + exclude: + labels: + - security - title: Other Changes 🦾 labels: diff --git a/.github/scripts/triage-label.sh b/.github/scripts/triage-label.sh new file mode 100755 index 000000000..c6bdda85c --- /dev/null +++ b/.github/scripts/triage-label.sh @@ -0,0 +1,80 @@ +#!/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