ci(release): nightly build against the latest master of every submodule - #1448
Conversation
A true nightly: pull only, nothing committed or published. Catches integration breakage between the submodules' masters before anyone bumps the bundle, instead of at bump time. - new cron 0 3 * * * (the weekly dev-release cron is unchanged), plus a workflow_dispatch input submodules=pinned|latest for on-demand runs - nightly-submodules: an always-run resolver job that, on the nightly cron or submodules=latest, moves every submodule (recursively) to its upstream master and reports whether anything moved relative to the committed pointers; skips the build entirely when nothing did - the four build jobs need it and, on nightly, run only when something moved; each checks out exactly the resolver's SHAs so every leg tests the same tips even minutes apart - build-tauri relocks src-tauri/Cargo.lock to the moved aw-server-rust revision so Tauri really builds latest and check_tauri_server.py still holds (installs a Rust toolchain for that step; the action is idempotent with the job's later setup) - preflight/create-tag are excluded from the nightly cron and from submodules=latest, so a nightly can never mint a dev prerelease
|
@greptileai review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bdbc87549
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| git -C "$path" fetch -q origin "$sha" | ||
| git -C "$path" checkout -q "$sha" | ||
| done | ||
| git submodule status --recursive |
There was a problem hiding this comment.
For nightly or submodules=latest runs in the manylinux container, this invokes top-level Git before the following safe.directory workaround. The adjacent workflow comment documents that the mounted workspace is not trusted until that configuration runs; unlike the later version probe, git submodule status --recursive exits nonzero under dubious ownership, so this build leg aborts before testing anything. Configure the workspace immediately after checkout, ahead of this step.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 16cc23a. Nested init now runs git submodule --quiet foreach 'git submodule update --init --recursive -q' — inside each direct submodule, so the direct tips survive. In the manylinux job the step moved after "Configure git safe directory" and carries a step-scoped safe.directory=* (GIT_CONFIG_* env), since the global entry covers the workspace but not the submodule worktrees the step runs git in.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Move each submodule (and nested submodules) to the tip of its | ||
| # upstream default branch. awatcher is third-party but is included: | ||
| # a nightly is exactly where its drift should surface. | ||
| git submodule update --init --recursive --remote --depth 1 2>&1 | tail -20 |
There was a problem hiding this comment.
The nightly resolver advances the third-party awatcher submodule to an unreviewed upstream tip. The Linux Tauri jobs then execute that revision through the recursive build while exposing TAURI_SIGNING_PRIVATE_KEY and its password. A compromised upstream revision could therefore exfiltrate the signing credentials. Nightly jobs should withhold these secrets or avoid executing mutable third-party code.
How this was verified: The resolver includes awatcher, its resolved SHA is replayed into build-tauri, and the resulting submodule build executes in steps whose environment contains the Tauri signing key and password.
| # which this job only installs later — the toolchain action is idempotent. | ||
| - name: Set up Rust for the nightly relock | ||
| if: needs.nightly-submodules.outputs.latest == 'true' | ||
| uses: dtolnay/rust-toolchain@stable |
There was a problem hiding this comment.
The new dtolnay/rust-toolchain@stable reference is mutable, unlike the three existing uses of this action that are pinned to a commit SHA. A repointed action could modify the workspace or toolchain later consumed by secret-bearing Tauri build steps. Pin this invocation to the reviewed commit as well.
How this was verified: This is the only mutable reference to this third-party action, and it executes before Tauri build steps that expose signing credentials.
| printf '%s\n' "$SUBMODULE_SHAS" | while read -r sha path; do | ||
| [ -n "$path" ] || continue | ||
| git -C "$path" fetch -q --depth 1 origin "$sha" | ||
| git -C "$path" checkout -q "$sha" | ||
| done |
There was a problem hiding this comment.
Consumers replay the resolver's recursive output onto a checkout initialized from the pinned submodule topology, using only git -C "$path" fetch and checkout. If an updated parent introduces a nested submodule, the resolver exports its new path, but that path does not exist in the consumer checkout. git -C then fails and aborts the affected nightly job. Synchronize and initialize submodules after updating parent revisions before replaying nested revisions.
| - uses: actions/checkout@v7 | ||
| with: | ||
| submodules: 'recursive' | ||
| fetch-depth: 1 |
There was a problem hiding this comment.
The resolver recursively checks out every submodule before it evaluates the LATEST guard. On ordinary pushes and pull requests, it then exits without using that checkout, while every build job waits for it through needs. This adds a complete redundant submodule checkout to every normal build. Initialize submodules only for latest-mode runs so the always-run resolver remains cheap.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🤖 AI code reviewAdds a nightly integration-build path to the release workflow: a second cron and a workflow_dispatch input select a 'latest' mode in which a new nightly-submodules job resolves every first-party submodule to its upstream master tip and the four build jobs check out those exact SHAs. The preflight/create-tag jobs are excluded from latest mode so no dev prerelease is minted, and build-tauri relocks its Cargo.lock to the moved aw-server-rust revision. Not safe to merge — 1 P1 openConfidence 3/5 ℹ️ Consensus was degraded on this run: 2 of 3 passes answered, so findings were filtered at 2-of-2 agreement rather than 2-of-3 — less filtered than usual; 2 of 3 fan-out jobs answered, so the findings above were filtered against less evidence than the threshold assumes.
1 finding · ❌ 1 P1 · 🔒 1 security❌ P1 high · 🔒 security — The resolver's How this was verified: Traced the resolver: before/after are built from Consensus: 2/2 passes agreed Files changed (1) — the diff as I read it
Previous review passes
Reviewed Maintainer commands
|
| # Move each submodule (and nested submodules) to the tip of its | ||
| # upstream default branch. awatcher is third-party but is included: | ||
| # a nightly is exactly where its drift should surface. | ||
| git submodule update --init --recursive --remote --depth 1 2>&1 | tail -20 |
There was a problem hiding this comment.
nightly-submodules resolver runs git submodule update --init --recursive --remote --depth 1 and then captures after=$(git submodule foreach --recursive --quiet "$tips"). The tips command is echo "$(git rev-parse HEAD) $displaypath". For a submodule whose upstream tip has not changed, git submodule update --remote leaves the working tree at the committed SHA, so before and after are equal and moved=false. However, the resolver's before is captured from the committed pointers, and the build jobs' Use latest submodule tips step checks out the resolved SHAs only when latest == 'true'. On a normal push/PR/dispatch with submodules=pinned, latest=false and the build jobs use the committed pointers — correct. The bug is that the resolver's moved comparison is done against the committed pointers, but the build jobs' if condition also requires moved == 'true' for the nightly cron. If a submodule's upstream tip moved but the resolver's git submodule update --remote fails to update it (e.g., because the submodule's remote is not configured or the default branch is not master), after equals before, moved=false, and the nightly build is skipped even though the submodule did move. This is a silent skip: the nightly integration build does not run, and the drift is not surfaced. The PR description says the resolver was rehearsed, but the failure mode of a submodule whose upstream default branch is not master (or whose remote is not origin) is not handled; git submodule update --remote uses the branch config or defaults to the remote's HEAD, and if that is not the intended upstream, the comparison is against the wrong tip.
…nit, exclude third-party awatcher - pin dtolnay/rust-toolchain to the commit the file already uses elsewhere - resolver job checks out without submodules; initialises them only in latest mode, so ordinary pushes/PRs no longer pay a recursive clone - consumers run two passes (direct, then `submodule update --init`, then nested) so a parent revision that introduces a new nested submodule cannot make `git -C <new path>` fail - the nightly moves only first-party submodules; awatcher (third-party) stays at its pinned revision so unreviewed upstream code never runs in jobs that carry Tauri signing secrets
|
Review findings addressed in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97742d7a65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| done | ||
| } | ||
| pass direct | ||
| git submodule update --init --recursive |
There was a problem hiding this comment.
Whenever a direct submodule has moved, this command undoes pass direct: the default checkout procedure for git submodule update checks out “the commit recorded in the superproject,” which is the pinned revision. pass nested then skips every path without /, and the final status command only reports the resulting mismatch, so all latest-mode jobs build pinned top-level modules (and the Tauri relock reads the old aw-server-rust SHA). Initialize nested modules from within the moved parents, or otherwise avoid updating the direct paths back to the index revisions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 16cc23a. Nested init now runs git submodule --quiet foreach 'git submodule update --init --recursive -q' — inside each direct submodule, so the direct tips survive. In the manylinux job the step moved after "Configure git safe directory" and carries a step-scoped safe.directory=* (GIT_CONFIG_* env), since the global entry covers the workspace but not the submodule worktrees the step runs git in.
…bmodule worktrees in the manylinux container Review findings (Codex): - `git submodule update --init --recursive` at the top level checks out the superproject-recorded revisions, undoing the direct-tip pass; every latest-mode job silently built pinned code. Initialise nested submodules from inside each direct submodule instead. - In the manylinux container the step ran before the safe.directory workaround and `git -C <submodule>` runs inside worktrees the global entry does not cover. Move it after, with a step-scoped safe.directory=* via GIT_CONFIG_*.
…ase gate Review finding (Bob's reviewer): preflight judges master by the names of its check-runs, and a nightly run's build jobs carried the same names as the push build against the same SHA — a red nightly on unreviewed submodule tips would have cancelled the weekly dev prerelease although the pinned tree was healthy. Latest-mode jobs are now named "... [latest submodules]" and the gate excludes that marker and the resolver job. Also refuse to run the resolver's `submodule update` with an empty first-party list, which would have moved every submodule, awatcher included.
|
@TimeToBuildBob P1 confirmed and fixed in Also merged master into the branch: the two red research legs were the pre-#1449 patcher failure, not this change. |
|
@TimeToBuildBob review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Round 2 review — HEAD Previously open findings — all confirmed fixed:
New findings: none at P1/P2. P3 advisory (non-blocking):
Verdict: LGTM. Ready to merge. |
|
On the automated reviewer's P1: the hardening point is fair as a follow-up, the verification is not. With Bob's manual round-2 LGTM and CI on |
…1457) * fix(ci): select nightly first-party submodules by URL, not by name Filter by whether the URL is under github.com/ActivityWatch/ (case- insensitive) instead of hard-coding a name exclusion list. This ensures newly added third-party submodules are excluded automatically, and prints the excluded set in every job run so the boundary stays visible. Closes #1456 Follow-up from #1448 Git-Session-Id: dc09 * fix(ci): anchor first-party URL match and derive submodule name from .gitmodules Two review findings on the URL-based classifier: - The `*github.com/activitywatch/*` glob matched the string anywhere in the URL, so a mirror like `https://mirror.example/github.com/ActivityWatch/x` would classify as first-party and get built with the signing key. Anchor the match against known URL prefixes instead. - `$sm` was populated from `git submodule foreach`'s `$sm_path`, but `.gitmodules` is keyed by submodule *name*, which git permits to differ from path. Read name/path pairs directly from `.gitmodules` instead of reusing the path as the name for the url lookup. Git-Session-Id: 1083368a-3a3e-5b8c-ac1e-c999007d3dc1
…gger, and name the build legs again (#1458) * ci(release): skip the nightly submodule resolver outside latest mode `nightly-submodules` ran on every trigger, including PRs and release tags, purely so that its dependents' `needs:` would not short-circuit. On a pinned build it checks out the repo, hits its own guard, writes latest=false and exits, so every ordinary run queued a runner and made four build jobs wait on a job that had nothing to do. Gate the job on the same condition its guard step already uses, and let the dependents survive a skipped dependency with `!cancelled()`. Their conditions and `name:` expressions already treat empty outputs as pinned mode, so a skipped resolver reads exactly like the latest=false it used to write. Nightly and `submodules=latest` behaviour is unchanged. A failed resolver in latest mode still cannot pull the build jobs in: their remaining condition requires latest == 'true' && moved == 'true', which empty outputs never satisfy. * ci(release): put the matrix leg back in the build job names Since #1448 gave build-qt and build-tauri an explicit `name:` for the `[latest submodules]` suffix, GitHub stopped appending the matrix values it adds only to unnamed jobs. Every leg rendered identically: four "Build Qt artifacts" and six "Build Tauri artifacts", so a red run did not say which platform failed without opening jobs one by one. Name the OS explicitly, plus the research flag, reusing the idiom the artifact upload steps already use. os and research fully disambiguate: python_version, node_version and the skip flags are single-valued in both matrices. * ci(release): keep a failed resolver blocking, and label the effective edition Greptile P1: with only !cancelled(), a workflow_dispatch run with submodules=latest whose resolver FAILED would still satisfy the workflow_dispatch disjunct, so the full matrix would build the pinned tree and report success for a run whose whole point was to validate the latest tips. The PR description claimed otherwise; it was wrong, because that disjunct short-circuits before the latest/moved check. Gate the dependents on the resolver's result being success or skipped, which restores the old failure semantics while keeping the new skip behaviour. Greptile P2: research tag pushes and edition=research dispatches enable Research Edition through AW_RESEARCH_EDITION while matrix.research stays false, so those legs were labelled as ordinary builds. Label the effective edition, mirroring each job's own AW_RESEARCH_EDITION expression. Applied to the manylinux job too, which Greptile did not flag but which builds the research edition on the same triggers.
2 participants
A true nightly, as discussed: pull only, nothing committed, nothing published. Every submodule moved to its upstream
master, then build + test — so integration breakage between the repos' masters surfaces the next morning instead of when someone bumps the bundle. Skips itself when nothing moved.Shape (a flag on the existing build jobs, not a parallel workflow)
0 3 * * *(identified viagithub.event.schedule; the weekly dev-release cron is untouched), plus aworkflow_dispatchinputsubmodules: pinned|latestfor on-demand runs.nightly-submodules— a cheap, always-run resolver job (soneeds:never short-circuits the build jobs). On the nightly cron orsubmodules=latestit runsgit submodule update --init --recursive --remoteand outputsmovedplus the fullsha pathlist (recursively, incl. nestedaw-webuiandmedia). On normal runs it outputslatest=falseand does nothing else.research-edition-checks,build-qt,build-qt-manylinux-2-28,build-tauri)needit and, on nightly, run only whenmoved == true. Right after their recursive checkout they check out exactly the resolver's SHAs, so every leg builds the same tips even when they start minutes apart.build-tauriadditionally relockssrc-tauri/Cargo.lockto the moved aw-server-rust revision (cargo update -p aw-server --precise <sha>), so Tauri genuinely builds latest and the existingcheck_tauri_server.pyverification still passes. That step installs a Rust toolchain because the job only sets one up later; the action is idempotent.preflight/create-tagare excluded from the nightly cron and fromsubmodules=latest, so neither path can mint a dev prerelease.release-notes/releaseremain tag-push only.Verified
actionlintclean for the changed regions (the 7 remaining findings are all in pre-existing scripts: preflight, manylinux, release-notes, release).$displaypathunderforeach --recursive,--remote --depth 1on initialised submodules, the before/after diff producingmoved=true, and the consume step fetching + checking out every SHA. It also surfaced a driftedaw-qt/media— exactly the class of thing this is for.Companion:
scripts/bump-submodules.sh(separate PR) is the human-driven counterpart that does commit.