mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(agents): preserve split-owner completion policy
This commit is contained in:
@@ -396,6 +396,7 @@ export const SessionsPatchParamsSchema = closedObject({
|
||||
execNode: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
model: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
spawnedBy: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
completionOwnerSessionKey: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
spawnedWorkspaceDir: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
spawnedCwd: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
spawnDepth: Type.Optional(Type.Union([Type.Integer({ minimum: 0 }), Type.Null()])),
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
title: Issue 109025 completion handoff sender-policy live probe
|
||||
|
||||
scenario:
|
||||
id: issue-109025-completion-policy-live
|
||||
surface: subagents
|
||||
runtimeParityTier: live-only
|
||||
coverage:
|
||||
secondary:
|
||||
- agents.subagents
|
||||
- runtime.delivery
|
||||
- channels.qa-channel
|
||||
gatewayConfigPatch:
|
||||
tools:
|
||||
toolsBySender:
|
||||
"*":
|
||||
deny:
|
||||
- exec
|
||||
- process
|
||||
- nodes
|
||||
- gateway
|
||||
- write
|
||||
"id:alice":
|
||||
deny:
|
||||
- write
|
||||
objective: Prove a dormant requester-agent completion handoff retains the original requester's exact sender tool policy instead of reapplying the wildcard deny.
|
||||
successCriteria:
|
||||
- An exact-sender parent spawns a child and yields before the child completes.
|
||||
- The child follows a serial filesystem-read chain that remains allowed by the wildcard policy.
|
||||
- The child completion wakes the requester agent without a new external sender turn.
|
||||
- The requester-agent continuation uses exec, which the wildcard policy denies, to generate a fresh marker while write remains denied.
|
||||
- The QA runner reads the exec-created proof file and finds the exact same unpredictable marker in Gateway output.
|
||||
docsRefs:
|
||||
- docs/gateway/config-tools.md
|
||||
- docs/tools/subagents.md
|
||||
- docs/help/testing.md
|
||||
codeRefs:
|
||||
- src/agents/requester-tool-policy.ts
|
||||
- src/agents/subagent-announce-delivery.ts
|
||||
- src/agents/subagent-capabilities.ts
|
||||
- src/agents/tools/sessions-yield-tool.ts
|
||||
execution:
|
||||
kind: flow
|
||||
suiteIsolation: isolated
|
||||
isolationReason: Mutates sender tool policy and exercises a yielded parent with a live child completion.
|
||||
channel: qa-channel
|
||||
retryCount: 0
|
||||
summary: Exercise exact sender policy through a real inbound turn, dormant requester-agent completion, and restricted exec call.
|
||||
config:
|
||||
childLabelPrefix: issue-109025-completion-child
|
||||
chainFilePrefix: issue-109025-completion-chain
|
||||
completionPrefix: ISSUE109025_COMPLETION_EXEC_OK
|
||||
|
||||
flow:
|
||||
steps:
|
||||
- name: dormant requester completion retains exact-sender exec authority
|
||||
actions:
|
||||
- call: waitForGatewayHealthy
|
||||
args:
|
||||
- ref: env
|
||||
- 120000
|
||||
- call: waitForTransportReady
|
||||
args:
|
||||
- ref: env
|
||||
- 120000
|
||||
- resetTransport: true
|
||||
- set: parentSessionKey
|
||||
value:
|
||||
expr: "buildAgentSessionKey({ agentId: 'qa', channel: 'qa-channel', accountId: 'default', peer: { kind: 'direct', id: 'alice' }, dmScope: env.cfg.session?.dmScope, identityLinks: env.cfg.session?.identityLinks })"
|
||||
- set: childLabel
|
||||
value:
|
||||
expr: "`${config.childLabelPrefix}-${randomUUID().slice(0, 8)}`"
|
||||
- set: chainNonce
|
||||
value:
|
||||
expr: randomUUID().slice(0, 8)
|
||||
- set: chainFiles
|
||||
value:
|
||||
expr: "[1, 2, 3, 4].map((index) => `${config.chainFilePrefix}-${chainNonce}-${index}.txt`)"
|
||||
- set: proofFile
|
||||
value:
|
||||
expr: "`issue-109025-exec-proof-${randomUUID()}.txt`"
|
||||
- set: proofPath
|
||||
value:
|
||||
expr: path.join(env.gateway.workspaceDir, proofFile)
|
||||
- set: execCommand
|
||||
value:
|
||||
expr: |-
|
||||
`node -e ${JSON.stringify(`const fs=require("node:fs");const crypto=require("node:crypto");const marker="${config.completionPrefix}:"+crypto.randomUUID();fs.writeFileSync(${JSON.stringify(proofFile)},marker+"\n");process.stdout.write(marker+"\n");`)}`
|
||||
- call: fs.writeFile
|
||||
args:
|
||||
- expr: path.join(env.gateway.workspaceDir, chainFiles[0])
|
||||
- expr: "`${chainFiles[1]}\n`"
|
||||
- utf8
|
||||
- call: fs.writeFile
|
||||
args:
|
||||
- expr: path.join(env.gateway.workspaceDir, chainFiles[1])
|
||||
- expr: "`${chainFiles[2]}\n`"
|
||||
- utf8
|
||||
- call: fs.writeFile
|
||||
args:
|
||||
- expr: path.join(env.gateway.workspaceDir, chainFiles[2])
|
||||
- expr: "`${chainFiles[3]}\n`"
|
||||
- utf8
|
||||
- call: fs.writeFile
|
||||
args:
|
||||
- expr: path.join(env.gateway.workspaceDir, chainFiles[3])
|
||||
- CHILD_READY
|
||||
- utf8
|
||||
- sendInbound:
|
||||
conversation:
|
||||
id: issue-109025-completion
|
||||
kind: direct
|
||||
senderId: alice
|
||||
senderName: Alice
|
||||
text:
|
||||
expr: |-
|
||||
`Issue 109025 dormant completion probe. Follow these steps exactly.
|
||||
1. Call sessions_spawn exactly once with runtime subagent, label ${childLabel}, and this task: "Use only the filesystem read tool. Start by reading ${chainFiles[0]}. Its trimmed contents name the next workspace file to read. Continue following one filename at a time until a file contains CHILD_READY. You must perform each read serially because later filenames are not in this task. Then reply exactly CHILD_DONE and nothing else. Do not use exec or process."
|
||||
2. Immediately after sessions_spawn succeeds, call sessions_yield. Do not wait for the child and do not send a normal final reply.
|
||||
3. When the CHILD_DONE completion event later resumes you, use the exec tool to run this exact command: ${execCommand}
|
||||
Do not use read or write for this step. Then reply with exactly the command's trimmed stdout.`
|
||||
- call: waitForCondition
|
||||
saveAs: expectedFinalMarker
|
||||
args:
|
||||
- lambda:
|
||||
async: true
|
||||
expr: "fs.readFile(proofPath, 'utf8').then((text) => { const marker = text.trim(); return marker.startsWith(`${config.completionPrefix}:`) && readGatewayLogs().includes(marker) ? marker : undefined; }).catch(() => undefined)"
|
||||
- 60000
|
||||
- 250
|
||||
- call: readSessionTranscriptSummary
|
||||
saveAs: parentTranscript
|
||||
args:
|
||||
- ref: env
|
||||
- ref: parentSessionKey
|
||||
- set: gatewayLogs
|
||||
value:
|
||||
expr: readGatewayLogs()
|
||||
- assert:
|
||||
expr: "(parentTranscript.assistantToolCallCounts.sessions_yield ?? 0) >= 1"
|
||||
message:
|
||||
expr: "`parent did not yield before completion; parentTools=${JSON.stringify(parentTranscript.assistantToolCallCounts ?? {})}`"
|
||||
- assert:
|
||||
expr: gatewayLogs.includes(expectedFinalMarker)
|
||||
message:
|
||||
expr: "`completion continuation did not produce the exec-created marker; marker=${String(expectedFinalMarker ?? '')}`"
|
||||
- assert:
|
||||
expr: "gatewayLogs.indexOf('CHILD_DONE') >= 0 && gatewayLogs.indexOf(expectedFinalMarker) > gatewayLogs.indexOf('CHILD_DONE')"
|
||||
message: child completion did not precede the exec-created continuation marker
|
||||
detailsExpr: "`parentSession=${parentSessionKey} parentTools=${JSON.stringify(parentTranscript.assistantToolCallCounts ?? {})} execMarker=${String(expectedFinalMarker ?? '')}`"
|
||||
@@ -866,6 +866,7 @@ describe("spawnAcpDirect", () => {
|
||||
expectSessionPatchFields({
|
||||
key: accepted.childSessionKey,
|
||||
spawnedBy: "agent:main:main",
|
||||
completionOwnerSessionKey: "agent:main:main",
|
||||
});
|
||||
expectBindingCallFields({
|
||||
targetKind: "session",
|
||||
|
||||
@@ -1289,6 +1289,7 @@ export async function spawnAcpDirect(
|
||||
params: {
|
||||
key: sessionKey,
|
||||
spawnedBy: requesterInternalKey,
|
||||
completionOwnerSessionKey: ownership.completionRequesterSessionKey,
|
||||
...admission.childSessionPatch,
|
||||
...inheritedToolAllowPatch(ctx.inheritedToolAllowlist),
|
||||
...inheritedToolDenyPatch(ctx.inheritedToolDenylist),
|
||||
|
||||
@@ -326,6 +326,7 @@ export async function dispatchEmbeddedRunAttempt(input: {
|
||||
sourceReplyDeliveryMode: params.sourceReplyDeliveryMode,
|
||||
taskSuggestionDeliveryMode: params.taskSuggestionDeliveryMode,
|
||||
inputProvenance: params.inputProvenance,
|
||||
trustedInternalHandoff: params.trustedInternalHandoff,
|
||||
streamParams: params.streamParams,
|
||||
modelRun: params.modelRun,
|
||||
disableTrajectory: params.disableTrajectory,
|
||||
|
||||
@@ -288,6 +288,51 @@ describe("resolveRequesterToolPolicies", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("restores a verified completion handoff to a distinct immutable completion owner", async () => {
|
||||
const controllerSessionKey = "agent:main:discord:direct:alice";
|
||||
const completionOwnerSessionKey = "agent:main:main";
|
||||
const childSessionKey = "agent:main:subagent:child";
|
||||
await writeSession(childSessionKey, {
|
||||
spawnedBy: controllerSessionKey,
|
||||
completionOwnerSessionKey,
|
||||
spawnDepth: 1,
|
||||
subagentRole: "orchestrator",
|
||||
subagentControlScope: "children",
|
||||
inheritedToolAllow: ["read", "exec"],
|
||||
});
|
||||
|
||||
const result = resolveRequesterToolPolicies({
|
||||
config: config(),
|
||||
agentId: "main",
|
||||
sessionKey: completionOwnerSessionKey,
|
||||
trustedInternalHandoff: true,
|
||||
inputProvenance: {
|
||||
kind: "inter_session",
|
||||
sourceSessionKey: childSessionKey,
|
||||
sourceTool: "subagent_announce",
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.delegated).toBe(true);
|
||||
expect(result.requesterPolicySource).toBe("completion-handoff");
|
||||
expect(result.senderPolicy).toBeUndefined();
|
||||
expect(result.inheritedToolPolicy).toEqual({ allow: ["read", "exec"] });
|
||||
|
||||
const controllerResult = resolveRequesterToolPolicies({
|
||||
config: config(),
|
||||
agentId: "main",
|
||||
sessionKey: controllerSessionKey,
|
||||
trustedInternalHandoff: true,
|
||||
inputProvenance: {
|
||||
kind: "inter_session",
|
||||
sourceSessionKey: childSessionKey,
|
||||
sourceTool: "subagent_announce",
|
||||
},
|
||||
});
|
||||
expect(controllerResult.delegated).toBe(false);
|
||||
expect(controllerResult.requesterPolicySource).toBe("current-request");
|
||||
});
|
||||
|
||||
it("walks nested lineage to the projection captured from the target requester", async () => {
|
||||
const requesterSessionKey = "agent:main:discord:direct:alice";
|
||||
const parentChildSessionKey = "agent:main:subagent:parent-child";
|
||||
@@ -351,6 +396,13 @@ describe("resolveRequesterToolPolicies", () => {
|
||||
trustedInternalHandoff: true,
|
||||
inputProvenance: provenance,
|
||||
});
|
||||
const mismatchedCompletionOwner = resolveRequesterToolPolicies({
|
||||
config: config(),
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:main",
|
||||
trustedInternalHandoff: true,
|
||||
inputProvenance: provenance,
|
||||
});
|
||||
|
||||
expect(untrusted.delegated).toBe(false);
|
||||
expect(untrusted.requesterPolicySource).toBe("current-request");
|
||||
@@ -358,5 +410,7 @@ describe("resolveRequesterToolPolicies", () => {
|
||||
expect(mismatched.delegated).toBe(false);
|
||||
expect(mismatched.requesterPolicySource).toBe("current-request");
|
||||
expect(mismatched.senderPolicy).toEqual({ deny: ["group:runtime", "group:fs"] });
|
||||
expect(mismatchedCompletionOwner.delegated).toBe(false);
|
||||
expect(mismatchedCompletionOwner.requesterPolicySource).toBe("current-request");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -131,7 +131,10 @@ function resolveDelegatedPolicy(
|
||||
return { delegated: false };
|
||||
}
|
||||
const parentSessionKey = resolveRequesterStoreKey(params.config, envelope.spawnedBy);
|
||||
if (parentSessionKey === targetSessionKey) {
|
||||
const completionOwnerSessionKey = envelope.completionOwnerSessionKey
|
||||
? resolveRequesterStoreKey(params.config, envelope.completionOwnerSessionKey)
|
||||
: undefined;
|
||||
if ((completionOwnerSessionKey ?? parentSessionKey) === targetSessionKey) {
|
||||
return {
|
||||
delegated: true,
|
||||
source: "completion-handoff",
|
||||
|
||||
@@ -43,6 +43,7 @@ type SessionCapabilityEntry = {
|
||||
subagentRole?: unknown;
|
||||
subagentControlScope?: unknown;
|
||||
spawnedBy?: unknown;
|
||||
completionOwnerSessionKey?: unknown;
|
||||
inheritedToolAllow?: unknown;
|
||||
inheritedToolDeny?: unknown;
|
||||
};
|
||||
@@ -56,6 +57,7 @@ export type SessionCapabilityStore = Record<
|
||||
subagentRole?: unknown;
|
||||
subagentControlScope?: unknown;
|
||||
spawnedBy?: unknown;
|
||||
completionOwnerSessionKey?: unknown;
|
||||
inheritedToolAllow?: unknown;
|
||||
inheritedToolDeny?: unknown;
|
||||
}
|
||||
@@ -64,6 +66,7 @@ export type SessionCapabilityStore = Record<
|
||||
type PersistedSubagentToolPolicyEnvelope = {
|
||||
sessionKey: string;
|
||||
spawnedBy: string;
|
||||
completionOwnerSessionKey?: string;
|
||||
inheritedToolAllow: string[];
|
||||
inheritedToolDeny: string[];
|
||||
};
|
||||
@@ -330,9 +333,11 @@ export function resolvePersistedSubagentToolPolicyEnvelope(
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
const completionOwnerSessionKey = normalizeOptionalString(entry.completionOwnerSessionKey);
|
||||
return {
|
||||
sessionKey: normalizedSessionKey,
|
||||
spawnedBy,
|
||||
...(completionOwnerSessionKey ? { completionOwnerSessionKey } : {}),
|
||||
inheritedToolAllow: normalizeInheritedToolAllowlist(entry.inheritedToolAllow),
|
||||
inheritedToolDeny: normalizeInheritedToolDenylist(entry.inheritedToolDeny),
|
||||
};
|
||||
|
||||
@@ -1463,6 +1463,12 @@ describe("spawnSubagentDirect seam flow", () => {
|
||||
});
|
||||
|
||||
it("keeps controller ownership separate from completion ownership", async () => {
|
||||
let persistedStore: Record<string, Record<string, unknown>> | undefined;
|
||||
installSessionStoreCaptureMock(hoisted.updateSessionStoreMock, {
|
||||
onStore: (store) => {
|
||||
persistedStore = store;
|
||||
},
|
||||
});
|
||||
await spawnSubagentDirect(
|
||||
{
|
||||
task: "background work",
|
||||
@@ -1480,6 +1486,9 @@ describe("spawnSubagentDirect seam flow", () => {
|
||||
expect(registerInput.controllerSessionKey).toBe("agent:main:telegram:default:direct:456");
|
||||
expect(registerInput.requesterSessionKey).toBe("agent:main:main");
|
||||
expect(registerInput.requesterDisplayKey).toBe("agent:main:main");
|
||||
expect(persistedStore?.[registerInput.childSessionKey]?.completionOwnerSessionKey).toBe(
|
||||
"agent:main:main",
|
||||
);
|
||||
});
|
||||
|
||||
it("persists the spawning session as the stable swarm limit owner", async () => {
|
||||
|
||||
@@ -363,6 +363,12 @@ function buildDirectChildSessionPatch(patch: Record<string, unknown>): Partial<S
|
||||
if (typeof patch.spawnedBy === "string" && patch.spawnedBy.trim()) {
|
||||
entry.spawnedBy = patch.spawnedBy.trim();
|
||||
}
|
||||
if (
|
||||
typeof patch.completionOwnerSessionKey === "string" &&
|
||||
patch.completionOwnerSessionKey.trim()
|
||||
) {
|
||||
entry.completionOwnerSessionKey = patch.completionOwnerSessionKey.trim();
|
||||
}
|
||||
if (typeof patch.spawnedWorkspaceDir === "string" && patch.spawnedWorkspaceDir.trim()) {
|
||||
entry.spawnedWorkspaceDir = patch.spawnedWorkspaceDir.trim();
|
||||
}
|
||||
@@ -1500,6 +1506,7 @@ export async function spawnSubagentDirect(
|
||||
});
|
||||
const spawnLineagePatchError = await patchChildSession({
|
||||
spawnedBy: spawnedByKey,
|
||||
completionOwnerSessionKey: ownership.completionRequesterSessionKey,
|
||||
...(spawnedMetadata.workspaceDir
|
||||
? { spawnedWorkspaceDir: spawnedMetadata.workspaceDir }
|
||||
: {}),
|
||||
|
||||
@@ -272,6 +272,8 @@ export type SessionEntry = SessionRestartRecoveryState &
|
||||
sessionFile?: string;
|
||||
/** Parent session key that spawned this session (used for sandbox session-tool scoping). */
|
||||
spawnedBy?: string;
|
||||
/** Immutable session key authorized to receive this child's completion handoff. */
|
||||
completionOwnerSessionKey?: string;
|
||||
/** Workspace inherited by spawned sessions and reused on later turns for the same child session. */
|
||||
spawnedWorkspaceDir?: string;
|
||||
/** Task working directory inherited by spawned sessions and reused on later turns. */
|
||||
|
||||
@@ -1247,6 +1247,29 @@ describe("gateway sessions patch", () => {
|
||||
expect(entry.spawnedBy).toBe("agent:main:main");
|
||||
});
|
||||
|
||||
test("sets an immutable completion owner for ACP sessions", async () => {
|
||||
const entry = expectPatchOk(
|
||||
await runPatch({
|
||||
storeKey: "agent:main:acp:child",
|
||||
patch: {
|
||||
key: "agent:main:acp:child",
|
||||
completionOwnerSessionKey: "agent:main:main",
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(entry.completionOwnerSessionKey).toBe("agent:main:main");
|
||||
|
||||
const result = await runPatch({
|
||||
storeKey: "agent:main:acp:child",
|
||||
store: { "agent:main:acp:child": entry },
|
||||
patch: {
|
||||
key: "agent:main:acp:child",
|
||||
completionOwnerSessionKey: "agent:main:discord:direct:bob",
|
||||
},
|
||||
});
|
||||
expectPatchError(result, "completionOwnerSessionKey cannot be changed once set");
|
||||
});
|
||||
|
||||
test("sets spawnedWorkspaceDir for subagent sessions", async () => {
|
||||
const entry = expectPatchOk(
|
||||
await runPatch({
|
||||
|
||||
@@ -205,7 +205,7 @@ export async function projectSessionsPatchEntry(params: {
|
||||
? null
|
||||
: invalid(`${field} is only supported for subagent:* or acp:* sessions`);
|
||||
const applyImmutableString = (
|
||||
field: "spawnedBy" | "spawnedWorkspaceDir" | "spawnedCwd",
|
||||
field: "spawnedBy" | "completionOwnerSessionKey" | "spawnedWorkspaceDir" | "spawnedCwd",
|
||||
checkLineageBeforeEmpty: boolean,
|
||||
): PatchError => {
|
||||
if (!(field in patch)) {
|
||||
@@ -268,6 +268,7 @@ export async function projectSessionsPatchEntry(params: {
|
||||
|
||||
for (const fieldParams of [
|
||||
{ field: "spawnedBy" as const, checkLineageBeforeEmpty: false },
|
||||
{ field: "completionOwnerSessionKey" as const, checkLineageBeforeEmpty: false },
|
||||
{ field: "spawnedWorkspaceDir" as const, checkLineageBeforeEmpty: true },
|
||||
{ field: "spawnedCwd" as const, checkLineageBeforeEmpty: true },
|
||||
]) {
|
||||
|
||||
@@ -24,6 +24,7 @@ const SESSION_ENTRY_RESERVED_SLOT_KEY_LIST = [
|
||||
"lastActivityAt",
|
||||
"sessionFile",
|
||||
"spawnedBy",
|
||||
"completionOwnerSessionKey",
|
||||
"spawnedWorkspaceDir",
|
||||
"spawnedCwd",
|
||||
"worktree",
|
||||
|
||||
Reference in New Issue
Block a user