chore: update dotagents and fix Codex configuration - #712
Conversation
…d config generation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR introduces configuration provisioning changes for Codex and OpenClaw components. It replaces symlink-based config management with activation-based copying for Codex, adds a new OpenClaw module with mode-aware configuration hydration (supporting both gateway and client modes), and updates related configurations and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant NixOS as NixOS/home-manager<br/>Activation
participant Module as OpenClaw<br/>Module
participant Script as Hydrate<br/>Script
participant Config as Config<br/>File
NixOS->>Module: Trigger home.activation.hydrateOpenclawConfig
Module->>Module: Create ${homeDir}/.openclaw directory
Module->>Module: Determine mode:<br/>host.isKyber?
alt Gateway Mode
Module->>Module: Set gateway paths<br/>(chromium, openclaw)
Module->>Script: Execute with MODE=gateway
Script->>Script: Load .env secrets
Script->>Script: Validate GATEWAY_TOKEN
Script->>Script: Load additional secrets<br/>(cliproxy, telegram, anthropic)
Script->>Script: Render full config<br/>from template
Script->>Config: Write rendered config
Script->>Script: Export ANTHROPIC_API_KEY
Script->>Script: Execute gateway binary
else Client Mode
Module->>Script: Execute with MODE=client
Script->>Script: Bypass template generation
Script->>Config: Write minimal remote<br/>gateway config
Script->>Script: Print client status
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines and enhances configuration management across the system. It specifically addresses robustness for Codex configurations by moving to an activation script for its Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DRUpdate dotagents submodule and fix Codex configuration issues. What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Pull request overview
This PR updates the dotagents submodule and improves OpenClaw configuration management by splitting gateway and client modes, while also fixing Codex configuration handling to avoid symlink issues.
Changes:
- Updated dotagents submodule to commit 25b1492f
- Refactored OpenClaw hydration script to support both gateway (Kyber) and client (macOS) modes
- Replaced Codex config symlink with activation script to handle atomic writes
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dotagents | Updated submodule reference to latest commit |
| spec/openclaw_hydrate_spec.sh | Removed test for legacy clawdbot fallback |
| nix-darwin/config/dock.nix | Added Codex app to persistent dock applications |
| home-manager/modules/openclaw/default.nix | Removed outdated comment about Node symlink |
| config/openclaw/hydrate.sh | Refactored to support gateway/client modes with MODE parameter |
| config/openclaw/default.nix | New configuration file implementing mode-based hydration |
| config/default.nix | Added openclaw to imported modules |
| config/codex/default.nix | Changed from symlink to activation script |
| config/codex/config.toml | Added suppress_unstable_features_warning flag |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # shellcheck source=/dev/null | ||
| set -euo pipefail | ||
|
|
||
| MODE="@mode@" |
There was a problem hiding this comment.
The MODE variable is set via template substitution but lacks validation. Consider adding a check after line 7 to ensure MODE is either 'gateway' or 'client' and exit with an error message if it's neither, preventing silent failures from misconfiguration.
| MODE="@mode@" | |
| MODE="@mode@" | |
| case "$MODE" in | |
| gateway|client) | |
| ;; | |
| *) | |
| echo "Error: invalid MODE '$MODE'. Expected 'gateway' or 'client'." >&2 | |
| exit 1 | |
| ;; | |
| esac |
| chromium = "/unused"; | ||
| openclaw = "/unused"; |
There was a problem hiding this comment.
Using '/unused' as placeholder values could cause confusion or issues if these paths are accidentally referenced. Consider using more explicit placeholder values like 'null' or empty strings, or restructure the code to only define these variables when host.isKyber is true.
| chromium = "/unused"; | |
| openclaw = "/unused"; | |
| chromium = ""; | |
| openclaw = ""; |
| $DRY_RUN_CMD cp -f ${./config.toml} ~/.codex/config.toml | ||
| $DRY_RUN_CMD chmod 600 ~/.codex/config.toml |
There was a problem hiding this comment.
The tilde (~) expansion may not work reliably in all contexts. Consider using '${config.home.homeDirectory}/.codex/config.toml' instead for consistency with other parts of the codebase.
| $DRY_RUN_CMD cp -f ${./config.toml} ~/.codex/config.toml | |
| $DRY_RUN_CMD chmod 600 ~/.codex/config.toml | |
| $DRY_RUN_CMD cp -f ${./config.toml} ${config.home.homeDirectory}/.codex/config.toml | |
| $DRY_RUN_CMD chmod 600 ${config.home.homeDirectory}/.codex/config.toml |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request updates the dotagents submodule, refactors the Codex configuration to use an activation script instead of a symlink, and introduces OpenClaw configuration. However, there are a few issues identified. The OpenClaw hydration script's activation process might silently fail due to an || true condition, and a critical token check exits with success even if the token is missing. Additionally, the change to nix-darwin/config/dock.nix contradicts the pull request description regarding the Codex app entry.
| exit 0 | ||
| fi |
There was a problem hiding this comment.
Exiting with 0 (success) when the GATEWAY_TOKEN is not set means that the OpenClaw configuration hydration will silently fail if this critical token is missing. This can lead to unexpected behavior or non-functional components without clear indication during the NixOS activation process. It's best to exit with a non-zero status to signal a failure.
| exit 0 | |
| fi | |
| echo "Warning: OPENCLAW_GATEWAY_TOKEN not set, skipping OpenClaw hydration" >&2 | |
| exit 1 |
| "/Applications/Ghostty.app" | ||
| "/Applications/Linear.app" | ||
| "/Applications/Tailscale.app" | ||
| "/Applications/Codex.app" |
There was a problem hiding this comment.
The pull request description states "Remove duplicate Codex app entry in persistent dock applications". However, this change adds "/Applications/Codex.app" to the list. This directly contradicts the stated purpose of the PR. Please clarify if the intention was to add or remove this entry, and adjust the code or description accordingly. If the goal was to remove a duplicate, this line should not be added.
"/Applications/Cursor.app"
| # Gateway mode on Kyber, client mode everywhere else | ||
| home.activation.hydrateOpenclawConfig = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| mkdir -p ${homeDir}/.openclaw | ||
| ${pkgs.bash}/bin/bash ${hydrateScript} || true |
There was a problem hiding this comment.
The || true at the end of the bash command can mask potential failures during the OpenClaw configuration hydration. If the hydrateScript fails for any reason, the NixOS activation will still report success, potentially leading to a silently misconfigured OpenClaw. It's generally better to let the activation fail if a critical setup script fails, so the user is immediately aware of the problem.
${pkgs.bash}/bin/bash ${hydrateScript}
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 35bf970...db0c880
Analysis
-
Service Lifecycle Management Issue: The OpenClaw gateway is started with
execduring home-manager activation, conflating configuration with service management. This will either orphan processes, block activation, or terminate prematurely when activation ends. -
Silent Failure Pattern: Error handling uses
|| trueandexit 0on missing secrets, creating silent failures that violate fail-fast principles. Users won't know OpenClaw isn't configured until they attempt to use it. -
Symlink to Activation Script Tradeoffs: While solving the Codex atomic write issue, this pattern loses symlink benefits (auto-sync with repo), requires manual activation on changes, and increases coupling between repo and runtime state.
-
Scope Mismatch: PR introduces a significant new OpenClaw configuration module but is titled as a "chore," understating architectural implications of the changes.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 3 comments | Edit Agent Settings • Read Docs
|
|
||
| # Chromium path (injected by nix) | ||
| CHROMIUM_PATH="@chromium@/bin/chromium" | ||
| if [ -z "${GATEWAY_TOKEN}" ]; then |
There was a problem hiding this comment.
The early exit with exit 0 when GATEWAY_TOKEN is missing will silently skip OpenClaw configuration without any persistent indication of failure. In client mode (macOS), this means users might not realize their OpenClaw isn't configured. Consider either:
- Creating a placeholder config file with an error marker that OpenClaw can detect
- Using a non-zero exit code and handling it gracefully in the activation script with
|| true - Logging to a file in ~/.openclaw/ that persists beyond stderr
The current approach could lead to confusing "OpenClaw not working" issues that are hard to debug.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#712
File: config/openclaw/hydrate.sh#L36
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The early exit with `exit 0` when GATEWAY_TOKEN is missing will silently skip OpenClaw configuration without any persistent indication of failure. In client mode (macOS), this means users might not realize their OpenClaw isn't configured. Consider either:
1. Creating a placeholder config file with an error marker that OpenClaw can detect
2. Using a non-zero exit code and handling it gracefully in the activation script with `|| true`
3. Logging to a file in ~/.openclaw/ that persists beyond stderr
The current approach could lead to confusing "OpenClaw not working" issues that are hard to debug.
| export ANTHROPIC_API_KEY | ||
| fi | ||
|
|
||
| exec @openclaw@/bin/openclaw gateway --port 18789 "$@" |
There was a problem hiding this comment.
In gateway mode, this exec replaces the shell process with the openclaw gateway command. However, this script is called during home-manager activation (line 31 of config/openclaw/default.nix), not as a persistent service starter. This means the gateway will start during activation and then be orphaned or terminated when activation completes.
Should this be a systemd service instead? Or should the exec be removed so this script only hydrates the config without starting the service?
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#712
File: config/openclaw/hydrate.sh#L64
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
In gateway mode, this `exec` replaces the shell process with the openclaw gateway command. However, this script is called during home-manager activation (line 31 of config/openclaw/default.nix), not as a persistent service starter. This means the gateway will start during activation and then be orphaned or terminated when activation completes.
Should this be a systemd service instead? Or should the exec be removed so this script only hydrates the config without starting the service?
| # Gateway mode on Kyber, client mode everywhere else | ||
| home.activation.hydrateOpenclawConfig = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| mkdir -p ${homeDir}/.openclaw | ||
| ${pkgs.bash}/bin/bash ${hydrateScript} || true |
There was a problem hiding this comment.
The || true here will silently swallow all errors from the hydration script, including legitimate failures like permission errors or invalid config generation. Combined with line 36 in hydrate.sh that does exit 0 on missing GATEWAY_TOKEN, this creates a double-silent failure mode.
Consider removing the || true and handling specific expected failure cases in the script with appropriate exit codes, or at minimum log failures to a persistent location.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#712
File: config/openclaw/default.nix#L31
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The `|| true` here will silently swallow all errors from the hydration script, including legitimate failures like permission errors or invalid config generation. Combined with line 36 in hydrate.sh that does `exit 0` on missing GATEWAY_TOKEN, this creates a double-silent failure mode.
Consider removing the `|| true` and handling specific expected failure cases in the script with appropriate exit codes, or at minimum log failures to a persistent location.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="nix-darwin/config/dock.nix">
<violation number="1" location="nix-darwin/config/dock.nix:33">
P2: This change adds `/Applications/Codex.app` to the dock persistent applications, but the PR description states the intention is to "Remove duplicate Codex app entry". The code contradicts the stated purpose. Please clarify if the intention was to add or remove this entry, and adjust the code or description accordingly.</violation>
</file>
<file name="config/default.nix">
<violation number="1" location="config/default.nix:19">
P2: The new module import `./openclaw` points to a path that does not exist in the repository, which will cause Nix evaluation to fail with a missing path error.</violation>
</file>
<file name="config/openclaw/default.nix">
<violation number="1" location="config/openclaw/default.nix:31">
P2: The `|| true` suffix will silently swallow all errors from the hydration script, including legitimate failures like permission errors or invalid config generation. Consider removing `|| true` and handling specific expected failure cases in the script with appropriate exit codes, or at minimum log failures to a persistent location for debugging.</violation>
</file>
<file name="config/openclaw/hydrate.sh">
<violation number="1" location="config/openclaw/hydrate.sh:38">
P2: Exiting with `exit 0` (success) when GATEWAY_TOKEN is not set means the OpenClaw configuration hydration will silently fail. This can lead to unexpected behavior or non-functional components without clear indication during the activation process. Consider using a non-zero exit code to signal failure, or log to a persistent location in `~/.openclaw/` for debugging.</violation>
<violation number="2" location="config/openclaw/hydrate.sh:64">
P1: Using `exec` to start the gateway during home-manager activation is problematic. This replaces the shell process with the openclaw gateway command, but since this runs during activation (not as a persistent service), the gateway will either be orphaned or terminated when activation completes. Consider either making this a systemd service, or removing the exec so the script only hydrates the config without attempting to start the gateway.</violation>
</file>
<file name="config/codex/default.nix">
<violation number="1" location="config/codex/default.nix:6">
P2: Activation script copies to ~/.codex/config.toml without ensuring ~/.codex exists, so on a clean install the activation will fail after removing home.file which previously created the directory.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| export ANTHROPIC_API_KEY | ||
| fi | ||
|
|
||
| exec @openclaw@/bin/openclaw gateway --port 18789 "$@" |
There was a problem hiding this comment.
P1: Using exec to start the gateway during home-manager activation is problematic. This replaces the shell process with the openclaw gateway command, but since this runs during activation (not as a persistent service), the gateway will either be orphaned or terminated when activation completes. Consider either making this a systemd service, or removing the exec so the script only hydrates the config without attempting to start the gateway.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/openclaw/hydrate.sh, line 64:
<comment>Using `exec` to start the gateway during home-manager activation is problematic. This replaces the shell process with the openclaw gateway command, but since this runs during activation (not as a persistent service), the gateway will either be orphaned or terminated when activation completes. Consider either making this a systemd service, or removing the exec so the script only hydrates the config without attempting to start the gateway.</comment>
<file context>
@@ -29,33 +30,53 @@ read_secret() {
+ export ANTHROPIC_API_KEY
+ fi
+
+ exec @openclaw@/bin/openclaw gateway --port 18789 "$@"
-# Start OpenClaw gateway
</file context>
| "/Applications/Ghostty.app" | ||
| "/Applications/Linear.app" | ||
| "/Applications/Tailscale.app" | ||
| "/Applications/Codex.app" |
There was a problem hiding this comment.
P2: This change adds /Applications/Codex.app to the dock persistent applications, but the PR description states the intention is to "Remove duplicate Codex app entry". The code contradicts the stated purpose. Please clarify if the intention was to add or remove this entry, and adjust the code or description accordingly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At nix-darwin/config/dock.nix, line 33:
<comment>This change adds `/Applications/Codex.app` to the dock persistent applications, but the PR description states the intention is to "Remove duplicate Codex app entry". The code contradicts the stated purpose. Please clarify if the intention was to add or remove this entry, and adjust the code or description accordingly.</comment>
<file context>
@@ -30,6 +30,7 @@
"/Applications/Ghostty.app"
"/Applications/Linear.app"
"/Applications/Tailscale.app"
+ "/Applications/Codex.app"
"/Applications/Cursor.app"
"/Applications/Visual Studio Code.app"
</file context>
| ./k3s | ||
| ./karabiner | ||
| ./llm | ||
| ./openclaw |
There was a problem hiding this comment.
P2: The new module import ./openclaw points to a path that does not exist in the repository, which will cause Nix evaluation to fail with a missing path error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/default.nix, line 19:
<comment>The new module import `./openclaw` points to a path that does not exist in the repository, which will cause Nix evaluation to fail with a missing path error.</comment>
<file context>
@@ -16,6 +16,7 @@
./k3s
./karabiner
./llm
+ ./openclaw
./opencode
./pi
</file context>
| # Gateway mode on Kyber, client mode everywhere else | ||
| home.activation.hydrateOpenclawConfig = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| mkdir -p ${homeDir}/.openclaw | ||
| ${pkgs.bash}/bin/bash ${hydrateScript} || true |
There was a problem hiding this comment.
P2: The || true suffix will silently swallow all errors from the hydration script, including legitimate failures like permission errors or invalid config generation. Consider removing || true and handling specific expected failure cases in the script with appropriate exit codes, or at minimum log failures to a persistent location for debugging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/openclaw/default.nix, line 31:
<comment>The `|| true` suffix will silently swallow all errors from the hydration script, including legitimate failures like permission errors or invalid config generation. Consider removing `|| true` and handling specific expected failure cases in the script with appropriate exit codes, or at minimum log failures to a persistent location for debugging.</comment>
<file context>
@@ -0,0 +1,33 @@
+ # Gateway mode on Kyber, client mode everywhere else
+ home.activation.hydrateOpenclawConfig = config.lib.dag.entryAfter [ "writeBoundary" ] ''
+ mkdir -p ${homeDir}/.openclaw
+ ${pkgs.bash}/bin/bash ${hydrateScript} || true
+ '';
+}
</file context>
| CHROMIUM_PATH="@chromium@/bin/chromium" | ||
| if [ -z "${GATEWAY_TOKEN}" ]; then | ||
| echo "Warning: OPENCLAW_GATEWAY_TOKEN not set, skipping OpenClaw hydration" >&2 | ||
| exit 0 |
There was a problem hiding this comment.
P2: Exiting with exit 0 (success) when GATEWAY_TOKEN is not set means the OpenClaw configuration hydration will silently fail. This can lead to unexpected behavior or non-functional components without clear indication during the activation process. Consider using a non-zero exit code to signal failure, or log to a persistent location in ~/.openclaw/ for debugging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/openclaw/hydrate.sh, line 38:
<comment>Exiting with `exit 0` (success) when GATEWAY_TOKEN is not set means the OpenClaw configuration hydration will silently fail. This can lead to unexpected behavior or non-functional components without clear indication during the activation process. Consider using a non-zero exit code to signal failure, or log to a persistent location in `~/.openclaw/` for debugging.</comment>
<file context>
@@ -29,33 +30,53 @@ read_secret() {
-CHROMIUM_PATH="@chromium@/bin/chromium"
+if [ -z "${GATEWAY_TOKEN}" ]; then
+ echo "Warning: OPENCLAW_GATEWAY_TOKEN not set, skipping OpenClaw hydration" >&2
+ exit 0
+fi
</file context>
| # Use activation script instead of home.file symlink | ||
| # Codex CLI uses atomic writes that break symlinks, so we force-copy on each switch | ||
| home.activation.codexConfig = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | ||
| $DRY_RUN_CMD cp -f ${./config.toml} ~/.codex/config.toml |
There was a problem hiding this comment.
P2: Activation script copies to ~/.codex/config.toml without ensuring ~/.codex exists, so on a clean install the activation will fail after removing home.file which previously created the directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/codex/default.nix, line 6:
<comment>Activation script copies to ~/.codex/config.toml without ensuring ~/.codex exists, so on a clean install the activation will fail after removing home.file which previously created the directory.</comment>
<file context>
@@ -1,7 +1,9 @@
+ # Use activation script instead of home.file symlink
+ # Codex CLI uses atomic writes that break symlinks, so we force-copy on each switch
+ home.activation.codexConfig = lib.hm.dag.entryAfter [ "writeBoundary" ] ''
+ $DRY_RUN_CMD cp -f ${./config.toml} ~/.codex/config.toml
+ $DRY_RUN_CMD chmod 600 ~/.codex/config.toml
+ '';
</file context>
| $DRY_RUN_CMD cp -f ${./config.toml} ~/.codex/config.toml | |
| $DRY_RUN_CMD mkdir -p ~/.codex | |
| $DRY_RUN_CMD cp -f ${./config.toml} ~/.codex/config.toml |
Changes
Technical Details
Testing
Generated with Claude Code by Claude
Summary by cubic
Reworked OpenClaw configuration with a new Nix module and a mode-aware hydrate script that generates gateway or client configs from secrets. Also fixes Codex setup (activation script, warning suppression, single Dock entry) and updates the dotagents submodule.
New Features
Migration
Written for commit db0c880. Summary will update on new commits.