fix(ci): surface the version, versionCode, track and branch of every Android release - #544
Conversation
…Android release
A failed release recorded nothing about what it was shipping. Run
29512207801 died in an early guard, and neither the run title ("Android
Release", identical for every run) nor the log said which version,
versionCode, track, or branch was being built, so reproducing it meant
guessing at the dispatch inputs.
Put the inputs in the run title via run-name, so `gh run list` and
`gh run view` identify a release without opening it, and add a Show
release parameters step that echoes them to the log and the job summary.
The step runs first, before disk-free and checkout, because the steps
that fail are all downstream of it and the values have to survive them.
Both carry the branch and the commit: re-dispatching wants the branch,
but reproducing a specific red run wants the SHA, since the branch will
have moved on. The step also prints a ready-to-paste `gh workflow run`
command that reconstructs the exact dispatch.
Verified by extracting the step from the workflow YAML and running it
against a build message containing quotes, $, & and ;: printf '%q'
escapes it, and the emitted command parses back to the original string
through a real shell, so it is safe to paste. run-name's support for the
inputs and github contexts confirmed against the GitHub Actions context
availability table.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sz19TStgQiJxZCZiE9tNsr
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
There was a problem hiding this comment.
Code Review: PR #544
Scope: PR #544 in thomasluizon/orbit-ui-mobile
Recommendation: APPROVE
Summary
Single-file, CI-only change to .github/workflows/android-release.yml: a run-name
expression that carries the dispatch inputs into the run title, and a new first step
that echoes the release parameters to the log and job summary plus a ready-to-paste
re-run command. The change does what the PR body claims and is well-verified (the
step was extracted and run verbatim against a message containing shell metacharacters).
Two non-blocking findings: an inconsistency between how message is escaped versus
app_version/android_version_code, and a comment that breaks the file's own
URL-backed WHY-comment convention.
Findings
Critical
None.
High
None.
Medium
[MEDIUM] RERUN_COMMAND and the summary table escape `message` but not `app_version` / `android_version_code`
· dimension: 1. Correctness / 12. Security (Injection)
· location: .github/workflows/android-release.yml:112-130
· issue: `BUILD_MESSAGE` is escaped with `printf '%q'` before being appended to `RERUN_COMMAND`,
and is deliberately kept out of the summary table because (per the PR body) "a `|` in
free-form text would break the markdown." `app_version` and `android_version_code` are
also free-form `workflow_dispatch` `type: string` inputs (validated only much later, at
the "Set Expo app version" step via `apps/mobile/scripts/set-app-version.js`'s
`/^\d+\.\d+\.\d+$/` / positive-integer checks) - but at the point "Show release
parameters" runs (deliberately the first step, before that validation), they are
interpolated into `RERUN_COMMAND` via plain `%s` and into the table row unescaped.
· risk: A value such as `1.4.2 (beta)` breaks the pasted re-run command (the exact
failure mode `%q`-escaping was added to prevent for `message`) or corrupts the summary
table with a stray `|`. The later version-format guard means a genuinely malformed
version still fails the whole build, but the corrupted summary/re-run command is
already emitted before that guard runs, undermining the PR's own "ready-to-paste"
and "cannot be broken" claims for the fields it didn't test.
· fix: Apply the same `printf '%q'` treatment to `APP_VERSION` and `ANDROID_VERSION_CODE`
when building `RERUN_COMMAND`, consistent with how `message` is already handled.
· reference: PR #544's own verification note (tested escaping only for `message`).
Low / Info
[LOW] New WHY comment breaks the file's own URL-backed-reference convention
· dimension: 4. Comment policy (not lint-gated - YAML is outside `local/no-comments` /
ORBIT0001's scope, so this is a local-convention note, not a rule violation)
· location: .github/workflows/android-release.yml:60-63
· issue: The "First step on purpose..." comment references "Run 29512207801" by number
only. The same file's pre-existing "Free disk space for the Android release build"
step links the run it references with a full URL
(`https://github.com/thomasluizon/orbit-ui-mobile/actions/runs/29030329772`), and the
PR body itself links `29512207801` the same way.
· risk: Minor - a reader has to reconstruct the URL to look up the referenced run.
· fix: `# ... Run https://github.com/thomasluizon/orbit-ui-mobile/actions/runs/29512207801 failed ...`
· reference: file's own precedent (the "Free disk space" step comment).
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A - no apps/web/** or apps/mobile/** file changed |
| i18n-syncer | N/A - no user-facing strings or i18n JSON changed |
| contract-aligner | N/A - no packages/shared/src/types/*, no second repo touched |
| security-reviewer | N/A - no orbit-api code changed (frontend/CI security categories reviewed directly in Phase 3 above) |
| design-reviewer | N/A - no apps/web/**, apps/mobile/**, or orbit-landing-page/src/** UI file changed |
Validation
| Check | Result |
|---|---|
| Lint | N/A - .github/workflows/*.yml is outside the npm turbo run lint globs (no .yml ESLint target in the repo) |
| Type check | N/A - same reason |
| Tests | N/A - no test surface changed; YAML confirmed to parse (python3 -c "yaml.safe_load(...)" succeeded) as a structural sanity check |
| Build (api) | N/A - orbit-api not touched |
Deferred - N/A dimensions & files not verdicted
- Dimensions 2 (Dead/stale code), 3 (SOLID/clean-arch), 5 (No-workaround), 6 (Type
safety), 7 (No console.log), 8 (DESIGN.md/AI-slop), 9 (Parity), 10 (i18n), 11
(Contract drift), 13 (Backend hard rules), 14 (FEATURES.md parity): all N/A - the
diff's only file (.github/workflows/android-release.yml) never touches any of
their gated surfaces (no TS/C# code, noapps/*UI, no shared types/DTOs, no
orbit-api, no user-facing feature surface). - No file left un-verdicted: the diff touches exactly one file
(.github/workflows/android-release.yml), and it received a full verdict above. - Adversarial skeptic pass (verification protocol Section 2) and the cross-model second
opinion (Section 2, Critical survivors) were not run: both are scoped to Critical/High
findings, and none survived Phase 3 - nothing to challenge. gh pr checks 544could not be run in this environment (command required
interactive approval that was not available); CI status for this PR was not
independently confirmed here.
What's good
- The parameter-echo step correctly uses the
env:mapping pattern (BUILD_MESSAGE: ${{ inputs.message }}->"${BUILD_MESSAGE}") instead of splicing${{ inputs.* }}
directly into therun:script - this is exactly the GitHub-recommended mitigation
for the classic Actions script-injection class, and it is applied to every input. run-nameonly usesinputs.*andgithub.ref_name, which the PR body confirms
against the GitHub Actions context-availability table forworkflow_dispatch.- Placing the parameters step first (before disk-cleanup/checkout) is the right call -
it guarantees the log/summary carries the shipped version even when the run fails on
an early guard, which is exactly the failure mode (run 29512207801) that motivated
the change. messageis correctly excluded from the summary table but included in the log
line and re-run command, avoiding a|-breaks-markdown bug while still keeping the
information available.- No secrets are echoed anywhere in the new step.
Recommendation
Approve as-is; the Medium finding is a real but low-blast-radius inconsistency (the
actor who could exploit it already needs repo write access to dispatch the workflow,
and a malformed version ultimately fails the build safely downstream) - fine to land
now with a follow-up to extend the %q escaping to app_version /
android_version_code, or fix inline if convenient before merge.
… follow-up) (#545) Android Release 1.3.22 (81) failed R8 eleven minutes into the Gradle build: Missing class expo.modules.kotlin.types.ColorCompat (referenced from: expo.modules.ui.UtilsKt.colorToComposeColorOrNull) expo-router depends on @expo/ui at ^57.0.4 and never declares a dependency on expo-modules-core, so npm has no way to see that @expo/ui 57.0.5+ calls ColorCompat, which expo-modules-core only added in 57.0.4. The release job installs the isolated workspace with --package-lock=false, so that unpinned range resolved to the newest publish (57.0.6) against expo-modules-core pinned at 57.0.3 (#543) and produced a set that cannot link. Verified against the registry rather than inferred: @expo/ui 57.0.4 has no reference to ColorCompat, 57.0.5 and 57.0.6 do, and ColorCompat.kt first appears in expo-modules-core 57.0.4. @expo/ui is the symptom. Reproducing the release install locally showed 13 of 57 native modules resolving to versions the repo-root lockfile does not hold, plus 7 more unpinned and matching only by luck of the current registry state: the AAB was compiled from native code no local install or CI job ever built, and #543 was the same class of failure a day earlier. So pin every native module to the version the root lockfile resolves (exact versions in dependencies for direct deps, overrides for the transitive ones) rather than chase them one release at a time. Only declared ranges move; no resolved version, and no lockfile version or integrity hash, changes. Replace the hand-maintained five-module guard with verify-release-native-modules.js, which walks the installed release tree, selects every module carrying an android/ Gradle project, and asserts each matches the root lockfile. It subsumes the old guard (all five modules are native and still checked), needs no upkeep when a native dep is added, and names the offender in about a minute instead of failing R8 eleven minutes in. The lockfile stays the single source of truth, so a deliberate dependency update passes as soon as the root lockfile carries it. Verified by reproducing the release step: prepare-release-workspace plus the same lockfile-free install now resolves all 57 native modules to the lockfile set and the guard exits 0; run against the pre-fix tree it exits 1 and lists all 13 drifted modules, @expo/ui among them. Residual, deliberately not bundled: 87 JS-only packages still drift the same way (mostly build-time tooling such as @expo/cli and @expo/config-plugins). Closing that means installing the release workspace from a lockfile instead of resolving fresh, which is a change to the Play release path that deserves its own PR. Claude-Session: https://claude.ai/code/session_01DDueAWEMzgcryVDyG7yBq2 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>



Why
Follow-up to #543. When run 29512207801 failed, the run recorded nothing about what it was shipping. The title was just
Android Release(identical for every run), and the failing step was an early guard, so the log never reached the version/track steps. Reproducing the run meant guessing at the dispatch inputs.What changed
1.
run-namecarries the inputs into the run title.gh run list/gh run viewnow identify a release without opening it:This matters most for the failure case:
gh run view <id> --log-failedshows only the failed step, so a passing params step wouldn't appear there. The title always does.2. A
Show release parametersstep echoes everything to the log and the job summary. It is the first step, before disk-free and checkout, because every step that can fail is downstream of it, and the values have to survive them.Both surfaces carry the branch and the commit SHA: re-dispatching wants the branch, but reproducing a specific red run wants the SHA, since the branch will have moved on by then.
3. A ready-to-paste re-run command, so an AI (or a human) can re-dispatch the identical build:
Job summary output
1.4.242productionmain3a13cb0f...falsefalsethomasluizonVerification
',",$,∧.printf '%q'escapes it, and the emittedgh workflow runcommand parses back to the original string through a real shell, so it is safe to paste and cannot be broken by a hostile message.run-namecontext support confirmed against the GitHub Actions context availability table (run-name→github, inputs, vars), not from memory. The expression uses onlyinputs.*andgithub.ref_name.Notes
messageis deliberately kept out of the summary table (a|in free-form text would break the markdown) but appears in the log line and the re-run command.🤖 Generated with Claude Code
https://claude.ai/code/session_01Sz19TStgQiJxZCZiE9tNsr