From 98f4f82db2ed121678f6259a9683c51adbb82d61 Mon Sep 17 00:00:00 2001 From: Millian Lamiaux Date: Sun, 19 Jul 2026 21:14:59 +0200 Subject: [PATCH 1/2] fix(ci): pr-iphone-deploy no longer self-merges on its own comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build-deploy job posts a 'Prêt à tester' comment whose body literally contains 'Reply **LGTM** pour merger' and 'Reply **KO**' as instructions to the reviewer. wait-approval re-fetches ALL comments every 30s and greps any body for \bLGTM\b (checked before KO) — so on the first poll (~30s after deploy) the bot matched its OWN comment and squash-merged automatically, with no human review. Regressed in #6937a36 (PR #12) which tightened the regex to catch 'Tested, LGTM!' but didn't notice the bot body matched. Fix (sentinel marker + python parse, identity-agnostic so it's safe whether PR_API_TOKEN is a bot or a personal account): - build-deploy: embed in the bot comment (HTML comment, invisible in rendered markdown, present in raw body). - wait-approval: replace the fragile grep-over-JSON with a python3 heredoc using json.loads + the existing \bLGTM\b / \bKO\b regexes. Skip any comment whose body contains the sentinel marker. Check KO BEFORE LGTM so a PR with both signals blocks (matches the existing 'KO blocks' intent). LGTM still does {"Do":"squash"}; KO still blocks; 2h timeout unchanged. Verified locally with three scenarios: bot-only -> PENDING (was LGTM before — the bug) bot + reviewer -> LGTM (squash-merge still fires) bot + KO -> KO (block still fires, even with bot LGTM in history) AGENTS.md §5 step 9, §5 pitfalls, and §8 anti-patterns updated so the doc describes the marker rule and warns against re-introducing the self-match. --- .github/workflows/pr-iphone-deploy.yml | 81 ++++++++++++++++++++------ AGENTS.md | 5 +- 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/.github/workflows/pr-iphone-deploy.yml b/.github/workflows/pr-iphone-deploy.yml index 05d977b..4021695 100644 --- a/.github/workflows/pr-iphone-deploy.yml +++ b/.github/workflows/pr-iphone-deploy.yml @@ -172,10 +172,16 @@ jobs: run: | PR="${{ github.event.pull_request.number }}" REPO="${{ github.repository }}" + # The HTML comment is invisible in Gitea's rendered markdown but is + # present in the raw body — wait-approval uses it as a sentinel to + # skip THIS bot comment when scanning for LGTM/KO (otherwise the + # instruction text "Reply LGTM pour merger" would self-trigger a + # merge ~30s after deploy). DO NOT remove or reword without also + # updating the scanner in the wait-approval job. curl -s -X POST \ -H "Authorization: token ${GT_TOKEN}" \ -H "Content-Type: application/json" \ - -d "{\"body\":\"## 📱 Prêt à tester !\\n\\nL'app est déployée sur l'iPhone (devicectl).\\n\\n- Teste les changements\\n- Reply **LGTM** pour merger\\n- Reply **KO** pour bloquer\"}" \ + -d "{\"body\":\"## 📱 Prêt à tester !\\n\\nL'app est déployée sur l'iPhone (devicectl).\\n\\n\\n\\n- Teste les changements\\n- Reply **LGTM** pour merger\\n- Reply **KO** pour bloquer\"}" \ "${GITEA_URL}/api/v1/repos/${REPO}/issues/${PR}/comments" wait-approval: @@ -202,6 +208,19 @@ jobs: # Re-fetch ALL comments each cycle (not since_id) so we also catch # edits — e.g. a reviewer changing "KO" → "LGTM". Negligible cost for # PRs with <50 comments. + # + # Parsing is done with python3 (preinstalled on the macOS runner and + # already used by scripts/ci-status.py) rather than grep over raw JSON: + # - the bot's "Ready to test" comment body literally contains the + # words LGTM/KO as instructions to the reviewer, so any naive + # body grep would self-match and auto-merge ~30s after deploy. + # We skip it via the sentinel + # embedded by the build-deploy job; + # - json.loads avoids false matches when the literal "body" key or + # trigger words appear inside another string field. + # KO is checked BEFORE LGTM so a PR with both signals blocks (matches + # the existing "KO blocks" intent — a reviewer who flip-flops shouldn't + # merge just because an older LGTM is still in the history). while [ $TRIES -lt $MAX ]; do sleep 30 @@ -210,23 +229,51 @@ jobs: COMMENTS=$(curl -s -H "Authorization: token ${GT_TOKEN}" \ "${API}/issues/${PR}/comments?limit=50&page=1") - # Match LGTM/KO anywhere in the body (with a word boundary so "KOM" - # or "LGTMX" don't trigger). Case-insensitive. - if echo "$COMMENTS" | grep -qiE '"body":[[:space:]]*"[^"]*\bLGTM\b'; then - echo "✅ LGTM reçu ! Squash-merge..." - curl -s -X POST \ - -H "Authorization: token ${GT_TOKEN}" \ - -H "Content-Type: application/json" \ - -d "{\"Do\":\"squash\"}" \ - "${API}/pulls/${PR}/merge" - echo "✅ Mergée (squash)." - exit 0 - fi + DECISION=$(python3 - "$COMMENTS" <<'PY' +import json, re, sys +MARKER = "" +try: + comments = json.loads(sys.argv[1] or "[]") +except Exception: + comments = [] +lgtm = re.compile(r"\bLGTM\b", re.IGNORECASE) +ko = re.compile(r"\bKO\b", re.IGNORECASE) +hit_lgtm = hit_ko = False +for c in comments: + body = c.get("body") or "" + if MARKER in body: + # Skip the bot's own "Ready to test" comment — its body mentions + # LGTM/KO as instructions and would otherwise self-trigger. + continue + if ko.search(body): + hit_ko = True + if lgtm.search(body): + hit_lgtm = True +if hit_ko: + print("KO") +elif hit_lgtm: + print("LGTM") +else: + print("PENDING") +PY +) - if echo "$COMMENTS" | grep -qiE '"body":[[:space:]]*"[^"]*\bKO\b'; then - echo "❌ KO reçu. Bloquée." - exit 1 - fi + case "$DECISION" in + LGTM) + echo "✅ LGTM reçu ! Squash-merge..." + curl -s -X POST \ + -H "Authorization: token ${GT_TOKEN}" \ + -H "Content-Type: application/json" \ + -d "{\"Do\":\"squash\"}" \ + "${API}/pulls/${PR}/merge" + echo "✅ Mergée (squash)." + exit 0 + ;; + KO) + echo "❌ KO reçu. Bloquée." + exit 1 + ;; + esac if [ $((TRIES % 4)) -eq 0 ]; then echo " ⏳ ... (${TRIES}/240, $(date +%H:%M))" diff --git a/AGENTS.md b/AGENTS.md index fc22ba4..06c9250 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,7 +167,7 @@ Complications : `TabataGoComplication`. 6. **xcodegen generate** → `xcodebuild -resolvePackageDependencies` → **build** (`-scheme TabataGo`, Debug, auto-provisioning, team `2MJF39L8VY`). Le scheme build les 4 targets : `TabataGo` (app iOS), `TabataGoWidget` (widget iOS, vrai target — ne pas confondre avec `TabataGoWatchWidget` qui est watchOS), `TabataGoWatch` (app watchOS), `TabataGoWatchWidget` (complication watchOS). 7. **Deploy iPhone** UDID `00008120-000925CE3672201E` : `xcrun devicectl device install app` uniquement. `devicectl` gère nativement la découverte WiFi (réseau) et filaire (USB-C/Thunderbolt) — pas de fallback `ios-deploy`. 8. **Post comment** "Prêt à tester" sur la PR. -9. **Job `wait-approval`** : poll (30s, max 240 = 2h). **Re-fetche TOUS les comments** chaque cycle (pas `since_id`) pour attraper aussi les edits (un reviewer passant de "KO" à "LGTM"). Match LGTM/KO n'importe où dans le body (regex `\bLGTM\b` / `\bKO\b`, case-insensitive) — "Tested, LGTM!" compte ; "KOM" ne compte pas. `LGTM` → **squash-merge** (`{"Do":"squash"}`). `KO` → blocage. Timeout → fail. +9. **Job `wait-approval`** : poll (30s, max 240 = 2h). **Re-fetche TOUS les comments** chaque cycle (pas `since_id`) pour attraper aussi les edits (un reviewer passant de "KO" à "LGTM"). Le parsing se fait en **python3** (`json.loads` + regex, pas de `grep` sur JSON brut). **Le commentaire bot "Prêt à tester" porte un marker sentinel ``** et est **ignoré** par le scanner — sinon son propre body (qui mentionne LGTM/KO comme instructions au reviewer) déclencherait un auto-merge ~30s après le deploy (bug historique). KO est checké **avant** LGTM (un PR avec les deux signaux bloque). Match LGTM/KO n'importe où dans le body (regex `\bLGTM\b` / `\bKO\b`, case-insensitive) — "Tested, LGTM!" compte ; "KOM" ne compte pas. `LGTM` → **squash-merge** (`{"Do":"squash"}`). `KO` → blocage. Timeout → fail. ### Secrets @@ -187,6 +187,7 @@ Complications : `TabataGoComplication`. - `-skipPackagePluginValidation -allowProvisioningUpdates`. - **Concurrency** : le bloc `concurrency:` est volontaire — ne pas le retirer, sinon les pushes successifs empilent des pipelines et tentent des double-merges. - **LGTM/KO regex** : matche n'importe où dans le body avec `\b...\b` (word boundary). Ne pas revenir à un `grep '"body": *"LGTM"'` ancré — il raterait "Tested, LGTM!". +- **Self-match sentinel** : le commentaire bot "Prêt à tester" contient littéralement `Reply **LGTM** pour merger` et `Reply **KO**` — sans précaution, le scanner matche son **propre** commentaire et auto-merge ~30s après le deploy. Le body porte donc un marker `` que `wait-approval` ignore. **Tout commentaire bot posté par ce workflow doit porter ce marker** ; ne pas le retirer ni poster d'autre commentaire contenant LGTM/KO sans marker. - **Squash-merge** (`{"Do":"squash"}`) — pas merge commit ni rebase. Garde l'historique `main` linéaire. - **Comment edits** : `wait-approval` re-fetche tous les comments chaque cycle (pas `since_id`) pour attraper les edits. @@ -249,6 +250,8 @@ Node.js (`server.js`, `package.json`, `Dockerfile`). Télécharge l'audio de pla | Compter sur un bloc `permissions:` natif Gitea Actions | Tout passe par l'API Gitea avec `PR_API_TOKEN` ; le bloc natif serait no-op | | Retirer le bloc `concurrency:` | Permet double-merge et pipelines empilés | | Ancre le grep LGTM au début du body (`'"body": *"LGTM"'`) | Regex `\bLGTM\b` n'importe où dans le body (attrape "Tested, LGTM!") | +| Poster un commentaire bot dont le body contient "LGTM"/"KO" sans le marker `` | Le scanner bot-scan ignore les comments portant le marker — tout commentaire bot du workflow doit l'inclure (sinon auto-merge en ~30s) | +| Scanner les comments en `grep` sur le JSON brut | `python3` + `json.loads` (gère unicode/quotes, pas de false match sur la clé `"body"`) | | `{"Do":"merge"}` (merge commit) | `{"Do":"squash"}` pour un `main` linéaire | | Supprimer le scheme explicite dans `project.yml` | xcodegen 2.45.4 n'en crée pas — scheme doit lister `TabataGo`/`TabataGoWidget`/`TabataGoWatch`/`TabataGoWatchWidget` | | Compter sur `admin-web/` pour la app iOS | Dashboard admin séparé, communique via Supabase uniquement | -- 2.49.1 From 27f9c6b7b63a8cb7f3776d8a53969d3510a3a8ee Mon Sep 17 00:00:00 2001 From: Millian Lamiaux Date: Sun, 19 Jul 2026 22:05:28 +0200 Subject: [PATCH 2/2] fix(ci): replace trigger-level paths: with dorny/paths-filter (Gitea ignores on.pull_request.paths) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gitea Actions does not reliably honor `on.pull_request.paths:` filters — the workflow silently fails to trigger even when changed files match the filter. PR #13 touches .github/workflows/pr-iphone-deploy.yml (explicitly listed in the paths filter) and AGENTS.md, but only ci.yml fired. The repo history shows this was already known: 65d85b6 'remplacer paths trigger par dorny/paths-filter' moved to job-level filtering, but later commits reverted to the trigger-level paths: block. The proven-working pattern already lives in ci.yml (dorny/paths-filter@v3 in a 'changes' job). Fix (mirror ci.yml): - Remove trigger-level `paths:` from on.pull_request. - Add a 'changes' job (ubuntu-latest, dorny/paths-filter@v3, single 'ios' filter covering tabatago-swift/** and the workflow itself). - Gate build-deploy: needs: changes + if: needs.changes.outputs.ios == 'true'. - Gate wait-approval: if: needs.build-deploy.result == 'success' so a filtered-out deploy doesn't launch a merge poll against an undeployed PR. Everything else unchanged: concurrency, squash-merge, 2h timeout, the self-merge fix from the previous commit on this branch (sentinel marker + python3 parser). This PR will self-validate: it touches the workflow file, so the ios filter matches, so the deploy runs. --- .github/workflows/pr-iphone-deploy.yml | 36 +++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr-iphone-deploy.yml b/.github/workflows/pr-iphone-deploy.yml index 4021695..65f4802 100644 --- a/.github/workflows/pr-iphone-deploy.yml +++ b/.github/workflows/pr-iphone-deploy.yml @@ -8,12 +8,11 @@ on: pull_request: branches: [main] types: [opened, synchronize, reopened] - # Only run the iOS build+deploy when the PR actually touches the Swift app - # or this workflow itself. Docs-only / backend-only / admin-web-only PRs - # skip the runner entirely (no point rebuilding an unchanged .app). - paths: - - 'tabatago-swift/**' - - '.github/workflows/pr-iphone-deploy.yml' + # NOTE: no trigger-level `paths:` here — Gitea Actions does not reliably + # honor `on.pull_request.paths:` filters (the workflow silently fails to + # trigger even when changed files match). Path gating is done at the job + # level via dorny/paths-filter@v3 in the `changes` job below (same pattern + # as ci.yml). Docs/backend-only PRs then skip the macOS runner. # Every mutation (comments, merge) goes through the Gitea API with PR_API_TOKEN, # never the runner's native GITHUB_TOKEN — so a `permissions:` block would be @@ -26,8 +25,29 @@ concurrency: jobs: + # ── Path filter — determines whether the macOS build+deploy is worth running ── + # Gitea Actions does not honor trigger-level `on.pull_request.paths:`, so we + # gate at the job level with dorny/paths-filter (same pattern as ci.yml). + changes: + name: Detect Changes + runs-on: ubuntu-latest + outputs: + ios: ${{ steps.filter.outputs.ios }} + steps: + - uses: actions/checkout@v4 + + - uses: dorny/paths-filter@v3 + id: filter + with: + filters: | + ios: + - 'tabatago-swift/**' + - '.github/workflows/pr-iphone-deploy.yml' + build-deploy: name: Build & Deploy to iPhone (devicectl) + needs: changes + if: needs.changes.outputs.ios == 'true' runs-on: macos timeout-minutes: 30 @@ -187,6 +207,10 @@ jobs: wait-approval: name: Wait for LGTM comment needs: build-deploy + # Only run when build-deploy actually deployed. Without this, a skipped + # build-deploy (filtered out by `changes`) would still launch this job and + # the merge poll would run against a PR that was never deployed to device. + if: needs.build-deploy.result == 'success' runs-on: macos timeout-minutes: 120 -- 2.49.1