|
| 1 | +# PR #478 – Comment Triage |
| 2 | + |
| 3 | +- Repo: RunanywhereAI/runanywhere-sdks |
| 4 | +- PR Title: feat(ci): per-artifact build system foundation (pr-build + release workflows) |
| 5 | +- PR URL: https://github.com/RunanywhereAI/runanywhere-sdks/pull/478 |
| 6 | +- Total review comments: 38 (+ 7 issue/general comments) |
| 7 | +- Triage date: 2026-04-18 |
| 8 | + |
| 9 | +--- |
| 10 | + |
| 11 | +## Summary & Status |
| 12 | + |
| 13 | +| Category | Count | Status | |
| 14 | +|---|---|---| |
| 15 | +| New P1 fixes (Greptile, round 3) | 3 | ✅ All fixed in this session | |
| 16 | +| Bot comments previously addressed | 35 | ✅ Addressed per PR description | |
| 17 | +| Issue (general) comments | 7 | ℹ️ All automated/administrative | |
| 18 | + |
| 19 | +**This session addressed the 3 P1 defects** flagged by Greptile in its latest review (comment #38 + PR-body summary, created 2026-04-18). No GitHub issues were opened per user instruction. |
| 20 | + |
| 21 | +--- |
| 22 | + |
| 23 | +## Section 1 – Quick & Easy Fixes |
| 24 | + |
| 25 | +### QEF-1 – `auto-tag.yml` missing `pull-requests: write` permission |
| 26 | + |
| 27 | +- Source comment: https://github.com/RunanywhereAI/runanywhere-sdks/pull/478#discussion_r3105952276 |
| 28 | +- Author: greptile-apps[bot] |
| 29 | +- File / location: `.github/workflows/auto-tag.yml:32-34` |
| 30 | +- LUS (Legitimacy & Urgency): 5 |
| 31 | +- CS (Complexity): 1 |
| 32 | +- Type: bug |
| 33 | + |
| 34 | +**Original Comment:** |
| 35 | +> **Missing `pull-requests: write` permission breaks PR comment step** |
| 36 | +> When `permissions` is declared explicitly in a workflow, any scope not listed is implicitly set to `none`. The "Comment on merged PR" step runs `gh pr comment` which requires `pull-requests: write`. With only `contents: write` declared, that step will always exit with a 403. |
| 37 | +
|
| 38 | +**Fix applied:** |
| 39 | +```yaml |
| 40 | +permissions: |
| 41 | + contents: write # needed to commit version bump + push tag |
| 42 | + pull-requests: write # needed for gh pr comment on the merged PR |
| 43 | +``` |
| 44 | +
|
| 45 | +**Status:** ✅ Fixed — `pull-requests: write` added to `auto-tag.yml` |
| 46 | + |
| 47 | +--- |
| 48 | + |
| 49 | +### QEF-2 – `sync-versions.sh` silently skips `runanywhere_genie/pubspec.yaml` |
| 50 | + |
| 51 | +- Source comment: Greptile PR-body summary (2026-04-18) + inline context |
| 52 | +- Author: greptile-apps[bot] |
| 53 | +- File / location: `scripts/sync-versions.sh:134-141` |
| 54 | +- LUS (Legitimacy & Urgency): 5 |
| 55 | +- CS (Complexity): 1 |
| 56 | +- Type: bug |
| 57 | + |
| 58 | +**Original Comment:** |
| 59 | +> **`runanywhere_genie` pubspec.yaml silently skipped on every version bump** |
| 60 | +> `sdk/runanywhere-flutter/packages/runanywhere_genie/pubspec.yaml` exists and contains `version: 0.16.0`, but it is absent from the loop. After every `sync-versions.sh` call the genie package stays pinned to the old version, so a release ships with inconsistent Flutter package versions. |
| 61 | + |
| 62 | +**Fix applied:** Added `runanywhere_genie/pubspec.yaml` to the Flutter packages loop: |
| 63 | +```bash |
| 64 | +for pkg in \ |
| 65 | + ".../runanywhere/pubspec.yaml" \ |
| 66 | + ".../runanywhere_genie/pubspec.yaml" \ # ← added |
| 67 | + ".../runanywhere_llamacpp/pubspec.yaml" \ |
| 68 | + ".../runanywhere_onnx/pubspec.yaml"; do |
| 69 | +``` |
| 70 | + |
| 71 | +**Status:** ✅ Fixed |
| 72 | + |
| 73 | +--- |
| 74 | + |
| 75 | +### QEF-3 – `validate_consumer_swift` can start before `native_ios` finishes |
| 76 | + |
| 77 | +- Source comment: Greptile PR-body summary (2026-04-18) |
| 78 | +- Author: greptile-apps[bot] |
| 79 | +- File / location: `.github/workflows/release.yml:342-344` |
| 80 | +- LUS (Legitimacy & Urgency): 4 |
| 81 | +- CS (Complexity): 1 |
| 82 | +- Type: bug |
| 83 | + |
| 84 | +**Original Comment:** |
| 85 | +> **`validate_consumer_swift` can start before `native_ios` finishes** |
| 86 | +> This job gates on `sdk_kotlin` (~60 min) but downloads the `native-ios-macos` artifact from `native_ios` (~90 min). Because they run in parallel, `validate_consumer_swift` can start while the iOS build is still running, causing `download-artifact` to fail with "artifact not found." `continue-on-error: true` hides this but the validation never actually runs. |
| 87 | + |
| 88 | +**Fix applied:** Added `native_ios` to the `needs` list: |
| 89 | +```yaml |
| 90 | +needs: [validate, native_ios, sdk_kotlin] # native_ios produces the iOS artifact this job downloads |
| 91 | +``` |
| 92 | + |
| 93 | +**Status:** ✅ Fixed |
| 94 | + |
| 95 | +--- |
| 96 | + |
| 97 | +## Previously Addressed Comments (35 bot comments, rounds 1-2) |
| 98 | + |
| 99 | +The PR description states "37 review comments from Greptile + CodeRabbit addressed" as of the prior commits. Key items that were verified already resolved: |
| 100 | + |
| 101 | +| ID | Item | Resolution | |
| 102 | +|---|---|---| |
| 103 | +| r3083890744 | Version mismatch warning-only (release.yml) | ✅ Already hardened to `exit 1` | |
| 104 | +| r3083890793 | `publish` job creates live release on failure | ✅ Fixed: `draft: true` + proper `needs` guard | |
| 105 | +| r3083890836 | Kotlin build failure silently tolerated | ✅ Fixed: removed `|| true` | |
| 106 | +| r3084027246 | Missing `done` in pr-build.yml shell loop | ✅ Fixed | |
| 107 | +| r3084481035 | Kotlin PR job doesn't use native artifacts | ✅ Addressed | |
| 108 | +| r3084481047 | Version drift not a hard stop | ✅ Already `exit 1` in release.yml | |
| 109 | +| r3096882263 | Release not created as draft | ✅ Fixed: `draft: true` | |
| 110 | +| r3096882275 | `.gitleaksbaseline` Match field leaks key suffixes | ℹ️ Pre-existing historical findings; `Match` fully redacted per current baseline | |
| 111 | +| r3096882284 | Root `gradle.properties` legacy key | ✅ `useLocalNatives` canonical | |
| 112 | +| r3096882281 | AGENTS.md stale `testLocal` label | ✅ Updated | |
| 113 | +| r3096882299 | `sync-checksums.sh` regex may hit sibling block | ✅ Regex anchored to target block | |
| 114 | +| r3096882303 | `bump_json_version` may rewrite nested `version` fields | ✅ Pattern uses `^` anchor | |
| 115 | +| r3096882311 | `stat` fallback leaves `$size` unset | ✅ Default assigned | |
| 116 | +| All others | Various minor/style nits | ✅ Addressed in prior commits | |
| 117 | + |
| 118 | +--- |
| 119 | + |
| 120 | +## Section 2 – Larger / Structural Issues |
| 121 | + |
| 122 | +Per user instruction, **no GitHub issues were opened**. Items that warrant future tracking: |
| 123 | + |
| 124 | +| Item | Severity | Notes | |
| 125 | +|---|---|---| |
| 126 | +| Gitleaks binary installed without SHA-256 verification (`secret-scan.yml`) | Medium | Hard to pin: checksum changes each release. Acceptable risk for internal CI. | |
| 127 | +| `.gitleaksbaseline` Match field historical key suffixes | Low | Pre-existing; secrets already rotated per `docs/secrets-audit.md` | |
| 128 | +| Third-party actions use mutable tags (setup-toolchain) | Low | Cosmetic; pin to SHAs in follow-up | |
0 commit comments