Skip to content

ci: widen the publish-attempt path list - #1560

Merged
rdimitrov merged 1 commit into
modelcontextprotocol:mainfrom
rdimitrov:rdimitrov/fix-auto-close-automation
Aug 20, 2026
Merged

ci: widen the publish-attempt path list#1560
rdimitrov merged 1 commit into
modelcontextprotocol:mainfrom
rdimitrov:rdimitrov/fix-auto-close-automation

Conversation

@rdimitrov

@rdimitrov rdimitrov commented Aug 20, 2026

Copy link
Copy Markdown
Member

#1524 tried to publish by adding data/servers/io.decisionrules/server.json. The detector ran fine and logged:

Non-publish file changed: data/servers/io.decisionrules/server.json
PR #1524 match=false

The path list only knew servers/** at the repo root and exactly data/seed.json. Putting the file under data/servers/ is a natural guess given data/seed.json exists, and it fell straight through.

Adds data/servers/** plus root-level server.json / servers.json. find confirms nothing legitimate lives at any of those paths today.

The list stays explicit rather than becoming a broad **/server.json glob. The classifier only flags a PR when every changed file matches, which makes widening asymmetric: too narrow silently misses spam, too broad auto-closes a legitimate single-file PR.

Both copies of the list (stage 1 and stage 2) are updated identically, with a comment on each pointing at the other.

Testing

actionlint clean. macOS ships bash 3.2, so I extracted the literal if [[ ... ]] condition from each workflow file and ran it under bash 5 in Docker — testing the shipped text rather than a retyped copy. Both copies are byte-identical and agree:

MATCH data/servers/io.decisionrules/server.json   # PR 1524
MATCH servers/com.aradia/x.json                   # PR 1549
MATCH data/seed.json
MATCH server.json
MATCH servers.json
keep  internal/api/handler.go
keep  internal/servers/service.go
keep  data/servers.md
keep  docs/README.md

Also dry-ran the full classifier against all 35 currently-open PRs from untrusted authors: 0 would be closed. No legitimate open PR is affected.

Known remaining gap (deliberately not fixed here)

The detector does not run at all for authors new to GitHub. This repo's Actions approval policy is first_time_contributors_new_to_github, so their fork PRs sit in action_required until a maintainer approves the run. That is why #1549 was never flagged.

I'm leaving that alone for now because the numbers don't justify the fix. Since the automation landed on 2026-07-09 it has had 71 detect runs: 70 ran, 1 was gated. It has correctly closed 3 publish attempts (SAGE, Diagram Generation API, Kairos Signal) and missed 2 — one to the path bug this PR fixes, one to the gate. So the gate costs roughly one missed PR every six weeks, and those get closed manually within a day.

The only ways around it are pull_request_target (runs untrusted PRs in a privileged context — one future actions/checkout from being a privilege-escalation hole) or a scheduled sweep (works, but trades the two-stage design for cron semantics and a ~15 min delay). Neither seems worth it at that rate. Happy to open a follow-up if you'd rather close the gap.

🤖 Generated with Claude Code

@rdimitrov rdimitrov changed the title ci: fix two silent misses in the invalid-publish-PR automation ci: fix the invalid-publish-PR automation (never fired for new accounts; wrong paths) Aug 20, 2026
PR modelcontextprotocol#1524 added `data/servers/io.decisionrules/server.json` and the detector
logged `match=false`, so it was never auto-closed. The path list only knew
`servers/**` at the repo root and exactly `data/seed.json`; putting the file
under `data/servers/` is a natural guess given `data/seed.json` exists.

Add `data/servers/**` plus root-level `server.json` / `servers.json`.
Nothing legitimate lives at any of these paths today (`find` finds no
server.json, servers.json or servers/ directory anywhere in the tree).

The list stays explicit rather than becoming a broad `**/server.json` glob.
The classifier only flags a PR when *every* changed file matches, which makes
widening asymmetric: too narrow silently misses spam, too broad auto-closes a
legitimate single-file PR. `internal/servers/service.go` and `data/servers.md`
correctly do not match.

Both copies of the list (stage 1 and stage 2) are updated identically, with a
comment on each pointing at the other.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rdimitrov
rdimitrov force-pushed the rdimitrov/fix-auto-close-automation branch from 291cb1f to 9c7dc43 Compare August 20, 2026 21:19
@rdimitrov rdimitrov changed the title ci: fix the invalid-publish-PR automation (never fired for new accounts; wrong paths) ci: widen the publish-attempt path list Aug 20, 2026
@rdimitrov
rdimitrov merged commit 5022e04 into modelcontextprotocol:main Aug 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant