diff --git a/.github/debug.keystore b/.github/debug.keystore new file mode 100644 index 000000000..6c5c50bf1 Binary files /dev/null and b/.github/debug.keystore differ diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..14dfee2e9 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,52 @@ +## Debug APK + + + +## Device test + + + +- [ ] Needs device test (labeled `needs-device-test`): +- [ ] Emulator is enough + +## Manual test checklist + + + +1. +2. +3. + +## What changed + + + +## Risks + + + +## Tested automatically + + + +## Code review + + + +- Reviewer used: +- [ ] Every finding addressed or answered + +## QA evidence + + + diff --git a/.github/workflows/pr-debug-apk.yml b/.github/workflows/pr-debug-apk.yml new file mode 100644 index 000000000..696be737c --- /dev/null +++ b/.github/workflows/pr-debug-apk.yml @@ -0,0 +1,168 @@ +name: PR debug APK + +# Builds an installable debug APK for every Android PR so it can be tested by +# hand on a phone. The PR build in android.yml is an UNSIGNED release APK, which +# Android refuses to install. This one is signed with .github/debug.keystore and +# installs on any device that allows sideloading. It is uploaded as an artifact, +# and a single sticky PR comment, updated on each push, links to the newest one. +# +# Tests and the release build still run in android.yml, whose `android-gate` is +# the required status check. Don't make this job required: it is path-filtered +# at the trigger, so on PRs that don't touch Android it never reports a status, +# and a required check would sit pending forever. + +on: + pull_request: + branches: [main, staging, dev] + paths: + - 'ft8af/**' + - '.github/workflows/pr-debug-apk.yml' + +permissions: + contents: read + pull-requests: write + +# A new push to the PR makes the previous APK stale, so cancel its build. +concurrency: + group: pr-debug-apk-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + debug-apk: + name: Debug APK for manual testing + runs-on: ubuntu-latest + + steps: + # Build the PR head commit itself, not the merge with dev, so the APK + # matches the SHA it is named after. + - name: Checkout repository + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + + - name: Cache Gradle dependencies + uses: actions/cache@v4 + with: + path: | + ~/.gradle/caches + ~/.gradle/wrapper + key: gradle-${{ runner.os }}-${{ hashFiles('ft8af/**/*.gradle*', 'ft8af/gradle/wrapper/gradle-wrapper.properties') }} + restore-keys: | + gradle-${{ runner.os }}- + + - name: Grant execute permission for gradlew + run: chmod +x ft8af/gradlew + + # Same retry as android.yml; keep NDK_VERSION in sync with it. + - name: Pre-install NDK (retry around flaky SDK downloads) + run: | + set -euo pipefail + NDK_VERSION=25.1.8937393 + SDKMANAGER="$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager" + MARKER="$ANDROID_HOME/ndk/$NDK_VERSION/source.properties" + for attempt in 1 2 3; do + echo "::group::NDK install attempt $attempt" + rm -rf "$ANDROID_HOME/ndk/$NDK_VERSION" "$HOME/.android/cache" + yes | "$SDKMANAGER" --install "ndk;$NDK_VERSION" || true + echo "::endgroup::" + if [[ -f "$MARKER" ]]; then + echo "NDK $NDK_VERSION ready at $ANDROID_HOME/ndk/$NDK_VERSION" + exit 0 + fi + echo "Attempt $attempt failed; retrying in 10s..." + sleep 10 + done + echo "::error::Failed to install NDK $NDK_VERSION after 3 attempts" + exit 1 + + # Sign with the shared .github/debug.keystore. Otherwise every runner makes + # its own debug key and each new APK refuses to install over the last one. + # The key is public on purpose: it only signs test builds, never releases. + # + # Maven Central rate-limits GitHub runner IPs (HTTP 429); retry like + # android.yml does. + - name: Build debug APK + working-directory: ft8af + run: | + set -euo pipefail + signing=( + -Pandroid.injected.signing.store.file="$GITHUB_WORKSPACE/.github/debug.keystore" + -Pandroid.injected.signing.store.password=android + -Pandroid.injected.signing.key.alias=androiddebugkey + -Pandroid.injected.signing.key.password=android + ) + for attempt in 1 2 3; do + if ./gradlew assembleDebug "${signing[@]}" --stacktrace; then + exit 0 + fi + echo "::warning::Gradle assemble failed (attempt ${attempt}/3) — retrying after backoff" + sleep $((attempt * 30)) + done + echo "::error::Gradle assemble failed after 3 attempts (likely Maven Central 429)" + exit 1 + + - name: Name the APK after the PR and commit + id: apk + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + name="FT8AF-pr${PR_NUMBER}-${HEAD_SHA:0:7}-debug" + mv ft8af/app/build/outputs/apk/debug/app-debug.apk "ft8af/app/build/outputs/apk/debug/${name}.apk" + echo "name=$name" >> "$GITHUB_OUTPUT" + + - name: Upload debug APK artifact + id: upload + uses: actions/upload-artifact@v7 + with: + name: ${{ steps.apk.outputs.name }} + path: ft8af/app/build/outputs/apk/debug/${{ steps.apk.outputs.name }}.apk + if-no-files-found: error + retention-days: 30 + + - name: Link the APK in the run summary + env: + ARTIFACT_URL: ${{ steps.upload.outputs.artifact-url }} + APK_NAME: ${{ steps.apk.outputs.name }} + run: | + echo "Debug APK: [${APK_NAME}.zip](${ARTIFACT_URL})" >> "$GITHUB_STEP_SUMMARY" + + # Fork PRs get a read-only token, so they can't comment. Their APK is + # still on the run page, through the summary link above. + - name: Post or update the APK comment on the PR + if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} + uses: actions/github-script@v7 + env: + ARTIFACT_URL: ${{ steps.upload.outputs.artifact-url }} + APK_NAME: ${{ steps.apk.outputs.name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + with: + script: | + const marker = ''; + const { ARTIFACT_URL, APK_NAME, HEAD_SHA, RUN_URL } = process.env; + const body = [ + marker, + '### Debug APK', + '', + `**[Download ${APK_NAME}.zip](${ARTIFACT_URL})** (built from ${HEAD_SHA.slice(0, 7)}, [run](${RUN_URL}))`, + '', + 'Unzip it and install the `.apk` on the phone. The download needs a signed-in GitHub account, and the link expires after 30 days.', + 'This build is signed with the shared debug key, so it installs over any earlier PR debug APK. If the Play Store build is installed, uninstall it first; Android will not install over an app signed with a different key.', + ].join('\n'); + const { owner, repo } = context.repo; + const issue_number = context.payload.pull_request.number; + const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number, per_page: 100 }); + const existing = comments.find(c => c.user.type === 'Bot' && c.body.includes(marker)); + if (existing) { + await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number, body }); + } diff --git a/AGENTS.md b/AGENTS.md index 59780008b..6aba5bf40 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,6 +68,64 @@ Remove the worktree when the branch is merged: `git worktree remove `. When nothing else is in flight in your checkout, a feature branch in place is fine, but a worktree per task is still the recommended default. +### Branch names + +`/`, where `` is `feat`, `fix`, `chore`, or `docs` +(e.g. `fix/civ-address-hex`). When the work comes from a tracked task, put its +id first in the slug, e.g. `fix/bur-12-cq-button-stuck`. + +### Pull requests + +- **Agents never merge.** Open the PR against `dev`, then stop. The maintainer + merges. They test on a phone only for `needs-device-test` PRs (see "Device + testing" below); for everything else they merge on the CI, code review, and + QA evidence. +- **Fill in every section of the PR template** + (`.github/pull_request_template.md`): the debug APK link, a 3-6 step + manual test checklist with expected results, what changed, risks, and what was + tested automatically. +- **The debug APK comes from CI.** For any PR that touches `ft8af/**`, + `.github/workflows/pr-debug-apk.yml` builds `FT8AF-pr--debug.apk` + and posts (or updates) one "Debug APK" comment on the PR with the download + link. Before you hand off, wait for that comment and link it in the + description. If the job fails, the PR isn't ready. +- **CI must be green.** That means `android-gate` (unit tests and the release + build) plus `Debug APK for manual testing`. +- **Get a code review before QA.** Once CI is green, request a Copilot review + first. If Copilot doesn't respond (it can run out of credits), run Claude's + `code-review` skill on the PR instead. Address or answer every finding before + handing off to QA, and fill in the "Code review" section of the template + with the reviewer you used. +- **Label PRs that come from Discord feedback** with `from-discord`, and link + the Discord message or the tracking task in "What changed". +- **QA attaches evidence.** After code review, QA installs the debug APK on an + emulator, runs the PR's manual test checklist, and posts screenshots or a + short screen recording to the PR. A PR without that evidence isn't ready + for the maintainer. + +### Device testing + +The maintainer doesn't install every PR's APK. Device testing happens once per +release batch, on the Play internal build that `staging` produces, which holds +every PR merged into `dev` since the last batch. Per-PR device testing is only +for changes an emulator can't check. Label those PRs `needs-device-test`: + +- transmit, PTT, or anything that keys the radio +- CAT / rig control (CI-V, Hamlib, FlexRadio, etc.) +- USB audio or USB serial +- audio routing, input/output device selection, or sample-rate handling +- runtime permissions, or what's declared in `AndroidManifest.xml` +- signing, build types, `versionCode`, or other release config + +The author applies the label and says why in the template's "Device test" +section. QA checks the label against the diff and adds it if it's missing. +When in doubt, add it. Everything else (UI, decode/display logic, settings, +CI, docs) merges on CI + code review + QA evidence. + +`from-discord` fixes are confirmed by the reporter: once the fix reaches a Play +testing track, `discord-play-track-watch` posts to Discord and the reporter +checks it on their own setup. + ## Testing **Every new code path requires a new test.** Any branch, helper, or behavior