fix: resolve issue #1753 - Add verified setup report fulfilling all acceptanc - #1801
seokwon-dev wants to merge 2 commits into
Conversation
… 回答"数值对(碰巧)但 Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>
Welcome to MisakaNet!Thanks for your first PR! 🔏 Fix DCO FirstSome commits are missing git commit --amend --signoff --no-edit # latest commit
git push --force-with-leaseFor several commits:
⏳ First PR: CI waits for one-time maintainer approvalOn your first PR from a fork, GitHub requires a maintainer to approve If you see a red Quick Links
Optional: Local Pre-commit HookFor instant DCO validation before pushing: pip install pre-commit
pre-commit install --hook-type commit-msgFor Agents & CrawlersFound a missing lesson? Submit via MCP (no account needed): curl -sS https://misakanet.org/mcp \
-H 'Content-Type: application/json' \
-H 'MCP-Protocol-Version: 2025-06-18' \
-H 'Origin: https://misakanet.org' \
-d '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"misakanet_submit_intake","arguments":{"problem":"YOUR PROBLEM","source":"your-agent"}}}'After the one-time approval, CI checks run automatically on every push. |
Signed-off-by: seokwon-dev <seokwon-dev@users.noreply.github.com>
c8cf86f to
3f7b47e
Compare
|
Heads-up on the base branch: this PR targets
Retargeting it to |
|
A second finding from the retarget, and it is about scope rather than mechanics: this PR is not Since the base is now The gate's own log says Suggested split, so each part can land by the route it belongs to:
(The earlier retarget comment still stands on its own: against the stale branch this PR could not have |
…label Option C from the triage discussion: keep the gate, give it a trigger and a human switch, and make it refuse the surfaces a person should decide. ## Why it never fired The gate keys on labels, and every label it looked at is applied *after* the PR opens: `.github/labeler.yml` adds `area:docs` seconds later, and the shape guard adds `docs-only` on its own run. With `types: [opened, synchronize, ready_for_review]` a PR that became eligible later was never examined again, so the channel produced nothing since it was written — Ikalus1988#1842 and Ikalus1988#1801, both external and genuinely prose-only, are still open. `labeled` is now in the trigger list: adding the label is what fires the check. ## The switch is a person The entry condition is now `auto-merge-eligible`, which is applied by hand and whose purpose is exactly this ("Docs-only PR, eligible for auto-merge when CI passes"; description refreshed). The previously load-bearing labels were automatic, and an automatic entry condition plus a merge to `main` (i.e. `docs.yml` publishing the site) means external text going live with nobody having read it. `area:docs` no longer admits a PR on its own, and the job still refuses `lessons-only` and `needs-human-review`. ## What is not eligible Beyond `lessons/` (already excluded, and the reason this file exists), two surfaces are now excluded because "docs" here means prose a human reads: * `docs/.well-known/**` — discovery cards other agents fetch and parse; * `docs/index.html` — the site itself (version badge, counts, claims). Eligible: prose under `docs/`, plus `README.md`, `CONTRIBUTING.md`, `JOIN.md`, `CHANGELOG.md`. ## Where a maintainer reads about it `docs/maintainer/pr-triage.md` gains the section: the label, what the gate re-checks, what it never touches, and why `docs/` is not covered by the injection scan (that runs on `lessons/` only) — the reason the switch is manual. `tests/test_auto_merge_docs.py` runs the real rule over 17 paths (including the two new exclusions and `lessons/contrib/x.md` → false) and asserts both halves of the design: the opt-in label is the entry condition (`area:docs` is not), and `labeled` is in the trigger types. The condition text is read with comments stripped, because the comment explaining the choice names the label it does not use. Also: `workers/register-proxy-sw.js`'s version comment now tells the truth about `env.MCP_VERSION` — nothing sets it, the constant is what release-please maintains, and the ad-hoc override is `wrangler deploy --var MCP_VERSION=…`. Deliberately not wired into wrangler.toml: a second owner for that number would be the untested one at runtime, and a stale var would win over the constant that R7 guards (Ikalus1988#1820). Signed-off-by: Ikalus1988 <136884451+Ikalus1988@users.noreply.github.com>
…sts for Testing the channel on a real external PR (Ikalus1988#1801) found the reason it has never merged anything — a second one, deeper than the trigger types: ``` head sha 3f7b47e, check suites: 2026-09-17T23:06:44Z Auto-Merge Docs PRs pull_request action_required 2026-09-17T23:06:44Z Cross-Platform Tests pull_request action_required … eight workflows, all created, none executed ``` A `pull_request` run from a fork is created in the **`action_required`** state and does not run a single step until a maintainer approves it (first-time contributors). So the gate's suites existed, read `action_required`, and never evaluated anything — for Ikalus1988#1801, for the seven workflows beside it, and for every contributor in that position. The population this gate is for is exactly the population whose runs are held. The trigger is now `pull_request_target`, which evaluates the *base* branch's workflow (so this file's `labeled` + `auto-merge-eligible` conditions apply to fork PRs for the first time) and does not need per-PR approval. **Why that is safe here, stated because it is the whole question:** this workflow never checks out the PR, never runs a file from it, and its only writes are a label-driven merge of a PR a maintainer already labelled. The `github-script` steps call the REST API with values GitHub supplies. The line not to cross — a future edit adding `actions/checkout` of the PR head, or executing anything from the diff — is now pinned by a test, so the classic `pull_request_target` vulnerability cannot be introduced quietly. `tests/test_auto_merge_docs.py` asserts the new trigger, the safety property (no checkout in code, comments stripped — the workflow's own comment names it), and keeps the earlier assertions: `labeled` in the types, `auto-merge-eligible` as the entry condition, and the file rule that refuses lessons/, `docs/.well-known/**` and `docs/index.html`. Signed-off-by: Ikalus1988 <136884451+Ikalus1988@users.noreply.github.com>
…a push Testing the channel on real PRs today exposed two ways a PR becomes (or stops being) eligible with no new commit, and the gate only listened for commits and labels: * **`edited`** — retargeting the base branch. Ikalus1988#1801 was opened against a stale feature branch (`agent/issue-1196-…`), and a `pull_request`/`pull_request_target` run evaluates the *base* branch's workflow file, so retargeting it to `main` changed which definition the PR ran. The gate would not look again until a human re-applied the label by hand — which is what I had to do. * **`unlabeled`** — a maintainer removing `needs-human-review` (or `lessons-only`) is a decision that the PR may proceed. Without this event the gate only ever saw the refusal, never its withdrawal, so "a person has looked and says it is fine" was indistinguishable from "nobody has looked". Both are one line in `types:`, and the test now asserts all three (added, removed, retargeted) rather than just `labeled` — the property is "the gate decides on the current state of the PR", not "the gate runs when a label appears". Signed-off-by: Ikalus1988 <136884451+Ikalus1988@users.noreply.github.com>
|
Label hygiene, since these were applied by automation and by me before the diff was read properly: It also has merge conflicts now, so the useful next step is the split described in the previous comment: |
…1873) Documents the two ways today's CI automation was silently inert, both from real runs: * a `GITHUB_TOKEN` push cannot trigger workflows, so the branch sync updated PRs and left their new heads with zero check runs (#1870 sat unmergeable until a human pushed an empty commit). The lesson also carries the second half found while testing the fix: a PAT push created fourteen runs that were all held as `action_required`, and the repository was carrying 1,804 such runs; * a fork PR's `pull_request` run is created as `action_required` and never executes a step, so a maintainer-side channel can be dead for exactly the contributors it exists for (#1801: eight suites, no logs, nothing to read) — with the `pull_request_target` route, its safety condition, and the stale-base trap that made the PR run an old workflow definition. Both include the fix and a verification step, and the count surfaces moved 43 -> 44 domains through `scripts/sync_lesson_count.py`. Gates: lesson_gate, validate_lessons, injection_scan, DCO, Lesson Quality Gate and Lesson Security Scan all green.
|
这条现在和 main 冲突( git fetch origin main
git rebase origin/main # 有冲突就解决后 git rebase --continue
git push --force-with-lease推上来后门禁会自动重跑;绿了我就会处理。如果这条分支上还叠着别的实验性改动,也可以先只挑这个改动重开一条干净分支——PR 越小越容易被合掉。 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
The report is the valuable part here, and it is the piece I would land — but the branch currently Keep: Drop two files, both inherited from the branch's first commit (Mr-Neutr0n's
And rebase onto A one-file branch (just the report, rebased) I will merge. One naming question I have raised with the repo |
…without a word Two gaps in the same file (issue Ikalus1988#1983), failing in opposite directions. **The merge was invisible to the rest of the repository.** `gh pr merge` used `secrets.GITHUB_TOKEN`, and a merge made with `GITHUB_TOKEN` produces a push that starts **no workflows**. Measured 2026-09-21 by counting push-triggered runs per commit: 8044678 Ikalus1988#1969, merged here (GITHUB_TOKEN) → 0 runs 8417494 Ikalus1988#1965, merged with the PAT → 5 runs dc50c25 Ikalus1988#1962, merged with the PAT → 5 runs `Release Please` and `Leaderboard Watch`'s last eight runs all correspond to PAT or manual pushes; none of the three `GITHUB_TOKEN` commits has one. At least ten workflows key on `push` — Release Please, Leaderboard Watch, Build Live Feed, Update Badge Counts, Deploy Documentation, Lesson Security Scan, Setup Package CI, MCP Endpoint Stress Tests, PyPI Wheel Smoke, fatal-guard CI — so every PR merged through this channel landed without any of them noticing. The repository has paid for this lesson twice (`auto-sync-prs.yml`, and `lessons/contrib/ci-github-token-push-does-not-trigger-workflows.md`); this was the third place. The token must **not** be widened to `workflow`: that scope is only needed for PRs that modify `.github/workflows/`, and this channel merges docs only — granting it would let an auto-merged PR change CI itself. **The refusal was invisible to the person who opted in.** This workflow had no comment step at all (the lesson channel has three), so a refusal existed as one `core.info` line inside a green run. Ikalus1988#1801 is the measured case: the maintainer applied `auto-merge-eligible` four times on 2026-09-19, the 13:23 run executed and refused it (`Docs-only: false (3 files)`), and nothing was posted and no run happened again — from the author's side, the label did nothing for three days. There is now a step on the refusal path that upserts a comment (capped at 20 offenders) naming the files that broke the rule and the way back in: re-apply `auto-merge-eligible`, which is not busywork, because the gate only re-evaluates on `labeled`/`unlabeled`/`edited`/`synchronize`/`ready_for_review`. Both checks are functions over the workflow text and the mutation cases feed a mutated copy through those same functions — asserting "the mutation took" and then asserting something trivially true is decoration, not a guard. Signed-off-by: Ikalus1988 <136884451+Ikalus1988@users.noreply.github.com>
Resolves #1753
Summary
Add verified setup report fulfilling all acceptance criteria for misakanet-setup.
Automated patch verified by seokwon-dev.