QVAC-24335 infra: install mobile prebuilds from a run id - #4591
Draft
tobi-legan wants to merge 11 commits into
Draft
tobi-legan wants to merge 11 commits into
tobi-legan wants to merge 11 commits into
Conversation
Contributor
Review StatusCurrent Status: ❌ PENDING Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member. |
Contributor
License compliance — cleanNo new dependency license findings in this PR. Warn-only (shadow) mode — this check does not block merges yet. Updated automatically by the canonical license compliance workflow. NOTICE presence (advisory)Missing NOTICE (advisory, does not block):
|
Contributor
Workflow security (shadow mode)zizmor found 1103 finding(s) in Findings are annotated inline on the changed files and listed in the job summary. Reproduce locally: pipx run zizmor==1.27.0 --offline .github/ |
Testing an unmerged NATIVE change on a device took four manual steps after mobile became dispatch-only: push a tmp-* branch, dispatch On Merge Trigger, wait for the publish, then hand-assemble @tetherto/<addon>-mono@<version>-tmp.runid-<id> as `package`. Skipping any of them silently tested the published release instead: one PR ran mobile on 5 addons and went green with `package` empty, so every run installed the published tarball rather than the ~300 lines of new C++ under review. Add a `prebuild_run_id` dispatch input that installs the prebuilds artifact a named run already built, so testing your own PR needs no tmp branch, no publish and no package name. It is the highest-precedence prebuild source and fails closed — a run id that is wrong, unreadable, unfinished or expired fails the run rather than falling back to @latest. - New `prebuild-run-id` input on run-mobile-integration-tests/setup, wired into the 12 mobile workflows that expose a pinned-package route, with `actions: read` on each build job. decoder-audio (no prebuild of its own) and inference-addon-cpp (compiles its own in-run) are exempt, and the test pins both reasons so a third exemption has to be a decision. - Every other prebuild source is gated on an empty run id, and prebuilds/ is cleared before extraction, so nothing can shadow the resolved run. - The download selects the artifact by ID, not name: a re-run leaves the earlier attempt's artifacts under the same run id, so a run can hold two live `prebuilds-<pkg>` rows. - Provenance names the run, artifact, head SHA, branch and the repository the binaries were built from. A fork-built run WARNS rather than failing: this repo is fork-first, and a fork's prebuilds only exist once the merge/release team approved `fork-ci` on that run. - audiogen-ggml pins its composite actions to the default branch, so it ignores an unknown input with only a warning and would silently install @latest. The setup action now exposes `prebuild-source-run-id` and that workflow asserts it matches the request, failing with instructions instead. A test requires the same assertion from any workflow pinning the composite to the default branch. - Node, not `gh`: these jobs run on self-hosted runners where the CLI is not guaranteed. The resolver sits beside the action so the callers' existing sparse checkout covers it. Also adds the qv-mobile-test-dispatch skill (.agents/skills, discovered directly by Codex and Cursor, linked into .claude/skills by scripts/agent-setup.sh) and a quick-start in docs/ci/MOBILE-ON-DEMAND.md, both covering how to find a run id, pick a valid test filter, and read the result. Verified against the live repo and real artifacts, not only unit tests: - 12 addons resolved their own bundle out of a 63-artifact multi-addon nx run. - Real downloads driven through the whole setup chain for 5 addons on both platforms; the correct per-addon .bare lands in the right dir. - 28 real Device Farm dispatches. Green end to end with a run id on llm-llamacpp and diffusion-cpp (both platforms), asr-ggml, ocr-ggml, bci-whispercpp, tts-ggml and translation-nmtcpp. - The published-npm and GPR routes still resolve unchanged, a wrong run id fails closed before any Device Farm spend, and setting both inputs is refused. Fixes found by review and covered by new tests: the main-module guard compared URL strings against argv[1], so through a symlinked $ACTION_PATH (macOS /tmp -> /private/tmp) the script exited 0 having done nothing while the following rm -rf still wiped prebuilds/; an unfinished source run was reported as "does not build prebuilds"; a failed or truncated artifact listing was reported as an empty run; and the not-found hint dumped all 65+ artifact names instead of naming which addons the run did build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oups.json
The on-device prestage step refuses a shard grep that matches no known runner,
because a workflow_call lane never runs validate-devices and an under-staged run
would otherwise ship silently. The oracle was built from MODEL_SHARDS — the
subset of runners that HAVE a model to stage — so every model-less runner read as
drift:
-f tests=runEsmNamedExportsTest
[prestage] FATAL: tests grep /runEsmNamedExportsTest/ matched no known runner
(test-groups <-> model-map drift)
observed on a real Device Farm run. runEsmNamedExportsTest is scheduled in
test-groups.json for both platforms, and the mobile-deferred runPi05Test that the
existing comment calls out as a legitimate network-fallback case would have
failed the same way. The existing test only covered runGrootTest, which is in
MODEL_SHARDS, so the gap survived.
Read the oracle from test-groups.json, the actual runner list, and keep
MODEL_SHARDS for deciding which models to stage. An unreadable test-groups.json
falls back to the model list rather than an empty oracle, which would fail every
grep closed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…bare-log flush
Appium's pull_file returns `value` as a base64 string on success and as an
error object ({error, message, stacktrace}) on failure. flushBareLog passed it
straight to Buffer.from, so a failed pull threw
[bare-log] after flush failed: The first argument must be of type string or an
instance of Buffer, ArrayBuffer, or Array or an Array-like Object
which replaced Appium's reason with a type error. Observed on every Android
Device Farm run, passing or failing; iOS logs 'flush ok' and is unaffected.
bare_console.log is where a failing runner says WHY it failed — the Device Farm
artifacts otherwise carry only the harness assertion at app.test.js and an OS
logcat. Masking the error therefore makes every Android mobile failure
untriageable, which is what blocked root-causing model-fit's runFitStubTest here.
Type-check the payload and raise Appium's own message instead. This does not by
itself make the Android pull succeed — the path is the iOS bundle form — but it
reports what Appium actually says so the next run shows whether an Android path
is available.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…not beside the source file
runFitStubTest failed on every mobile run, Android and iOS alike, while
runFitTest passed. The reason was invisible on Android (the app-side log is not
captured there); an iOS dispatch produced it:
FileError: ENOENT: no such file or directory, stat "/app.bundle"
at mkdirSync (bare-fs)
at ensureFixtures (test/integration/fit-stub.test.js:45)
The test writes its fixtures 'beside the downloaded model', but derived that
location a second time as path.resolve(__dirname, '../model'). On a device the
file is loaded out of the read-only app bundle, so that expression resolves
inside the bundle and the directory cannot be created — while the model's own
directory is writable and had just been downloaded into, which is why only the
stub test failed.
Pass the directory the model actually landed in. fixtureDir/fixturePath take an
optional base and default to the checkout layout, so desktop behaviour is
unchanged and 'beside the model' becomes true by construction on both.
No new dependency: bare-path is already declared. An earlier attempt used
bare-os for a platform check, which model-fit does not depend on and would have
replaced the ENOENT with MODULE_NOT_FOUND on device.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the iOS container form flushBareLog only ever asked for '@<bundle>:documents/bare_console.log', the iOS (XCUITest) container form. On Android that can never resolve, and every run logged Cannot access the container of 'io.tether.test.qvac:documents' application so the app-side log was never captured there. That log is the only place a failing runner says WHY it failed — the Device Farm artifacts otherwise carry the harness assertion and an OS logcat with no bare output — which is what makes an Android mobile failure untriageable. iOS logs 'flush ok' and is unaffected. Try platform-appropriate candidates: the container form on iOS, and on Android the UiAutomator2 forms that perf-extract.js already pulls perf-report.json with successfully. A run that exhausts every candidate reports the last error and how many paths were tried, instead of a bare failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ole.log pull With the Android path forms in place the pull reaches adb and then stops at adb pull /data/user/0/io.tether.test.qvac/files/bare_console.log adb: error: failed to stat remote object: Permission denied adb cannot read another app's sandbox. perf-extract.js already solves this for perf-report.json with 'run-as <bundleId> cat <relative>', so use the same mechanism as a last resort before reporting failure. This runs through the WDIO command queue rather than the raw-HTTP pull the earlier candidates use, so it is confined to the normal end-of-run path and does not affect the crash path the raw-HTTP approach exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rying doomed pulls Chasing the Android app-side log to its end: adb cannot read the app's private data dir adb pull /data/user/0/<pkg>/files/bare_console.log adb: error: failed to stat remote object: Permission denied and run-as is refused on the Device Farm artifact run-as: package not debuggable: <pkg> both observed on real runs. With a release-signed APK there is no route to the app's private files, so no harness change can recover that log. Keep one Android candidate — the world-readable external-files path, which adb CAN read and which the app could be changed to write to — and otherwise log a line saying the log is unavailable and why, pointing at an iOS run for app-side output. Drops five failing pulls and a run-as call from every Android run. This is why an Android mobile failure cannot be triaged from its artifacts: logcat carries no bare output either. Closing the gap means making the app write bare_console.log to the external files dir, which belongs to the mobile test framework rather than here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An earlier commit here claimed logcat carries no bare output and that an Android
mobile failure therefore cannot be triaged. That is wrong. The bare runtime logs
to logcat under the `bare` tag, so logcat_full.txt carries the TAP lines and the
real failure, e.g.
E bare: Test 'runFitStubTest' failed: AddonError: ADDON_NOT_FOUND:
Cannot find addon '.' from @qvac/model-fit/binding.js
Candidates: - linked:libqvac__model-fit.0.12.0.so
[cause]: Error: dlopen fail
which is the actual reason model-fit fails on Android, visible in artifacts that
were there all along. The earlier searches looked for TAP markers and the app
package name rather than the `bare` tag and the smaller Logcat.logcat file
rather than logcat_full.txt.
bare_console.log remains unavailable on Android for the reasons already
documented; it is simply the iOS channel for the same output, not a gap. The
skill now says where to look per platform.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e C++ output test-results.json only carries the harness assertion, which is identical for every failure. The reason is in the app's own output: logcat_full.txt under the `bare` tag on Android, bare_console.log on iOS. Native engine output rides the same channel on both, prefixed [C++ TEST]. Documents the traps that cost time in practice: logcat_full.txt is not the smaller Logcat.logcat, and the runtime prints through logcat so TAP markers never appear as raw lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- selectArtifact now takes the NEWEST matching row, not the first listed. A re-run leaves the earlier attempt's artifacts under the same run id, and the API lists id-ascending, so first-match pinned the pre-re-run binary while the provenance line printed the same head_sha either way. created_at breaks the tie when ids are not comparable. - The skill and docs described the vla prestage drift as a live trap, which commit 7acc75c in this PR removes. That FATAL now only fires on a name in neither list, i.e. a typo. - The documented quick-start could dispatch an EMPTY prebuild_run_id when no run carries the bundle, which is the unchanged path and quietly resolves @latest — the failure this route exists to close, reachable by copy-pasting the snippet. Guarded in both the docs and the skill. - audiogen's honoured-run-id assert compared the raw dispatch input against the resolver's trimmed output, so a run id pasted with a stray space installed correctly and then failed the assert after the build was paid for. - Catalogued qv-mobile-test-dispatch in qv-skill-list; it is manual-only, so nothing else surfaces it. Not changed: the review flagged the 11th workflow_dispatch input on integration-mobile-test-llm-llamacpp.yml as exceeding GitHub's 10-input cap. That cap is no longer enforced — this branch's 11-input workflow was dispatched successfully twice (runs 35235330030 and 35238417351, both passing -f prebuild_run_id), and test-sdk.yml runs with 25 inputs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
model-fit and vla-ggml run prettier --check in their lint:js gate, and three added lines exceeded the configured print width, failing model-fit-pr-head-ts-checks and vla-pr-head-ts-checks. Wrap them the way each file already wraps its siblings. Also trims the comment blocks added since the last pass, which had grown to 38-50% of the added lines in the files touched after it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tobi-legan
force-pushed
the
infra/QVAC-24335-mobile-dispatch-prebuild-run-id
branch
from
September 18, 2026 16:17
ae2c3d5 to
75a8080
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 What problem does this PR solve?
Testing an unmerged native change on a device took four manual steps after mobile became dispatch-only (QVAC-23665): push a
tmp-*branch, dispatch On Merge Trigger, wait for the publish, then hand-assemble@tetherto/<addon>-mono@<version>-tmp.runid-<id>aspackage.Skipping any of them silently tested the published release instead. QVAC-24253 ran mobile on 5 addons and went green with
packageempty, so every run installed the published tarball rather than the ~300 lines of new C++ under review. Nobody noticed, because the input help said empty meant "this branch's prebuild artifact".@qvac/asr-ggmland@qvac/audiogen-ggmlpublish no mobile prebuilds at all, so for those two an empty input could never work — it fails withNo prebuilds directory found in package.📝 How does it solve it?
New
prebuild_run_iddispatch input installs theprebuilds-<pkg>artifact a named run already built (falling back to the legacyprebuildsname). Point it at the on-pr run that built your prebuilds — no tmp branch, no publish, no package name.Highest-precedence prebuild source. Every other source is gated on an empty run id, and
prebuilds/is cleared before extraction, so nothing can shadow the resolved run.Fails closed. A run id that is wrong, unreadable, unfinished or expired fails the run with the reason instead of sliding back to
@latest. Setting bothprebuild_run_idandpackageis refused rather than silently resolved.The download selects the artifact by id, not name: a re-run leaves the earlier attempt's artifacts under the same run id, so a run can hold two live
prebuilds-<pkg>rows.Provenance names the run, artifact, head SHA, branch and the repository the binaries were built from. A fork-built run warns rather than failing — this repo is fork-first (
docs/gitflow.md), and a fork's prebuilds only exist once the merge/release team approvedfork-cion that run.audiogen-ggmlpins its composite actions to the default branch, so it ignores an unknown input with only a warning and would silently install@latest. The setup action now exposesprebuild-source-run-idand that workflow asserts it matches the request, failing with instructions instead. A test requires the same assertion from any workflow pinning the composite to the default branch.Node, not
gh: these jobs run on self-hosted runners where the CLI is not guaranteed. The resolver sits beside the action so the callers' existing sparse checkout covers it.Adds the
qv-mobile-test-dispatchskill (.agents/skills/, discovered directly by Codex and Cursor, linked into.claude/skillsbyscripts/agent-setup.sh) and a quick-start indocs/ci/MOBILE-ON-DEMAND.md, both covering how to find a run id, pick a valid test filter, and where the Android and iOS logs are, including the native C++ output.Not applicable to two addons, by design:
decoder-audiohas no native prebuild of its own (it ridesbare-ffmpegfrom npm) andinference-addon-cppcompiles its own prebuilds in-run from the dispatched ref. A test pins both reasons so a third exemption has to be a decision.Interface change is additive:
prebuild-run-idis optional and defaults to"", andprebuild-source-run-idis a new output. Every existing caller is unchanged — verified by control runs that still resolve@qvac/<addon>@latestfrom npm. Not tagged[bc].Drive-by fixes found while testing
vla-ggmlprestage drift — the on-device drift oracle was built fromMODEL_SHARDS(the runners that have a model) instead oftest-groups.json(the runners that exist), so any model-less runner failed closed with[prestage] FATAL: matched no known runner.runEsmNamedExportsTestis scheduled on both platforms and the mobile-deferredrunPi05Testwould have failed the same way.model-fitfixture path —fit-stub.test.jswrote fixtures to a second derivation of "beside the model" (path.resolve(__dirname, '../model')), which on device points inside the read-only app bundle:ENOENT: ... stat "/app.bundle". Now derived from the model's own path.flushBareLogfed Appium's error object toBuffer.from, so every failure surfaced asThe first argument must be of type string…, masking the real reason. Now type-checked and reported; the Android branch states plainly thatbare_console.logdoes not exist there and points atlogcat_full.txtunder thebaretag.🧪 How was it tested?
Unit and policy suites, plus 28 real AWS Device Farm dispatches across 12 addons and both platforms. Every result below is read from the run's
test-results.json, not from the workflow conclusion — each green row istotal=1 passed=1 failed=0with the named runner recordedpassed. Each run exercises one filtered test, deliberately, to keep Device Farm cost down; this is not full-suite coverage.node --test .github/actions/run-mobile-integration-tests/setup/test/*.test.mjs— 38 pass (injected HTTP client, no network or token)node --test .github/scripts/test/mobile-prebuild-registry.test.mjs— 104 passnode --test packages/vla-ggml/scripts/__tests__/*.test.js— 39 passnode scripts/ci/validate-agent-config.mjs— 36 skills validatednode .github/scripts/lint-workflows.mjs— actionlint structural gate cleanGreen end-to-end with
prebuild_run_idExisting paths unchanged
Verified: prebuilds come from @qvac/<addon>@<ver> (npm (registry.npmjs.org))Verified: prebuilds come from @tetherto/<addon>-mono@<ver>-tmp.runid-<id> (pinned, GitHub Packages)Fail-closed behaviour
Run … has no 'prebuilds-ocr-ggml' … That run built prebuilds for: vla-ggml.—Build mobile app,Upload to Device Farm,Schedule Device Farm test runsall skipped, zero Device Farm costprebuild_run_idandpackageprebuild_run_id and a pinned package are mutually exclusive (…)— Device Farm steps skippedAlso exercised against captured real API payloads for paths the live repo cannot produce: expired artifacts, an unfinished run, a fork-built run, a legacy bare
prebuildsname, and a run that published nothing.🔐 Action pinning
actions/download-artifact: no version change. The new cross-run download step reuses the SHA already pinned throughout this repo —3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # 8.0.1— which is the same pin thebenchmark-perf-*.ymlworkflows already use withrun-id. No third-party action was added, bumped or repinned.🛡️ Permissions changes
build-and-testin the 12integration-mobile-test-*.ymlworkflows that expose a pinned-package route.contents: read,packages: read,pull-requests: write,id-token: writeactions: readactions: read. Without it the lookup 403s after the dispatcher has already waited for a build; the resolver detects that case and says so explicitly. No other scope changed, and the token remainssecrets.GITHUB_TOKEN(not a long-lived PAT). A security review confirmed the run-id path actually reduces token exposure on that lane, because it skips the npm/GPR step that materialises a credentialed.npmrc.🐛 Bugs found that need a dev (not caused by this PR)
Testing surfaced three failures with isolating evidence. None is introduced by this change, and none blocks it.
1.
model-fit— native addon fails todlopenon Androidrun 35358616440 —
logcat_full.txt,baretag:Android only — the same test passes on iOS after the fixture fix (35354826155). Predates this PR: identical failure on a
maindispatch from before this branch existed (35212397340).2.
embed-llamacpp— the #4519 binary fails on Android only@qvac/embed-llamacpp@0.41.1→ passes (35235878040)Two variables, each isolated: change the platform or change the binary and it passes. So it is specifically the Android build of the run-
35217306279binary, which comes from PR #4519 ("assert addons pin fabric's C++ runtime and narrow exports") — merged, and the published0.41.1predates it. Worth checking before embed's next release. Caveat: that run is a pre-merge build of #4519, so confirming means a run id from a post-merge build.3.
classification-ggml— dev builds differ from the published release0.26.2build) → fails (35248219256)@tetherto/classification-ggml-mono@0.26.2-tmp.runid-35247678767→ fails the same way (35250768920)@qvac/classification-ggml@0.26.1→ passes (35234478391)It fails through the pre-existing GPR route as well as the new one, so this is a real difference in its dev builds rather than anything introduced here.
Related, worth its own ticket
@qvac/asr-ggmland@qvac/audiogen-ggmlpublish no mobile prebuilds, so an empty dispatch input can never work for them. Their@tetherto/*-monodev builds do carry prebuilds, andprebuild_run_idworks — asr is proven green above.audiogen-ggmlcannot useprebuild_run_iduntil this merges, because it loads its composite actions from the default branch. It now fails with an explicit message instead of silently installing@latest.MODEL_SHARDSvstest-groups.jsondrift pattern fixed here forvla-ggmlcould recur in any addon whose prestage generator keeps its own runner list.🔎 Reviews
node -eliteral cannot break out (the identifier regex rejects quotes, backslashes, backticks and$(...)). Noted that the run-id path reduces token exposure, since it skips the step that materialises a credentialed.npmrc.ae2c3d5c9: newest-artifact tie-break, stale prestage-trap docs, an empty-RUN_IDfootgun in the documented snippet, the audiogen trim comparison, and the skill catalog entry. One finding not actioned: the 11thworkflow_dispatchinput on the LLM workflow was flagged as exceeding GitHub's 10-input cap, but that cap is no longer enforced — this branch's 11-input workflow dispatched successfully twice (35235330030, 35238417351), andtest-sdk.ymlruns with 25.