From 59eb3a6d0635e4880a20926eab902a48e0c91ba9 Mon Sep 17 00:00:00 2001 From: saimskywalker <15120878+saimskywalker@users.noreply.github.com> Date: Tue, 10 Nov 2026 21:55:38 +0800 Subject: [PATCH 1/2] Add AGENTS.md and a PR rules check --- .github/pull_request_template.md | 2 + .github/workflows/pr-rules.yml | 95 +++++++++++++++++++ AGENTS.md | 152 +++++++++++++++++++++++++++++++ CONTRIBUTING.md | 34 ++++++- 4 files changed, 282 insertions(+), 1 deletion(-) 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 de2083e..cf4ece0 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -6,6 +6,8 @@ - [ ] In `packages/wherefrom`: `dart format --output=none --set-exit-if-changed .` and `flutter analyze` are clean, `flutter test` passes - [ ] `dart test` passes in `packages/wherefrom_builder` if it changed +- [ ] Closes an issue labelled `accepted`; at most 400 changed lines; no `// ignore:`, `@Suppress(` or `swiftlint:disable` (checked) +- [ ] Changes to Dart, Kotlin or Swift sources come with a test (checked) - [ ] If this fixes a bug: there is a test that fails without the fix - [ ] If this touches native code: the example builds (`flutter build apk --debug`, `flutter build ios --simulator --no-codesign`) - [ ] User-visible change: `CHANGELOG.md` entry, and README or TESTING.md updated diff --git a/.github/workflows/pr-rules.yml b/.github/workflows/pr-rules.yml new file mode 100644 index 0000000..9028a5d --- /dev/null +++ b/.github/workflows/pr-rules.yml @@ -0,0 +1,95 @@ +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 }} + # Both packages and their examples: Dart, Kotlin, Java and Swift sources. + CODE: '^packages/(wherefrom_builder/(lib|example)|wherefrom/(lib|android/src/main|ios/wherefrom/Sources|example/(lib|android/app/src/main|ios/Runner)))/.*\.(dart|kt|java|swift)$' + TESTS: '^packages/(wherefrom|wherefrom_builder)/(test|android/src/(test|androidTest)|ios/wherefrom/Tests|example/(test|integration_test|ios/RunnerTests|android/app/src/(test|androidTest)))/' + 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 a package's source but no test. Add a test under that package's test/ (or android/src/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|\\.lockfile|(^|/)Package\\.resolved)$") | 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 source lines that silence the Dart analyzer, Kotlin, SwiftLint or ShellCheck. + hits="$(jq -r '.[] | select(.patch) | .filename as $f | .patch | split("\n")[] | select(startswith("+")) + | select((($f | endswith(".dart")) and test("//\\s*ignore(_for_file)?:")) + or (($f | test("\\.(kt|kts|java)$")) and test("@(file:)?Suppress(Warnings)?\\(")) + or (($f | endswith(".swift")) and test("swiftlint:disable")) + or (($f | endswith(".sh")) and test("#\\s*shellcheck\\s+disable"))) + | "\($f): \(.)"' "$RUNNER_TEMP/files.json")" + changed="$(jq -r '.[].filename | select(test("(^|/)(analysis_options\\.yaml|\\.shellcheckrc)$"))' "$RUNNER_TEMP/files.json")" + if [ -n "$changed" ]; then + hits="${hits}"$'\n'"lint configuration is changed: ${changed//$'\n'/, }" + 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..8a39d85 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,152 @@ +# 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 wherefrom is + +Two packages that share one set of test vectors: + +- `packages/wherefrom`: a Flutter plugin that reads the Play Install Referrer + (Kotlin, `android/src/main/kotlin/`) and the Apple AdServices token (Swift, + `ios/wherefrom/Sources/wherefrom/`) and turns it into a typed result and + the deep link the user wanted (`lib/src/`). +- `packages/wherefrom_builder`: pure Dart, no runtime dependencies; builds + referrer strings and Play Store URLs on a server. + +Read before you touch: + +- parsing or building: the "Shared vectors" and "Never throw on input" rules + in CONTRIBUTING.md, `packages/wherefrom/doc/decoding.md` and + `packages/wherefrom/test/vectors/*.json`, which both packages' tests read. +- `readOnce()`, `claimOnce()` and the stored state: + `packages/wherefrom/doc/consume-once.md`. +- Android build setup: the "No Kotlin Gradle Plugin" rule in CONTRIBUTING.md + and the comment at the top of `packages/wherefrom/android/build.gradle.kts`. +- what real devices return: `packages/wherefrom/TESTING.md`. +- releases: `RELEASING.md`. + +## Commands + +CI uses Flutter 3.47.7 and Java 17. Every one of these must pass before you +open or update a pull request. CI runs the same ones. + +The plugin, in `packages/wherefrom`: + +```bash +flutter pub get +dart format --output=none --set-exit-if-changed . +flutter analyze +flutter test +(cd example && flutter test) +dart pub publish --dry-run +(cd example && flutter build apk --debug) +(cd example/android && ./gradlew :wherefrom:testDebugUnitTest) +(cd example && flutter build ios --simulator --debug --no-codesign) +``` + +The builder, in `packages/wherefrom_builder` (Dart 3.8.0 and stable): + +```bash +dart pub get +dart format --output=none --set-exit-if-changed . +dart analyze --fatal-infos +dart test +dart run example/wherefrom_builder_example.dart +dart pub publish --dry-run +``` + +CI also builds and checks release builds (R8 bundle, merged manifest, 16 KB +alignment, an emulator smoke test, the iOS privacy manifest); CONTRIBUTING.md +lists the commands to run them locally. + +Run one test while iterating: `flutter test test/install_referrer_test.dart` +(plugin), `dart test test/play_store_url_test.dart` (builder), or +`./gradlew :wherefrom:testDebugUnitTest --tests 'io.github.saimskywalker.wherefrom.ReferrerReaderTest'` +(in `packages/wherefrom/example/android`). + +## 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: + +- **Real Play or Apple output.** A referrer string, response code, install + source or AdServices answer described as "what Play returns" or "what the + device reports" comes from a real install, with the device, OS version and + Play Store version (see TESTING.md). Never write one from memory or guess + how an OEM or Play Store version behaves. +- **API behaviour.** Cite the Play Install Referrer, Android or Apple + AdServices documentation or the source that explains a behaviour. If you + cannot find one, say so; do not guess. +- **Release build results.** The merged-manifest snapshot + (`doc/merged-manifest.snapshot.xml`), R8, 16 KB alignment and privacy + manifest results come from a real release build. Never update the snapshot + without running `merged_manifest_snapshot.py --update` after one. +- **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 versions in the manifests: the plugin Dart 3.12 and + Flutter 3.44 (`packages/wherefrom/pubspec.yaml`), the builder Dart 3.8; + Android minSdk 21 and JVM 17 with AGP built-in Kotlin + (`android/build.gradle.kts`); iOS 15.0 (`ios/wherefrom.podspec`, + `ios/wherefrom/Package.swift`). Do not use features newer than that. +- Fixtures and examples use invented package names, campaigns and domains + such as `com.example.app`. Never use real data. +- 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 the Dart, Kotlin or Swift sources of either package (or + their examples) comes with a test change: `test/` of that package, + `packages/wherefrom/android/src/test/`, the example's `test/`, + `integration_test/` or `ios/RunnerTests/`. A bug fix has a test that fails + without the fix; check that it does before you open the pull request. +- A change in parsing or building behaviour is a change to the shared + vectors; a parser fix adds a vector for the string that broke it. + +## Never + +- Never silence the analyzer or a linter (checked): no `// ignore:`, + `// ignore_for_file:`, `@Suppress(`, `swiftlint:disable`, + `# shellcheck disable`, no changes to any `analysis_options.yaml` or + `.shellcheckrc`. Fix the finding instead. The existing + `@Suppress("DEPRECATION")` fallbacks for old Android versions stay; a new + one needs a maintainer's waiver. +- Never delete, skip, loosen or rewrite an existing test, expectation or + vector to make CI pass. If a test looks wrong, say so in the issue. +- Never change what users depend on without an issue that asks for it: the + exported API in `lib/wherefrom.dart` and `wherefrom_builder.dart`, the + method channel `io.github.saimskywalker/wherefrom`, the + `UnavailableReason` names, and the stored state file names + (`imaji_install_attribution.handled`, `.failed_reads`, `.first_seen`). +- Never apply the Kotlin Gradle Plugin, or give `wherefrom_builder` a + runtime dependency. +- Never bump a version (`pubspec.yaml`, `android/build.gradle.kts`, the + podspec), edit a released `CHANGELOG.md` section, publish, tag or release. + Add your entry at the top of the package's `CHANGELOG.md` under + `## Unreleased` (create the heading if it is missing). +- Never add a dependency or change CI workflows or `.github/scripts/` 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 06355ce..61c3725 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,6 +57,34 @@ reviews and merges, including changes from forks. For anything larger than a small fix, open an issue first so the approach can be agreed before you spend time on it. +## Pull requests + +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 the Dart, Kotlin or Swift sources of either + package (or their examples) needs a test change: the package's `test/`, + `packages/wherefrom/android/src/test/`, or the example's tests. A bug fix + needs a test that fails without the fix. +- **Small.** At most 400 changed lines (lock files not counted), one issue + per pull request. +- **No suppressions.** No `// ignore:`, `@Suppress(`, `swiftlint:disable` or + `# shellcheck disable`, no changes to any `analysis_options.yaml` or + `.shellcheckrc`. 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)). + ## Ground rules - **No Kotlin Gradle Plugin.** The Android plugin uses AGP built-in Kotlin; @@ -82,7 +110,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 6bcb1273ec35a7cc8d3f7a27b78d73bcedb29d33 Mon Sep 17 00:00:00 2001 From: saimskywalker <15120878+saimskywalker@users.noreply.github.com> Date: Tue, 10 Nov 2026 22:31:27 +0800 Subject: [PATCH 2/2] CONTRIBUTING: every pull request needs an accepted issue first --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 61c3725..011359a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,9 +53,9 @@ To see a real referrer on a device, follow ## How changes land Every change reaches `main` through a pull request that the maintainer -reviews and merges, including changes from forks. For anything larger than a -small fix, open an issue first so the approach can be agreed before you spend -time on it. +reviews and merges, including changes from forks. Every pull request, small +fixes included, needs an issue that a maintainer has labelled `accepted` +first, so the approach can be agreed before you spend time on it. ## Pull requests