codex: address PR review feedback (#26461)

2026-11-10 21:00:47 +00:00 · 2026-06-04 21:14:27 +00:00
parent dc73635f01
commit d1855d9117
3 changed files with 135 additions and 85 deletions
@@ -0,0 +1,109 @@
+name: finish-cargo-ci
+description: Save shared Cargo CI caches and report sccache statistics.
+inputs:
+  target:
+    description: Rust target triple used for cache namespacing.
+    required: true
+  profile:
+    description: Cargo profile used for cache namespacing.
+    required: true
+  cache-runner:
+    description: Runner name used for cache namespacing.
+    required: true
+  cargo-home-cache-hit:
+    description: Whether the exact Cargo home cache key was restored.
+    required: true
+  lockfile-hash:
+    description: SHA-256 hash of codex-rs/Cargo.lock.
+    required: true
+  toolchain-hash:
+    description: SHA-256 hash of codex-rs/rust-toolchain.toml.
+    required: true
+  summary-label:
+    description: Label shown beside the target in the sccache summary.
+    required: true
+  use-hermetic-cargo-home:
+    description: Save the workspace-local Cargo home used by musl builds.
+    required: false
+    default: "false"
+  use-sccache:
+    description: Save and report sccache state.
+    required: false
+    default: "true"
+  supplemental-cache-path:
+    description: Optional workload-specific cache path to save.
+    required: false
+    default: ""
+  supplemental-cache-key:
+    description: Cache key for the optional workload-specific cache.
+    required: false
+    default: ""
+  supplemental-cache-hit:
+    description: Whether the optional workload-specific cache was restored exactly.
+    required: false
+    default: "false"
+
+runs:
+  using: composite
+  steps:
+    - name: Save cargo home cache
+      if: ${{ !cancelled() && inputs.cargo-home-cache-hit != 'true' && inputs.use-hermetic-cargo-home != 'true' }}
+      continue-on-error: true
+      uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
+      with:
+        path: |
+          ~/.cargo/bin/
+          ~/.cargo/registry/index/
+          ~/.cargo/registry/cache/
+          ~/.cargo/git/db/
+        key: cargo-home-${{ inputs.cache-runner }}-${{ inputs.target }}-${{ inputs.profile }}-${{ inputs.lockfile-hash }}-${{ inputs.toolchain-hash }}
+
+    - name: Save cargo home cache
+      if: ${{ !cancelled() && inputs.cargo-home-cache-hit != 'true' && inputs.use-hermetic-cargo-home == 'true' }}
+      continue-on-error: true
+      uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
+      with:
+        path: |
+          ~/.cargo/bin/
+          ~/.cargo/registry/index/
+          ~/.cargo/registry/cache/
+          ~/.cargo/git/db/
+          ${{ github.workspace }}/.cargo-home/bin/
+          ${{ github.workspace }}/.cargo-home/registry/index/
+          ${{ github.workspace }}/.cargo-home/registry/cache/
+          ${{ github.workspace }}/.cargo-home/git/db/
+        key: cargo-home-${{ inputs.cache-runner }}-${{ inputs.target }}-${{ inputs.profile }}-${{ inputs.lockfile-hash }}-${{ inputs.toolchain-hash }}
+
+    - name: Save sccache cache (fallback)
+      if: ${{ !cancelled() && inputs.use-sccache == 'true' && env.SCCACHE_GHA_ENABLED != 'true' }}
+      continue-on-error: true
+      uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
+      with:
+        path: ${{ env.SCCACHE_DIR }}
+        key: sccache-${{ inputs.cache-runner }}-${{ inputs.target }}-${{ inputs.profile }}-${{ inputs.lockfile-hash }}-${{ github.run_id }}
+
+    - name: sccache stats
+      if: ${{ inputs.use-sccache == 'true' }}
+      continue-on-error: true
+      shell: bash
+      run: sccache --show-stats || true
+
+    - name: sccache summary
+      if: ${{ inputs.use-sccache == 'true' }}
+      shell: bash
+      run: |
+        {
+          echo "### sccache stats — ${{ inputs.target }} (${{ inputs.summary-label }})";
+          echo;
+          echo '```';
+          sccache --show-stats || true;
+          echo '```';
+        } >> "$GITHUB_STEP_SUMMARY"
+
+    - name: Save supplemental cache
+      if: ${{ !cancelled() && inputs.supplemental-cache-path != '' && inputs.supplemental-cache-hit != 'true' }}
+      continue-on-error: true
+      uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
+      with:
+        path: ${{ inputs.supplemental-cache-path }}
+        key: ${{ inputs.supplemental-cache-key }}
@@ -185,42 +185,18 @@ jobs:
          if-no-files-found: error
          retention-days: 1

-      - name: Save cargo home cache
-        if: always() && !cancelled() && steps.cargo_setup.outputs.cargo-home-cache-hit != 'true'
-        continue-on-error: true
-        uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
+      - name: Finish Cargo CI
+        if: always()
+        uses: ./.github/actions/finish-cargo-ci
        with:
-          path: |
-            ~/.cargo/bin/
-            ~/.cargo/registry/index/
-            ~/.cargo/registry/cache/
-            ~/.cargo/git/db/
-          key: cargo-home-${{ env.ARCHIVE_CACHE_RUNNER }}-${{ inputs.target }}-${{ inputs.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ steps.cargo_setup.outputs.toolchain-hash }}
-
-      - name: Save sccache cache (fallback)
-        if: always() && !cancelled() && env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true'
-        continue-on-error: true
-        uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
-        with:
-          path: ${{ env.SCCACHE_DIR }}
-          key: sccache-${{ env.ARCHIVE_CACHE_RUNNER }}-${{ inputs.target }}-${{ inputs.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ github.run_id }}
-
-      - name: sccache stats
-        if: always() && env.USE_SCCACHE == 'true'
-        continue-on-error: true
-        run: sccache --show-stats || true
-
-      - name: sccache summary
-        if: always() && env.USE_SCCACHE == 'true'
-        shell: bash
-        run: |
-          {
-            echo "### sccache stats — ${{ inputs.target }} (tests)";
-            echo;
-            echo '```';
-            sccache --show-stats || true;
-            echo '```';
-          } >> "$GITHUB_STEP_SUMMARY"
+          target: ${{ inputs.target }}
+          profile: ${{ inputs.profile }}
+          cache-runner: ${{ env.ARCHIVE_CACHE_RUNNER }}
+          cargo-home-cache-hit: ${{ steps.cargo_setup.outputs.cargo-home-cache-hit }}
+          lockfile-hash: ${{ steps.cargo_setup.outputs.lockfile-hash }}
+          toolchain-hash: ${{ steps.cargo_setup.outputs.toolchain-hash }}
+          summary-label: tests
+          use-sccache: ${{ env.USE_SCCACHE }}

  shard:
    name: Tests shard ${{ matrix.shard }}/4
@@ -337,57 +337,22 @@ jobs:
          path: codex-rs/target/**/cargo-timings/cargo-timing.html
          if-no-files-found: warn

-      # Save caches explicitly; make non-fatal so cache packaging
-      # never fails the overall job. Only save when key wasn't hit.
-      - name: Save cargo home cache
-        if: always() && !cancelled() && steps.cargo_setup.outputs.cargo-home-cache-hit != 'true'
-        continue-on-error: true
-        uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
+      - name: Finish Cargo CI
+        if: always()
+        uses: ./.github/actions/finish-cargo-ci
        with:
-          path: |
-            ~/.cargo/bin/
-            ~/.cargo/registry/index/
-            ~/.cargo/registry/cache/
-            ~/.cargo/git/db/
-            ${{ github.workspace }}/.cargo-home/bin/
-            ${{ github.workspace }}/.cargo-home/registry/index/
-            ${{ github.workspace }}/.cargo-home/registry/cache/
-            ${{ github.workspace }}/.cargo-home/git/db/
-          key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ steps.cargo_setup.outputs.toolchain-hash }}
-
-      - name: Save sccache cache (fallback)
-        if: always() && !cancelled() && env.USE_SCCACHE == 'true' && env.SCCACHE_GHA_ENABLED != 'true'
-        continue-on-error: true
-        uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
-        with:
-          path: ${{ env.SCCACHE_DIR }}
-          key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ steps.cargo_setup.outputs.lockfile-hash }}-${{ github.run_id }}
-
-      - name: sccache stats
-        if: always() && env.USE_SCCACHE == 'true'
-        continue-on-error: true
-        run: sccache --show-stats || true
-
-      - name: sccache summary
-        if: always() && env.USE_SCCACHE == 'true'
-        shell: bash
-        run: |
-          {
-            echo "### sccache stats — ${{ matrix.target }} (${{ matrix.profile }})";
-            echo;
-            echo '```';
-            sccache --show-stats || true;
-            echo '```';
-          } >> "$GITHUB_STEP_SUMMARY"
-
-      - name: Save APT cache (musl)
-        if: always() && !cancelled() && (matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl') && steps.cache_apt_restore.outputs.cache-hit != 'true'
-        continue-on-error: true
-        uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4
-        with:
-          path: |
-            /var/cache/apt
-          key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1
+          target: ${{ matrix.target }}
+          profile: ${{ matrix.profile }}
+          cache-runner: ${{ matrix.runner }}
+          cargo-home-cache-hit: ${{ steps.cargo_setup.outputs.cargo-home-cache-hit }}
+          lockfile-hash: ${{ steps.cargo_setup.outputs.lockfile-hash }}
+          toolchain-hash: ${{ steps.cargo_setup.outputs.toolchain-hash }}
+          summary-label: ${{ matrix.profile }}
+          use-hermetic-cargo-home: ${{ contains(matrix.target, 'musl') && 'true' || 'false' }}
+          use-sccache: ${{ env.USE_SCCACHE }}
+          supplemental-cache-path: ${{ contains(matrix.target, 'musl') && '/var/cache/apt' || '' }}
+          supplemental-cache-key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1
+          supplemental-cache-hit: ${{ steps.cache_apt_restore.outputs.cache-hit }}

  tests_macos_aarch64:
    name: Tests — macos-15-xlarge - aarch64-apple-darwin