Repository navigation
feat(ci): two-branch CI — changed-files-only PR lane, full-suite release lane, release-branch cuts, cancellation watchdog - #4486
Conversation
…lane vitest-changed-coverage.sh runs 'vitest related' with coverage against only the changed app/src files; rust-coverage-changed.sh scopes cargo-llvm-cov to domain-level libtest filters + changed integration-test targets. Both fall back to the full suite whenever scoping is not clearly safe. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
PRs targeting main now run only the unit tests related to the changed files: vitest related for app/src changes and domain-scoped cargo-llvm-cov for Rust, both falling back to the full suite on config-level changes. The Rust mock- backend E2E and Playwright lanes move to the new release CI (main→release PRs). diff-cover still gates >= 80% on changed lines. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
…release PR release-ci.yml runs the complete matrix on PRs targeting the release branch: full unit suites (test-reusable), Rust mock-backend E2E, Playwright web lane, and the full desktop E2E shards on Linux/macOS/Windows, gated by a single 'Release CI Gate' check. prepare-release-pr.yml keeps a standing main→release PR open on every push to main. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
…sion - release-staging/production run from the release branch: enforce/checkout/ bump/commit/tag/push all target release instead of main - version is supplied explicitly via a required workflow input (X.Y.Z, must be strictly greater than current) — bump-version.js accepts an exact version alongside patch/minor/major - after the cut, the version-bump commit is merged back into main (continue-on-error + warning on conflicts so a tagged release is never stranded) Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
Evidence from run 28692500745: after cancel-in-progress fired, Rust E2E and Playwright ran to 'success' 4-23 min later and Rust Core Coverage kept building for 28 min — the runner's SIGINT/SIGTERM is swallowed by the host-side docker exec client and never reaches processes inside container jobs, so ci-cancel-aware.sh's traps never fire. Add a cancellation watchdog to ci-cancel-aware.sh that polls the Actions API (GET /actions/runs/<id>) and TERMs the build tree from inside the container when the run is cancelled. Grant actions:read + workflow-level GH_TOKEN in every wrapper-using workflow, and route the Vitest lane through the wrapper. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR introduces a two-branch CI model (main fast-lane, release full-lane) with new prepare-release-pr and release-ci workflows, back-merge-to-main steps in release-production/release-staging, a cancellation watchdog in scripts/ci-cancel-aware.sh wired via GH_TOKEN/actions:read across many workflows, and changed-files-scoped coverage scripts for frontend Vitest and Rust llvm-cov, plus explicit X.Y.Z version input support. ChangesCancellation watchdog rollout
PR CI scoped coverage lanes
Release branch model with release-ci and back-merge
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![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: 6856eed4d8
ℹ️ 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".
|
|
||
| - name: Open or refresh the main→release PR | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Because this step authenticates gh pr create with the default GITHUB_TOKEN, GitHub puts pull_request runs created or updated by that token into an approval-required state instead of starting them unattended (see GitHub's triggering-a-workflow docs: https://docs.github.com/actions/using-workflows/triggering-a-workflow#triggering-a-workflow-from-a-workflow). In the normal path where a push to main first opens the standing main→release PR, the new release-ci.yml gate can remain pending until someone manually approves workflows, so the automated full-lane release PR does not green itself. Use the existing GitHub App token/PAT for PR creation if this is meant to run automatically.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c0417f1 — prepare-release-pr.yml now mints a token via actions/create-github-app-token@v2 (same XGITHUB_APP_ID/XGITHUB_APP_PRIVATE_KEY App the release workflows push with) and uses it for gh pr create, so the standing main→release PR triggers Release CI unattended.
| RELEASE_SHA="$(git rev-parse HEAD)" | ||
| git fetch origin main | ||
| git checkout -B main origin/main | ||
| if git merge --no-ff "$RELEASE_SHA" -m "chore(release): merge release v${VERSION} back into main"; then |
There was a problem hiding this comment.
When release has just advanced from main with only the version-bump commit, origin/main is already an ancestor of RELEASE_SHA, but --no-ff still creates a new merge commit on main. The new prepare-release-pr workflow then compares release...main, sees that merge commit as main-only, and opens another main→release PR that runs the full release lane even though the trees are already synced; the staging workflow has the same pattern. Allow a fast-forward when possible (or otherwise suppress pure back-merge commits) so every cut does not create a no-op release PR.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c0417f1 — the production back-merge now tries git merge --ff-only first: when main hasn't moved since the cut, main fast-forwards to exactly the release SHA, so prepare-release-pr sees ahead_by=0 and opens no PR. The --no-ff path only remains for the case where main has genuinely diverged (in which case a new release PR is warranted anyway). The staging workflow no longer back-merges at all (removed in e4af430 per maintainer direction).
coderabbitai
Bot
added
the
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/pr-ci.yml (1)
85-138: 📐 Maintainability & Code Quality | 🔵 TrivialMinor asymmetry:
scripts/ci-cancel-aware.shinvalidates rust-core-full but not frontend-full.
rust-core-full(line 130) treats a change toscripts/ci-cancel-aware.shas invalidating scoping, butfrontend-full(lines 86-99) does not, even thoughscripts/ci/vitest-changed-coverage.shalso wraps its Vitest invocation throughscripts/ci-cancel-aware.sh. Not a correctness bug (the watchdog script doesn't affect test selection), but worth aligning for consistency if the intent is "any change to shared CI infra falls back to full".♻️ Optional consistency fix
frontend-full: - '.github/workflows/pr-ci.yml' - 'scripts/ci/vitest-changed-coverage.sh' + - 'scripts/ci-cancel-aware.sh' - 'package.json'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pr-ci.yml around lines 85 - 138, frontend-full is missing the shared CI infra dependency that rust-core-full already includes, so a change to scripts/ci-cancel-aware.sh does not currently force the full frontend Vitest run. Update the frontend-full path list in pr-ci.yml to include scripts/ci-cancel-aware.sh, keeping it aligned with rust-core-full and the scripts/ci/vitest-changed-coverage.sh wrapper so shared CI changes consistently invalidate scoped test runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-packages.yml:
- Around line 16-27: The release workflow is exposing a highly privileged
GH_TOKEN and broad write permissions to every job/step by defining them at the
workflow level. Move the permissions and GH_TOKEN setup down to the specific
job(s), or even only the step(s) that call scripts/ci-cancel-aware.sh, and keep
the workflow default permissions minimal while granting write scopes only where
needed. Use the workflow’s permissions block and the env usage around the
watchdog invocation to localize access.
In `@scripts/ci/rust-coverage-changed.sh`:
- Around line 49-52: The current CHANGED_FILES parsing in the rust coverage
script uses eval to rebuild the files array, which treats PR-derived filenames
as shell code. Update the parsing logic in the changed-files handling block to
use a structured, non-evaluating parser or safe delimiter-based read/loop
approach so filenames are treated as data only. Keep the logging/counting
behavior in the same script, but remove any shell-code re-parsing around the
files array initialization.
---
Nitpick comments:
In @.github/workflows/pr-ci.yml:
- Around line 85-138: frontend-full is missing the shared CI infra dependency
that rust-core-full already includes, so a change to scripts/ci-cancel-aware.sh
does not currently force the full frontend Vitest run. Update the frontend-full
path list in pr-ci.yml to include scripts/ci-cancel-aware.sh, keeping it aligned
with rust-core-full and the scripts/ci/vitest-changed-coverage.sh wrapper so
shared CI changes consistently invalidate scoped test runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df17a310-4e26-4b6b-a3d3-c7eec1f4a188
📒 Files selected for processing (21)
.github/workflows/android-compile.yml.github/workflows/build-desktop.yml.github/workflows/e2e-agent-review.yml.github/workflows/e2e-playwright.yml.github/workflows/e2e-reusable.yml.github/workflows/e2e.yml.github/workflows/ios-appstore.yml.github/workflows/ios-compile.yml.github/workflows/pr-ci.yml.github/workflows/prepare-release-pr.yml.github/workflows/release-ci.yml.github/workflows/release-packages.yml.github/workflows/release-production.yml.github/workflows/release-staging.yml.github/workflows/test-reusable.ymlAGENTS.mdscripts/ci-cancel-aware.shscripts/ci/rust-coverage-changed.shscripts/ci/vitest-changed-coverage.shscripts/release/bump-version.jsscripts/test-release-act.sh
Production takes patch/minor/major again (no manual version string); staging reverts to its automatic patch bump and no longer merges release back into main — only the production cut does. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4af430640
ℹ️ 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".
| touch app/.env | ||
|
|
||
| - name: Run Rust E2E suite | ||
| run: pnpm test:rust:e2e |
There was a problem hiding this comment.
This new Release CI job runs inside container: and starts the long Rust E2E suite directly, bypassing scripts/ci-cancel-aware.sh even though AGENTS.md now requires long build/test commands to use that wrapper because docker exec does not forward cancellation signals. When a main→release PR run is superseded or manually cancelled, this test process can keep consuming the runner until it times out or finishes naturally; wrap this command (and the analogous Playwright build/run commands in this workflow) with the cancel-aware script.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c0417f1 — all three long-running Release CI steps are now wrapped: pnpm test:rust:e2e (the inner script only wrapped its cargo segments; the wrapper now covers the mock server + node harness too), the Playwright artifact build, and the Playwright web session.
…al, scoped GH_TOKEN, wrapped release-ci steps - prepare-release-pr: create the main→release PR with the GitHub App token; PRs created by GITHUB_TOKEN never trigger pull_request workflows, so the standing PR would sit without a Release CI run (codex) - release-production back-merge: fast-forward main when it hasn't moved so the back-merge doesn't spawn a no-op main→release PR (codex) - changed-files scripts: parse the paths-filter list with xargs instead of eval — PR-controlled filenames are never re-parsed as shell code; hostile quoting degrades to the full-suite fallback (coderabbit) - write-permission workflows (release-packages/staging/production, build-desktop): GH_TOKEN env moved from workflow level to only the jobs that run ci-cancel-aware.sh (coderabbit/zizmor) - release-ci: wrap Rust E2E, Playwright build, and Playwright session with ci-cancel-aware.sh (codex) Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
…erit The prepare-release-pr minted token is restricted to pull-requests:write + contents:read — it can open the main→release PR but cannot push commits. release-ci no longer inherits secrets: neither reusable workflow it calls uses any, and it is pull_request-triggered, so its runs stay secret-free. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
…nment XGITHUB_APP_* only exist as environment secrets, so the job needs an environment to read them at all; Release-PR-Automation holds only those two secrets and is branch-policied to main, keeping the Production environment (signing keys etc.) out of reach of push-to-main automation. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
The check job runs on every merge to main with only the default read token and no environment; the common case (PR already open / nothing to merge) ends there. The open-pr job — the only one entering Release-PR-Automation and minting the App token — runs solely when a PR must actually be created, and re-checks for a racing PR under the concurrency group before creating. Claude-Session: https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff44d2c87f
ℹ️ 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".
| env: | ||
| # Consumed by scripts/ci-cancel-aware.sh's cancellation watchdog. | ||
| GH_TOKEN: ${{ github.token }} |
There was a problem hiding this comment.
This job already has an env mapping immediately above; adding a second env: at the same job level makes the reusable desktop workflow invalid in Actions or leaves only the second map in YAML parsers. When staging/production call build-desktop.yml, the desktop matrix either will not start or will silently lose the job-level GITHUB_TOKEN and Telegram bot variables, so put GH_TOKEN into the existing env block instead of creating a duplicate key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in follow-up PR #4488 (commit 91728b8) — the stray second env: is merged into the job's existing env block, and workflow validation now uses a strict duplicate-key YAML loader so this class of break can't pass locally again. (This bug did ship with #4486 and made every push produce a failed invalid-workflow run for build-desktop.yml.)
| # Changes here invalidate per-file test scoping → full Vitest run. | ||
| frontend-full: | ||
| - '.github/workflows/pr-ci.yml' | ||
| - 'scripts/ci/vitest-changed-coverage.sh' |
There was a problem hiding this comment.
This path only appears in frontend-full, but the frontend job and coverage gate are guarded by needs.changes.outputs.frontend; a PR that changes only scripts/ci/vitest-changed-coverage.sh sets frontend-full=true while frontend=false, so the changed runner is not exercised at all. Add this path to the frontend filter too, or include frontend-full in the job/gate conditions.
Useful? React with 👍 / 👎.
| # Changes here invalidate per-module test scoping → full suite. | ||
| rust-core-full: | ||
| - '.github/workflows/pr-ci.yml' | ||
| - 'scripts/ci/rust-coverage-changed.sh' |
There was a problem hiding this comment.
This path only sets rust-core-full, while rust-quality, rust-core-coverage, and the coverage gate are all keyed off rust-core; a PR that only changes scripts/ci/rust-coverage-changed.sh will therefore skip the Rust lane instead of running the full suite that this flag requests. Include this script in the rust-core filter as well, or make the job conditions consider rust-core-full.
Useful? React with 👍 / 👎.
…n/consume
The core moved the login-token consume call to `POST /auth/login-token/consume`
(token in the JSON body, response `{ success, data: { jwt } }`) back in tinyhumansai#3794;
the removed legacy route was `/telegram/login-tokens/:token/consume`. The E2E
mock server still only served the legacy path and returned `jwtToken`, and every
login/auth spec asserted on the legacy URL substring — so once the full desktop
E2E suite began running in the new Release CI lane (tinyhumansai#4486), `performFullLogin`
never saw a consume call, no JWT was returned, and every shard cascaded
("Auth consume call missing", "Welcome screen still not visible").
- mock (scripts/mock-api/routes/auth.mjs): accept the new path (legacy kept as a
harmless alias) and return `data.jwt` (the field the core parses), not jwtToken.
- specs: swap the consume matcher substring to `/auth/login-token/consume` across
auth-access-control, login-flow, mega-flow, voice-mode, local-model-runtime,
logout-relogin-onboarding, and the Playwright login-flow spec.
Pure test-fixture fix; no product code changes.
Claude-Session: https://claude.ai/code/session_01UAetDHQCyFtWnyP7RwGdJU
Summary
mainnow run unit tests only for the changed files —vitest relatedforapp/srcchanges, domain-scopedcargo llvm-cov(libtest filters derived fromsrc/<a>/<b>/…) for Rust — instead of the full ~13k-test instrumented suite. diff-cover still gates ≥ 80% on changed lines.release-ci.ymlruns the complete matrix (full unit suites, Rust mock-backend E2E, Playwright, desktop E2E on 3 OSes) on PRs targeting the long-livedreleasebranch, gated by a singleRelease CI Gatecheck.prepare-release-pr.ymlkeeps a standing main→release PR open on every push to main.release:release-staging.yml/release-production.ymlnow enforce/checkout/bump/tag/push onrelease, and merge the release branch back intomainafter a production cut (staging does not back-merge). Production takes arelease_typeincrement (patch/minor/major); staging auto patch-bumps.ci-cancel-aware.shgains an Actions-API watchdog that stops builds insidecontainer:jobs when the run is cancelled — the runner's SIGINT/SIGTERM is swallowed by the host-sidedocker execclient and never reaches the build.Problem
28692697989's sibling28692500745, jobs kept building 23–28 minutes after cancel-in-progress fired, concludingsuccess/failureinstead ofcancelled, holding runner slots hostage.mainwith auto-guessed version bumps.Solution
scripts/ci/vitest-changed-coverage.sh+scripts/ci/rust-coverage-changed.sh: changed-files-only test selection with conservative full-suite fallbacks (config/lockfile/lib.rs/nested-test-support changes, empty or oversized change sets). dorny/paths-filterlist-files: shellfeeds the changed-file lists.release-ci.ymlcomposes the existingtest-reusable.ymlande2e-reusable.yml(full, 3 OS) plus the Rust E2E and Playwright lanes moved out ofpr-ci.yml.release_type(patch/minor/major) input; after the tag, a post-cut step mergesreleaseback intomain(continue-on-error + warning so a tagged release is never stranded on conflicts). Staging keeps its automatic patch bump and does not back-merge.ci-cancel-aware.shpollsGET /repos/{repo}/actions/runs/{run_id}(needs the newactions: readpermission + workflow-levelGH_TOKEN) and TERM→KILLs the build tree from inside the container. Verified locally against a faked API: cancelled runs kill the build within one poll interval; healthy runs are untouched.Known tradeoff, called out explicitly: on the fast lane, changed Rust lines must be covered by tests in their own domain (or a changed integration-test target) — coverage contributed by unrelated suites no longer counts. The full suite still runs on every main→release PR.
Follow-up (repo settings, not in this diff): create the
releasebranch, allow it in the "Disallow creating branches" ruleset, and add a "Protect release" ruleset requiringRelease CI Gate.Submission Checklist
N/A: CI workflow + shell script change; scripts exercised via stubbed dry-runs (classification, fallback, watchdog kill paths) documented in the PRN/A: no app/Rust source changed; workflows and CI shell scripts are outside the coverage gateN/A: behaviour-only CI change## Related—N/A: no feature rows affectedN/A: release flow changed but smoke checklist steps themselves are unchanged; release workflows now run from the release branchCloses #NNN—N/A: no tracking issueImpact
releasebranch; version increments stay patch/minor/major.Related
releasebranch + rulesets (repo settings); consider sharding release-ci if main→release PRs become slow.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
https://claude.ai/code/session_01FsaameYvpZMKZfUeWqZi7x