[pull] main from danny-avila:main

docs/tool-approval-replay.md

@@ -61,16 +61,22 @@ Keep it stable through every approval resume,
 including rebuilt `Run` instances, and change it for each new generation. A
 conversation id alone is insufficient; edits that reuse a response id also need
 a generation epoch. Explicit scoping removes LangGraph's changing task namespace
-from the composite owner while retaining the executing agent id.
+from the composite owner while retaining the executing agent id, principal id,
+and conversation id.
 
 Carried evidence is active only for its resumed interrupt and consuming task.
 The interrupt id is checked against LangGraph's resume map, and the task's
 scratchpad distinguishes a resumed node from later steps of the same invocation.
+Missing or malformed resume internals fail closed whenever a resume value could
+otherwise authorize execution; absence outside a consuming task is treated as
+stale evidence.
 This applies to every ToolNode interrupt, including `ask_user_question` pauses
 that have replay records but no approval-review evidence.
 A validated parent replaying a child approval can forward that child's evidence
-without a local resume value. Stale review and settled-batch evidence are ignored
-together. An active approval with a different execution owner still fails closed;
+without a local resume value. Stale authorization is removed separately from
+settled results for the active owner and batch, so clearing an old review cannot
+repeat checkpointed completed work. An active approval with a different
+execution owner still fails closed;
 starting a fresh generation may select a different agent without inheriting the
 prior approval.
 
@@ -97,6 +103,10 @@ third-party tools as exactly-once across that crash window.
   Reviewed rejection and allowlist restrictions still apply. Unreviewed direct
   siblings fail closed when their prior policy cannot be reconstructed.
 - Existing public approval payload and resume-decision shapes are unchanged.
+- Three-field owners from the preceding replay format are bound to the current
+  principal and conversation when restored through the checkpoint entry point.
+  Hosts must authorize checkpoint/thread access before calling the SDK; the SDK
+  cannot recover a principal that was never recorded in a legacy checkpoint.
 - Old SDK consumers cannot restore the new completion record. Do not route a
   paused run between SDK versions indiscriminately: drain or version-pin paused
   runs during rollout and before rollback. A durable checkpointer alone is not

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@librechat/agents",
-  "version": "3.8.4",
+  "version": "3.8.5",
   "reova": {
     "enabled": true,
     "endpoint": "https://telemetry.reo.dev/data"

src/tools/ToolNode.ts

@@ -128,8 +128,9 @@ import {
   attachToolBatchReplayState,
   restoreToolBatchReplayState,
   getToolBatchReplayState,
+  clearStaleToolApprovalConfig,
   isChildToolReplayOwner,
-  isToolReplayResume,
+  getToolReplayResumeStatus,
 } from '@/tools/toolBatchReplay';
 
 function stripToolApprovalReviewConfig(
@@ -925,15 +926,24 @@ export class ToolNode<T = any> extends RunnableCallable<T, T> {
             ? isChildToolReplayOwner(config, reviewEvidence.owner, trustedParentCallIds)
             : replayState.records.some((record) => isChildToolReplayOwner(config, record.owner, trustedParentCallIds));
         }
-        if ((reviewEvidence?.owner != null || replayState != null) &&
-          !isToolReplayResume(config, reviewEvidence?.interruptId ?? replayState?.interruptId, isChildReplay)) {
+        const hasApprovalAuthorization =
+          reviewEvidence?.owner != null || replayState?.approvalOwner != null;
+        const replayResumeStatus = hasApprovalAuthorization || replayState != null
+          ? getToolReplayResumeStatus(
+            config,
+            reviewEvidence?.interruptId ?? replayState?.interruptId,
+            isChildReplay
+          )
+          : undefined;
+        if (replayResumeStatus === 'unverifiable' && hasApprovalAuthorization) {
+          throw new Error('Cannot verify the active tool approval resume — failing closed');
+        }
+        if (replayResumeStatus === 'stale') {
+          const configurable = { ...config.configurable };
+          clearStaleToolApprovalConfig(configurable, replayOwner, activeReplayKey);
           config = {
             ...config,
-            configurable: {
-              ...config.configurable,
-              [TOOL_APPROVAL_REVIEW_CONFIG_KEY]: undefined,
-              [TOOL_BATCH_REPLAY_KEY]: undefined,
-            },
+            configurable,
           };
           reviewEvidence = undefined;
         }

src/tools/__tests__/ToolNode.approvalScope.test.ts

@@ -24,7 +24,8 @@ import { ToolNode } from '../ToolNode';
 function createHarness(
   carryCheckpointConfig = false,
   question = false,
-  completedSibling = false
+  completedSibling = false,
+  resumeInternalMode: 'supported' | 'missing' | 'mismatched' = 'supported'
 ) {
   let carriedConfigurable: RunnableConfig['configurable'];
   const checkpointer = new MemorySaver();
@@ -99,7 +100,7 @@ function createHarness(
                         {
                           id: `${runId}-sibling-${completed}`,
                           name: 'sibling',
-                          args: { value: `${runId}-sibling` },
+                          args: { value: `${runId}-sibling-${completed}` },
                         },
                       ]
                       : []),
@@ -130,9 +131,19 @@ function createHarness(
             ),
           });
         }
+        const configurable = { ...carriedConfigurable, ...config.configurable };
+        if (configurable[TOOL_APPROVAL_REVIEW_CONFIG_KEY] != null &&
+          resumeInternalMode !== 'supported') {
+          if (resumeInternalMode === 'missing') {
+            delete configurable.__pregel_resume_map;
+            delete configurable.__pregel_scratchpad;
+          } else {
+            configurable.__pregel_resume_map = {};
+          }
+        }
         const nodeConfig = {
           ...config,
-          configurable: { ...carriedConfigurable, ...config.configurable },
+          configurable,
         };
         if (
           carryCheckpointConfig &&
@@ -178,9 +189,10 @@ function createHarness(
   return { createRun, executions, namespaces, resumeInternals };
 }
 
-function config(scope?: string): RunnableConfig & { version: 'v2' } {
+function config(scope?: string, userId = 'user-a'): RunnableConfig & { version: 'v2' } {
   return {
     configurable: {
+      user_id: userId,
       thread_id: 'same-conversation',
       ...(scope == null
         ? {}
@@ -232,16 +244,51 @@ describe('approval execution scope', () => {
       { messages: [new HumanMessage('start')] },
       runConfig
     );
-    expect(executions).toEqual(['run-1-sibling']);
+    expect(executions).toEqual(['run-1-sibling-0']);
 
     await run.resume([{ type: 'approve' }], runConfig);
-    expect(executions).toEqual(['run-1-sibling', 'run-1-0']);
+    expect(executions).toEqual(['run-1-sibling-0', 'run-1-0']);
     expect(resumeInternals).toEqual([
       { resumeMap: true, scratchpad: true },
     ]);
     expect(run.getInterrupt()).toBeUndefined();
   });
 
+  it.each(['missing', 'mismatched'] as const)(
+    'fails closed without replaying a completed mutation when resume internals are %s',
+    async (resumeInternalMode) => {
+      const { createRun, executions } = createHarness(false, false, true, resumeInternalMode);
+      const runId = `changed-internals-${resumeInternalMode}`;
+      const run = await createRun('a', runId);
+      const runConfig = config(runId);
+      await run.processStream({ messages: [new HumanMessage('start')] }, runConfig);
+      expect(executions).toEqual([`${runId}-sibling-0`]);
+
+      await expect(run.resume([{ type: 'approve' }], runConfig)).rejects.toThrow(
+        'Cannot verify the active tool approval resume'
+      );
+      expect(executions).toEqual([`${runId}-sibling-0`]);
+    }
+  );
+
+  it('preserves completed results through two consecutive approval cycles', async () => {
+    const { createRun, executions } = createHarness(true, false, true);
+    const run = await createRun('a', 'two-cycle-replay', 4);
+    const runConfig = config('two-cycle-replay');
+    await run.processStream({ messages: [new HumanMessage('start')] }, runConfig);
+    await run.resume([{ type: 'approve' }], runConfig);
+    expect(run.getInterrupt()?.payload).toMatchObject({ type: 'tool_approval' });
+    await run.resume([{ type: 'approve' }], runConfig);
+
+    expect(executions).toEqual([
+      'two-cycle-replay-sibling-0',
+      'two-cycle-replay-0',
+      'two-cycle-replay-sibling-2',
+      'two-cycle-replay-2',
+    ]);
+    expect(run.getInterrupt()).toBeUndefined();
+  });
+
   it('continues to a new tool batch after a question resume with fresh instances', async () => {
     const { createRun, executions } = createHarness(false, true);
     const run = await createRun('a', 'question-run', 2);
@@ -256,7 +303,7 @@ describe('approval execution scope', () => {
     expect(rebuilt.getInterrupt()).toBeUndefined();
   });
 
-  it.each(['agent', 'scope'])(
+  it.each(['agent', 'scope', 'principal'])(
     'fails closed when the %s changes during the pending resume',
     async (changedIdentity) => {
       const { createRun, executions } = createHarness();
@@ -272,7 +319,10 @@ describe('approval execution scope', () => {
       await expect(
         resumed.resume(
           [{ type: 'approve' }],
-          config(changedIdentity === 'scope' ? 'run-2' : 'run-1')
+          config(
+            changedIdentity === 'scope' ? 'run-2' : 'run-1',
+            changedIdentity === 'principal' ? 'user-b' : 'user-a'
+          )
         )
       ).rejects.toThrow('Tool approval execution owner changed');
       expect(executions).toEqual([]);

src/tools/subagent/SubagentExecutor.ts

@@ -1768,7 +1768,10 @@ export class SubagentExecutor {
             channel === INTERRUPT && approvalScope != null &&
             value != null && typeof value === 'object' && 'value' in value
               ? { ...value, value: rebindToolBatchReplayPayload(
-                value.value, approvalScope.source, approvalScope.target
+                value.value,
+                approvalScope.source,
+                approvalScope.target,
+                targetThreadId
               ) }
               : value;
           writes.push([channel, reboundValue]);
@@ -2666,7 +2669,12 @@ export class SubagentExecutor {
           activeChildRun.pendingInterrupts = resumeExecution == null ? persistedInterrupts :
             persistedInterrupts.map((pending) => ({
               ...pending,
-              value: rebindToolBatchReplayPayload(pending.value, resumeExecution.approvalExecutionScope, approvalExecutionScope),
+              value: rebindToolBatchReplayPayload(
+                pending.value,
+                resumeExecution.approvalExecutionScope,
+                approvalExecutionScope,
+                childThreadId
+              ),
             }));
           execution.markStarted();
           childAlreadyStarted = true;
@@ -2709,7 +2717,8 @@ export class SubagentExecutor {
           rebindToolBatchReplayScope(
             childConfigurable,
             resumeExecution?.approvalExecutionScope ?? approvalExecutionScope,
-            approvalExecutionScope
+            approvalExecutionScope,
+            childThreadId
           );
           childInput = new Command({ resume: childResumeMap });
         } else if (recoveredInProgress) {