Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

apps/controller/tests/openclaw-runtime-plugin-writer.test.ts (1)

23-26: Consider documenting the partial mock intent.

The type assertion as ControllerEnv creates a partial mock with only the required paths. This is acceptable for focused unit tests, but a brief comment could clarify that other ControllerEnv properties aren't needed for this test.

📝 Optional: Add clarifying comment

   beforeEach(async () => {
     rootDir = await mkdtemp(path.join(tmpdir(), "nexu-runtime-plugin-writer-"));
+    // Partial mock: only runtimePluginTemplatesDir and openclawExtensionsDir are used
     env = {
       runtimePluginTemplatesDir: path.join(rootDir, "runtime-plugins"),
       openclawExtensionsDir: path.join(rootDir, "extensions"),
     } as ControllerEnv;
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/controller/tests/openclaw-runtime-plugin-writer.test.ts` around lines 23
- 26, The test creates a partial mock by asserting env = {
runtimePluginTemplatesDir: path.join(rootDir, "runtime-plugins"),
openclawExtensionsDir: path.join(rootDir, "extensions") } as ControllerEnv but
doesn't document that other ControllerEnv properties are intentionally omitted;
add a brief inline comment above this assignment explaining that this is an
intentional partial/mock ControllerEnv used only to supply the two path
properties required by the openclaw-runtime-plugin-writer tests so readers won't
expect the full environment object.

apps/web/src/components/ui/dialog.tsx (1)

19-21: Unify modal layering with a dedicated z-index scale

DialogOverlay/DialogContent share z-50, which collides with other overlays/popovers already using z-50 (e.g. select content and channel-connect modal). This can produce inconsistent stacking when both are active.

Proposed fix

-        "fixed inset-0 z-50 bg-black/40 backdrop-blur-sm",
+        "fixed inset-0 z-[60] bg-black/40 backdrop-blur-sm",
...
-          "fixed left-1/2 top-1/2 z-50 w-full max-w-md -translate-x-1/2 -translate-y-1/2",
+          "fixed left-1/2 top-1/2 z-[61] w-full max-w-md -translate-x-1/2 -translate-y-1/2",

Also applies to: 40-44

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/components/ui/dialog.tsx` around lines 19 - 21, DialogOverlay
and DialogContent both use the generic "z-50" which conflicts with other
overlays/popovers; update the dialog components to use a dedicated, higher
z-index token (e.g., replace "z-50" with a unique class like "z-dialog" or a
higher numeric value such as "z-60") so dialogs consistently stack above other
UI layers; change the class on both DialogOverlay and DialogContent (look for
the strings "DialogOverlay" and "DialogContent" or the array entries containing
"z-50") and ensure any CSS/tailwind config defines the new z-dialog value so it
does not collide with other z-50 elements.

apps/web/tests/import-skill-modal-state.test.ts (1)

7-17: Add edge-case assertions for .ZIP and nullish input.

Current tests verify happy-path lowercase and one invalid extension, but they don’t lock behavior for uppercase extension or missing input.

Proposed test additions

 describe("getSelectedZipFile", () => {
   it("keeps valid zip files", () => {
     const file = { name: "skill.zip" };

     expect(getSelectedZipFile(file)).toBe(file);
   });

+  it("keeps valid zip files with uppercase extension", () => {
+    const file = { name: "skill.ZIP" };
+
+    expect(getSelectedZipFile(file)).toBe(file);
+  });
+
   it("clears the selection for invalid files", () => {
     expect(getSelectedZipFile({ name: "skill.txt" })).toBeNull();
   });
+
+  it("returns null when no file is provided", () => {
+    expect(getSelectedZipFile(null)).toBeNull();
+    expect(getSelectedZipFile(undefined)).toBeNull();
+  });
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web/tests/import-skill-modal-state.test.ts` around lines 7 - 17, Tests
for getSelectedZipFile miss two edge cases: an uppercase extension and nullish
input. Add two assertions in apps/web/tests/import-skill-modal-state.test.ts:
one test that passes a file object with name "skill.ZIP" and expects
getSelectedZipFile(...) to return that file, and another test that calls
getSelectedZipFile(undefined) (and optionally null) and expects null; reference
the getSelectedZipFile function to locate where to add these it blocks alongside
the existing cases.

tests/desktop/openclaw-runtime-plugin-writer.test.ts (1)

17-118: Add one test for missing templates directory (ENOENT) early-return path.

ensurePlugins() has explicit ENOENT handling; adding a dedicated assertion would complete branch coverage for this method.

Proposed test addition

 describe("OpenClawRuntimePluginWriter", () => {
@@
   it("excludes all node_modules/.bin directories including real ones", async () => {
@@
     ).rejects.toMatchObject({ code: "ENOENT" });
   });
+
+  it("returns without error when runtime plugin templates directory is missing", async () => {
+    await rm(env.runtimePluginTemplatesDir, { recursive: true, force: true });
+
+    const writer = new OpenClawRuntimePluginWriter(env);
+    await expect(writer.ensurePlugins()).resolves.toBeUndefined();
+  });
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/desktop/openclaw-runtime-plugin-writer.test.ts` around lines 17 - 118,
Add a test that verifies OpenClawRuntimePluginWriter.ensurePlugins() handles a
missing templates directory by early-returning: in
tests/desktop/openclaw-runtime-plugin-writer.test.ts create an it block that
does NOT create env.runtimePluginTemplatesDir (leave it nonexistent),
instantiate new OpenClawRuntimePluginWriter(env), call await
writer.ensurePlugins() and assert it resolves (doesn't throw) and that
env.openclawExtensionsDir was not created (e.g.,
access(env.openclawExtensionsDir) rejects with { code: "ENOENT" }); reference
ensurePlugins, OpenClawRuntimePluginWriter, and env.runtimePluginTemplatesDir in
the test to locate the code.

apps/controller/src/lib/openclaw-config-compiler.ts (1)

371-374: Couple auth.token emission to the effective auth mode.

At Line 371 you now compute/use gatewayAuthMode, but Lines 372-374 still emit token whenever env token exists. This can produce mode: "none" plus a token in the same compiled block. I’d gate token emission by gatewayAuthMode === "token".

Proposed patch

       auth: {
         mode: gatewayAuthMode,
-        ...(env.openclawGatewayToken
-          ? { token: env.openclawGatewayToken }
-          : {}),
+        ...(gatewayAuthMode === "token" && env.openclawGatewayToken
+          ? { token: env.openclawGatewayToken }
+          : {}),
       },

As per coding guidelines, apps/controller/src/lib/openclaw-config-compiler.ts: Config generator output must match specs/references/openclaw-config-schema.md.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/controller/src/lib/openclaw-config-compiler.ts` around lines 371 - 374,
The emitted auth block can include a token even when gatewayAuthMode is "none";
update the config generator in openclaw-config-compiler.ts so token is only
emitted when gatewayAuthMode === "token" and env.openclawGatewayToken is
present: replace the unconditional spread of { token: env.openclawGatewayToken }
with a conditional that checks both gatewayAuthMode === "token" and
env.openclawGatewayToken (affecting the auth output where mode: gatewayAuthMode
is set) so the compiled config conforms to the openclaw-config schema.

apps/controller/src/runtime/openclaw-runtime-plugin-writer.ts (1)

5-12: LGTM! Filter logic correctly addresses the .bin symlink crash.

The implementation now properly:

1. Skips symlinked node_modules/.bin entries (preventing the EINVAL crash from fs.cp)
2. Preserves real node_modules/.bin directories
3. Materializes non-.bin symlinks as real directories via dereference: true

This addresses the past review concern about all symlinks being skipped.

Nitpick: The basename(source) !== ".bin" check on line 47 is redundant—isNodeModulesBinPath already guarantees segments.at(-1) === ".bin", which is equivalent to basename(source) === ".bin". Consider simplifying:

♻️ Optional simplification

         filter: async (source) => {
-          if (!isNodeModulesBinPath(source) || basename(source) !== ".bin") {
+          if (!isNodeModulesBinPath(source)) {
             return true;
           }

           const stat = await lstat(source);
           return !stat.isSymbolicLink();
         },

,

Also applies to: 46-53

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/controller/src/runtime/openclaw-runtime-plugin-writer.ts` around lines 5
- 12, Remove the redundant basename check by relying solely on
isNodeModulesBinPath(source): the helper isNodeModulesBinPath already returns
true when the path ends with "node_modules/.bin", so drop any additional checks
like basename(source) !== ".bin" wherever you guarded logic (e.g., inside the
code that filters or handles symlinks prior to fs.cp); update the conditional to
call isNodeModulesBinPath(source) (or negate it) instead of combining it with
basename-based logic to simplify and avoid duplicate checks.