Merge remote-tracking branch 'upstream/main'

Author: lee101

.bazelrc

@@ -60,3 +60,51 @@ common:remote --jobs=800
 # Enable pipelined compilation since we are not bound by local CPU count.
 #common:remote --@rules_rust//rust/settings:pipelined_compilation
 
+# GitHub Actions CI configs.
+common:ci --remote_download_minimal
+common:ci --keep_going
+common:ci --verbose_failures
+common:ci --build_metadata=REPO_URL=https://github.com/openai/codex.git
+common:ci --build_metadata=ROLE=CI
+common:ci --build_metadata=VISIBILITY=PUBLIC
+
+# Disable disk cache in CI since we have a remote one and aren't using persistent workers.
+common:ci --disk_cache=
+
+# Shared config for the main Bazel CI workflow.
+common:ci-bazel --config=ci
+common:ci-bazel --build_metadata=TAG_workflow=bazel
+
+# Rearrange caches on Windows so they're on the same volume as the checkout.
+common:ci-windows --config=ci-bazel
+common:ci-windows --build_metadata=TAG_os=windows
+common:ci-windows --repo_contents_cache=D:/a/.cache/bazel-repo-contents-cache
+common:ci-windows --repository_cache=D:/a/.cache/bazel-repo-cache
+
+# We prefer to run the build actions entirely remotely so we can dial up the concurrency.
+# We have platform-specific tests, so we want to execute the tests on all platforms using the strongest sandboxing available on each platform.
+
+# On linux, we can do a full remote build/test, by targeting the right (x86/arm) runners, so we have coverage of both.
+# Linux crossbuilds don't work until we untangle the libc constraint mess.
+common:ci-linux --config=ci-bazel
+common:ci-linux --build_metadata=TAG_os=linux
+common:ci-linux --config=remote
+common:ci-linux --strategy=remote
+common:ci-linux --platforms=//:rbe
+
+# On mac, we can run all the build actions remotely but test actions locally.
+common:ci-macos --config=ci-bazel
+common:ci-macos --build_metadata=TAG_os=macos
+common:ci-macos --config=remote
+common:ci-macos --strategy=remote
+common:ci-macos --strategy=TestRunner=darwin-sandbox,local
+
+# Linux-only V8 CI config.
+common:ci-v8 --config=ci
+common:ci-v8 --build_metadata=TAG_workflow=v8
+common:ci-v8 --build_metadata=TAG_os=linux
+common:ci-v8 --config=remote
+common:ci-v8 --strategy=remote
+
+# Optional per-user local overrides.
+try-import %workspace%/user.bazelrc

.github/workflows/bazel.yml

@@ -78,30 +78,17 @@ jobs:
         shell: bash
         run: ./scripts/check-module-bazel-lock.sh
 
-      # TODO(mbolin): Bring this back once we have caching working. Currently,
-      # we never seem to get a cache hit but we still end up paying the cost of
-      # uploading at the end of the build, which takes over a minute!
-      #
-      # Cache build and external artifacts so that the next ci build is incremental.
-      # Because github action caches cannot be updated after a build, we need to
-      # store the contents of each build in a unique cache key, then fall back to loading
-      # it on the next ci run. We use hashFiles(...) in the key and restore-keys- with
-      # the prefix to load the most recent cache for the branch on a cache miss. You
-      # should customize the contents of hashFiles to capture any bazel input sources,
-      # although this doesn't need to be perfect. If none of the input sources change
-      # then a cache hit will load an existing cache and bazel won't have to do any work.
-      # In the case of a cache miss, you want the fallback cache to contain most of the
-      # previously built artifacts to minimize build time. The more precise you are with
-      # hashFiles sources the less work bazel will have to do.
-      # - name: Mount bazel caches
-      #   uses: actions/cache@v5
-      #   with:
-      #     path: |
-      #       ~/.cache/bazel-repo-cache
-      #       ~/.cache/bazel-repo-contents-cache
-      #     key: bazel-cache-${{ matrix.os }}-${{ hashFiles('**/BUILD.bazel', '**/*.bzl', 'MODULE.bazel') }}
-      #     restore-keys: |
-      #       bazel-cache-${{ matrix.os }}
+      # Restore bazel repository cache so we don't have to redownload all the external dependencies
+      # on every CI run.
+      - name: Restore bazel repository cache
+        id: cache_bazel_repository_restore
+        uses: actions/cache/restore@v5
+        with:
+          path: |
+            ~/.cache/bazel-repo-cache
+          key: bazel-cache-${{ matrix.target }}-${{ hashFiles('MODULE.bazel', 'codex-rs/Cargo.lock', 'codex-rs/Cargo.toml') }}
+          restore-keys: |
+            bazel-cache-${{ matrix.target }}
 
       - name: Configure Bazel startup args (Windows)
         if: runner.os == 'Windows'
@@ -157,10 +144,7 @@ jobs:
           bazel_args=(
             test
             --test_verbose_timeout_warnings
-            --build_metadata=REPO_URL=https://github.com/openai/codex.git
             --build_metadata=COMMIT_SHA=$(git rev-parse HEAD)
-            --build_metadata=ROLE=CI
-            --build_metadata=VISIBILITY=PUBLIC
           )
 
           bazel_targets=(
@@ -177,6 +161,13 @@ jobs:
             bazel_args+=("--test_env=CODEX_JS_REPL_NODE_PATH=${node_bin}")
           fi
 
+          ci_config=ci-linux
+          if [[ "${RUNNER_OS:-}" == "macOS" ]]; then
+            ci_config=ci-macos
+          elif [[ "${RUNNER_OS:-}" == "Windows" ]]; then
+            ci_config=ci-windows
+          fi
+
           if [[ -n "${BUILDBUDDY_API_KEY:-}" ]]; then
             echo "BuildBuddy API key is available; using remote Bazel configuration."
             # Work around Bazel 9 remote repo contents cache / overlay materialization failures
@@ -186,8 +177,8 @@ jobs:
             set +e
             bazel $BAZEL_STARTUP_ARGS \
               --noexperimental_remote_repo_contents_cache \
-              --bazelrc=.github/workflows/ci.bazelrc \
               "${bazel_args[@]}" \
+              "--config=${ci_config}" \
               "--remote_header=x-buildbuddy-api-key=$BUILDBUDDY_API_KEY" \
               -- \
               "${bazel_targets[@]}" \
@@ -229,3 +220,14 @@ jobs:
             print_failed_bazel_test_logs "$bazel_console_log"
             exit "$bazel_status"
           fi
+
+      # Save bazel repository cache explicitly; make non-fatal so cache uploading
+      # never fails the overall job. Only save when key wasn't hit.
+      - name: Save bazel repository cache
+        if: always() && !cancelled() && steps.cache_bazel_repository_restore.outputs.cache-hit != 'true'
+        continue-on-error: true
+        uses: actions/cache/save@v5
+        with:
+          path: |
+            ~/.cache/bazel-repo-cache
+          key: bazel-cache-${{ matrix.target }}-${{ hashFiles('MODULE.bazel', 'codex-rs/Cargo.lock', 'codex-rs/Cargo.toml') }}

.github/workflows/ci.bazelrc

@@ -547,7 +547,10 @@ jobs:
   tests:
     name: Tests — ${{ matrix.runner }} - ${{ matrix.target }}${{ matrix.remote_env == 'true' && ' (remote)' || '' }}
     runs-on: ${{ matrix.runs_on || matrix.runner }}
-    timeout-minutes: ${{ matrix.runner == 'windows-arm64' && 35 || 30 }}
+    # Perhaps we can bring this back down to 30m once we finish the cutover
+    # from tui_app_server/ to tui/. Incidentally, windows-arm64 was the main
+    # offender for exceeding the timeout.
+    timeout-minutes: 45
     needs: changed
     if: ${{ needs.changed.outputs.codex == 'true' || needs.changed.outputs.workflows == 'true' || github.event_name == 'push' }}
     defaults:

.github/workflows/rust-ci.yml

@@ -116,8 +116,8 @@ jobs:
 
           bazel \
             --noexperimental_remote_repo_contents_cache \
-            --bazelrc=.github/workflows/v8-ci.bazelrc \
             "${bazel_args[@]}" \
+            --config=ci-v8 \
             "--remote_header=x-buildbuddy-api-key=${BUILDBUDDY_API_KEY}"
 
       - name: Stage release pair

.github/workflows/rusty-v8-release.yml

@@ -108,8 +108,8 @@ jobs:
 
           bazel \
             --noexperimental_remote_repo_contents_cache \
-            --bazelrc=.github/workflows/v8-ci.bazelrc \
             "${bazel_args[@]}" \
+            --config=ci-v8 \
             "--remote_header=x-buildbuddy-api-key=${BUILDBUDDY_API_KEY}"
 
       - name: Stage release pair

.github/workflows/v8-canary.yml

@@ -10,6 +10,7 @@ node_modules
 # build
 dist/
 bazel-*
+user.bazelrc
 build/
 out/
 storybook-static/

.github/workflows/v8-ci.bazelrc

@@ -11,6 +11,11 @@ In the codex-rs folder where the rust code lives:
 - Always collapse if statements per https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
 - Always inline format! args when possible per https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
 - Use method references over closures when possible per https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls
+- Avoid bool or ambiguous `Option` parameters that force callers to write hard-to-read code such as `foo(false)` or `bar(None)`. Prefer enums, named methods, newtypes, or other idiomatic Rust API shapes when they keep the callsite self-documenting.
+- When you cannot make that API change and still need a small positional-literal callsite in Rust, follow the `argument_comment_lint` convention:
+  - Use an exact `/*param_name*/` comment before opaque literal arguments such as `None`, booleans, and numeric literals when passing them by position.
+  - Do not add these comments for string or char literals unless the comment adds real clarity; those literals are intentionally exempt from the lint.
+  - If you add one of these comments, the parameter name must exactly match the callee signature.
 - When possible, make `match` statements exhaustive and avoid wildcard arms.
 - When writing tests, prefer comparing the equality of entire objects over fields one by one.
 - When making a change that adds or changes an API, ensure that the documentation in the `docs/` folder is up to date if applicable.
@@ -19,7 +24,23 @@ In the codex-rs folder where the rust code lives:
   repo root to refresh `MODULE.bazel.lock`, and include that lockfile update in the same change.
 - After dependency changes, run `just bazel-lock-check` from the repo root so lockfile drift is caught
   locally before CI.
+- Bazel does not automatically make source-tree files available to compile-time Rust file access. If
+  you add `include_str!`, `include_bytes!`, `sqlx::migrate!`, or similar build-time file or
+  directory reads, update the crate's `BUILD.bazel` (`compile_data`, `build_script_data`, or test
+  data) or Bazel may fail even when Cargo passes.
 - Do not create small helper methods that are referenced only once.
+- Avoid large modules:
+  - Prefer adding new modules instead of growing existing ones.
+  - Target Rust modules under 500 LoC, excluding tests.
+  - If a file exceeds roughly 800 LoC, add new functionality in a new module instead of extending
+    the existing file unless there is a strong documented reason not to.
+  - This rule applies especially to high-touch files that already attract unrelated changes, such
+    as `codex-rs/tui/src/app.rs`, `codex-rs/tui/src/bottom_pane/chat_composer.rs`,
+    `codex-rs/tui/src/bottom_pane/footer.rs`, `codex-rs/tui/src/chatwidget.rs`,
+    `codex-rs/tui/src/bottom_pane/mod.rs`, and similarly central orchestration modules.
+  - When extracting code from a large module, move the related tests and module/type docs toward
+    the new implementation so the invariants stay close to the code that owns them.
+- When running Rust commands (e.g. `just fix` or `cargo test`) be patient with the command and never try to kill them using the PID. Rust lock can make the execution slow, this is expected.
 
 Run `just fmt` (in `codex-rs` directory) automatically after you have finished making Rust code changes; do not ask for approval to run it. Additionally, run the tests:
 
@@ -30,6 +51,19 @@ Before finalizing a large change to `codex-rs`, run `just fix -p <project>` (in
 
 Also run `just argument-comment-lint` to ensure the codebase is clean of comment lint errors.
 
+## The `codex-core` crate
+
+Over time, the `codex-core` crate (defined in `codex-rs/core/`) has become bloated because it is the largest crate, so it is often easier to add something new to `codex-core` rather than refactor out the library code you need so your new code neither takes a dependency on, nor contributes to the size of, `codex-core`.
+
+To that end: **resist adding code to codex-core**!
+
+Particularly when introducing a new concept/feature/API, before adding to `codex-core`, consider whether:
+
+- There is an existing crate other than `codex-core` that is an appropriate place for your new code to live.
+- It is time to introduce a new crate to the Cargo workspace for your new functionality. Refactor existing code as necessary to make this happen.
+
+Likewise, when reviewing code, do not hesitate to push back on PRs that would unnecessarily add code to `codex-core`.
+
 ## TUI style conventions
 
 See `codex-rs/tui/styles.md`.