From 9f5ba09a0dc2139a969f3729ed4d9db86f4808db Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 06:17:51 -0400 Subject: [PATCH] fix(sessions): prevent lifecycle races and unsafe session management (#114477) * fix(sessions): harden lifecycle, ownership, and subagent spawning * test(agents): refresh session tool prompt snapshots * fix(sessions): await rebuilding transcript projections --------- Co-authored-by: Peter Steinberger --- docs/concepts/session-tool.md | 11 +- docs/tools/subagents.md | 5 +- ...tools.create-openclaw-coding-tools.test.ts | 17 ++ src/agents/agent-tools.ts | 4 +- .../run/attempt-bundle-tools.test.ts | 78 ++++- .../run/attempt-bundle-tools.ts | 8 + .../run/attempt-tool-base-prepare.ts | 3 + src/agents/tool-policy.ts | 2 +- src/agents/tools/sessions-spawn-tool.test.ts | 132 ++++++-- src/agents/tools/sessions-spawn-tool.ts | 25 +- src/agents/tools/sessions-spawn-visible.ts | 6 +- src/agents/tools/sessions-tool.test.ts | 140 ++++++++- src/agents/tools/sessions-tool.ts | 53 +++- .../session-accessor.sqlite-active-events.ts | 13 +- src/config/sessions/session-accessor.ts | 1 + src/gateway/server-methods/sessions-create.ts | 1 + src/gateway/server-methods/sessions-delete.ts | 8 +- .../server-methods/sessions-mutations.ts | 26 ++ src/gateway/server-methods/sessions-shared.ts | 28 +- src/gateway/server.sessions.create.test.ts | 185 +++++++++++ .../server.sessions.plugin-ownership.test.ts | 286 ++++++++++++++++++ src/gateway/session-create-service.ts | 120 ++++++-- src/gateway/session-plugin-ownership.ts | 30 ++ src/gateway/session-reset-service.ts | 41 +++ .../session-transcript-readers.test.ts | 39 ++- src/gateway/session-transcript-readers.ts | 15 +- .../codex-dynamic-tools.discord-group.json | 5 + .../codex-dynamic-tools.heartbeat-turn.json | 5 + .../codex-dynamic-tools.telegram-direct.json | 5 + .../discord-group-codex-message-tool.md | 8 +- .../telegram-direct-codex-message-tool.md | 8 +- .../telegram-heartbeat-codex-tool.md | 8 +- 32 files changed, 1214 insertions(+), 102 deletions(-) create mode 100644 src/gateway/server.sessions.plugin-ownership.test.ts create mode 100644 src/gateway/session-plugin-ownership.ts diff --git a/docs/concepts/session-tool.md b/docs/concepts/session-tool.md index 6f5566ae2c4c..d4861b194591 100644 --- a/docs/concepts/session-tool.md +++ b/docs/concepts/session-tool.md @@ -13,7 +13,7 @@ OpenClaw gives agents tools to work across sessions, inspect status, and orchest | Tool | What it does | | -------------------- | --------------------------------------------------------------------------- | -| `sessions` | Patch visible session settings and manage the global session-group catalog | +| `sessions` | Patch, reset, or delete visible sessions and manage session groups | | `sessions_list` | List sessions with optional filters (kind, label, agent, archive, preview) | | `sessions_search` | Search visible session transcripts and return matching excerpts | | `sessions_history` | Read the transcript of a specific session | @@ -58,11 +58,15 @@ Use [`sessions_search`](/concepts/session-search) for exact full-text recall acr ## Managing session settings and groups -The owner-gated `sessions` tool exposes two bounded self-service surfaces: +The owner-gated `sessions` tool exposes bounded self-service surfaces: -- `action: "patch"` changes the current session by default, or another visible session selected by `sessionKey`. It can set the label, sidebar icon, pin/archive state, model, and thinking level. It does not expose reset, delete, or compact actions. +- `action: "patch"` changes the current session by default, or another visible session selected by `sessionKey`. It can set the label, sidebar icon, pin/archive state, model, and thinking level. +- `action: "reset"` resets another visible session selected by `sessionKey`. +- `action: "delete"` first archives and then deletes the exact same generation of another visible session selected by `sessionKey`. By default its transcript is retained as a deleted archive; pass `deleteTranscript: false` to leave the transcript state untouched. Resetting or deleting the session currently running the tool is rejected. - `group_list`, `group_set`, `group_rename`, and `group_delete` manage the global ordered session-group catalog. `group_set` replaces the ordered name list rather than patching one entry. +Use `sessions_spawn` with `visible: true` to create a persistent dashboard session. This keeps session creation on the controlled spawn path, which enforces the parent's tool policy, sandbox, concurrency limits, and run timeout. + An agent-selected model patch stays reversible until that selection completes a successful run. If the selected model is definitively unusable because of authentication, billing, or model-not-found failure, OpenClaw restores the previous model and writes a visible system note. Transient rate-limit, overload, timeout, network, and server failures do not undo the selection. ## Sessions versus conversations @@ -120,6 +124,7 @@ Key options: - `runtime: "subagent"` (default) or `"acp"` for external harness agents. - `model` and `thinking` overrides for the child session. +- `runTimeoutSeconds` to override the configured child-run timeout; `0` disables it. - `thread: true` to bind the spawn to a chat thread (Discord, Slack, etc.). - `sandbox: "require"` to enforce sandboxing on the child. - `context: "fork"` for native sub-agents when the child needs the current requester transcript; omit it or use `context: "isolated"` for a clean child. `context: "fork"` is only valid with `runtime: "subagent"`. Thread-bound native sub-agents default to `context: "fork"` unless `threadBindings.defaultSpawnContext` says otherwise. diff --git a/docs/tools/subagents.md b/docs/tools/subagents.md index a62eb8f9bafb..3e2337fed378 100644 --- a/docs/tools/subagents.md +++ b/docs/tools/subagents.md @@ -147,7 +147,7 @@ session to confirm the effective tool list. - **Model:** native sub-agents inherit the caller unless you set `agents.defaults.subagents.model` (or per-agent `agents.entries.*.subagents.model`). ACP runtime spawns use the same configured subagent model when present; otherwise the ACP harness keeps its own default. An explicit `sessions_spawn.model` still wins. - **Thinking:** native sub-agents inherit the caller unless you set `agents.defaults.subagents.thinking` (or per-agent `agents.entries.*.subagents.thinking`). ACP runtime spawns also apply `agents.defaults.models["provider/model"].params.thinking` for the selected model. An explicit `sessions_spawn.thinking` still wins. -- **Run timeout:** OpenClaw uses `agents.defaults.subagents.runTimeoutSeconds` when set; otherwise it falls back to `0` (no timeout). `sessions_spawn` does not accept per-call timeout overrides. +- **Run timeout:** pass `runTimeoutSeconds` to set a timeout for a specific native, ACP, or visible sub-agent run. When omitted, OpenClaw uses `agents.defaults.subagents.runTimeoutSeconds` if configured; otherwise it falls back to `0` (no timeout). An explicit `0` disables the timeout for that run. - **Process lifetime:** a detached OpenClaw sub-agent has its own run lifecycle. A background task created inside an external CLI backend is different: it shares the parent CLI subprocess and stops if that parent reaches `agents.defaults.timeoutSeconds`. - **Task delivery:** native sub-agents receive the delegated task in their first visible `[Subagent Task]` message. The sub-agent system prompt carries runtime rules and routing context, not a hidden duplicate of the task. @@ -212,6 +212,9 @@ Per-agent override: `agents.entries.*.subagents.delegationMode`. Override the sub-agent model. Invalid values are skipped and the sub-agent runs on the default model with a warning in the tool result. + + Override the configured run timeout for this child. Must be a non-negative integer; `0` disables the timeout. Applies to native, ACP, and visible sessions. + Override thinking level for the sub-agent run. Not available with `visible: true`. diff --git a/src/agents/agent-tools.create-openclaw-coding-tools.test.ts b/src/agents/agent-tools.create-openclaw-coding-tools.test.ts index ed2f3abc8d14..b028566adda1 100644 --- a/src/agents/agent-tools.create-openclaw-coding-tools.test.ts +++ b/src/agents/agent-tools.create-openclaw-coding-tools.test.ts @@ -535,6 +535,23 @@ describe("createOpenClawCodingTools", () => { expect(inheritedAllow?.includes("exec")).toBe(false); }); + it("keeps restricted spawn inheritance in the caller-owned runtime snapshot", () => { + const createOpenClawToolsMock = vi.mocked(createOpenClawTools); + createOpenClawToolsMock.mockClear(); + const inheritedToolAllowlistRef: string[] = []; + + createOpenClawCodingTools({ + config: { tools: { allow: ["read", "sessions_spawn"] } }, + inheritedToolAllowlistRef, + }); + + expect(latestCreateOpenClawToolsOptions().inheritedToolAllowlist).toBe( + inheritedToolAllowlistRef, + ); + expectListIncludes(inheritedToolAllowlistRef, ["read", "sessions_spawn"]); + expect(inheritedToolAllowlistRef).not.toContain("exec"); + }); + it("preserves runtime-allowed message through restrictive profiles", () => { const tools = createOpenClawCodingTools({ config: { tools: { profile: "minimal" } }, diff --git a/src/agents/agent-tools.ts b/src/agents/agent-tools.ts index a14b6072de29..139d6f99fdd1 100644 --- a/src/agents/agent-tools.ts +++ b/src/agents/agent-tools.ts @@ -404,6 +404,8 @@ type OpenClawCodingToolsOptions = { runtimeToolAllowlist?: string[]; /** True when runtimeToolAllowlist is real parent authority that child sessions inherit. */ inheritRuntimeToolAllowlist?: boolean; + /** Mutable spawn capability snapshot refreshed after late-bound runtime tools are authorized. */ + inheritedToolAllowlistRef?: string[]; /** Mutable cron creator cap ref for callers that append final runtime tools later. */ cronCreatorToolAllowlistRef?: CronCreatorToolAllowlistEntry[]; /** If true, the model has native vision capability */ @@ -805,7 +807,7 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions) const inheritedToolDenylist = [...pluginToolDenylist]; // Passed by reference to sessions_spawn and populated after the final policy // pass so child sessions inherit the actual parent tool surface. - const inheritedToolAllowlist: string[] = []; + const inheritedToolAllowlist = options?.inheritedToolAllowlistRef ?? []; const toolPolicyInheritanceSources = capabilityProfile.policy.inheritancePolicies; const shouldInheritEffectiveToolAllowlist = toolPolicyInheritanceSources.some(hasRestrictiveAllowPolicy); diff --git a/src/agents/embedded-agent-runner/run/attempt-bundle-tools.test.ts b/src/agents/embedded-agent-runner/run/attempt-bundle-tools.test.ts index dced1df5cbfd..7521e0297828 100644 --- a/src/agents/embedded-agent-runner/run/attempt-bundle-tools.test.ts +++ b/src/agents/embedded-agent-runner/run/attempt-bundle-tools.test.ts @@ -17,7 +17,15 @@ vi.mock("../../agent-bundle-mcp-tools.js", () => ({ })); vi.mock("../../runtime-plan/tools.js", () => ({ - normalizeAgentRuntimeTools: vi.fn(() => []), + normalizeAgentRuntimeTools: vi.fn(({ tools }: { tools: unknown[] }) => tools), +})); + +vi.mock("../../local-model-lean.js", () => ({ + filterLocalModelLeanTools: vi.fn(({ tools }: { tools: unknown[] }) => tools), +})); + +vi.mock("../../tool-schema-projection.js", () => ({ + filterRuntimeCompatibleTools: vi.fn((tools: unknown[]) => ({ tools, diagnostics: [] })), })); vi.mock("../effective-tool-policy.js", () => ({ @@ -35,6 +43,74 @@ import { prepareEmbeddedAttemptBundleTools } from "./attempt-bundle-tools.js"; describe("prepareEmbeddedAttemptBundleTools", () => { beforeEach(() => { vi.clearAllMocks(); + mocks.createBundleLspToolRuntime.mockReset().mockResolvedValue(undefined); + mocks.getOrCreateSessionMcpRuntime.mockReset().mockResolvedValue(undefined); + mocks.materializeBundleMcpToolsForRun.mockReset().mockResolvedValue(undefined); + mocks.applyFinalEffectiveToolPolicy + .mockReset() + .mockImplementation(({ bundledTools }: { bundledTools: unknown[] }) => bundledTools); + }); + + function createInput(inheritedToolAllowlist: string[], toolsRaw: unknown[]) { + return { + agentDir: "/tmp/agent", + attempt: { + config: {}, + model: {}, + modelId: "model", + provider: "provider", + runId: "run", + runtimePlan: {}, + sessionId: "session", + }, + effectiveWorkspace: "/tmp/workspace", + getCurrentAttemptPluginMetadataSnapshot: () => undefined, + getProviderRuntimeHandle: () => undefined, + isRawModelRun: false, + preparedToolBase: { + cronCreatorToolAllowlist: [], + effectiveToolsAllow: undefined, + inheritedToolAllowlist, + localModelLeanPreserveToolNames: [], + runtimeCapabilityProfile: undefined, + toolsEnabled: true, + toolsRaw, + }, + sessionAgentId: "main", + } as unknown as Parameters[0]; + } + + it("refreshes spawned-child inheritance after authorized MCP tools materialize", async () => { + const inheritedToolAllowlist = ["sessions_spawn"]; + mocks.getOrCreateSessionMcpRuntime.mockResolvedValue({}); + mocks.materializeBundleMcpToolsForRun.mockResolvedValue({ + tools: [{ name: "server__read" }], + }); + + await prepareEmbeddedAttemptBundleTools( + createInput(inheritedToolAllowlist, [{ name: "sessions_spawn" }]), + ); + + expect(inheritedToolAllowlist).toEqual(["sessions_spawn", "server__read"]); + }); + + it("never adds policy-denied bundled tools to spawned-child inheritance", async () => { + const inheritedToolAllowlist = ["sessions_spawn"]; + mocks.getOrCreateSessionMcpRuntime.mockResolvedValue({}); + mocks.materializeBundleMcpToolsForRun.mockResolvedValue({ + tools: [{ name: "server__read" }, { name: "server__delete" }], + }); + mocks.applyFinalEffectiveToolPolicy.mockImplementation( + ({ bundledTools }: { bundledTools: Array<{ name: string }> }) => + bundledTools.filter((tool) => tool.name !== "server__delete"), + ); + + await prepareEmbeddedAttemptBundleTools( + createInput(inheritedToolAllowlist, [{ name: "sessions_spawn" }]), + ); + + expect(inheritedToolAllowlist).toEqual(["sessions_spawn", "server__read"]); + expect(inheritedToolAllowlist).not.toContain("server__delete"); }); it("disposes prepared bundle runtimes when later policy setup fails", async () => { diff --git a/src/agents/embedded-agent-runner/run/attempt-bundle-tools.ts b/src/agents/embedded-agent-runner/run/attempt-bundle-tools.ts index f45ab9404be9..9e4b0a1530ff 100644 --- a/src/agents/embedded-agent-runner/run/attempt-bundle-tools.ts +++ b/src/agents/embedded-agent-runner/run/attempt-bundle-tools.ts @@ -6,6 +6,7 @@ import { } from "../../agent-bundle-mcp-tools.js"; import { filterLocalModelLeanTools } from "../../local-model-lean.js"; import { normalizeAgentRuntimeTools } from "../../runtime-plan/tools.js"; +import { replaceWithEffectiveToolAllowlist } from "../../tool-policy.js"; import { filterRuntimeCompatibleTools } from "../../tool-schema-projection.js"; import { logRuntimeToolSchemaQuarantine } from "../../tool-schema-quarantine.js"; import { replaceWithEffectiveCronCreatorToolAllowlist } from "../../tools/cron-tool.js"; @@ -36,6 +37,7 @@ export async function prepareEmbeddedAttemptBundleTools(params: { const { cronCreatorToolAllowlist, effectiveToolsAllow, + inheritedToolAllowlist, localModelLeanPreserveToolNames, runtimeCapabilityProfile, toolsEnabled, @@ -199,6 +201,12 @@ export async function prepareEmbeddedAttemptBundleTools(params: { ); } const schemaProjection = filterRuntimeCompatibleTools(projectedTools); + if (inheritedToolAllowlist?.length) { + // Spawn tools close over this ref before MCP/LSP materialize. Refresh it + // only after final policy and schema projection so children inherit the + // parent's complete authorized surface, never denied bundled tools. + replaceWithEffectiveToolAllowlist(inheritedToolAllowlist, schemaProjection.tools); + } logRuntimeToolSchemaQuarantine({ diagnostics: schemaProjection.diagnostics, tools: projectedTools, diff --git a/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts b/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts index 101d194569f6..898884b89217 100644 --- a/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts +++ b/src/agents/embedded-agent-runner/run/attempt-tool-base-prepare.ts @@ -118,6 +118,7 @@ export function prepareEmbeddedAttemptToolBase(params: { : undefined; const toolSearchTargetTranscriptProjections: ToolSearchTargetTranscriptProjection[] = []; const cronCreatorToolAllowlist: CronCreatorToolAllowlistEntry[] = []; + const inheritedToolAllowlist: string[] = []; const spawnWorkspaceDir = params.effectiveCwd !== params.effectiveWorkspace ? params.resolvedWorkspace @@ -303,6 +304,7 @@ export function prepareEmbeddedAttemptToolBase(params: { enableHeartbeatTool: attempt.enableHeartbeatTool, forceHeartbeatTool: attempt.forceHeartbeatTool, runtimeToolAllowlist: effectiveToolsAllow, + inheritedToolAllowlistRef: inheritedToolAllowlist, cronCreatorToolAllowlistRef: cronCreatorToolAllowlist, authProfileStore: attempt.authProfileStore, recordToolPrepStage: params.markCoreToolStage, @@ -335,6 +337,7 @@ export function prepareEmbeddedAttemptToolBase(params: { computerContextEpoch, cronCreatorToolAllowlist, effectiveToolsAllow, + inheritedToolAllowlist, localModelLeanEnabled, localModelLeanPreserveToolNames, replaySafetyOptions, diff --git a/src/agents/tool-policy.ts b/src/agents/tool-policy.ts index 26bddacee308..f5fc03dfe672 100644 --- a/src/agents/tool-policy.ts +++ b/src/agents/tool-policy.ts @@ -71,7 +71,7 @@ export function hasRestrictiveAllowPolicy(policy?: { allow?: string[] }): boolea /** Replaces an allowlist with the normalized names of an effective tool array. */ export function replaceWithEffectiveToolAllowlist( target: string[], - tools: Array<{ name: string }>, + tools: ReadonlyArray<{ name: string }>, ): void { target.length = 0; const seen = new Set(); diff --git a/src/agents/tools/sessions-spawn-tool.test.ts b/src/agents/tools/sessions-spawn-tool.test.ts index 66422759443a..249227801f17 100644 --- a/src/agents/tools/sessions-spawn-tool.test.ts +++ b/src/agents/tools/sessions-spawn-tool.test.ts @@ -250,16 +250,17 @@ describe("sessions_spawn tool", () => { expect(schema.properties?.runtime?.enum).toEqual(["subagent", "acp"]); }); - it("does not expose timeout override fields to the model", () => { + it("exposes the canonical per-run timeout without advertising a wait-timeout alias", () => { const tool = createSessionsSpawnTool(); const schema = tool.parameters as { properties?: { - runTimeoutSeconds?: unknown; + runTimeoutSeconds?: { type?: string; minimum?: number; description?: string }; timeoutSeconds?: unknown; }; }; - expect(schema.properties?.runTimeoutSeconds).toBeUndefined(); + expect(schema.properties?.runTimeoutSeconds).toMatchObject({ type: "integer", minimum: 0 }); + expect(schema.properties?.runTimeoutSeconds?.description).toContain("configured subagent"); expect(schema.properties?.timeoutSeconds).toBeUndefined(); }); @@ -494,6 +495,38 @@ describe("sessions_spawn tool", () => { expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); }); + it("applies a per-run timeout to visible dashboard sessions", async () => { + const callGateway = vi.fn(async () => ({ + key: "agent:main:dashboard:timed-child", + runStarted: true, + runId: "run-visible-timed", + })); + const registerRun = vi.fn(); + const tool = createSessionsSpawnTool({ + agentSessionKey: "agent:main:main", + config: { + agents: { + defaults: { subagents: { runTimeoutSeconds: 120 } }, + list: [{ id: "main" }], + }, + }, + callGateway: callGateway as never, + registerRun, + countActiveRuns: () => 0, + }); + + const result = await tool.execute("visible-timeout", { + task: "inspect issue", + visible: true, + runTimeoutSeconds: 7, + }); + + expect(result.details).toMatchObject({ status: "accepted", runId: "run-visible-timed" }); + expect(registerRun).toHaveBeenCalledWith( + expect.objectContaining({ runId: "run-visible-timed", runTimeoutSeconds: 7 }), + ); + }); + it("uses the target agent model for cross-agent visible sessions", async () => { const callGateway = vi.fn(async () => ({ key: "agent:reviewer:dashboard:child", @@ -1234,28 +1267,68 @@ describe("sessions_spawn tool", () => { expect(result.details).not.toHaveProperty("role"); }); - it.each([ - "runTimeoutSeconds", - "timeoutSeconds", - "run_timeout_seconds", - "timeout_seconds", - ] as const)("rejects stale timeout override argument %s", async (timeoutParam) => { - const tool = createSessionsSpawnTool({ - agentSessionKey: "agent:main:main", - }); + it.each(["runTimeoutSeconds", "run_timeout_seconds"] as const)( + "forwards native per-run timeout argument %s", + async (timeoutParam) => { + const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main" }); - await expect( - tool.execute("call-stale-timeout-override", { + await tool.execute("call-run-timeout", { task: "do thing", [timeoutParam]: 2, - }), - ).rejects.toThrow( - `sessions_spawn does not support per-call "${timeoutParam}". Configure agents.defaults.subagents.runTimeoutSeconds instead.`, - ); + }); - expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); + expect(hoisted.spawnSubagentDirectMock).toHaveBeenCalledWith( + expect.objectContaining({ task: "do thing", runTimeoutSeconds: 2 }), + expect.any(Object), + ); + }, + ); + + it("forwards a zero native timeout so a child can explicitly disable the default", async () => { + const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main" }); + + await tool.execute("call-no-timeout", { task: "do thing", runTimeoutSeconds: 0 }); + + expect(hoisted.spawnSubagentDirectMock).toHaveBeenCalledWith( + expect.objectContaining({ task: "do thing", runTimeoutSeconds: 0 }), + expect.any(Object), + ); }); + it.each([-1, 0.5, Number.NaN, Number.POSITIVE_INFINITY, "not-a-number"])( + "rejects invalid native timeout %s before spawning", + async (runTimeoutSeconds) => { + const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main" }); + + await expect( + tool.execute("call-invalid-timeout", { task: "do thing", runTimeoutSeconds }), + ).rejects.toThrow("runTimeoutSeconds must be a non-negative integer"); + + expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); + expect(hoisted.spawnAcpDirectMock).not.toHaveBeenCalled(); + }, + ); + + it.each(["timeoutSeconds", "timeout_seconds"] as const)( + "rejects ambiguous wait-timeout argument %s", + async (timeoutParam) => { + const tool = createSessionsSpawnTool({ + agentSessionKey: "agent:main:main", + }); + + await expect( + tool.execute("call-stale-timeout-override", { + task: "do thing", + [timeoutParam]: 2, + }), + ).rejects.toThrow( + `sessions_spawn does not support "${timeoutParam}". Use "runTimeoutSeconds" for a per-run timeout.`, + ); + + expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); + }, + ); + it("passes inherited workspaceDir from tool context, not from tool args", async () => { const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main", @@ -1491,6 +1564,27 @@ describe("sessions_spawn tool", () => { expect(spawnArgs.model).toBe("github-copilot/claude-sonnet-4.6"); }); + it("forwards a per-run timeout to ACP runtime spawns", async () => { + registerAcpBackendForTest(); + const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main" }); + + await tool.execute("call-acp-timeout", { + runtime: "acp", + task: "investigate the failing CI run", + agentId: "codex", + runTimeoutSeconds: 45, + }); + + expect(hoisted.spawnAcpDirectMock).toHaveBeenCalledWith( + expect.objectContaining({ + task: "investigate the failing CI run", + agentId: "codex", + runTimeoutSeconds: 45, + }), + expect.any(Object), + ); + }); + it("adds requested role to forwarded ACP failures", async () => { registerAcpBackendForTest(); hoisted.spawnAcpDirectMock.mockResolvedValueOnce({ diff --git a/src/agents/tools/sessions-spawn-tool.ts b/src/agents/tools/sessions-spawn-tool.ts index c096912716b9..5ca395dfda07 100644 --- a/src/agents/tools/sessions-spawn-tool.ts +++ b/src/agents/tools/sessions-spawn-tool.ts @@ -44,6 +44,7 @@ import type { AnyAgentTool } from "./common.js"; import { jsonResult, normalizeToolModelOverride, + readNonNegativeIntegerParam, readStringParam, ToolInputError, } from "./common.js"; @@ -67,11 +68,6 @@ const UNSUPPORTED_SESSIONS_SPAWN_PARAM_KEYS = [ "replyTo", "reply_to", ] as const; -const UNSUPPORTED_SESSIONS_SPAWN_TIMEOUT_PARAM_KEYS = [ - "runTimeoutSeconds", - "timeoutSeconds", -] as const; - type AcpSpawnModule = typeof import("../acp-spawn.js"); const acpSpawnModuleLoader = createLazyImportLoader( @@ -151,6 +147,13 @@ function createSessionsSpawnToolSchema(params: { ), agentId: Type.Optional(Type.String()), model: Type.Optional(Type.String()), + runTimeoutSeconds: Type.Optional( + Type.Integer({ + minimum: 0, + description: + "Per-run timeout in seconds; overrides the configured subagent default. Zero disables the timeout.", + }), + ), thinking: Type.Optional( Type.String({ description: "Thinking override; unavailable with visible=true." }), ), @@ -324,17 +327,14 @@ export function createSessionsSpawnTool( `sessions_spawn does not support "${unsupportedParam}". Use "message" or "sessions_send" for channel delivery.`, ); } - const unsupportedTimeoutParam = UNSUPPORTED_SESSIONS_SPAWN_TIMEOUT_PARAM_KEYS.find((key) => - resolveSnakeCaseParamKey(params, key), - ); + const unsupportedTimeoutParam = resolveSnakeCaseParamKey(params, "timeoutSeconds"); if (unsupportedTimeoutParam) { - const providedTimeoutParam = - resolveSnakeCaseParamKey(params, unsupportedTimeoutParam) ?? unsupportedTimeoutParam; throw new ToolInputError( - `sessions_spawn does not support per-call "${providedTimeoutParam}". Configure agents.defaults.subagents.runTimeoutSeconds instead.`, + `sessions_spawn does not support "${unsupportedTimeoutParam}". Use "runTimeoutSeconds" for a per-run timeout.`, ); } const task = readStringParam(params, "task", { required: true }); + const runTimeoutSeconds = readNonNegativeIntegerParam(params, "runTimeoutSeconds"); const taskNameResult = normalizeSubagentTaskName(params.taskName); if (taskNameResult.error) { return jsonResult({ @@ -370,6 +370,7 @@ export function createSessionsSpawnTool( label, runtime, requestedAgentId, + runTimeoutSeconds, sandbox, options: opts, }); @@ -445,6 +446,7 @@ export function createSessionsSpawnTool( resumeSessionId, model: modelOverride, thinking: thinkingOverrideRaw, + ...(runTimeoutSeconds !== undefined ? { runTimeoutSeconds } : {}), cwd, mode: mode === "run" || mode === "session" ? mode : undefined, thread, @@ -485,6 +487,7 @@ export function createSessionsSpawnTool( agentId: requestedAgentId, model: modelOverride, thinking: thinkingOverrideRaw, + ...(runTimeoutSeconds !== undefined ? { runTimeoutSeconds } : {}), collect: hasCollectParam ? collect : undefined, outputSchema: params.outputSchema && typeof params.outputSchema === "object" diff --git a/src/agents/tools/sessions-spawn-visible.ts b/src/agents/tools/sessions-spawn-visible.ts index af10076e7b84..0bc4a0edfbb6 100644 --- a/src/agents/tools/sessions-spawn-visible.ts +++ b/src/agents/tools/sessions-spawn-visible.ts @@ -92,6 +92,7 @@ export async function maybeSpawnVisibleSession(params: { label: string; runtime: "subagent" | "acp"; requestedAgentId?: string; + runTimeoutSeconds?: number; sandbox: "inherit" | "require"; options?: VisibleSessionsSpawnOptions; }): Promise | undefined> { @@ -227,7 +228,10 @@ export async function maybeSpawnVisibleSession(params: { } const resolvedModel = modelOverride ?? resolveSubagentSpawnModelSelection({ cfg, agentId: targetAgentId }); - const runTimeoutSeconds = resolveConfiguredSubagentRunTimeoutSeconds({ cfg }); + const runTimeoutSeconds = resolveConfiguredSubagentRunTimeoutSeconds({ + cfg, + runTimeoutSeconds: params.runTimeoutSeconds, + }); const requesterRuntime = resolveSandboxRuntimeStatus({ cfg, sessionKey: requesterKey }); const childRuntime = resolveSandboxRuntimeStatus({ cfg, diff --git a/src/agents/tools/sessions-tool.test.ts b/src/agents/tools/sessions-tool.test.ts index 7ac499ed8435..2f9075d7831b 100644 --- a/src/agents/tools/sessions-tool.test.ts +++ b/src/agents/tools/sessions-tool.test.ts @@ -45,8 +45,17 @@ describe("sessions tool", () => { properties: { action: { type: "string", - enum: ["patch", "group_list", "group_set", "group_rename", "group_delete"], + enum: [ + "patch", + "reset", + "delete", + "group_list", + "group_set", + "group_rename", + "group_delete", + ], }, + deleteTranscript: { type: "boolean" }, label: { type: "string", description: expect.stringContaining("Empty string clears") }, statusNote: { type: "string", maxLength: 120 }, attention: { @@ -57,6 +66,135 @@ describe("sessions tool", () => { archived: { type: "boolean", description: expect.stringContaining("without deleting") }, }, }); + expect(tool.parameters).not.toHaveProperty("properties.message"); + }); + + it("does not expose direct session creation outside controlled spawning", async () => { + const callGateway = vi.fn(); + const tool = createSessionsTool({ + agentSessionKey: "agent:main:main", + config: {}, + callGateway, + }); + + await expect(tool.execute("create-uncontrolled-session", { action: "create" })).rejects.toThrow( + "Unknown action: create", + ); + expect(tool.parameters).not.toHaveProperty("properties.parentSessionKey"); + expect(tool.parameters).not.toHaveProperty("properties.agentId"); + expect(tool.parameters).not.toHaveProperty("properties.fork"); + expect(callGateway).not.toHaveBeenCalled(); + }); + + it("archives a visible target before write-scoped session deletion", async () => { + const sessionKey = "agent:main:dashboard:finished"; + const sessionId = "finished-session"; + const lifecycleRevision = "finished-revision"; + const callGateway = vi.fn(async (method: string) => + method === "sessions.patch" + ? { ok: true, entry: { sessionId, lifecycleRevision } } + : { ok: true, deleted: true }, + ); + const tool = createSessionsTool({ + agentSessionKey: "agent:main:main", + config: { tools: { sessions: { visibility: "agent" } } }, + callGateway: callGateway as never, + }); + + await tool.execute("delete-session", { action: "delete", sessionKey }); + + expect(callGateway.mock.calls).toEqual([ + ["sessions.patch", { key: sessionKey, archived: true }], + [ + "sessions.delete", + { + key: sessionKey, + archivedOnly: true, + expectedSessionId: sessionId, + expectedLifecycleRevision: lifecycleRevision, + deleteTranscript: true, + }, + ], + ]); + }); + + it("forwards an explicit transcript-preservation choice on deletion", async () => { + const sessionKey = "agent:main:dashboard:finished"; + const sessionId = "finished-session"; + const callGateway = vi.fn(async (method: string) => + method === "sessions.patch" + ? { ok: true, entry: { sessionId } } + : { ok: true, deleted: true }, + ); + const tool = createSessionsTool({ + agentSessionKey: "agent:main:main", + config: { tools: { sessions: { visibility: "agent" } } }, + callGateway: callGateway as never, + }); + + await tool.execute("delete-preserve", { + action: "delete", + sessionKey, + deleteTranscript: false, + }); + + expect(callGateway).toHaveBeenLastCalledWith("sessions.delete", { + key: sessionKey, + archivedOnly: true, + expectedSessionId: sessionId, + deleteTranscript: false, + }); + }); + + it("does not delete a session when archive cannot identify its generation", async () => { + const sessionKey = "agent:main:dashboard:finished"; + const callGateway = vi.fn(async () => ({ ok: true })); + const tool = createSessionsTool({ + agentSessionKey: "agent:main:main", + config: { tools: { sessions: { visibility: "agent" } } }, + callGateway: callGateway as never, + }); + + await expect( + tool.execute("delete-missing-generation", { action: "delete", sessionKey }), + ).rejects.toThrow("archive did not return its session identity"); + + expect(callGateway).toHaveBeenCalledTimes(1); + expect(callGateway).toHaveBeenCalledWith("sessions.patch", { + key: sessionKey, + archived: true, + }); + }); + + it("resets another visible session through the canonical gateway method", async () => { + const sessionKey = "agent:main:dashboard:reset-me"; + const callGateway = vi.fn(async () => ({ ok: true })); + const tool = createSessionsTool({ + agentSessionKey: "agent:main:main", + config: { tools: { sessions: { visibility: "agent" } } }, + callGateway: callGateway as never, + }); + + await tool.execute("reset-session", { action: "reset", sessionKey }); + + expect(callGateway).toHaveBeenCalledWith("sessions.reset", { + key: sessionKey, + reason: "reset", + }); + }); + + it.each(["delete", "reset"])("refuses to %s its currently running session", async (action) => { + const callGateway = vi.fn(); + const tool = createSessionsTool({ + agentSessionKey: "agent:main:main", + config: {}, + callGateway, + }); + + await expect( + tool.execute(`self-${action}`, { action, sessionKey: "agent:main:main" }), + ).rejects.toThrow(`Cannot ${action} the session running this tool`); + expect(callGateway).not.toHaveBeenCalled(); }); it("patches its session, then reverts a failed agent-selected model", async () => { diff --git a/src/agents/tools/sessions-tool.ts b/src/agents/tools/sessions-tool.ts index 6955fdad3078..6fc30710c5d2 100644 --- a/src/agents/tools/sessions-tool.ts +++ b/src/agents/tools/sessions-tool.ts @@ -23,7 +23,15 @@ import { import { resolveSessionToolContext } from "./sessions-helpers.js"; import { resolveSessionReference } from "./sessions-resolution.js"; -const ACTIONS = ["patch", "group_list", "group_set", "group_rename", "group_delete"] as const; +const ACTIONS = [ + "patch", + "reset", + "delete", + "group_list", + "group_set", + "group_rename", + "group_delete", +] as const; const GROUP_NAME_MAX_LENGTH = 512; const GROUP_NAMES_MAX_ITEMS = 200; @@ -31,6 +39,9 @@ const SessionsToolSchema = Type.Object( { action: stringEnum(ACTIONS, { description: "Action" }), sessionKey: Type.Optional(Type.String({ description: "Target session. Default: current" })), + deleteTranscript: Type.Optional( + Type.Boolean({ description: "Archive the deleted session transcript. Default: true." }), + ), label: Type.Optional( Type.String({ description: "Sidebar title override. Empty string clears it." }), ), @@ -185,11 +196,49 @@ export function createSessionsTool(opts: SessionsToolOptions = {}): AnyAgentTool label: "Sessions", name: "sessions", description: - "Session settings and groups: patch label/icon/status, pin, archive/restore, model/thinking override; group_list/group_set/group_rename/group_delete.", + "Session settings, reset, delete, and groups: patch label/icon/status, pin, archive/restore, model/thinking override; reset/delete visible sessions; group_list/group_set/group_rename/group_delete.", parameters: SessionsToolSchema, execute: async (_toolCallId, rawArgs) => { const params = rawArgs as Record; const action = readStringParam(params, "action", { required: true }); + if (action === "reset" || action === "delete") { + const rawKey = readStringParam(params, "sessionKey", { required: true }); + const { key } = await resolvePatchTarget( + { ...opts, config: opts.config ?? getRuntimeConfig() }, + rawKey, + ); + const context = resolveSessionToolContext({ + ...opts, + config: opts.config ?? getRuntimeConfig(), + }); + if (key === context.effectiveRequesterKey) { + throw new ToolInputError(`Cannot ${action} the session running this tool`); + } + if (action === "reset") { + return jsonResult(await gatewayCall("sessions.reset", { key, reason: "reset" })); + } + // Archive returns the exact row generation. Carry it into the locked + // delete so a concurrent reset cannot delete a replacement session. + const archived = await gatewayCall<{ + entry?: { sessionId?: string; lifecycleRevision?: string }; + }>("sessions.patch", { key, archived: true }); + const expectedSessionId = normalizeOptionalString(archived.entry?.sessionId); + if (!expectedSessionId) { + throw new ToolInputError("Session archive did not return its session identity"); + } + const expectedLifecycleRevision = normalizeOptionalString( + archived.entry?.lifecycleRevision, + ); + return jsonResult( + await gatewayCall("sessions.delete", { + key, + archivedOnly: true, + expectedSessionId, + ...(expectedLifecycleRevision ? { expectedLifecycleRevision } : {}), + deleteTranscript: readBoolean(params, "deleteTranscript") ?? true, + }), + ); + } if (action === "group_list") { return jsonResult(await gatewayCall("sessions.groups.list", {})); } diff --git a/src/config/sessions/session-accessor.sqlite-active-events.ts b/src/config/sessions/session-accessor.sqlite-active-events.ts index 7f2ff0002158..c75ce9321a5d 100644 --- a/src/config/sessions/session-accessor.sqlite-active-events.ts +++ b/src/config/sessions/session-accessor.sqlite-active-events.ts @@ -27,7 +27,10 @@ import { toDatabaseOptions, } from "./session-accessor.sqlite-scope.js"; import type { SessionTranscriptProjectionState } from "./session-transcript-index.js"; -import { startSessionTranscriptIndexReconcile } from "./session-transcript-reconcile.js"; +import { + startSessionTranscriptIndexReconcile, + waitForSessionTranscriptIndexReconcile, +} from "./session-transcript-reconcile.js"; type ActiveTranscriptDatabase = Pick< OpenClawAgentKyselyDatabase, @@ -83,6 +86,14 @@ export function isSessionTranscriptProjectionUnavailableError( return error instanceof SessionTranscriptProjectionUnavailableError; } +/** Waits for a projection rebuild already scheduled by a failed transcript read. */ +export async function waitForSessionTranscriptProjection( + scope: SessionTranscriptReadScope, +): Promise { + const resolved = resolveSqliteTranscriptReadScope(scope); + await waitForSessionTranscriptIndexReconcile(toDatabaseOptions(resolved)); +} + type CurrentProjection = { database: OpenClawAgentDatabase; resolved: ReturnType; diff --git a/src/config/sessions/session-accessor.ts b/src/config/sessions/session-accessor.ts index d56b05f77bea..57e52f6cf8a9 100644 --- a/src/config/sessions/session-accessor.ts +++ b/src/config/sessions/session-accessor.ts @@ -216,6 +216,7 @@ export { readSessionTranscriptMessageEvents, readSessionTranscriptVisibleMessageDelta, SessionTranscriptProjectionUnavailableError, + waitForSessionTranscriptProjection, } from "./session-accessor.sqlite-active-events.js"; export type { SessionTranscriptMessageAnchorPage, diff --git a/src/gateway/server-methods/sessions-create.ts b/src/gateway/server-methods/sessions-create.ts index af57ad07c9e3..3a822ff2274c 100644 --- a/src/gateway/server-methods/sessions-create.ts +++ b/src/gateway/server-methods/sessions-create.ts @@ -360,6 +360,7 @@ export const sessionCreateHandlers: GatewayRequestHandlers = { resetMainWhenUnspecified: !hasInitialTurn, commandSource: "webchat", creation: resolveOperatorSessionCreation(client, { allowTrustedHint: true }), + authorizedPluginId: normalizeOptionalString(client?.internal?.pluginRuntimeOwnerId), loadGatewayModelCatalog: context.loadGatewayModelCatalog, afterCreate: hasInitialTurn ? async ({ key, agentId, entry, storePath }) => { diff --git a/src/gateway/server-methods/sessions-delete.ts b/src/gateway/server-methods/sessions-delete.ts index 94c89609e2fa..d70245ba3350 100644 --- a/src/gateway/server-methods/sessions-delete.ts +++ b/src/gateway/server-methods/sessions-delete.ts @@ -32,7 +32,7 @@ import { emitSessionsChanged } from "./session-change-event.js"; import { loadAccessorSessionEntryForGatewayTarget, loadSessionsRuntimeModule, - rejectPluginRuntimeDeleteMismatch, + rejectPluginRuntimeSessionOwnershipMismatch, requireSessionKey, resolveGatewaySessionTargetFromKey, resolveSessionWorkerPlacementMutationError, @@ -181,7 +181,8 @@ export const sessionDeleteHandlers: GatewayRequestHandlers = { return; } if ( - rejectPluginRuntimeDeleteMismatch({ + rejectPluginRuntimeSessionOwnershipMismatch({ + action: "delete", client, key: target.canonicalKey ?? key, entry: initialDeleteEntry, @@ -304,7 +305,8 @@ export const sessionDeleteHandlers: GatewayRequestHandlers = { return undefined; } if ( - rejectPluginRuntimeDeleteMismatch({ + rejectPluginRuntimeSessionOwnershipMismatch({ + action: "delete", client, key: canonicalKey ?? key, entry, diff --git a/src/gateway/server-methods/sessions-mutations.ts b/src/gateway/server-methods/sessions-mutations.ts index 7c1200330be5..fceddfb246ea 100644 --- a/src/gateway/server-methods/sessions-mutations.ts +++ b/src/gateway/server-methods/sessions-mutations.ts @@ -45,6 +45,7 @@ import { resolveOperatorSessionCreation } from "./session-creation-provenance.js import { isAgentMainSessionKey, loadSessionsRuntimeModule, + rejectPluginRuntimeSessionOwnershipMismatch, requireSessionKey, resolveGatewaySessionTargetFromKey, resolveSessionWorkerPlacementPatchError, @@ -76,6 +77,17 @@ export const sessionMutationHandlers: GatewayRequestHandlers = { }); const canonicalKey = target.canonicalKey ?? key; const lifecycleEntry = loadSessionEntry(key, { agentId: requestedAgentId }).entry; + if ( + rejectPluginRuntimeSessionOwnershipMismatch({ + action: "patch", + client, + key: canonicalKey, + entry: lifecycleEntry, + respond, + }) + ) { + return; + } const missingHarnessSessionError = resolveMissingAgentHarnessSessionError( canonicalKey, lifecycleEntry, @@ -126,6 +138,19 @@ export const sessionMutationHandlers: GatewayRequestHandlers = { }; const applyPatch = async () => { const currentLifecycleEntry = loadSessionEntry(key, { agentId: requestedAgentId }).entry; + // Recheck inside the lifecycle lock so a replaced row cannot switch + // plugin ownership between authorization and the committed patch. + if ( + rejectPluginRuntimeSessionOwnershipMismatch({ + action: "patch", + client, + key: canonicalKey, + entry: currentLifecycleEntry, + respond, + }) + ) { + return null; + } // A reset queued ahead of archive can rotate the row before this mutation starts. // Never apply stale destructive intent to the replacement session identity. const lifecycleEntryRemoved = @@ -450,6 +475,7 @@ export const sessionMutationHandlers: GatewayRequestHandlers = { reason, commandSource: "gateway:sessions.reset", creation: resolveOperatorSessionCreation(client), + authorizedPluginId: normalizeOptionalString(client?.internal?.pluginRuntimeOwnerId), assertAuthorizedInstance: sessionMutationAuthorization?.assertCurrent, }); if (!result.ok) { diff --git a/src/gateway/server-methods/sessions-shared.ts b/src/gateway/server-methods/sessions-shared.ts index 3a4797437c8e..5a03b9a81010 100644 --- a/src/gateway/server-methods/sessions-shared.ts +++ b/src/gateway/server-methods/sessions-shared.ts @@ -13,6 +13,10 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { normalizeAgentId, parseAgentSessionKey } from "../../routing/session-key.js"; import { createLazyRuntimeModule } from "../../shared/lazy-runtime.js"; +import { + resolvePluginSessionOwnershipError, + type PluginSessionOwnershipAction, +} from "../session-plugin-ownership.js"; import { resolveSessionStoreAgentId, resolveSessionStoreKey } from "../session-store-key.js"; import { resolveFreshestSessionEntryFromStoreKeys, @@ -161,27 +165,23 @@ export function requireSessionKey(key: unknown, respond: RespondFn): string | nu return normalized; } -export function rejectPluginRuntimeDeleteMismatch(params: { +export function rejectPluginRuntimeSessionOwnershipMismatch(params: { + action: PluginSessionOwnershipAction; client: GatewayClient | null; key: string; entry: SessionEntry | undefined; respond: RespondFn; }): boolean { - const pluginOwnerId = normalizeOptionalString(params.client?.internal?.pluginRuntimeOwnerId); - if (!pluginOwnerId || !params.entry) { + const error = resolvePluginSessionOwnershipError({ + action: params.action, + entry: params.entry, + key: params.key, + pluginOwnerId: params.client?.internal?.pluginRuntimeOwnerId, + }); + if (!error) { return false; } - if (normalizeOptionalString(params.entry.pluginOwnerId) === pluginOwnerId) { - return false; - } - params.respond( - false, - undefined, - errorShape( - ErrorCodes.INVALID_REQUEST, - `Plugin "${pluginOwnerId}" cannot delete session "${params.key}" because it did not create it.`, - ), - ); + params.respond(false, undefined, error); return true; } diff --git a/src/gateway/server.sessions.create.test.ts b/src/gateway/server.sessions.create.test.ts index 5b8abd8d7aac..17f29d063fe1 100644 --- a/src/gateway/server.sessions.create.test.ts +++ b/src/gateway/server.sessions.create.test.ts @@ -62,6 +62,72 @@ const { createSessionStoreDir, createSelectedGlobalSessionStore, openClient } = const execFileAsync = promisify(execFile); const tempDirs = useAutoCleanupTempDirTracker(afterEach); +test("sessions.create and sessions.delete preserve every concurrent session lifecycle", async () => { + const { storePath } = await createSessionStoreDir(); + const sessionCount = 24; + + const created = await Promise.all( + Array.from({ length: sessionCount }, (_, index) => + directSessionReq<{ key: string; sessionId: string }>("sessions.create", { + agentId: "main", + label: `Concurrent session ${index}`, + }), + ), + ); + + expect(created.every((result) => result.ok)).toBe(true); + const sessionKeys = created.map((result) => + requireNonEmptyString(result.payload?.key, "concurrent session key"), + ); + const sessionIds = created.map((result) => + requireNonEmptyString(result.payload?.sessionId, "concurrent session id"), + ); + expect(new Set(sessionKeys).size).toBe(sessionCount); + expect(new Set(sessionIds).size).toBe(sessionCount); + for (const [index, sessionKey] of sessionKeys.entries()) { + expect(loadSessionEntry({ sessionKey, storePath })).toMatchObject({ + sessionId: sessionIds[index], + label: `Concurrent session ${index}`, + }); + } + + const deleted = await Promise.all( + sessionKeys.map((key) => + directSessionReq<{ deleted: boolean }>("sessions.delete", { + key, + deleteTranscript: false, + }), + ), + ); + + expect(deleted.every((result) => result.ok && result.payload?.deleted === true)).toBe(true); + for (const sessionKey of sessionKeys) { + expect(loadSessionEntry({ sessionKey, storePath })).toBeUndefined(); + } +}); + +test("concurrent sessions.create requests adopt one canonical keyed session", async () => { + const { storePath } = await createSessionStoreDir(); + const key = "agent:main:dashboard:concurrent-keyed-session"; + + const created = await Promise.all( + Array.from({ length: 16 }, () => + directSessionReq<{ key: string; sessionId: string }>("sessions.create", { + agentId: "main", + key, + }), + ), + ); + + expect(created.every((result) => result.ok)).toBe(true); + expect(new Set(created.map((result) => result.payload?.key))).toEqual(new Set([key])); + const sessionIds = new Set(created.map((result) => result.payload?.sessionId)); + expect(sessionIds.size).toBe(1); + expect(loadSessionEntry({ sessionKey: key, storePath })?.sessionId).toBe( + created[0]?.payload?.sessionId, + ); +}); + test("sessions.create keeps incognito rows process-local through list, spawn, reset, and delete", async () => { const { storePath } = await createSessionStoreDir(); try { @@ -2549,6 +2615,125 @@ test("sessions.create sends selected global initial tasks to the requested agent ws.close(); }); +test("sessions.create gives plugin runtimes an owned root without linking operator sessions", async () => { + const { storePath } = await createSessionStoreDir(); + await writeSessionStore({ + entries: { main: sessionStoreEntry("operator-owned-main") }, + }); + const pluginClient = { + connect: { scopes: ["operator.write"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + const created = await directSessionReq<{ + key: string; + entry: { parentSessionKey?: string; pluginOwnerId?: string }; + }>("sessions.create", { agentId: "main" }, { client: pluginClient }); + + expect(created.ok, JSON.stringify(created.error)).toBe(true); + const key = requireNonEmptyString(created.payload?.key, "plugin-owned root session key"); + expect(created.payload?.entry).toMatchObject({ pluginOwnerId: "memory-core" }); + expect(created.payload?.entry.parentSessionKey).toBeUndefined(); + expect(loadSessionEntry({ sessionKey: key, storePath })).toMatchObject({ + pluginOwnerId: "memory-core", + }); + + const patched = await directSessionReq( + "sessions.patch", + { key, label: "Plugin-owned root" }, + { client: pluginClient }, + ); + + expect(patched.ok, JSON.stringify(patched.error)).toBe(true); + expect(loadSessionEntry({ sessionKey: key, storePath })).toMatchObject({ + label: "Plugin-owned root", + pluginOwnerId: "memory-core", + }); +}); + +test.each([ + { + name: "forking a foreign plugin transcript", + params: { parentSessionKey: "agent:main:dashboard:foreign-owned", fork: true }, + action: "fork", + target: "agent:main:dashboard:foreign-owned", + }, + { + name: "linking a foreign plugin parent", + params: { parentSessionKey: "agent:main:dashboard:foreign-owned" }, + action: "link", + target: "agent:main:dashboard:foreign-owned", + }, + { + name: "forking an operator-owned transcript", + params: { parentSessionKey: "agent:main:dashboard:operator-owned", fork: true }, + action: "fork", + target: "agent:main:dashboard:operator-owned", + }, + { + name: "adopting a foreign plugin session", + params: { key: "agent:main:dashboard:foreign-owned" }, + action: "adopt", + target: "agent:main:dashboard:foreign-owned", + }, + { + name: "adopting an operator-owned session", + params: { key: "agent:main:dashboard:operator-owned" }, + action: "adopt", + target: "agent:main:dashboard:operator-owned", + }, +])("sessions.create prevents plugin runtimes from $name", async ({ params, action, target }) => { + const { storePath } = await createSessionStoreDir(); + await writeSessionStore({ + entries: { + "agent:main:dashboard:foreign-owned": sessionStoreEntry("foreign-session", { + pluginOwnerId: "other-plugin", + }), + "agent:main:dashboard:operator-owned": sessionStoreEntry("operator-session"), + }, + }); + const pluginClient = { + connect: { scopes: ["operator.write"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + const created = await directSessionReq("sessions.create", params, { client: pluginClient }); + + expect(created.ok).toBe(false); + expect(created.error).toMatchObject({ + code: "INVALID_REQUEST", + message: `Plugin "memory-core" cannot ${action} session "${target}" because it did not create it.`, + }); + expect(loadSessionEntry({ sessionKey: target, storePath })).toBeDefined(); +}); + +test("sessions.create allows plugin runtimes to link their own parent session", async () => { + const { storePath } = await createSessionStoreDir(); + const parentSessionKey = "agent:main:dashboard:memory-core-parent"; + await writeSessionStore({ + entries: { + [parentSessionKey]: sessionStoreEntry("memory-core-parent", { + pluginOwnerId: "memory-core", + }), + }, + }); + const pluginClient = { + connect: { scopes: ["operator.write"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + const created = await directSessionReq<{ + key: string; + entry: { parentSessionKey?: string }; + }>("sessions.create", { parentSessionKey }, { client: pluginClient }); + + expect(created.ok, JSON.stringify(created.error)).toBe(true); + expect(created.payload?.entry.parentSessionKey).toBe(parentSessionKey); + expect(loadSessionEntry({ sessionKey: parentSessionKey, storePath })?.pluginOwnerId).toBe( + "memory-core", + ); +}); + test("sessions.create rejects unknown parentSessionKey", async () => { await createSessionStoreDir(); diff --git a/src/gateway/server.sessions.plugin-ownership.test.ts b/src/gateway/server.sessions.plugin-ownership.test.ts new file mode 100644 index 000000000000..67c4778e082a --- /dev/null +++ b/src/gateway/server.sessions.plugin-ownership.test.ts @@ -0,0 +1,286 @@ +import { expect, test } from "vitest"; +import { + loadSessionEntry, + replaceSessionEntry, + replaceSessionEntrySync, +} from "../config/sessions/session-accessor.js"; +import { runExclusiveSessionLifecycleMutation } from "../sessions/session-lifecycle-admission.js"; +import { writeSessionStore } from "./test-helpers.js"; +import { + directSessionReq, + sessionStoreEntry, + setupGatewaySessionsTestHarness, +} from "./test/server-sessions.test-helpers.js"; + +const { createSessionStoreDir } = setupGatewaySessionsTestHarness(); + +test.each([ + { + name: "another plugin's session", + key: "agent:main:dreaming-narrative-foreign", + entry: sessionStoreEntry("foreign-plugin-session", { pluginOwnerId: "other-plugin" }), + }, + { + name: "an operator-owned session", + key: "agent:main:dashboard:operator-owned", + entry: sessionStoreEntry("operator-owned-session"), + }, +])("sessions.reset prevents a plugin from resetting $name", async ({ key, entry }) => { + const { storePath } = await createSessionStoreDir(); + await writeSessionStore({ entries: { [key]: entry } }); + const pluginClient = { + connect: { scopes: ["operator.write"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + const reset = await directSessionReq("sessions.reset", { key }, { client: pluginClient }); + + expect(reset.ok).toBe(false); + expect(reset.error).toMatchObject({ + code: "INVALID_REQUEST", + message: `Plugin "memory-core" cannot reset session "${key}" because it did not create it.`, + }); + expect(loadSessionEntry({ sessionKey: key, storePath })).toMatchObject({ + sessionId: entry.sessionId, + ...(entry.pluginOwnerId ? { pluginOwnerId: entry.pluginOwnerId } : {}), + }); +}); + +test("sessions.reset preserves ownership when a plugin resets its own session", async () => { + const { storePath } = await createSessionStoreDir(); + const sessionKey = "agent:main:dreaming-narrative-owned"; + await writeSessionStore({ + entries: { + [sessionKey]: sessionStoreEntry("owned-plugin-session", { pluginOwnerId: "memory-core" }), + }, + }); + const pluginClient = { + connect: { scopes: ["operator.write"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + const reset = await directSessionReq( + "sessions.reset", + { key: sessionKey }, + { client: pluginClient }, + ); + + expect(reset.ok, JSON.stringify(reset.error)).toBe(true); + expect(loadSessionEntry({ sessionKey, storePath })).toMatchObject({ + pluginOwnerId: "memory-core", + }); +}); + +test("sessions.reset stamps plugin ownership when it materializes a missing session", async () => { + const { storePath } = await createSessionStoreDir(); + const sessionKey = "agent:main:dreaming-narrative-new"; + const pluginClient = { + connect: { scopes: ["operator.write"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + const reset = await directSessionReq( + "sessions.reset", + { key: sessionKey }, + { client: pluginClient }, + ); + + expect(reset.ok, JSON.stringify(reset.error)).toBe(true); + expect(loadSessionEntry({ sessionKey, storePath })).toMatchObject({ + pluginOwnerId: "memory-core", + }); + + const patched = await directSessionReq( + "sessions.patch", + { key: sessionKey, label: "Reset-created plugin session" }, + { client: pluginClient }, + ); + + expect(patched.ok, JSON.stringify(patched.error)).toBe(true); +}); + +test("sessions.reset rechecks plugin ownership inside lifecycle admission", async () => { + const { storePath } = await createSessionStoreDir(); + const sessionKey = "agent:main:dreaming-narrative-owned"; + const sessionId = "owned-plugin-session"; + await writeSessionStore({ + entries: { + [sessionKey]: sessionStoreEntry(sessionId, { pluginOwnerId: "memory-core" }), + }, + }); + const { performGatewaySessionReset } = await import("./session-reset-service.js"); + let replaced = false; + + const reset = await performGatewaySessionReset({ + key: sessionKey, + reason: "reset", + commandSource: "gateway:sessions.reset", + authorizedPluginId: "memory-core", + assertCurrent: () => { + if (replaced) { + return; + } + replaced = true; + replaceSessionEntrySync( + { sessionKey, storePath }, + sessionStoreEntry(sessionId, { pluginOwnerId: "other-plugin" }), + ); + }, + }); + + expect(reset.ok).toBe(false); + if (!reset.ok) { + expect(reset.error.message).toBe( + `Plugin "memory-core" cannot reset session "${sessionKey}" because it did not create it.`, + ); + } + expect(loadSessionEntry({ sessionKey, storePath })).toMatchObject({ + pluginOwnerId: "other-plugin", + sessionId, + }); +}); + +test("sessions.patch limits plugin-runtime mutations to sessions owned by that plugin", async () => { + const { storePath } = await createSessionStoreDir(); + const ownedKey = "agent:main:dreaming-narrative-owned"; + const foreignKey = "agent:main:dreaming-narrative-foreign"; + const operatorKey = "agent:main:dashboard:operator-owned"; + await writeSessionStore({ + entries: { + [ownedKey]: sessionStoreEntry("sess-owned", { pluginOwnerId: "memory-core" }), + [foreignKey]: sessionStoreEntry("sess-foreign", { pluginOwnerId: "other-plugin" }), + [operatorKey]: sessionStoreEntry("sess-operator"), + }, + }); + const pluginClient = { + connect: { scopes: ["operator.admin"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + for (const key of [foreignKey, operatorKey]) { + const denied = await directSessionReq( + "sessions.patch", + { key, label: "unauthorized mutation" }, + { client: pluginClient }, + ); + + expect(denied.ok).toBe(false); + expect(denied.error).toMatchObject({ + code: "INVALID_REQUEST", + message: `Plugin "memory-core" cannot patch session "${key}" because it did not create it.`, + }); + expect(loadSessionEntry({ sessionKey: key, storePath })?.label).toBeUndefined(); + } + + const patched = await directSessionReq( + "sessions.patch", + { key: ownedKey, label: "authorized mutation" }, + { client: pluginClient }, + ); + + expect(patched.ok, JSON.stringify(patched.error)).toBe(true); + expect(loadSessionEntry({ sessionKey: ownedKey, storePath })?.label).toBe("authorized mutation"); +}); + +test("sessions.patch rechecks plugin ownership after waiting for lifecycle admission", async () => { + const { storePath } = await createSessionStoreDir(); + const sessionKey = "agent:main:dreaming-narrative-owned"; + const sessionId = "sess-owned"; + await writeSessionStore({ + entries: { + [sessionKey]: sessionStoreEntry(sessionId, { pluginOwnerId: "memory-core" }), + }, + }); + const pluginClient = { + connect: { scopes: ["operator.admin"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + let releaseMutation = () => {}; + let markMutationStarted = () => {}; + const mutationStarted = new Promise((resolve) => { + markMutationStarted = resolve; + }); + const mutation = runExclusiveSessionLifecycleMutation({ + scope: storePath, + identities: [sessionKey, sessionId], + run: async () => { + markMutationStarted(); + await new Promise((release) => { + releaseMutation = release; + }); + }, + }); + await mutationStarted; + const patch = directSessionReq( + "sessions.patch", + { key: sessionKey, label: "unauthorized raced mutation" }, + { client: pluginClient }, + ); + await Promise.resolve(); + await replaceSessionEntry( + { sessionKey, storePath }, + sessionStoreEntry(sessionId, { pluginOwnerId: "other-plugin" }), + ); + releaseMutation(); + + const [patched] = await Promise.all([patch, mutation]); + + expect(patched.ok).toBe(false); + expect(patched.error?.message).toContain("did not create it"); + expect(loadSessionEntry({ sessionKey, storePath })).toMatchObject({ + pluginOwnerId: "other-plugin", + }); + expect(loadSessionEntry({ sessionKey, storePath })?.label).toBeUndefined(); +}); + +test("sessions.delete protects the archived session generation from a replacement", async () => { + const { storePath } = await createSessionStoreDir(); + const sessionKey = "agent:main:dreaming-narrative-owned"; + const originalSessionId = "original-plugin-session"; + await writeSessionStore({ + entries: { + [sessionKey]: sessionStoreEntry(originalSessionId, { pluginOwnerId: "memory-core" }), + }, + }); + const pluginClient = { + connect: { scopes: ["operator.write"] }, + internal: { pluginRuntimeOwnerId: "memory-core" }, + } as never; + + const archived = await directSessionReq<{ + entry: { sessionId: string; lifecycleRevision?: string }; + }>("sessions.patch", { key: sessionKey, archived: true }, { client: pluginClient }); + + expect(archived.ok, JSON.stringify(archived.error)).toBe(true); + expect(archived.payload?.entry.sessionId).toBe(originalSessionId); + + await replaceSessionEntry( + { sessionKey, storePath }, + sessionStoreEntry("replacement-plugin-session", { + archivedAt: Date.now(), + lifecycleRevision: "replacement-plugin-revision", + pluginOwnerId: "memory-core", + }), + ); + + const deleted = await directSessionReq( + "sessions.delete", + { + key: sessionKey, + archivedOnly: true, + expectedSessionId: originalSessionId, + ...(archived.payload?.entry.lifecycleRevision + ? { expectedLifecycleRevision: archived.payload.entry.lifecycleRevision } + : {}), + }, + { client: pluginClient }, + ); + + expect(deleted.ok).toBe(false); + expect(deleted.error?.message).toBe(`Session ${sessionKey} changed before deletion. Retry.`); + expect(loadSessionEntry({ sessionKey, storePath })).toMatchObject({ + lifecycleRevision: "replacement-plugin-revision", + pluginOwnerId: "memory-core", + sessionId: "replacement-plugin-session", + }); +}); diff --git a/src/gateway/session-create-service.ts b/src/gateway/session-create-service.ts index cd6fe14cedbb..ac0928d03e30 100644 --- a/src/gateway/session-create-service.ts +++ b/src/gateway/session-create-service.ts @@ -72,6 +72,7 @@ import { normalizeSessionDeliveryState } from "../utils/delivery-context.shared. import { ADMIN_SCOPE } from "./operator-scopes.js"; import { buildForkedGatewaySessionEntry } from "./session-create-fork-entry.js"; import { shouldPreserveSessionAuthProfileOverride } from "./session-model-patch-origin.js"; +import { resolvePluginSessionOwnershipError } from "./session-plugin-ownership.js"; import { isSessionVisibilityAllowed, resolveSessionVisibility } from "./session-sharing.js"; import { resolveSessionStoreAgentId, resolveSessionStoreKey } from "./session-store-key.js"; import { loadSessionEntryReadOnly, resolveGatewaySessionStoreTarget } from "./session-utils.js"; @@ -524,6 +525,15 @@ export async function createGatewaySession(params: { ), }; } + const parentOwnershipError = resolvePluginSessionOwnershipError({ + action: params.fork === true ? "fork" : "link", + entry: parent.entry, + key: parent.canonicalKey, + pluginOwnerId: params.authorizedPluginId, + }); + if (parentOwnershipError) { + return { ok: false, error: parentOwnershipError }; + } if (isModelSelectionLocked(parent.entry)) { return { ok: false, @@ -588,11 +598,17 @@ export async function createGatewaySession(params: { } const targetSessionKey = explicitTargetKey ?? buildDashboardSessionKey(agentId, { incognito }); + const creationTarget = resolveGatewaySessionStoreTarget({ + cfg: params.cfg, + key: targetSessionKey, + agentId, + }); const agentMainSessionKey = resolveAgentMainSessionKey({ cfg: params.cfg, agentId }); // Durable dashboard sessions parent to main for flow-up notices and sidebar threads. // Incognito roots omit durable lineage so notices cannot cross the storage boundary. const dashboardParentSessionKey = !parentSessionKey && + !params.authorizedPluginId && !incognito && params.fork !== true && (params.cfg.session?.dmScope ?? "main") === "main" && @@ -674,7 +690,9 @@ export async function createGatewaySession(params: { if ( canonicalParentSessionKey && parentSessionTarget && - (params.emitCommandHooks === true || params.fork === true) + (params.emitCommandHooks === true || + params.fork === true || + params.authorizedPluginId !== undefined) ) { const currentParent = loadSessionEntryReadOnly( canonicalParentSessionKey, @@ -694,18 +712,31 @@ export async function createGatewaySession(params: { }; } currentParentSessionEntry = currentParentEntry; - if (isModelSelectionLocked(currentParentEntry)) { + const parentOwnershipError = resolvePluginSessionOwnershipError({ + action: params.fork === true ? "fork" : "link", + entry: currentParentEntry, + key: canonicalParentSessionKey, + pluginOwnerId: params.authorizedPluginId, + }); + if (parentOwnershipError) { + return { ok: false, error: parentOwnershipError }; + } + if ( + (params.emitCommandHooks === true || params.fork === true) && + isModelSelectionLocked(currentParentEntry) + ) { return { ok: false, error: errorShape(ErrorCodes.INVALID_REQUEST, MODEL_SELECTION_LOCKED_PARENT_FORK_MESSAGE), }; } const parentHasActiveWork = - isEmbeddedAgentRunActive(currentParentEntry.sessionId) || - isSessionWorkAdmissionActive(parentSessionTarget.storePath, [ - canonicalParentSessionKey, - currentParentEntry.sessionId, - ]); + (params.emitCommandHooks === true || params.fork === true) && + (isEmbeddedAgentRunActive(currentParentEntry.sessionId) || + isSessionWorkAdmissionActive(parentSessionTarget.storePath, [ + canonicalParentSessionKey, + currentParentEntry.sessionId, + ])); if (parentHasActiveWork) { return { ok: false, @@ -747,8 +778,7 @@ export async function createGatewaySession(params: { }); } - const key = targetSessionKey; - const target = resolveGatewaySessionStoreTarget({ cfg: params.cfg, key, agentId }); + const target = creationTarget; const created = await createSessionEntryWithTranscript( { agentId: target.agentId, @@ -758,6 +788,15 @@ export async function createGatewaySession(params: { async ({ existingEntry, sessionEntries }) => { // This callback owns generated and explicit keys alike; no existing row // is the canonical signal that this request will actually create one. + const existingOwnershipError = resolvePluginSessionOwnershipError({ + action: "adopt", + entry: existingEntry, + key: target.canonicalKey, + pluginOwnerId: params.authorizedPluginId, + }); + if (existingOwnershipError) { + return { ok: false, error: existingOwnershipError }; + } if ( isAgentHarnessSessionKey(target.canonicalKey) && !authorizedHarnessCreation && @@ -939,8 +978,8 @@ export async function createGatewaySession(params: { ...(params.worktree ? { worktree: params.worktree } : {}), ...(execNode ? { execHost: "node", execNode, ...(execCwd ? { execCwd } : {}) } : {}), ...(initialAgentHarnessId ? { agentHarnessId: initialAgentHarnessId } : {}), - ...(authorizedPluginCreation - ? { pluginOwnerId: params.initialEntry?.pluginOwnerId } + ...(createdNewEntry && params.authorizedPluginId && !params.catalogTarget + ? { pluginOwnerId: params.authorizedPluginId } : {}), ...(authorizedPluginCreation && params.initialEntry?.providerOverride ? { providerOverride: params.initialEntry.providerOverride } @@ -1115,32 +1154,55 @@ export async function createGatewaySession(params: { }; }; + const runWithCreationTargetLock = async () => + await runExclusiveSessionLifecycleMutation({ + scope: creationTarget.storePath, + identities: [creationTarget.canonicalKey], + run: createChildSession, + }); + + let result: CreateGatewaySessionResult; if ( canonicalParentSessionKey && parentSessionEntry?.sessionId && parentSessionTarget && - (params.emitCommandHooks === true || params.fork === true) + (params.emitCommandHooks === true || + params.fork === true || + params.authorizedPluginId !== undefined) ) { - const result = await runExclusiveSessionLifecycleMutation({ - scope: parentSessionTarget.storePath, - identities: [canonicalParentSessionKey, parentSessionEntry.sessionId], - run: createChildSession, - }); - if (result.ok && !result.resetExisting && createdContext) { - // Adoption still runs post-create work (initial chat.send, plugin hooks); - // only the created journal event is reserved for genuinely new rows. - if (createdNewEntry) { - recordSessionCreated({ - sessionKey: createdContext.key, - agentId: createdContext.agentId, - entry: createdContext.entry, + if (parentSessionTarget.storePath === creationTarget.storePath) { + result = await runExclusiveSessionLifecycleMutation({ + scope: creationTarget.storePath, + identities: [ + canonicalParentSessionKey, + parentSessionEntry.sessionId, + creationTarget.canonicalKey, + ], + run: createChildSession, + }); + } else { + const runWithParentLock = async (run: () => Promise) => + await runExclusiveSessionLifecycleMutation({ + scope: parentSessionTarget.storePath, + identities: [canonicalParentSessionKey, parentSessionEntry.sessionId], + run, }); - } - await params.afterCreate?.(createdContext); + // Cross-agent forks touch two stores. Acquire their locks in canonical + // store order so simultaneous opposite-direction forks cannot deadlock. + result = + parentSessionTarget.storePath < creationTarget.storePath + ? await runWithParentLock(runWithCreationTargetLock) + : await runExclusiveSessionLifecycleMutation({ + scope: creationTarget.storePath, + identities: [creationTarget.canonicalKey], + run: async () => await runWithParentLock(createChildSession), + }); } - return result; + } else { + // Keyed creates must observe and adopt the winning row under the same + // lifecycle fence; otherwise concurrent callers mint divergent session IDs. + result = await runWithCreationTargetLock(); } - const result = await createChildSession(); if (result.ok && !result.resetExisting && createdContext) { if (createdNewEntry) { recordSessionCreated({ diff --git a/src/gateway/session-plugin-ownership.ts b/src/gateway/session-plugin-ownership.ts new file mode 100644 index 000000000000..7af03ad93a24 --- /dev/null +++ b/src/gateway/session-plugin-ownership.ts @@ -0,0 +1,30 @@ +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; +import { + ErrorCodes, + errorShape, + type ErrorShape, +} from "../../packages/gateway-protocol/src/index.js"; +import type { SessionEntry } from "../config/sessions.js"; + +export type PluginSessionOwnershipAction = "adopt" | "delete" | "fork" | "link" | "patch" | "reset"; + +/** Plugin callers may access an existing session only when they own its exact row. */ +export function resolvePluginSessionOwnershipError(params: { + action: PluginSessionOwnershipAction; + entry: SessionEntry | undefined; + key: string; + pluginOwnerId: string | null | undefined; +}): ErrorShape | undefined { + const pluginOwnerId = normalizeOptionalString(params.pluginOwnerId); + if ( + !pluginOwnerId || + !params.entry || + normalizeOptionalString(params.entry.pluginOwnerId) === pluginOwnerId + ) { + return undefined; + } + return errorShape( + ErrorCodes.INVALID_REQUEST, + `Plugin "${pluginOwnerId}" cannot ${params.action} session "${params.key}" because it did not create it.`, + ); +} diff --git a/src/gateway/session-reset-service.ts b/src/gateway/session-reset-service.ts index 83872ee36843..55a49acba8a7 100644 --- a/src/gateway/session-reset-service.ts +++ b/src/gateway/session-reset-service.ts @@ -86,6 +86,7 @@ import { noteActiveSessionForShutdown, } from "./active-sessions-shutdown-tracker.js"; import { findDirectChildSessionsForParent } from "./session-child-sessions.js"; +import { resolvePluginSessionOwnershipError } from "./session-plugin-ownership.js"; import { notifyGatewaySessionReset } from "./session-reset-notifications.js"; import { archiveSessionTranscriptsDetailed, @@ -929,6 +930,8 @@ export async function performGatewaySessionReset(params: { commandSource: string; /** Trusted provenance for a reset that materializes a previously missing row. */ creation?: { via: SessionCreatedVia; actor?: SessionCreatedActor }; + /** Exact plugin namespace authorized by the scoped plugin runtime. */ + authorizedPluginId?: string; assertCurrent?: () => void; assertAuthorizedInstance?: () => void; onCommitted?: (commit: { key: string; sessionId: string }) => void; @@ -991,6 +994,15 @@ export async function performGatewaySessionReset(params: { params.key, resetTarget.requestedAgentId ? { agentId: resetTarget.requestedAgentId } : undefined, ).entry; + const initialOwnershipError = resolvePluginSessionOwnershipError({ + action: "reset", + entry: initialResetEntry, + key: resetTarget.target.canonicalKey, + pluginOwnerId: params.authorizedPluginId, + }); + if (initialOwnershipError) { + return { ok: false, error: initialOwnershipError }; + } const missingHarnessSessionError = resolveMissingAgentHarnessSessionError( resetTarget.target.canonicalKey, initialResetEntry, @@ -1045,6 +1057,7 @@ export async function performGatewaySessionReset(params: { }; } let admittedWorkReleased = true; + let resetOwnershipError: ReturnType | undefined; return await runExclusiveSessionLifecycleMutation({ scope: resetTarget.storePath, identities: resetLifecycleIdentities, @@ -1053,6 +1066,21 @@ export async function performGatewaySessionReset(params: { prepare: async () => { params.assertCurrent?.(); params.assertAuthorizedInstance?.(); + const currentEntry = loadSessionEntry( + params.key, + resetTarget.requestedAgentId ? { agentId: resetTarget.requestedAgentId } : undefined, + ).entry; + // Check the locked generation before interrupting any work; a replaced + // foreign row must not be reset or have its admitted run cancelled. + resetOwnershipError = resolvePluginSessionOwnershipError({ + action: "reset", + entry: currentEntry, + key: resetTarget.target.canonicalKey, + pluginOwnerId: params.authorizedPluginId, + }); + if (resetOwnershipError) { + return; + } admittedWorkReleased = await interruptSessionWorkAdmissions({ scope: resetTarget.storePath, identities: resetLifecycleIdentities, @@ -1061,6 +1089,9 @@ export async function performGatewaySessionReset(params: { }, run: async () => { const { cfg, target, storePath, requestedAgentId } = resetTarget; + if (resetOwnershipError) { + return { ok: false, error: resetOwnershipError }; + } if (!admittedWorkReleased) { return { ok: false, @@ -1074,6 +1105,15 @@ export async function performGatewaySessionReset(params: { params.key, requestedAgentId ? { agentId: requestedAgentId } : undefined, ); + const currentOwnershipError = resolvePluginSessionOwnershipError({ + action: "reset", + entry, + key: canonicalKey, + pluginOwnerId: params.authorizedPluginId, + }); + if (currentOwnershipError) { + return { ok: false, error: currentOwnershipError }; + } const placementBlock = entry?.sessionId ? resolveSessionPlacementResetBlock(entry.sessionId) : undefined; @@ -1415,6 +1455,7 @@ export async function performGatewaySessionReset(params: { subject: currentEntry?.subject, groupChannel: currentEntry?.groupChannel, space: currentEntry?.space, + pluginOwnerId: currentEntry?.pluginOwnerId ?? params.authorizedPluginId, cliSessionBindings: currentEntry?.cliSessionBindings, cliSessionIds: currentEntry?.cliSessionIds, claudeCliSessionId: currentEntry?.claudeCliSessionId, diff --git a/src/gateway/session-transcript-readers.test.ts b/src/gateway/session-transcript-readers.test.ts index db847c376a0f..762c23718c95 100644 --- a/src/gateway/session-transcript-readers.test.ts +++ b/src/gateway/session-transcript-readers.test.ts @@ -8,7 +8,10 @@ import { } from "../config/sessions/session-accessor.js"; import { waitForSessionTranscriptIndexReconcile } from "../config/sessions/session-transcript-reconcile.js"; import { formatSqliteSessionFileMarker } from "../config/sessions/sqlite-marker.js"; -import { closeOpenClawAgentDatabasesForTest } from "../state/openclaw-agent-db.js"; +import { + closeOpenClawAgentDatabasesForTest, + openOpenClawAgentDatabase, +} from "../state/openclaw-agent-db.js"; import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; import { captureEnv, setTestEnvValue } from "../test-utils/env.js"; import { readSessionMessagesAroundIdWithStatsAsync } from "./session-transcript-anchor-reader.js"; @@ -362,6 +365,40 @@ describe("session transcript reader facade", () => { ).resolves.toMatchObject({ found: true, seq: 1 }); }); + test("waits for an in-flight SQLite projection before counting messages", async () => { + const sessionId = "reader-sqlite-rebuilding-count"; + const scope = { + agentId: "main", + sessionId, + sessionKey: `agent:main:${sessionId}`, + storePath, + }; + await persistSessionTranscriptTurn(scope, { + messages: [ + { + eventId: "root", + parentId: null, + message: { role: "user", content: "cross-client prompt" }, + }, + { + eventId: "reply", + parentId: "root", + message: { role: "assistant", content: "cross-client reply" }, + }, + ], + touchSessionEntry: false, + }); + const database = openOpenClawAgentDatabase({ + agentId: "main", + path: path.join(tempDir, "openclaw-agent.sqlite"), + }); + database.db + .prepare("UPDATE session_transcript_index_state SET needs_rebuild = 1 WHERE session_id = ?") + .run(sessionId); + + await expect(readSessionMessageCountAsync(scope)).resolves.toBe(2); + }); + test("projects SQLite transcript reads to the active branch", async () => { const sessionId = "reader-sqlite-branch"; const scope = { diff --git a/src/gateway/session-transcript-readers.ts b/src/gateway/session-transcript-readers.ts index 75d24397d484..451ed7f6cd5d 100644 --- a/src/gateway/session-transcript-readers.ts +++ b/src/gateway/session-transcript-readers.ts @@ -1,10 +1,12 @@ import { + isSessionTranscriptProjectionUnavailableError, readRecentSessionTranscriptMessageEvents, readSessionTranscriptMessageEventById, readSessionTranscriptMessageEventCount, readSessionTranscriptMessageEventPage, readSessionTranscriptMessageEvents, resolveSessionTranscriptReadTarget, + waitForSessionTranscriptProjection, type SessionTranscriptMessageEvent, type SessionTranscriptReadScope, type TranscriptEvent, @@ -459,7 +461,18 @@ export async function readSessionMessageCountAsync( ): Promise { const target = resolveTranscriptReadTarget(scope); if (isSqliteReadTarget(target)) { - return readSessionTranscriptMessageEventCount(toTranscriptReadScope(target)); + const transcriptScope = toTranscriptReadScope(target); + try { + return readSessionTranscriptMessageEventCount(transcriptScope); + } catch (error) { + if (!isSessionTranscriptProjectionUnavailableError(error)) { + throw error; + } + // The failed read already scheduled the rebuild; wait before assigning + // a sequence so a concurrent send cannot fail or reuse a stale count. + await waitForSessionTranscriptProjection(transcriptScope); + return readSessionTranscriptMessageEventCount(transcriptScope); + } } return await readSessionMessageCountAsyncFile( target.sessionId, diff --git a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.discord-group.json b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.discord-group.json index 070a98644301..4f27a10ebef0 100644 --- a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.discord-group.json +++ b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.discord-group.json @@ -211,6 +211,11 @@ "enum": ["subagent"], "type": "string" }, + "runTimeoutSeconds": { + "description": "Per-run timeout in seconds; overrides the configured subagent default. Zero disables the timeout.", + "minimum": 0, + "type": "integer" + }, "sandbox": { "enum": ["inherit", "require"], "type": "string" diff --git a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.heartbeat-turn.json b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.heartbeat-turn.json index 0b069209df1a..a71031046fbd 100644 --- a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.heartbeat-turn.json +++ b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.heartbeat-turn.json @@ -211,6 +211,11 @@ "enum": ["subagent"], "type": "string" }, + "runTimeoutSeconds": { + "description": "Per-run timeout in seconds; overrides the configured subagent default. Zero disables the timeout.", + "minimum": 0, + "type": "integer" + }, "sandbox": { "enum": ["inherit", "require"], "type": "string" diff --git a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.telegram-direct.json b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.telegram-direct.json index 35527d0e7df8..7e7dfc98c902 100644 --- a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.telegram-direct.json +++ b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/codex-dynamic-tools.telegram-direct.json @@ -211,6 +211,11 @@ "enum": ["subagent"], "type": "string" }, + "runTimeoutSeconds": { + "description": "Per-run timeout in seconds; overrides the configured subagent default. Zero disables the timeout.", + "minimum": 0, + "type": "integer" + }, "sandbox": { "enum": ["inherit", "require"], "type": "string" diff --git a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md index a1ba04c4f1d1..b3294006b4d9 100644 --- a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md +++ b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md @@ -220,8 +220,8 @@ This is the deterministic model-bound layer stack OpenClaw can snapshot for the "roughTokens": 0 }, "dynamicToolsJson": { - "chars": 59624, - "roughTokens": 14906 + "chars": 59844, + "roughTokens": 14961 }, "openClawDeveloperInstructions": { "chars": 3471, @@ -232,8 +232,8 @@ This is the deterministic model-bound layer stack OpenClaw can snapshot for the "roughTokens": 6964 }, "totalWithDynamicToolsJson": { - "chars": 87480, - "roughTokens": 21870 + "chars": 87700, + "roughTokens": 21925 }, "userInputText": { "chars": 1300, diff --git a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md index e700fed8eb54..b74db8d4edbe 100644 --- a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md +++ b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md @@ -220,8 +220,8 @@ This is the deterministic model-bound layer stack OpenClaw can snapshot for the "roughTokens": 0 }, "dynamicToolsJson": { - "chars": 59316, - "roughTokens": 14829 + "chars": 59536, + "roughTokens": 14884 }, "openClawDeveloperInstructions": { "chars": 2362, @@ -232,8 +232,8 @@ This is the deterministic model-bound layer stack OpenClaw can snapshot for the "roughTokens": 6594 }, "totalWithDynamicToolsJson": { - "chars": 85692, - "roughTokens": 21423 + "chars": 85912, + "roughTokens": 21478 }, "userInputText": { "chars": 929, diff --git a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md index f5cbe884aa71..7014de66a1c9 100644 --- a/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md +++ b/test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md @@ -221,8 +221,8 @@ This is the deterministic model-bound layer stack OpenClaw can snapshot for the "roughTokens": 0 }, "dynamicToolsJson": { - "chars": 60878, - "roughTokens": 15220 + "chars": 61098, + "roughTokens": 15275 }, "openClawDeveloperInstructions": { "chars": 2381, @@ -233,8 +233,8 @@ This is the deterministic model-bound layer stack OpenClaw can snapshot for the "roughTokens": 6706 }, "totalWithDynamicToolsJson": { - "chars": 87702, - "roughTokens": 21926 + "chars": 87922, + "roughTokens": 21981 }, "userInputText": { "chars": 1284,