fix(ci): init every vendored submodule in the release Docker builds - #5596
Conversation
The Docker job named eight submodule paths by hand while `.gitmodules` declares ten, so `vendor/tinyhosts` and `vendor/tinywallet` were never checked out. The Dockerfile COPYs `vendor/`, so both arrived in the image as empty directories and cargo failed on the missing manifest before compiling anything. Production releases have been blocked since 2026-08-07 and staging carries a byte-identical list. Fixing the two names would have restored the build and left the defect. The list's stated purpose was to skip "the large tauri-cef fork the core image doesn't need", and that fork stopped being a submodule in `1843706c3 refactor(tauri): replace CEF runtime with upstream Wry`. Nothing remains for an enumeration to exclude: all ten entries are `path` dependencies of the root crate, and cargo reads every one of their manifests during resolution whether or not the feature is enabled — which is why an `optional = true` crate broke the build. A subset is therefore always wrong. It had already gone stale three times, each surfacing at release time rather than in PR CI. So the enumeration is removed rather than corrected again. Nothing heavy is pulled in: the two missing crates are 508K and 836K, and their only nested submodule is `tinybus`, which the old list already named.
|
@coderabbitai review |
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughBoth production and staging Docker release workflows now initialize all vendored Rust submodules recursively. The obsolete hard-coded path lists and ChangesRelease submodule initialization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The workflows now initialize every declared vendored submodule, addressing the missing manifests without changing shipped application behavior. The PR is mergeable with explicit maintainer follow-up because it targets main while the blocked release jobs run from release and must be promoted or cherry-picked. Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A submodule pointer bump currently skips every openhuman Rust lane while the aggregate gate still reports success. Reproduced on the tip of main: 92bab8d "bump tinycortex" - the only change is the vendor/tinycortex pointer CI Lite run 32358998415: success TinyCortex Memory Tests success PR CI Gate skipped Rust Quality (fmt, clippy) skipped Rust Core Coverage (cargo-llvm-cov) skipped Rust Feature-Gate Smoke (gates off) skipped Rust Tauri Coverage (cargo-llvm-cov) So the current tip of main has never been compiled by CI, and main is what gets promoted to release. Cause: the `rust-core` and `rust-core-full` filters enumerated exactly two vendored paths - `vendor/motosan-ai-oauth/**` and `vendor/tinychannels` - out of ten submodules, and `rust-tauri` named none at all. The `tinycortex` filter that did match feeds only TinyCortex's own test lane, which is why that ran while nothing compiled openhuman against the new pin. Every vendored crate is force-resolved by path through `[patch.crates-io]`, in BOTH cargo worlds, so any pointer bump changes what the core and the shell compile against. This is the same defect the release Docker job had: a hand-maintained submodule list that went stale three times before fe5bcb2 (#5596) replaced it with `--init --recursive`. Enumerating the ten paths here would fail the same way on the eleventh, so this matches on `vendor/**` and `.gitmodules` instead. `rust-core-full` gets them too: a dependency-graph change invalidates per-module test scoping exactly as a Cargo.lock change does, so a bump should run the full suite rather than a scoped subset. Cost: a vendored bump now runs the full Rust lanes rather than nothing. That is the point - eight of the ten crates could previously move with zero verification. Refs #5595
Summary
.gitmodulesdeclares 10, sovendor/tinyhostsandvendor/tinywalletwere never checked out and cargo could not resolve two path dependencies.release. It targetsmainper the fleet default; a maintainer will need to promote or cherry-pick it, sincereleaseis where the blocked pipeline actually runs.Problem
The Dockerfile does
COPY vendor/ vendor/, so an uninitialised submodule arrives in the image as an empty directory. Cargo then fails on the missing manifest before compiling anything:optional = truedoes not exempt these: cargo reads everypathdependency's manifest to build the lockfile whether or not the feature is enabled. The failing build proves it —tinyhostsis declaredoptional = trueand still broke resolution.Fixing only
tinyhostswould not have worked. Cargo stops at the first unresolvable dependency, andtinywallet(Cargo.toml:524) sits behindtinyhosts(:493). Both were missing; both are added.Solution
I removed the list rather than adding two names to it, and the deciding evidence is that nothing is left for it to exclude:
tauri-cefstopped being a submodule in1843706c3 refactor(tauri): replace CEF runtime with upstream Wry. It is not in.gitmodulestoday, so a recursive init cannot pull it — the ~28 remainingtauri-cefreferences in the tree are docs, scripts and TS, plus exactly oneCargo.tomlline that is a comment, not a dependency..gitmodulesis apathdependency of the root crate. The only othervendor/path dep,motosan-ai-oauth, is a committed directory rather than a submodule (git ls-fileslists its sources), so it is unaffected either way. A correct list is therefore always all ten — the enumeration had no legitimate degrees of freedom left, only the ability to be wrong.tinybus, which the old list already named.tinymemory(fixed in54f8710f), thentinywalletandtinyhoststogether (this issue). Correcting it a third time would schedule a fourth.This also makes the proposed "add a CI guard that asserts the list matches
Cargo.toml" follow-up unnecessary for this class: there is no list left to drift.Both files get the identical change, so staging and production cannot diverge again.
Verification — the pipeline is the only real test
Stated plainly: a
.github/workflowschange cannot be proven locally. It only executes on aworkflow_dispatchof the release pipelines, which I am not authorised to run, and this repo's rule is to verify Rust through CI rather than building locally.What I could check, I did:
git submodule update --init --recursivein thebuild-dockerjob of each (11 jobs parsed in production, 5 in staging)..gitmodulesdeclares 10 paths; the old list named 8; the two missing names are exactlyvendor/tinyhostsandvendor/tinywallet.git submodule update --init vendor/tinyhosts vendor/tinywalletsucceeds and both contain aCargo.toml— so the checkout the workflow will now perform does produce the manifests cargo was looking for.tauri-cefabsent from.gitmodules, and the commit that removed it identified.The honest confirmation is the next
Release Stagingdispatch reaching the Docker build step. Nothing in this PR is exercised by PR CI, so a green run here is not evidence the fix works.Submission Checklist
.github/workflowschange with no application code; there is no unit-testable surface, and the only execution path is a release dispatch. — Tests added or updated (happy path + at least one failure / edge case)diff-coverhas nothing to measure on this diff. — Diff coverage ≥ 80%## Related.gitmodulesand already fetched by every other job that usessubmodules: recursive; this adds no new remote.Closes #NNNin the## RelatedsectionImpact
Platform: Linux/Docker (
openhuman-coreimage) on the production and staging release pipelines. No runtime, application or user-visible change — nothing ships differently, the builder simply checks out the sources it already required.Performance: two additional submodules and their nested
tinybus(already fetched). 508K + 836K, negligible against the image build.Security / compatibility / migration: none. No new remotes, no dependency version changes, no lockfile change.
Risk if wrong: contained to the release pipeline, which is already failing — this cannot regress a passing state.
Related
timeout-minutes: 90on macOS and Linux while Windows passes with 9 seconds of margin, so a release still cannot go green on this fix alone. Filed separately with the measurement that the researcher's write-up did not have: desktop build time roughly doubled between 08-13 and 08-19 (Windows, the uncensored control leg: 51m58s → 89m01s → 89m51s), so the ceiling is the symptom rather than the cause.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/5594-release-submodule-listfe5bcb224Validation Run
app/are touched. —pnpm --filter openhuman-app format:checkpnpm typecheckbuild-dockerjob. — Focused testssrc-taurifiles changed. — Tauri fmt/check (if changed)Validation Blocked
command:the release pipelines (Release Staging/Release Production,workflow_dispatch)error:not run — dispatching a release is a maintainer action, and PR CI does not execute these workflowsimpact:the fix is unverified end-to-end until a release dispatch reaches the Docker build step. The failure mode if it is still wrong is identical and immediate (cargo cannot read avendor/*/Cargo.toml), so it fails loudly rather than shipping a bad image.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit