Skip to content

Commit 74deba1

Browse files
committed
fix(electron,web): run desktop backend outside production mode, keep save token fresh
The packaged desktop app launched its backend with NODETOOL_ENV=production only to make the pack loader require an allowlist. The server treats production as "hosted cloud" and disables local-only features the desktop is built around: the Python bridge (every Python node failed with "Python bridge is disabled in production"), the vector/RAG nodes, the file browser, workspaces, MCP config, local model scanning, and the /mcp mount the bundled Claude Desktop extension connects to. Give pack trust a dedicated flag (NODETOOL_PACKS_REQUIRE_ALLOWLIST=1), set that from Electron instead, and leave production mode to hosted deployments. Also adopt the server's updated_at after a save that raced a user edit: saveWorkflow kept the whole stale workflow object in that branch, so the client's concurrency token never advanced past its own successful write and every later save and autosave failed with an optimistic-concurrency conflict. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANBmpjQdGvEasgGZfbuyBs
1 parent d8ad55a commit 74deba1

8 files changed

Lines changed: 100 additions & 21 deletions

File tree

docs/configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,7 @@ the run is a separate concern; see
431431
| `NODETOOL_ENABLE_FAKE_PROVIDER` | Register the `fake` provider id as a builtin, so a workflow can select it | no | `1` only, and ignored when `NODETOOL_ENV=production`. Separate from `NODETOOL_FAKE_PROVIDERS`, which fakes the providers that are already registered |
432432
| `NODETOOL_PACK_SEARCH_PATHS` | Extra `node_modules` directories to load node packs from | no | Comma-, semicolon-, or `PATH`-separator-delimited (`:` is not a separator on Windows, so drive letters survive). Paths that do not exist are dropped. Searched before the walk up from the working directory. See [Node Packs](node-packs.md) |
433433
| `NODETOOL_OPTIONAL_NODE_MODULES` | A single extra `node_modules` directory for pack loading | no | The one-path form of `NODETOOL_PACK_SEARCH_PATHS`; both are read, and the desktop app uses this to point the loader at its bundled install root |
434+
| `NODETOOL_PACKS_REQUIRE_ALLOWLIST` | Default `allowUnlisted` to false without production mode | no | `1` only. Same trust default `NODETOOL_ENV=production` gives, without disabling the local-only features production mode turns off. The packaged desktop app sets it — its optional-node directory holds user-installed code, but it needs the Python bridge, file browser, and the rest of the local surface. An explicit `allowUnlisted` in `packs.json` still wins |
434435
| `NODETOOL_CHAT_DETACH_GRACE_MS` | How long a running chat turn survives with no client attached | no | Default `600000` (10 minutes), then the turn is aborted so an abandoned client cannot leave an agent working forever. See [Chat turn replay](#chat-turn-replay) |
435436
| `NODETOOL_CHAT_REPLAY_RETENTION_MS` | How long a finished turn is kept for a late reconnect | no | Default `300000` (5 minutes) |
436437
| `NODETOOL_CHAT_REPLAY_BUFFER_EVENTS` | Frames buffered per turn for replay | no | Default `2000`. A client whose `last_seq` predates the buffer is told the replay is incomplete and refetches thread history over REST |

docs/developer/custom-nodes-guide.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ To avoid silently running whatever happens to be in `node_modules` in production
160160
- **Allowlist** — a list of trusted pack names; `"*"` allows everything. Set via:
161161
- The env var `NODETOOL_PACKS_ALLOWLIST` (comma-separated names), or
162162
- The `allow` field of `~/.config/nodetool/packs.json` (path overridable with the `NODETOOL_PACKS_CONFIG` env var).
163-
- **`allowUnlisted`** — whether packs not on the allowlist load anyway. Defaults to **`true` in development** (so installing a pack just works) and **`false` in production** (`NODETOOL_ENV=production`). Override via the config file.
163+
- **`allowUnlisted`** — whether packs not on the allowlist load anyway. Defaults to **`true` in development** (so installing a pack just works) and **`false`** when either `NODETOOL_ENV=production` or `NODETOOL_PACKS_REQUIRE_ALLOWLIST=1` is set. The packaged desktop app sets the latter — it needs the allowlist without production mode, which would disable local-only features. Override via the config file.
164164

165165
Two further guards protect the registry regardless of trust:
166166

electron/src/__tests__/server.spawn.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,17 +63,21 @@ describe("backend utilityProcess spawn contract", () => {
6363
"STATIC_FOLDER",
6464
"NODETOOL_PYTHON",
6565
"NODE_ENV",
66-
"NODETOOL_ENV",
66+
"NODETOOL_PACKS_REQUIRE_ALLOWLIST",
6767
"NODE_OPTIONS",
6868
"NODE_PATH",
6969
];
70+
// NODETOOL_ENV must stay OUT of this shape: production mode disables
71+
// local-only server features (Python bridge, file browser, vector nodes)
72+
// that the desktop app depends on. Pack trust comes from the dedicated
73+
// NODETOOL_PACKS_REQUIRE_ALLOWLIST flag instead.
7074
const backendEnv: Record<string, string> = {
7175
PORT: "7777",
7276
HOST: "127.0.0.1",
7377
STATIC_FOLDER: "/mock/web",
7478
NODETOOL_PYTHON: "",
7579
NODE_ENV: "production",
76-
NODETOOL_ENV: "production",
80+
NODETOOL_PACKS_REQUIRE_ALLOWLIST: "1",
7781
NODE_OPTIONS: "--conditions=nodetool-dev",
7882
NODE_PATH: "/mock/backend/node_modules",
7983
};
@@ -110,7 +114,7 @@ describe("backend utilityProcess spawn contract", () => {
110114
PORT: "7777",
111115
HOST: "127.0.0.1",
112116
NODE_ENV: "production",
113-
NODETOOL_ENV: "production",
117+
NODETOOL_PACKS_REQUIRE_ALLOWLIST: "1",
114118
}),
115119
}),
116120
);

electron/src/server.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,17 @@ async function startServer(): Promise<void> {
307307
NODETOOL_PYTHON: pythonPath,
308308
NODE_ENV: isDevMode() ? "development" : "production",
309309
// Electron's optional-node directory holds user-installed code in every
310-
// build mode; host packs need an explicit allowlist even during app development.
311-
NODETOOL_ENV: "production",
312-
// The line above asks for the pack allowlist, not for the curated cloud
313-
// catalog. Without an explicit profile, production alone would activate it
314-
// and unregister every local and OAuth-backed provider — Ollama, LM Studio,
315-
// llama.cpp, vLLM, and the Claude subscription — on the one surface where
316-
// they are the point. Set `NODETOOL_NODE_PROFILE=cloud` to opt back in.
310+
// build mode; host packs need an explicit allowlist even during app
311+
// development. This dedicated flag asks for exactly that. The backend must
312+
// NOT run with NODETOOL_ENV=production — the server treats production as
313+
// "hosted cloud" and disables local-only features the desktop app is built
314+
// around: the Python bridge, vector nodes, the file browser, workspaces,
315+
// MCP config, and local model scanning.
316+
NODETOOL_PACKS_REQUIRE_ALLOWLIST: "1",
317+
// Keep the full node catalog and the local providers (Ollama, LM Studio,
318+
// llama.cpp, vLLM, the Claude subscription) — the desktop app is the one
319+
// surface where they are the point. Set `NODETOOL_NODE_PROFILE=cloud` in
320+
// the launching environment to opt into the curated cloud catalog.
317321
NODETOOL_NODE_PROFILE: getProcessEnv()["NODETOOL_NODE_PROFILE"] ?? "full",
318322
NODE_OPTIONS: nodeOptionsParts.filter(Boolean).join(" "),
319323
NODE_PATH: backendNodePath,

packages/node-sdk/src/pack-loader.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,18 @@ export function resolvePackTrust(
267267
const allowlist =
268268
options.allowlist ?? envList ?? fromFile.allow ?? [];
269269

270+
// Unlisted packs are trusted by default only in development. Production
271+
// servers (NODETOOL_ENV=production) and the packaged desktop app
272+
// (NODETOOL_PACKS_REQUIRE_ALLOWLIST=1 — set by Electron, which must NOT run
273+
// in production mode because that disables local-only features like the
274+
// Python bridge and the file browser) both require an explicit allowlist.
270275
const isProd = process.env["NODETOOL_ENV"] === "production";
276+
const requireAllowlist =
277+
process.env["NODETOOL_PACKS_REQUIRE_ALLOWLIST"] === "1";
271278
const allowUnlisted =
272-
options.allowUnlisted ?? fromFile.allowUnlisted ?? !isProd;
279+
options.allowUnlisted ??
280+
fromFile.allowUnlisted ??
281+
!(isProd || requireAllowlist);
273282

274283
return { allowlist, allowUnlisted };
275284
}

packages/node-sdk/tests/pack-loader.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ beforeEach(() => {
6868
mkdirSync(nodeModules, { recursive: true });
6969
for (const key of [
7070
"NODETOOL_ENV",
71+
"NODETOOL_PACKS_REQUIRE_ALLOWLIST",
7172
"NODETOOL_PACKS_ALLOWLIST",
7273
"NODETOOL_PACKS_CONFIG"
7374
]) {
@@ -300,6 +301,14 @@ describe("trust / allowlist", () => {
300301
expect(resolvePackTrust().allowUnlisted).toBe(true);
301302
});
302303

304+
// The packaged desktop app must not run in production mode (that disables
305+
// local-only features), so it requests the allowlist with this flag instead.
306+
it("defaults to allowUnlisted=false when NODETOOL_PACKS_REQUIRE_ALLOWLIST=1", () => {
307+
delete process.env["NODETOOL_ENV"];
308+
process.env["NODETOOL_PACKS_REQUIRE_ALLOWLIST"] = "1";
309+
expect(resolvePackTrust().allowUnlisted).toBe(false);
310+
});
311+
303312
it("reads the allowlist from NODETOOL_PACKS_ALLOWLIST", () => {
304313
process.env["NODETOOL_PACKS_ALLOWLIST"] = "@acme/a, @acme/b";
305314
expect(resolvePackTrust().allowlist).toEqual(["@acme/a", "@acme/b"]);

web/src/stores/WorkflowManagerStore.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,14 @@ export const createWorkflowManagerStore = (queryClient: QueryClient) => {
393393
workflow: persistedWorkflow
394394
});
395395
nodeStore.getState().setWorkflowDirty(false);
396+
} else {
397+
// The save itself succeeded, so the server row now carries this
398+
// response's updated_at. Adopt it as the concurrency token —
399+
// keeping the old one makes every later save and autosave fail
400+
// with an optimistic-concurrency conflict.
401+
nodeStore
402+
.getState()
403+
.setWorkflowUpdatedAt(persistedWorkflow.updated_at);
396404
}
397405
}
398406

web/src/stores/__tests__/WorkflowManagerStore.save.test.ts

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,24 @@ jest.mock("../../trpc/client", () => ({
2222

2323
jest.mock("../NodeStore", () => ({
2424
createNodeStore: (workflow: Workflow) =>
25-
create((_set, get: () => { workflow: Workflow }) => ({
26-
workflow,
27-
nodes: [],
28-
edges: [],
29-
workflowIsDirty: false,
30-
getWorkflow: () => get().workflow,
31-
setWorkflowDirty: jest.fn(),
32-
cleanup: jest.fn()
33-
}))
25+
create(
26+
(
27+
set: (fn: (state: { workflow: Workflow }) => object) => void,
28+
get: () => { workflow: Workflow }
29+
) => ({
30+
workflow,
31+
nodes: [],
32+
edges: [],
33+
workflowIsDirty: false,
34+
getWorkflow: () => get().workflow,
35+
setWorkflowDirty: jest.fn(),
36+
setWorkflowUpdatedAt: (updatedAt: string) =>
37+
set((state) => ({
38+
workflow: { ...state.workflow, updated_at: updatedAt }
39+
})),
40+
cleanup: jest.fn()
41+
})
42+
)
3443
}));
3544

3645
jest.mock("../workflowUpdates", () => ({
@@ -110,6 +119,41 @@ describe("saveWorkflow first save", () => {
110119
expect(updateMutate.mock.calls[1][0].expected_updated_at).toBeUndefined();
111120
});
112121

122+
it("adopts the server revision even when the user edited during the save", async () => {
123+
const store = createWorkflowManagerStore(new QueryClient());
124+
const serverWorkflow: Workflow = {
125+
id: "wf-edited",
126+
name: "Existing",
127+
description: "",
128+
access: "private",
129+
graph: { nodes: [], edges: [] },
130+
created_at: "2026-08-01T00:00:00.000Z",
131+
updated_at: "2026-08-02T00:00:00.000Z"
132+
};
133+
store.getState().addWorkflow(serverWorkflow);
134+
135+
const savedUpdatedAt = "2026-08-03T00:00:00.000Z";
136+
const nodeStore = store.getState().getNodeStore("wf-edited");
137+
// Simulate an edit landing while the save is in flight: the mutation
138+
// replaces the edges array reference before resolving.
139+
updateMutate.mockImplementation(async () => {
140+
nodeStore?.setState({ edges: [] });
141+
return { ...serverWorkflow, updated_at: savedUpdatedAt };
142+
});
143+
144+
await store.getState().saveWorkflow(serverWorkflow);
145+
146+
// The edit survives, but the concurrency token moves to the server's new
147+
// revision — otherwise every later save conflicts.
148+
expect(nodeStore?.getState().workflow.updated_at).toBe(savedUpdatedAt);
149+
await store
150+
.getState()
151+
.saveWorkflow(store.getState().getWorkflow("wf-edited") as Workflow);
152+
expect(updateMutate.mock.calls[1][0].expected_updated_at).toBe(
153+
savedUpdatedAt
154+
);
155+
});
156+
113157
it("sends expected_updated_at for a workflow that came from the server", async () => {
114158
const store = createWorkflowManagerStore(new QueryClient());
115159
const serverWorkflow: Workflow = {

0 commit comments

Comments
 (0)