addressing codex review

pgondhi987 · pgondhi987 · commit 5c9e75ce088a · 2026-05-07T08:47:21.000Z
diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts
@@ -430,14 +430,15 @@ function resolveOwnerAuthorizationState(params: {
 
 function resolveCommandSenderAuthorization(params: {
   commandAuthorized: boolean;
+  enforceOwnerForCommands: boolean;
   nativeCommandAuthorized: boolean;
   isOwnerForCommands: boolean;
   senderCandidates: string[];
   commandsAllowFromList: string[] | null;
   providerResolutionError: boolean;
   commandsAllowFromConfigured: boolean;
 }): boolean {
-  if (!params.isOwnerForCommands) {
+  if (params.enforceOwnerForCommands && !params.isOwnerForCommands) {
     return false;
   }
   if (
@@ -713,6 +714,7 @@ export function resolveCommandAuthorization(params: {
     commandAuthorized && ctx.CommandSource === "native" && !requireOwner;
   const isAuthorizedSender = resolveCommandSenderAuthorization({
     commandAuthorized,
+    enforceOwnerForCommands: enforceOwner,
     nativeCommandAuthorized,
     isOwnerForCommands,
     senderCandidates,
diff --git a/src/auto-reply/command-control.test.ts b/src/auto-reply/command-control.test.ts
@@ -674,6 +674,34 @@ describe("resolveCommandAuthorization", () => {
       expect(auth.isAuthorizedSender).toBe(false);
     });
 
+    it("keeps commands.allowFrom available to non-owner command users when an owner allowlist is configured", () => {
+      const cfg = {
+        commands: {
+          ownerAllowFrom: ["discord:owner"],
+          allowFrom: {
+            discord: ["helper"],
+          },
+        },
+        channels: { discord: { allowFrom: ["*"] } },
+      } as OpenClawConfig;
+
+      const auth = resolveCommandAuthorization({
+        ctx: {
+          Provider: "discord",
+          Surface: "discord",
+          ChatType: "group",
+          From: "discord:helper",
+          SenderId: "helper",
+          CommandSource: "native",
+        } as MsgContext,
+        cfg,
+        commandAuthorized: true,
+      });
+
+      expect(auth.senderIsOwner).toBe(false);
+      expect(auth.isAuthorizedSender).toBe(true);
+    });
+
     it("does not treat conversation ids in From as sender identities", () => {
       const cfg = {
         commands: {
diff --git a/src/auto-reply/reply/commands-stop-target.test.ts b/src/auto-reply/reply/commands-stop-target.test.ts
@@ -1,6 +1,10 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig } from "../../config/config.js";
-import { setActivePluginRegistry } from "../../plugins/runtime.js";
+import {
+  getActivePluginRegistry,
+  resetPluginRuntimeStateForTest,
+  setActivePluginRegistry,
+} from "../../plugins/runtime.js";
 import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js";
 import { resolveCommandAuthorization } from "../command-auth.js";
 import type { MsgContext } from "../templating.js";
@@ -55,6 +59,8 @@ vi.mock("./reply-run-registry.js", () => ({
 const formatAllowFrom = ({ allowFrom }: { allowFrom: Array<string | number> }) =>
   allowFrom.map((entry) => String(entry).trim()).filter(Boolean);
 
+let previousPluginRegistry: ReturnType<typeof getActivePluginRegistry>;
+
 function registerOwnerEnforcingTelegramPlugin() {
   setActivePluginRegistry(
     createTestRegistry([
@@ -116,10 +122,19 @@ function buildStopParams(): HandleCommandsParams {
 
 describe("handleStopCommand target fallback", () => {
   beforeEach(() => {
+    previousPluginRegistry = getActivePluginRegistry();
     vi.clearAllMocks();
     persistAbortTargetEntryMock.mockResolvedValue(true);
   });
 
+  afterEach(() => {
+    if (previousPluginRegistry) {
+      setActivePluginRegistry(previousPluginRegistry);
+    } else {
+      resetPluginRuntimeStateForTest();
+    }
+  });
+
   it("does not fall back to the wrapper session when a distinct target session is missing from store", async () => {
     const params = buildStopParams();
 
diff --git a/src/auto-reply/reply/commands-subagents-routing.test.ts b/src/auto-reply/reply/commands-subagents-routing.test.ts
@@ -1,6 +1,10 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig } from "../../config/config.js";
-import { setActivePluginRegistry } from "../../plugins/runtime.js";
+import {
+  getActivePluginRegistry,
+  resetPluginRuntimeStateForTest,
+  setActivePluginRegistry,
+} from "../../plugins/runtime.js";
 import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js";
 import { resolveCommandAuthorization } from "../command-auth.js";
 import type { MsgContext } from "../templating.js";
@@ -31,6 +35,8 @@ vi.mock("./commands-subagents-control.runtime.js", () => ({
 const formatAllowFrom = ({ allowFrom }: { allowFrom: Array<string | number> }) =>
   allowFrom.map((entry) => String(entry).trim()).filter(Boolean);
 
+let previousPluginRegistry: ReturnType<typeof getActivePluginRegistry>;
+
 function registerOwnerEnforcingTelegramPlugin() {
   setActivePluginRegistry(
     createTestRegistry([
@@ -104,9 +110,18 @@ function buildParams(
 
 describe("subagents command dispatch", () => {
   beforeEach(() => {
+    previousPluginRegistry = getActivePluginRegistry();
     vi.clearAllMocks();
   });
 
+  afterEach(() => {
+    if (previousPluginRegistry) {
+      setActivePluginRegistry(previousPluginRegistry);
+    } else {
+      resetPluginRuntimeStateForTest();
+    }
+  });
+
   it("prefers native command target session keys", () => {
     const params = buildParams("/subagents list", {
       CommandSource: "native",
