chore: retro-review PR #103 — ci: include dev branch in dual-loop-review PR-target gate

[Artic0din]

Synthetic retro-review of merged PR #103. Diff = original ee0aaba..58ee0b2 (same as original PR). Do NOT merge; close after @claude review posts.

[Artic0din]

@claude review

[claude]

Claude finished @Artic0din's task in 8m 6s —— View job

---

Code Review — PR #135 (Retro-review of PR #103)

• Gather context and read changed files
• Analyze the diff
• Post review findings

---

Summary

Single-line change to .github/workflows/dual-loop-review.yml: adds dev to the branches filter on the pull_request trigger so Loop 1 (automated first-pass review) fires on PRs targeting dev in addition to main and develop.

-    branches: [main, develop]
+    branches: [main, develop, dev]

---

Findings

✅ Correct and targeted — The change does exactly what it says. Loop 1 (auto-review job) is now gated on three target branches instead of two. No logic, security, or correctness issues introduced.

✅ AEGIS rules satisfied

• No ${{ }} interpolation in run: blocks (there are no run: blocks in this file).
• Permissions are explicit (contents: read, pull-requests: write, issues: write, id-token: write) — not write-all.

🟡 Minor: branch naming ambiguity — The list now contains both develop and dev. If these refer to two distinct long-lived branches, that's intentional. If one is a legacy alias for the other, the stale entry is dead weight and will silently never match. Not blocking, but worth a comment in the branch strategy docs or removing the unused name.

ℹ️ Pre-existing asymmetry (not introduced here) — Loop 2 (deep-review) is triggered by issue_comment/pull_request_review_comment events and carries no branch filter, so @claude review fires on any PR regardless of target branch. This is a reasonable design (deep reviews are on-demand), but it means Loop 1 and Loop 2 have different coverage envelopes. Something to keep in mind if dev-targeting PRs start generating unexpected deep-review runs via comment trigger.

---

Verdict

LGTM. The change is minimal, correct, and follows existing CI conventions. The only actionable note is the develop vs dev naming question, which should be clarified if both branches aren't actively used.

Score: 9/10

[claude]

Claude finished @Artic0din's task in 13m 36s —— View job

---

PR #103 Retro-Review

• Gather context
• Read diff and changed file
• Analyze change
• Post review findings

---

Review — ci: include dev branch in dual-loop-review PR-target gate

Diff summary: Single-line change adding dev to the branches filter in the pull_request trigger of .github/workflows/dual-loop-review.yml.

-    branches: [main, develop]
+    branches: [main, develop, dev]

---

✅ Correctness

The change is mechanically correct. GitHub's branches: filter under pull_request matches the target (base) branch of a PR, not the source branch. Adding dev means Loop 1 (auto-review) now fires when a PR targets main, develop, or dev. That is the stated intent and it works.

Loop 2 (deep-review on @claude review) is triggered by issue_comment/pull_request_review_comment events, which carry no branch filter — so this change has no effect on Loop 2. That's fine and expected.

---

🟡 Mild Concern — develop vs dev naming ambiguity

The list now contains both develop and dev. If this repo actually has two long-lived integration branches with these names, listing both is correct. But if one is a rename or alias of the other, the dead entry is silent noise. Worth a quick confirmation that both branches are in active use — no functional breakage either way, since GitHub silently ignores a filter entry for a branch that doesn't exist.

---

🔒 CI/CD Rules (CLAUDE.md)

Rule	Status
No ${{ }} interpolation in run: blocks	✅ No run: steps in this workflow
No permissions: write-all	✅ Uses scoped permissions

One existing (pre-PR) observation: id-token: write is present on the workflow. This permission is typically needed for OIDC cloud auth, which this workflow doesn't use. It's not introduced by this PR but is worth cleaning up separately if OIDC is not needed.

---

Verdict: LGTM

The change is minimal, safe, and does exactly what it says. The develop/dev naming question is worth a comment in the commit or PR description, but it does not block the change.

[Artic0din]

Retro-review complete. Findings (if any) tracked in fix/retro-review-batch (#143) or noted in .planning/retro-review-findings.md. Closing synthetic PR + cleaning branches.