diff --git a/.claude/skills/review-issue/SKILL.md b/.claude/skills/review-issue/SKILL.md index dc3ee110a..384aa137c 100644 --- a/.claude/skills/review-issue/SKILL.md +++ b/.claude/skills/review-issue/SKILL.md @@ -31,7 +31,7 @@ merge, not a courtesy. - 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: `martian-triage-issue` (investigates + +- 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. diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index 93d0ebbf8..ae37ee33a 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.claude/skills/review-pr/SKILL.md @@ -96,12 +96,9 @@ Codex sometimes re-posts old comments that reference code you've already fixed ( ## Labels — never apply or invent them -**Do not apply labels to PRs or issues programmatically, and never create new ones.** Labeling is the maintainer's call (and is often automated). Two hard rules: +**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. -- **Never invent a label.** GitHub's "add labels" API *auto-creates* any label name that doesn't already exist — so a typo or a guessed name silently pollutes the repo's label list with a stray, uncolored duplicate. Adding `breaking` (which does not exist) creates it alongside the real `breaking change` label. -- **Use only labels that already exist.** If you genuinely need to confirm a label, look it up first (`get_label` / the repo's label list) and match the exact name. The canonical names here are specific — e.g. the breaking-change label is **`breaking change`**, not `breaking`; enhancements is **`enhancements`**, features is **`features`**, bugs is **`bugs`**. - -When a change warrants a label (e.g. it's breaking), **say so in the PR body and let the maintainer apply the label** rather than applying it yourself. There is no MCP tool to delete a label, so a mistaken creation can only be cleaned up by hand in repo settings — the cost of guessing is high and one-directional. +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 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 b7e5509e5..c82e9c0bd 100644 --- a/.github/actions/run-pytest/action.yml +++ b/.github/actions/run-pytest/action.yml @@ -19,7 +19,7 @@ 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" @@ -29,17 +29,33 @@ runs: MAX_PROCS="0" EXTRA_FLAGS="-x" else - MARKER="not integration and not client_process and not conformance" + 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 \ 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/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