fix(check-inbox): deliver with exit 0 when the poll failed part-way (… #1162
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
| name: tests | |
| # Run the full Bats suite on every PR and on pushes to main. This repo is | |
| # public, so GitHub-hosted runners are free — running everything every time is | |
| # simpler and safer than selectively running "only the changed tests", which | |
| # would miss regressions in shared libs (lib/*.sh) that fan out across many | |
| # scripts. What we do instead is run the whole suite in parallel shards. | |
| # | |
| # Sharding: the suite has grown to ~790 tests, 11 min on ubuntu and 12-17 on | |
| # macOS, which is the dominant wait on every PR. `bats-shard` splits it across | |
| # SHARD_TOTAL runners per OS. The split is computed from the tree by | |
| # .github/scripts/shard-tests.sh, NOT from a list in this file: a static list | |
| # would put a newly added test file in no shard at all, and CI would go on | |
| # reporting green while silently not running it. tests/test_ci_sharding.bats | |
| # pins that the shards stay a partition of tests/*.bats. | |
| # | |
| # Docs-only skip (without the paths-ignore trap): | |
| # the bats check is REQUIRED on main. A naive `paths-ignore` would mean it | |
| # never reports on a docs-only PR, leaving it stuck "pending" forever and | |
| # unmergeable. So instead the shard jobs ALWAYS run and ALWAYS complete green — | |
| # a `changes` job decides whether the diff is docs-only, and only the heavy | |
| # steps (install + run) are skipped when it is. The required context is | |
| # therefore always reported; on docs-only PRs it just reports green in seconds | |
| # without running the suite. | |
| # | |
| # The required context is the `bats` SUMMARY job, not the shards. Shard names | |
| # carry their index, so changing SHARD_TOTAL would rename every required check | |
| # and silently stop enforcing the ones branch protection still names. The | |
| # summary job's name is stable across any future re-shard, and it fails unless | |
| # the whole matrix succeeded — including when the matrix was skipped, which | |
| # must never read as green. | |
| # | |
| # What counts as "docs-only" (= safe to skip the bash suite): the docs tree | |
| # (`docs/**`), the marketing site (`site/**` — a standalone Astro app with no | |
| # bearing on the bash suite), the desktop app (`app/**` — Rust/TS the bats | |
| # suite never reads; it gets its own `app typecheck` job below instead), | |
| # `llms.txt` / `llms-full.txt`, and the top-level prose docs (README, | |
| # CHANGELOG, etc). SKILL.md is deliberately EXCLUDED — the suite asserts on it | |
| # (tests/test_install.bats checks it has no unsubstituted placeholder), so a | |
| # SKILL.md change must run the suite. Anything not on the allowlist runs it too. | |
| on: | |
| push: | |
| branches: [main] | |
| pull_request: | |
| branches: [main] | |
| permissions: | |
| contents: read | |
| env: | |
| # Number of parallel shards per OS. Kept in one place because the matrix, the | |
| # job name, and the argument to shard-tests.sh must agree — if they drift, a | |
| # shard silently runs the wrong slice of the suite. Raising it is safe (the | |
| # split is recomputed, not hardcoded); it only changes the shard job names, | |
| # which is exactly why the required check is the summary job below. | |
| SHARD_TOTAL: 4 | |
| jobs: | |
| # Cheap gate: is this PR's diff entirely documentation? Pushes to main always | |
| # run the full suite (docs_only=false) — main must stay fully verified. | |
| changes: | |
| name: detect docs-only | |
| runs-on: ubuntu-latest | |
| outputs: | |
| docs_only: ${{ steps.detect.outputs.docs_only }} | |
| app_changed: ${{ steps.detect.outputs.app_changed }} | |
| steps: | |
| - uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 0 # need base + head history to diff the PR range | |
| - id: detect | |
| env: | |
| EVENT: ${{ github.event_name }} | |
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | |
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | |
| run: | | |
| set -euo pipefail | |
| if [ "$EVENT" != "pull_request" ]; then | |
| echo "Event is '$EVENT' (not pull_request) — running the full suite." | |
| echo "docs_only=false" >> "$GITHUB_OUTPUT" | |
| echo "app_changed=true" >> "$GITHUB_OUTPUT" | |
| exit 0 | |
| fi | |
| changed=$(git diff --name-only "$BASE_SHA...$HEAD_SHA") | |
| echo "Changed files:" | |
| echo "$changed" | |
| docs_only=true | |
| app_changed=false | |
| while IFS= read -r f; do | |
| [ -z "$f" ] && continue | |
| case "$f" in | |
| docs/*) ;; | |
| site/*) ;; | |
| app/*) app_changed=true ;; | |
| llms.txt|llms-full.txt) ;; | |
| README.md|CHANGELOG.md|CONTRIBUTING.md|PRIVACY.md|ARCHITECTURE.md|RELEASING.md|LICENSE) ;; | |
| # NOTE: SKILL.md is intentionally not here — the suite tests it. | |
| *) docs_only=false ;; | |
| esac | |
| done <<< "$changed" | |
| echo "Result: docs_only=$docs_only app_changed=$app_changed" | |
| echo "docs_only=$docs_only" >> "$GITHUB_OUTPUT" | |
| echo "app_changed=$app_changed" >> "$GITHUB_OUTPUT" | |
| bats-shard: | |
| name: bats (${{ matrix.os }} ${{ matrix.shard }}/4) | |
| needs: changes | |
| # Run even if `changes` somehow failed/was skipped — fail open to the full | |
| # suite rather than leaving the required check unreported (which would | |
| # block the PR). When changes failed, docs_only is empty → heavy steps run. | |
| # | |
| # This is also why the matrix is a literal list rather than something the | |
| # `changes` job computes: a dynamic matrix cannot be evaluated when the job | |
| # that feeds it failed, so a broken `changes` would take the whole suite | |
| # down instead of falling back to running everything. | |
| if: ${{ !cancelled() }} | |
| runs-on: ${{ matrix.os }} | |
| # Bound a hung runner/test so a stall fails fast (and is re-runnable) instead | |
| # of pinning this REQUIRED check for the GitHub default of 6h. | |
| # | |
| # Keep this comfortably above the real suite time, not just above it. The | |
| # cap was 15 when the suite ran in ~5 min; the suite has since grown to 728 | |
| # tests and macOS measures 11m57s-14m10s, leaving 1-3 min of headroom. The | |
| # result was that ~47% of macOS runs (9 of 19 sampled) were killed mid-suite | |
| # at ~15m18s, at whatever test happened to be running — a REQUIRED check | |
| # behaving like a coin flip on PRs that changed nothing relevant. 25 matches | |
| # the app-test-windows job below. | |
| # | |
| # If a shard starts firing this cap, re-shard (raise SHARD_TOTAL) rather | |
| # than re-running until it passes: a job that always dies at the same | |
| # elapsed time is a timeout, not a flaky test. | |
| # bats-windows sets its own timeout; this covers ubuntu/macos. | |
| # | |
| # Left at the pre-sharding value on purpose. A shard is a quarter of the | |
| # suite, so this is now generous headroom rather than a tight bound — and | |
| # a cap that only ever fires on a genuine hang is the one that carries no | |
| # false positives. (It was 15 against a 12-14 min macOS suite, and killed | |
| # ~47% of macOS runs mid-suite at whatever test was running.) | |
| timeout-minutes: 25 | |
| strategy: | |
| # Cover both GNU (Linux) and BSD (macOS) userlands — the scripts shell out | |
| # to sed/stat/mktemp/ps etc. whose flags differ between them. | |
| fail-fast: false | |
| matrix: | |
| os: [ubuntu-latest, macos-latest] | |
| # Must list exactly 1..SHARD_TOTAL. The `bats` summary job verifies | |
| # end-to-end that the shards actually covered every test file, so a | |
| # drift here fails the required check instead of silently dropping a | |
| # slice of the suite. | |
| shard: [1, 2, 3, 4] | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Docs-only change — skipping suite | |
| if: needs.changes.outputs.docs_only == 'true' | |
| run: echo "Diff is documentation-only; skipping the bats suite and reporting green to satisfy the required check." | |
| - name: Compute this shard's test files | |
| if: needs.changes.outputs.docs_only != 'true' | |
| env: | |
| SHARD: ${{ matrix.shard }} | |
| run: | | |
| set -euo pipefail | |
| if [ "$SHARD" -gt "$SHARD_TOTAL" ]; then | |
| echo "::error::matrix shard $SHARD exceeds SHARD_TOTAL=$SHARD_TOTAL — the matrix and SHARD_TOTAL have drifted apart" | |
| exit 1 | |
| fi | |
| .github/scripts/shard-tests.sh "$SHARD" "$SHARD_TOTAL" > shard-files.txt | |
| if [ ! -s shard-files.txt ]; then | |
| echo "::error::shard $SHARD of $SHARD_TOTAL was assigned no test files" | |
| exit 1 | |
| fi | |
| echo "Shard $SHARD/$SHARD_TOTAL runs $(wc -l < shard-files.txt) file(s):" | |
| cat shard-files.txt | |
| - name: Ensure sqlite3 is available (Linux) | |
| if: runner.os == 'Linux' && needs.changes.outputs.docs_only != 'true' | |
| run: sqlite3 --version >/dev/null 2>&1 || { sudo apt-get update && sudo apt-get install -y sqlite3; } | |
| # Install a modern bats-core (the suite uses BATS_TEST_TMPDIR) into a | |
| # HOME-local npm prefix. This avoids both the ubuntu runner's EACCES on | |
| # `npm i -g` under /usr/local and the macOS SIP block on /usr/lib that the | |
| # bats-core action's default install paths hit. | |
| - name: Install bats | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: | | |
| npm config set prefix "$HOME/.npm-global" | |
| npm install -g bats | |
| echo "$HOME/.npm-global/bin" >> "$GITHUB_PATH" | |
| - name: Show tool versions | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: | | |
| bash --version | head -1 | |
| sqlite3 --version | |
| bats --version | |
| - name: Run bats suite (this shard) | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: | | |
| set -euo pipefail | |
| # Paths come from shard-tests.sh over tests/*.bats and contain no | |
| # spaces; xargs keeps them as separate arguments to one bats run. | |
| xargs bats --print-output-on-failure < shard-files.txt | |
| # What the summary job checks the partition against. Uploaded only when | |
| # the suite actually ran, so a docs-only PR produces none — and the | |
| # summary skips the check for exactly that case. | |
| - name: Record which files this shard ran | |
| if: needs.changes.outputs.docs_only != 'true' | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: bats-manifest-${{ matrix.os }}-${{ matrix.shard }} | |
| path: shard-files.txt | |
| retention-days: 1 | |
| # The REQUIRED status check. Shard job names carry their index, so making the | |
| # shards required would mean branch protection had to be re-pointed every | |
| # time SHARD_TOTAL changed — and until someone did, protection would be | |
| # enforcing check names that no longer run. This job's name never changes. | |
| # | |
| # It fails unless the whole matrix succeeded. `skipped` is treated as failure | |
| # too: a suite that did not run is not a suite that passed, and the whole | |
| # point of the always-report pattern above is that green means something. | |
| bats: | |
| name: bats | |
| needs: [changes, bats-shard] | |
| if: ${{ !cancelled() }} | |
| runs-on: ubuntu-latest | |
| timeout-minutes: 10 | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Check shard results | |
| env: | |
| RESULT: ${{ needs.bats-shard.result }} | |
| run: | | |
| set -euo pipefail | |
| echo "bats shard matrix result: $RESULT" | |
| case "$RESULT" in | |
| success) ;; | |
| skipped) echo "::error::the bats shards did not run — refusing to report green"; exit 1 ;; | |
| *) echo "::error::the bats shards did not all pass (result: $RESULT)"; exit 1 ;; | |
| esac | |
| - name: Docs-only change — no shard manifests to verify | |
| if: needs.changes.outputs.docs_only == 'true' | |
| run: echo "Diff is documentation-only; the shards reported green without running, so there is no partition to verify." | |
| - name: Download shard manifests | |
| if: needs.changes.outputs.docs_only != 'true' | |
| uses: actions/download-artifact@v4 | |
| with: | |
| pattern: bats-manifest-* | |
| path: manifests | |
| # The end-to-end guarantee: every tests/*.bats file was actually run, on | |
| # every OS. A partition bug in shard-tests.sh, a matrix entry someone | |
| # deleted, a new test file that landed nowhere — all of them show up here | |
| # as a named missing file rather than as a green run that quietly tested | |
| # less than it did yesterday. | |
| - name: Verify the shards covered the whole suite | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: | | |
| set -euo pipefail | |
| expected="$(find tests -maxdepth 1 -name '*.bats' | LC_ALL=C sort)" | |
| status=0 | |
| for os in ubuntu-latest macos-latest; do | |
| actual="$(cat manifests/bats-manifest-"$os"-*/shard-files.txt | LC_ALL=C sort)" | |
| if [ "$actual" = "$expected" ]; then | |
| echo "$os: all $(printf '%s\n' "$expected" | wc -l | tr -d ' ') test file(s) ran." | |
| continue | |
| fi | |
| status=1 | |
| missing="$(comm -23 <(printf '%s\n' "$expected") <(printf '%s\n' "$actual" | uniq))" | |
| extra="$(comm -13 <(printf '%s\n' "$expected") <(printf '%s\n' "$actual" | uniq))" | |
| dupes="$(printf '%s\n' "$actual" | uniq -d)" | |
| # Plain `if`s, not `[ -n "$x" ] && echo`: under `set -e` a false | |
| # test at the end of a && list exits the step, which would report | |
| # only the first kind of discrepancy and swallow the rest. | |
| if [ -n "$missing" ]; then | |
| printf '::error::%s: test file(s) run by no shard:\n%s\n' "$os" "$missing" | |
| fi | |
| if [ -n "$extra" ]; then | |
| printf '::error::%s: shard(s) ran file(s) not in tests/:\n%s\n' "$os" "$extra" | |
| fi | |
| if [ -n "$dupes" ]; then | |
| printf '::error::%s: file(s) run by more than one shard:\n%s\n' "$os" "$dupes" | |
| fi | |
| done | |
| exit "$status" | |
| # Windows coverage for the native helpers shipped in #103 (Git Bash runner, | |
| # sqlite3 compatibility shim, cygpath normalization). The POSIX-assuming suite | |
| # is not fully green on Windows yet, so this is staged (#125): a single focused | |
| # leg targeting the Windows-relevant install tests. Not a required check yet — | |
| # promote it to required once it is reliably green. The required checks stay | |
| # bats (ubuntu/macos). | |
| # | |
| # The old informational "full" leg (the whole suite, continue-on-error) was | |
| # removed: it ran POSIX-assuming tests that hang on Windows and hit the job | |
| # timeout. A timed-out job reports "cancelled", and continue-on-error does NOT | |
| # absorb a cancellation (only a failure), so it turned the whole tests run red. | |
| # Windows-relevant tests get fixed in separate PRs and folded into this leg. | |
| bats-windows: | |
| name: bats (windows-latest, ${{ matrix.leg }}) | |
| needs: changes | |
| if: ${{ !cancelled() }} | |
| runs-on: windows-latest | |
| # Bound the job so an unexpected hang can't pin a runner. The focused leg is | |
| # fast and green today; this is just a safety net. | |
| timeout-minutes: 12 | |
| strategy: | |
| fail-fast: false | |
| matrix: | |
| include: | |
| # Focused leg: the install-side native Windows helpers (#103). These | |
| # are green on windows-latest today and are the candidate to promote | |
| # to a required check. | |
| - leg: install helpers | |
| filter: "[Ww]indows" | |
| target: tests/test_install.bats | |
| # The Windows runtime path, which nothing here covered until #567: a | |
| # change to how liveness is decided on Windows shipped through a CI | |
| # that never decided liveness on Windows. Everything else about that | |
| # class is proved against a `tasklist` stub on a POSIX host, which | |
| # shows what the code does when a probe says "not found" -- not that | |
| # Git Bash says it, and not that a launch survives it. | |
| # | |
| # These two run with the real tasklist and the real MSYS pid space, | |
| # and they assert the EFFECT rather than the premise: the TUI reaches | |
| # its bridged --remote handoff, and a bridge is actually started. | |
| # Reaching the handoff is not delivering a message -- they are | |
| # separate failure points and each has its own test. | |
| - leg: windows runtime (#567) | |
| filter: "windows-native" | |
| target: tests/test_codex_monitor.bats tests/test_codex_bridge_launcher.bats | |
| defaults: | |
| run: | |
| # bats is bash; on Windows runners `shell: bash` is Git Bash, the same | |
| # environment native Windows users run agmsg under. | |
| shell: bash | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Docs-only change — skipping suite | |
| if: needs.changes.outputs.docs_only == 'true' | |
| run: echo "Diff is documentation-only; skipping the Windows bats legs." | |
| # chocolatey's community feed intermittently returns 504, which leaves the | |
| # runner with no sqlite3 at all ("Chocolatey installed 0/0 packages"). | |
| # Retry rather than let a transient feed outage redden an unrelated PR. | |
| - name: Install sqlite3 | |
| if: needs.changes.outputs.docs_only != 'true' | |
| shell: pwsh | |
| run: | | |
| for ($attempt = 1; $attempt -le 3; $attempt++) { | |
| choco install sqlite -y --no-progress | |
| if ($LASTEXITCODE -eq 0) { exit 0 } | |
| Write-Host "::warning::choco install sqlite failed (attempt $attempt of 3); retrying" | |
| Start-Sleep -Seconds 15 | |
| } | |
| Write-Host "::error::sqlite3 could not be installed after 3 attempts" | |
| exit 1 | |
| - name: Put chocolatey shims on PATH (Git Bash form) | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: echo "/c/ProgramData/chocolatey/bin" >> "$GITHUB_PATH" | |
| - name: Install bats | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: | | |
| npm config set prefix "$HOME/.npm-global" | |
| npm install -g bats | |
| echo "$HOME/.npm-global/bin" >> "$GITHUB_PATH" | |
| # Fail HERE if a tool is missing, not three minutes later inside the suite. | |
| # The trailing `|| echo` this used to end with turned a missing sqlite3 | |
| # into a passing step, so the real symptom surfaced as unrelated install | |
| # tests failing with "sqlite3 is required but not found" — which reads as | |
| # a defect in whatever PR happened to be running. | |
| - name: Show tool versions | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: | | |
| bash --version | head -1 | |
| sqlite3 --version || sqlite3.exe --version | |
| bats --version | |
| - name: Run tests | |
| if: needs.changes.outputs.docs_only != 'true' | |
| run: | | |
| # matrix.target may name multiple files (space-separated) — leave it | |
| # unquoted so the shell word-splits it into separate bats path args. | |
| # It's a fixed value from the matrix above, not external input. | |
| if [ -n "${{ matrix.filter }}" ]; then | |
| bats --print-output-on-failure --filter "${{ matrix.filter }}" ${{ matrix.target }} | |
| else | |
| bats --print-output-on-failure ${{ matrix.target }} | |
| fi | |
| # The desktop app (app/**) is Rust + TypeScript — the bats suite never | |
| # reads it, and the full Tauri build only runs on app-v* release tags via | |
| # app-release.yml. Without this job an app-only PR would merge with zero | |
| # compile verification (the bats legs skip via the app/* allowlist above). | |
| # cargo check + tsc --noEmit is a few minutes, not the full-bundle cost. | |
| # Same always-report pattern as bats: the job always completes, and only | |
| # the heavy steps are gated, so it can be promoted to a required check | |
| # without ever leaving a PR stuck pending. Pushes to main always run it. | |
| # Fail-open mirrors bats too: steps skip only on an EXPLICIT | |
| # app_changed=false — if the changes job breaks and its output is empty, | |
| # the typecheck runs rather than green-washing a possibly-app diff. | |
| app-check: | |
| name: app typecheck | |
| needs: changes | |
| if: ${{ !cancelled() }} | |
| # macOS: Tauri's crates (wry etc.) compile against system frameworks that | |
| # are already on the runner — ubuntu would need the webkit2gtk/gtk dev | |
| # stack installed first. Also matches the release build environment. | |
| runs-on: macos-latest | |
| timeout-minutes: 20 | |
| defaults: | |
| run: | |
| working-directory: app | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: No app changes — skipping typecheck | |
| if: needs.changes.outputs.app_changed == 'false' | |
| run: echo "Diff does not touch app/ — reporting green without running the typecheck." | |
| - uses: pnpm/action-setup@v4 | |
| if: needs.changes.outputs.app_changed != 'false' | |
| with: | |
| version: 10 | |
| - uses: actions/setup-node@v4 | |
| if: needs.changes.outputs.app_changed != 'false' | |
| with: | |
| node-version: 20 | |
| cache: pnpm | |
| cache-dependency-path: app/pnpm-lock.yaml | |
| - uses: dtolnay/rust-toolchain@stable | |
| if: needs.changes.outputs.app_changed != 'false' | |
| - uses: Swatinem/rust-cache@v2 | |
| if: needs.changes.outputs.app_changed != 'false' | |
| with: | |
| workspaces: app/src-tauri | |
| - name: Install frontend deps | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: pnpm install --frozen-lockfile | |
| - name: TypeScript typecheck | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: pnpm exec tsc --noEmit | |
| - name: Frontend tests | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: pnpm test | |
| - name: Bundle pinned agmsg-core | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: scripts/bundle-core.sh | |
| - name: Rust check | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: cargo check --manifest-path src-tauri/Cargo.toml | |
| - name: Rust tests | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: cargo test --manifest-path src-tauri/Cargo.toml | |
| # Windows leg for the app: the command layer's bash resolution and path | |
| # conversion are all behind cfg(windows) and had never run in CI — the very | |
| # 0.1.1→0.1.3 regressions (WSL bash.exe, backslash argv). This runs cargo test | |
| # on windows-latest so those paths are actually exercised. Same fail-open | |
| # convention as app-check: run unless the diff provably doesn't touch app/. | |
| app-test-windows: | |
| name: app test (windows-latest) | |
| needs: changes | |
| if: ${{ !cancelled() }} | |
| runs-on: windows-latest | |
| timeout-minutes: 25 | |
| defaults: | |
| run: | |
| working-directory: app | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: No app changes — skipping app tests | |
| if: needs.changes.outputs.app_changed == 'false' | |
| run: echo "Diff does not touch app/ — reporting green without running the app tests." | |
| - uses: dtolnay/rust-toolchain@stable | |
| if: needs.changes.outputs.app_changed != 'false' | |
| - uses: Swatinem/rust-cache@v2 | |
| if: needs.changes.outputs.app_changed != 'false' | |
| with: | |
| workspaces: app/src-tauri | |
| - name: Bundle pinned agmsg-core | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: scripts/bundle-core.sh | |
| shell: bash | |
| - name: Rust tests | |
| if: needs.changes.outputs.app_changed != 'false' | |
| run: cargo test --manifest-path src-tauri/Cargo.toml |