From 746849957fba051ccccedfb4e9f20cc6252f064e Mon Sep 17 00:00:00 2001 From: saimskywalker <15120878+saimskywalker@users.noreply.github.com> Date: Tue, 10 Nov 2026 21:56:08 +0800 Subject: [PATCH 1/2] Add AGENTS.md and a PR rules check --- .github/pull_request_template.md | 2 + .github/workflows/pr-rules.yml | 96 +++++++++++++++++++++++ AGENTS.md | 126 +++++++++++++++++++++++++++++++ CONTRIBUTING.md | 31 +++++++- 4 files changed, 252 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/pr-rules.yml create mode 100644 AGENTS.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index ba4078d..003b375 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -6,6 +6,8 @@ - [ ] `dart format --output=none --set-exit-if-changed .` and `dart analyze --fatal-infos` are clean - [ ] `dart test` passes +- [ ] Closes an issue labelled `accepted`; at most 400 changed lines; no `// ignore:` (checked) +- [ ] Changes to `lib/` or `bin/` come with a test (checked) - [ ] If this fixes a bug: there is a test that fails without the fix - [ ] New or changed rule: a passing and a failing synthetic fixture, the documented source in `docs`, and a README rule table row - [ ] User-visible change: `CHANGELOG.md` entry diff --git a/.github/workflows/pr-rules.yml b/.github/workflows/pr-rules.yml new file mode 100644 index 0000000..742e133 --- /dev/null +++ b/.github/workflows/pr-rules.yml @@ -0,0 +1,96 @@ +name: PR rules + +# The rules in CONTRIBUTING.md and AGENTS.md, enforced. Each rule fails the PR; +# only a maintainer can waive one, with the label named in its step (only +# maintainers can add labels). Reads the GitHub API only: no checkout. +on: + pull_request: + types: [opened, reopened, synchronize, edited, labeled, unlabeled] + +permissions: + pull-requests: read + issues: read + +env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} + AUTHOR: ${{ github.event.pull_request.user.login }} + CODE: '^(lib|bin)/' + TESTS: '^test/' + MAX_LINES: 400 + +jobs: + rules: + name: PR rules + runs-on: ubuntu-latest + steps: + - name: Read the changed files + run: gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR/files" > "$RUNNER_TEMP/files.json" + + - name: Code changes come with tests (waive with no-tests-needed) + if: ${{ !cancelled() && !contains(github.event.pull_request.labels.*.name, 'no-tests-needed') }} + run: | + files="$(jq -r '.[].filename' "$RUNNER_TEMP/files.json")" + if grep -Eq "$CODE" <<<"$files" && ! grep -Eq "$TESTS" <<<"$files"; then + echo "::error title=Tests required::This PR changes lib/ or bin/ but no test. Add a test under test/." + exit 1 + fi + + - name: At most MAX_LINES changed lines (waive with large-change) + if: ${{ !cancelled() && !contains(github.event.pull_request.labels.*.name, 'large-change') }} + run: | + lines="$(jq '[.[] | select(.filename | test("\\.lock$") | not) | .additions + .deletions] | add // 0' "$RUNNER_TEMP/files.json")" + if [ "$lines" -gt "$MAX_LINES" ]; then + echo "::error title=PR too large::$lines changed lines (limit $MAX_LINES). Split it into smaller PRs, one issue each." + exit 1 + fi + + - name: No lint suppressions (waive with suppression-ok) + if: ${{ !cancelled() && !contains(github.event.pull_request.labels.*.name, 'suppression-ok') }} + run: | + # Added Dart lines that silence the analyzer, or storehound config that turns a rule + # off or ignores a finding, outside the fixture that tests those options. + hits="$(jq -r '.[] | select(.patch) | .filename as $f + | ($f | endswith(".dart")) as $dart + | select($dart or (($f | test("(^|/)(storehound|imaji_store_preflight)\\.yaml$")) + and ($f | startswith("test/fixtures/rejected_app/") | not))) + | .patch | split("\n")[] + | select(startswith("+") and (if $dart then test("//\\s*ignore(_for_file)?:") + else test("^\\+\\s*(ignore\\s*:|-\\s*rule\\s*:|enabled\\s*:\\s*false\\b)") end)) + | "\($f): \(.)"' "$RUNNER_TEMP/files.json")" + if jq -e 'any(.[]; .filename | test("(^|/)analysis_options\\.yaml$"))' "$RUNNER_TEMP/files.json" >/dev/null; then + hits="${hits}"$'\n'"analysis_options.yaml is changed" + fi + if [ -n "${hits//[[:space:]]/}" ]; then + echo "::error title=Lint suppression::Fix the finding instead of silencing it:" + echo "$hits" + exit 1 + fi + + - name: Linked to an accepted issue (maintainers exempt) + if: ${{ !cancelled() && !contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.pull_request.author_association) && github.event.pull_request.user.login != 'dependabot[bot]' }} + env: + QUERY: >- + query($owner: String!, $name: String!, $pr: Int!) { repository(owner: $owner, name: $name) { + nameWithOwner pullRequest(number: $pr) { closingIssuesReferences(first: 10) { nodes { + repository { nameWithOwner } labels(first: 20) { nodes { name } } } } } } } + FILTER: >- + .data.repository as $r | [$r.pullRequest.closingIssuesReferences.nodes[] + | select(.repository.nameWithOwner == $r.nameWithOwner) + | select(any(.labels.nodes[]; .name == "accepted"))] | length + run: | + accepted="$(gh api graphql -F owner="${GITHUB_REPOSITORY%/*}" -F name="${GITHUB_REPOSITORY#*/}" \ + -F pr="$PR" -f query="$QUERY" --jq "$FILTER")" + if [ "$accepted" -eq 0 ]; then + echo "::error title=Issue first::Open an issue first and wait until a maintainer labels it 'accepted', then write 'Closes #N' in the PR description. Editing the description re-runs this check." + exit 1 + fi + + - name: At most 2 open PRs per person (maintainers exempt) + if: ${{ !cancelled() && !contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.pull_request.author_association) && github.event.pull_request.user.login != 'dependabot[bot]' }} + run: | + open="$(gh pr list -R "$GITHUB_REPOSITORY" --author "$AUTHOR" --state open --json number --jq length)" + if [ "$open" -gt 2 ]; then + echo "::error title=Too many open PRs::You have $open open PRs here (limit 2). Finish or close one first." + exit 1 + fi diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..4a3d188 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,126 @@ +# AGENTS.md + +Rules for AI coding agents working in this repository. People should read +[CONTRIBUTING.md](CONTRIBUTING.md); everything here applies to them too. The +`PR rules` check enforces the rules marked (checked); a pull request that +breaks one cannot be merged. + +## What storehound is + +A Dart CLI and library that lints a Flutter project's `Info.plist`, privacy +manifest, entitlements, ExportOptions and `AndroidManifest.xml` for mistakes +that get App Store or Google Play builds refused. Rules live in +`lib/src/rules/` (registered in `all_rules.dart`), the store data they rely on +in `lib/src/data/`, configuration (`storehound.yaml`) in `lib/src/config.dart` +and the reporters in `lib/src/report/`. + +Read before you touch: + +- a rule: the "Adding a rule" section of CONTRIBUTING.md, `doc/rules.md`, + and `test/support/fixtures.dart`. +- a data table: the "Updating a data table" section of CONTRIBUTING.md and + the sources cited at the top of each file in `lib/src/data/`. +- configuration or the CLI: `doc/configuration.md`. +- the fixture apps: `test/fixtures/README.md`, including how to regenerate + `test/fixtures/production_app.golden.txt`. + +## Commands + +Use the Dart SDK, 3.8 or newer. Every one of these must pass before you open +or update a pull request. CI runs the same ones: + +```bash +dart pub get +dart format --output=none --set-exit-if-changed . +dart analyze --fatal-infos +dart test +dart pub publish --dry-run +dart run storehound --project test/fixtures/clean_app +dart run storehound --project test/fixtures/rejected_app --sarif --output rejected.sarif || test $? -eq 1 +dart run storehound --project test/fixtures/production_app --fail-on info --today 2026-10-01 --brief --no-color > production.txt +diff -u test/fixtures/production_app.golden.txt production.txt +``` + +CI runs them on Dart 3.8.0 and 3.13.5 (formatting on 3.13.5 only). + +Run one test file while iterating: `dart test test/rules/android_test.dart`. + +## Stop and ask the human + +Some things must come from the real world. An agent must never invent them. +Stop and ask the person you work for to provide them: + +- **A real rejection.** A new rule starts from an App Review guideline number, + an `ITMS-` code, the reviewer's wording (anonymised) or observed store or + platform behaviour, provided by the human, plus the public Apple or Google + page that documents it. Never invent a rejection, a code or a guideline + number. +- **Store data.** Every privacy data type, purpose and required-reason code, + every SDK data type mapping, every purpose string a plugin needs (from that + plugin's README) and every Play target API deadline is copied from the + public source cited in `lib/src/data/`. Never invent a value, a date or an + API level; if you cannot find the source, say so. +- **Real Xcode, Gradle or device output.** What Xcode writes into + `project.pbxproj`, what AGP or the NDK does, or what a binary links (for + example `nm -u`) must come from a real run the human did. +- **An accepted issue.** If the change has no issue labelled `accepted`, stop + and ask the human to open one and wait for a maintainer. + +## Code + +- **English only** for code, identifiers, comments, commit messages and docs. + Answer the person you work for in their language. +- Target the minimum SDK in `pubspec.yaml` (Dart 3.8). Do not use language or + library features newer than that. +- Rules read files only through `context.project`, and prefer false + negatives over false positives; heuristics are `Severity.warning`. +- Public API has doc comments (`public_member_api_docs`). Other comments are + sparse: only where the code is not self-evident. Do not add comments, doc + comments or type annotations to code you did not change. + +## Tests (checked) + +- Every change to `lib/` or `bin/` comes with a test change under `test/`. A + bug fix has a test that fails without the fix; check that it does before + you open the pull request. +- A new or changed rule has tests in `test/rules/` with a passing and a + failing synthetic fixture, and a row in the README rule table (`test/version_test.dart` + checks it). +- Fixtures are invented: no real app, company, bundle id, hostname, key or + person (`com.example.*`, `example.com`, placeholder Firebase values). + +## Never + +- Never silence the analyzer or storehound (checked): no `// ignore:`, + `// ignore_for_file:`, no changes to `analysis_options.yaml`, and no + `ignore:` entries or `enabled: false` in a `storehound.yaml` outside + `test/fixtures/rejected_app/`. Fix the finding instead. +- Never delete, skip, loosen or rewrite an existing test, expectation or + fixture to make CI pass, and never regenerate + `production_app.golden.txt` to accept a new finding: on that fixture it is + a false positive until shown otherwise. If a test looks wrong, say so in + the issue. +- Never change what users depend on: rule `id`s and `check:` ids, + `storehound.yaml` keys (and the legacy `imaji_store_preflight.yaml` name), + CLI flags, exit codes (0 clean, 1 findings, 2 usage or configuration + problem), and the JSON and SARIF output. +- Never bump the version in `pubspec.yaml`, edit a released `CHANGELOG.md` + section, publish, tag or release. Add your entry at the top of + `CHANGELOG.md` under `## Unreleased` (create the heading if it is missing). +- Never add a dependency or change CI workflows unless the issue asks for it. +- Never add unrelated "improvements": no drive-by refactors, renames, + reformatting of untouched code or dead code. +- Never open pull requests, issues or comments on your own. The human reviews + your work and posts it. + +## Pull requests (checked) + +- First an issue, labelled `accepted` by a maintainer. The pull request says + `Closes #N`. +- One issue per pull request, at most 400 changed lines. +- At most 2 open pull requests per person. +- Fill in the pull request template, including `### AI use` with the tool and + model. +- The person submitting has read, run and understood every line, and writes + the description in their own words. Pull requests that are raw, unreviewed + agent output will be closed. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 86b4c66..d605a14 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,8 +59,29 @@ by default, only deadlines Google has published). ## Pull requests -Small, focused commits with clear messages. Run the commands above before -pushing. +Small, focused commits with clear messages, in English. Run the commands +above before pushing; every CI check must pass before a pull request can be +merged. The `PR rules` check enforces these: + +- **Issue first.** Open an issue and wait until a maintainer labels it + `accepted`, then write `Closes #N` in the pull request. +- **Tests.** Every change to `lib/` or `bin/` needs a test change under + `test/`. A bug fix needs a test that fails without the fix. +- **Small.** At most 400 changed lines, one issue per pull request. +- **No suppressions.** No `// ignore:`, no changes to + `analysis_options.yaml`, and no `ignore:` entries or `enabled: false` in a + `storehound.yaml` outside `test/fixtures/rejected_app/`. Fix the finding. +- **At most 2 open pull requests** per person. + +If a rule really does not fit your change, say why in the description. A +maintainer can waive it with a label (`no-tests-needed`, `large-change` or +`suppression-ok`). + +## Legal notice + +By submitting a contribution you confirm that you wrote it, that you have the +right to submit it, and that it may be distributed under the project license +(see [LICENSE](LICENSE)). ## AI-assisted contributions @@ -73,7 +94,11 @@ Using AI tools is fine; please just be open about it. It shows us what you understood about the change. Please don't paste generated text. - Read, run and test every line you submit, and be ready to explain it in - review. + review. If you don't understand what a line does, don't submit it. +- Keep the change focused. Agents like to add unrelated "improvements", + long comments and dead code; remove them before you open the pull request. +- Pull requests that are raw, unreviewed agent output will be closed. +- If you use an agent, point it at [AGENTS.md](AGENTS.md). - If the disclosure is missing, a check adds a reminder to the PR. It does not fail the PR, but a maintainer will ask about it before merging. From d4cfcf7bf8055f04358cdba887d5ae44bf248115 Mon Sep 17 00:00:00 2001 From: saimskywalker <15120878+saimskywalker@users.noreply.github.com> Date: Tue, 10 Nov 2026 22:31:01 +0800 Subject: [PATCH 2/2] Flag fail_on and severity changes in storehound.yaml --- .github/workflows/pr-rules.yml | 5 +++-- AGENTS.md | 5 +++-- CONTRIBUTING.md | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr-rules.yml b/.github/workflows/pr-rules.yml index 742e133..13e29e2 100644 --- a/.github/workflows/pr-rules.yml +++ b/.github/workflows/pr-rules.yml @@ -49,14 +49,15 @@ jobs: if: ${{ !cancelled() && !contains(github.event.pull_request.labels.*.name, 'suppression-ok') }} run: | # Added Dart lines that silence the analyzer, or storehound config that turns a rule - # off or ignores a finding, outside the fixture that tests those options. + # off, ignores a finding or sets a severity or fail_on (any change: a diff line alone + # cannot tell a downgrade from an upgrade), outside the fixture that tests them. hits="$(jq -r '.[] | select(.patch) | .filename as $f | ($f | endswith(".dart")) as $dart | select($dart or (($f | test("(^|/)(storehound|imaji_store_preflight)\\.yaml$")) and ($f | startswith("test/fixtures/rejected_app/") | not))) | .patch | split("\n")[] | select(startswith("+") and (if $dart then test("//\\s*ignore(_for_file)?:") - else test("^\\+\\s*(ignore\\s*:|-\\s*rule\\s*:|enabled\\s*:\\s*false\\b)") end)) + else test("^\\+\\s*(ignore\\s*:|fail_on\\s*:|severity\\s*:|-\\s*rule\\s*:|enabled\\s*:\\s*false\\b)") end)) | "\($f): \(.)"' "$RUNNER_TEMP/files.json")" if jq -e 'any(.[]; .filename | test("(^|/)analysis_options\\.yaml$"))' "$RUNNER_TEMP/files.json" >/dev/null; then hits="${hits}"$'\n'"analysis_options.yaml is changed" diff --git a/AGENTS.md b/AGENTS.md index 4a3d188..c40e710 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,8 +93,9 @@ Stop and ask the person you work for to provide them: - Never silence the analyzer or storehound (checked): no `// ignore:`, `// ignore_for_file:`, no changes to `analysis_options.yaml`, and no - `ignore:` entries or `enabled: false` in a `storehound.yaml` outside - `test/fixtures/rejected_app/`. Fix the finding instead. + `ignore:` entries, `enabled: false`, `fail_on:` or `severity:` lines in a + `storehound.yaml` outside `test/fixtures/rejected_app/`. Fix the finding + instead. - Never delete, skip, loosen or rewrite an existing test, expectation or fixture to make CI pass, and never regenerate `production_app.golden.txt` to accept a new finding: on that fixture it is diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d605a14..f67524a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,8 +69,9 @@ merged. The `PR rules` check enforces these: `test/`. A bug fix needs a test that fails without the fix. - **Small.** At most 400 changed lines, one issue per pull request. - **No suppressions.** No `// ignore:`, no changes to - `analysis_options.yaml`, and no `ignore:` entries or `enabled: false` in a - `storehound.yaml` outside `test/fixtures/rejected_app/`. Fix the finding. + `analysis_options.yaml`, and no `ignore:` entries, `enabled: false`, + `fail_on:` or `severity:` lines in a `storehound.yaml` outside + `test/fixtures/rejected_app/`. Fix the finding. - **At most 2 open pull requests** per person. If a rule really does not fit your change, say why in the description. A