From ec74a85876b7ddefc414c698398d1fd2feee03c9 Mon Sep 17 00:00:00 2001 From: latenighthackathon Date: Sun, 31 May 2026 02:36:58 +0000 Subject: [PATCH] fix(policy): report custom presets that cannot be recorded locally applyPresetContent applied a custom preset to the gateway and then returned true even when the sandbox had no local registry entry, so addCustomPolicy never ran and the preset was not recorded under customPolicies. Because policy-list and status surface custom presets only from the registry (listCustomPresets and getGatewayPresets both read registry.getCustomPolicies), the preset stayed invisible while policy-add --from-file still exited 0. Return false and warn when a custom preset reaches the gateway but cannot be recorded locally, so the command no longer reports success for a preset that policy-list and status cannot show. Fixes #4510 Signed-off-by: latenighthackathon --- src/lib/policy/index.ts | 14 ++++++ test/policies.test.ts | 99 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/src/lib/policy/index.ts b/src/lib/policy/index.ts index e7218398de..87ee19ce7c 100644 --- a/src/lib/policy/index.ts +++ b/src/lib/policy/index.ts @@ -833,6 +833,20 @@ function applyPresetContent( } registry.updateSandbox(sandboxName, { policies: pols }); } + } else if (options.custom) { + // The preset reached the gateway, but sandbox `sandboxName` has no local + // registry entry, so it cannot be recorded under `customPolicies`. Custom + // presets are surfaced only from the registry (both `listCustomPresets` + // and `getGatewayPresets` read `registry.getCustomPolicies`), so an + // unrecorded custom preset never appears in `policy-list` or `status`. + // Report the gap instead of exiting 0 as if the preset were fully applied. (#4510) + console.error( + ` Warning: '${presetName}' was applied to the gateway but could not be ` + + `recorded locally because sandbox '${sandboxName}' is not in the ` + + `registry, so it will not appear in policy-list or status. Recover or ` + + `re-onboard the sandbox, then re-apply.`, + ); + return false; } return true; diff --git a/test/policies.test.ts b/test/policies.test.ts index efafe5b48b..c7095fcd44 100644 --- a/test/policies.test.ts +++ b/test/policies.test.ts @@ -1033,6 +1033,105 @@ exit 1 }); }); + describe("issue 4510: policy-add --from-file false success when the sandbox is absent from the registry", () => { + const registryModule = requireForTest( + path.join(REPO_ROOT, "dist", "lib", "state", "registry.js"), + ) as Record; + const CUSTOM_CONTENT = "network_policies:\n slack-files-upload:\n host: files.slack.com\n"; + const SOURCE_PATH = "/tmp/slack-files-upload-case.yaml"; + + let tmpHome: string; + let fakeOpenshell: string; + let origHome: string | undefined; + let resolveSpy: ReturnType; + let savedGetSandbox: any; + let savedAddCustomPolicy: any; + + beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-issue4510-")); + const localBin = path.join(tmpHome, ".local", "bin"); + fs.mkdirSync(localBin, { recursive: true }); + fakeOpenshell = path.join(localBin, "openshell"); + fs.writeFileSync(fakeOpenshell, "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + origHome = process.env.HOME; + process.env.HOME = tmpHome; + resolveSpy = vi + .spyOn(resolveOpenshellModule, "resolveOpenshell") + .mockReturnValue(fakeOpenshell); + savedGetSandbox = registryModule.getSandbox; + savedAddCustomPolicy = registryModule.addCustomPolicy; + }); + + afterEach(() => { + if (origHome === undefined) delete process.env.HOME; + else process.env.HOME = origHome; + resolveSpy.mockRestore(); + registryModule.getSandbox = savedGetSandbox; + registryModule.addCustomPolicy = savedAddCustomPolicy; + fs.rmSync(tmpHome, { recursive: true, force: true }); + }); + + it("returns false and warns when a custom preset cannot be recorded locally", () => { + // Sandbox is Ready on the gateway but missing from the local registry + // (e.g. after stale-registry pruning), so addCustomPolicy cannot persist. + registryModule.getSandbox = () => null; + const addSpy = vi.fn(() => false); + registryModule.addCustomPolicy = addSpy; + const errors: string[] = []; + const errSpy = vi.spyOn(console, "error").mockImplementation((...a: unknown[]) => { + errors.push(a.map((x) => String(x)).join(" ")); + }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + try { + const result = policies.applyPresetContent( + "my-assistant", + "slack-files-upload", + CUSTOM_CONTENT, + { custom: { sourcePath: SOURCE_PATH } }, + ); + // Pre-fix this returned true (silent exit 0) while policy-list/status + // never showed the preset. The command must not claim success. + expect(result).toBe(false); + expect(addSpy).not.toHaveBeenCalled(); + const combined = errors.join("\n"); + expect(combined).toContain("my-assistant"); + expect(combined).toMatch(/could not be\s+recorded locally/); + expect(combined).toMatch(/policy-list or status/); + } finally { + errSpy.mockRestore(); + logSpy.mockRestore(); + } + }); + + it("records the custom preset and returns true when the sandbox is registered", () => { + registryModule.getSandbox = (name: string) => ({ name }); + const addSpy = vi.fn(() => true); + registryModule.addCustomPolicy = addSpy; + const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + const errSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + try { + const result = policies.applyPresetContent( + "my-assistant", + "slack-files-upload", + CUSTOM_CONTENT, + { custom: { sourcePath: SOURCE_PATH } }, + ); + expect(result).toBe(true); + expect(addSpy).toHaveBeenCalledWith( + "my-assistant", + expect.objectContaining({ + name: "slack-files-upload", + content: CUSTOM_CONTENT, + sourcePath: SOURCE_PATH, + }), + ); + } finally { + logSpy.mockRestore(); + errSpy.mockRestore(); + } + }); + }); + describe("extractPresetEntries", () => { it("returns null for null input", () => { expect(policies.extractPresetEntries(null)).toBe(null);