millianlmx/tabatago
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 5 Wiki Activity

fix(ci): pr-iphone-deploy no longer self-merges on its own comment #13

Merged
millianlmx merged 2 commits from fix/ci-lgtm-self-match into main 2026-07-19 22:16:13 +02:00
Conversation 0 Commits 2 Files Changed 2 +98 -24
millianlmx commented 2026-07-19 21:17:41 +02:00
Owner
Copy Link

Bug

pr-iphone-deploy auto-merges every PR ~30s after deploy, with no human review. The bot posts a « Prêt à tester » comment, then wait-approval immediately matches that same comment and squash-merges.

Root cause

The build-deploy job posts a comment whose body literally contains the trigger words as instructions to the reviewer:

  • Reply LGTM pour merger
  • Reply KO pour bloquer

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 matches its own comment and squash-merges automatically — no human review ever happens.

Regressed in #6937a36 (PR #12), which tightened the regex to catch "Tested, LGTM!" but didn't notice the bot's own body matched.

Fix — sentinel marker + python parse

Identity-agnostic (safe whether `PR_API_TOKEN` is a bot or a personal account):

  • `.github/workflows/pr-iphone-deploy.yml`
    • `build-deploy`: the bot comment now embeds `` — an HTML comment, invisible in Gitea's rendered markdown but present in the raw body.
    • `wait-approval`: replaced the fragile `grep`-over-JSON with a `python3` heredoc using `json.loads` + the existing `\bLGTM\b` / `\bKO\b` regexes. Any comment containing the sentinel is skipped. KO is now checked BEFORE LGTM, so a PR with both signals blocks (matches the existing "KO blocks" intent).
    • Merge style (`squash`), 2h timeout, concurrency group, and secrets handling are unchanged.
  • `AGENTS.md`: §5 step 9, §5 pitfalls, and §8 anti-patterns updated so the canonical doc describes the sentinel rule and warns future agents against re-introducing the self-match.

Verified locally

Ran the new parser against three simulated payloads:

Scenario Before fix After fix
bot-only (just "Prêt à tester") LGTM ❌ (auto-merge — the bug) PENDING ✅
bot + reviewer LGTM LGTM LGTM ✅ (merge still fires)
bot + reviewer KO LGTM (LGTM wins) KO ✅ (block still fires, even with bot LGTM in history)

Validation plan on merge

  1. After CI runs, confirm `wait-approval` stays in `PENDING` after the bot posts its comment (no auto-merge within 30s).
  2. Reply `LGTM` from a reviewer account → confirm squash-merge still fires.
  3. On another PR, reply `KO` → confirm block still fires.

Files

  • `.github/workflows/pr-iphone-deploy.yml` — sentinel marker + python3 parser
  • `AGENTS.md` — doc sync (§5 step 9, §5 pitfalls, §8 anti-patterns)
## Bug `pr-iphone-deploy` auto-merges every PR ~30s after deploy, with no human review. The bot posts a « Prêt à tester » comment, then `wait-approval` immediately matches that same comment and squash-merges. ## Root cause The `build-deploy` job posts a comment whose body literally contains the trigger words as instructions to the reviewer: > - Reply **LGTM** pour merger > - Reply **KO** pour bloquer `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 matches its **own** comment and squash-merges automatically — no human review ever happens. Regressed in #6937a36 (PR #12), which tightened the regex to catch \"Tested, LGTM!\" but didn't notice the bot's own body matched. ## Fix — sentinel marker + python parse Identity-agnostic (safe whether \`PR_API_TOKEN\` is a bot or a personal account): - **\`.github/workflows/pr-iphone-deploy.yml\`** - \`build-deploy\`: the bot comment now embeds \`<!-- tabatago:ready-to-test -->\` — an HTML comment, invisible in Gitea's rendered markdown but present in the raw body. - \`wait-approval\`: replaced the fragile \`grep\`-over-JSON with a \`python3\` heredoc using \`json.loads\` + the existing \`\\bLGTM\\b\` / \`\\bKO\\b\` regexes. Any comment containing the sentinel is skipped. **KO is now checked BEFORE LGTM**, so a PR with both signals blocks (matches the existing \"KO blocks\" intent). - Merge style (\`squash\`), 2h timeout, concurrency group, and secrets handling are **unchanged**. - **\`AGENTS.md\`**: §5 step 9, §5 pitfalls, and §8 anti-patterns updated so the canonical doc describes the sentinel rule and warns future agents against re-introducing the self-match. ## Verified locally Ran the new parser against three simulated payloads: | Scenario | Before fix | After fix | |---|---|---| | bot-only (just \"Prêt à tester\") | LGTM ❌ (auto-merge — the bug) | **PENDING** ✅ | | bot + reviewer LGTM | LGTM | **LGTM** ✅ (merge still fires) | | bot + reviewer KO | LGTM (LGTM wins) | **KO** ✅ (block still fires, even with bot LGTM in history) | ## Validation plan on merge 1. After CI runs, confirm \`wait-approval\` stays in \`PENDING\` after the bot posts its comment (no auto-merge within 30s). 2. Reply \`LGTM\` from a reviewer account → confirm squash-merge still fires. 3. On another PR, reply \`KO\` → confirm block still fires. ## Files - \`.github/workflows/pr-iphone-deploy.yml\` — sentinel marker + python3 parser - \`AGENTS.md\` — doc sync (§5 step 9, §5 pitfalls, §8 anti-patterns)
millianlmx added 1 commit 2026-07-19 21:17:41 +02:00
fix(ci): pr-iphone-deploy no longer self-merges on its own comment
All checks were successful
CI / Detect Changes (pull_request) Successful in 3s
Details
CI / YouTube Worker (pull_request) Has been skipped
Details
CI / Deploy (pull_request) Has been skipped
Details
98f4f82db2 
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 <!-- tabatago:ready-to-test --> 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.
millianlmx added 1 commit 2026-07-19 22:06:02 +02:00
fix(ci): replace trigger-level paths: with dorny/paths-filter (Gitea ignores on.pull_request.paths)
All checks were successful
CI / Detect Changes (pull_request) Successful in 3s
Details
CI / YouTube Worker (pull_request) Has been skipped
Details
CI / Deploy (pull_request) Has been skipped
Details
27f9c6b7b6 
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.
millianlmx merged commit e2532a8136 into main 2026-07-19 22:16:13 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
millianlmx
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: millianlmx/tabatago#13
Delete Branch "fix/ci-lgtm-self-match"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?