-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(core): 异常终止时保存 ReActAgent 状态 #2713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -486,24 +486,49 @@ private Mono<Void> saveStateToSession(CallExecution scope) { | |
| if (stateStore == null) { | ||
| return Mono.empty(); | ||
| } | ||
| return Mono.<Void>fromRunnable(() -> persistState(scope)) | ||
| .subscribeOn(Schedulers.boundedElastic()); | ||
| } | ||
|
|
||
| private void persistState(CallExecution scope) { | ||
| syncToolkitToState(scope.state); | ||
| SlotRef ref = SlotRef.parse(scope.slotKey); | ||
| AgentState toSave = scope.state; | ||
| return Mono.<Void>fromRunnable( | ||
| () -> { | ||
| long newVersion = | ||
| persistAgentStateCas( | ||
| ref.userId, | ||
| ref.sessionId, | ||
| scope.slotKey, | ||
| toSave, | ||
| scope.loadedVersion, | ||
| scope.loadedContextSize); | ||
| if (newVersion != AgentStateStore.UNVERSIONED) { | ||
| scope.loadedVersion = newVersion; | ||
| } | ||
| }) | ||
| .subscribeOn(Schedulers.boundedElastic()); | ||
| long newVersion = | ||
| persistAgentStateCas( | ||
| ref.userId, | ||
| ref.sessionId, | ||
| scope.slotKey, | ||
| scope.state, | ||
| scope.loadedVersion, | ||
| scope.loadedContextSize); | ||
| if (newVersion != AgentStateStore.UNVERSIONED) { | ||
| scope.loadedVersion = newVersion; | ||
| } | ||
| } | ||
|
|
||
| private void repairStateBeforeAbnormalCheckpoint(CallExecution scope) { | ||
| try { | ||
| scope.dropUncommittedToolCallsFromCurrentCall(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Good defensive design: wrapping both
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Verified as addressed in |
||
| } catch (RuntimeException repairError) { | ||
| log.warn("Failed to repair agent state before checkpoint", repairError); | ||
| } | ||
| } | ||
|
|
||
| private void checkpointStateAfterCancellation(CallExecution scope) { | ||
| repairStateBeforeAbnormalCheckpoint(scope); | ||
| try { | ||
| if (stateStore != null) { | ||
| persistState(scope); | ||
| } | ||
| } catch (RuntimeException saveError) { | ||
| log.warn("Failed to save agent state after abnormal termination", saveError); | ||
| } | ||
| } | ||
|
|
||
| private <T> Mono<T> checkpointOnAbnormalTermination(CallExecution scope, Mono<T> execution) { | ||
| return execution | ||
| .doOnCancel(() -> checkpointStateAfterCancellation(scope)) | ||
| .onErrorResume(error -> saveStateAfterCallFailure(scope, error)); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -522,6 +547,7 @@ private <T> Mono<T> saveStateAfterCallFailure(CallExecution scope, Throwable cal | |
| if (ExceptionUtils.containsInterruptedException(callFailure)) { | ||
| return Mono.error(callFailure); | ||
| } | ||
| repairStateBeforeAbnormalCheckpoint(scope); | ||
| return saveStateToSession(scope) | ||
| .onErrorResume( | ||
| saveFailure -> { | ||
|
|
@@ -1223,8 +1249,7 @@ protected Mono<Msg> doCall(List<Msg> msgs) { | |
| AgentEventEmitter.fromForwardingContext(cv) | ||
| .ifPresent(ae -> scope.externalEventEmitter = ae); | ||
| } | ||
| return scope.doCallInner(msgs) | ||
| .onErrorResume(error -> saveStateAfterCallFailure(scope, error)) | ||
| return checkpointOnAbnormalTermination(scope, scope.doCallInner(msgs)) | ||
| .flatMap(result -> saveStateToSession(scope).thenReturn(result)); | ||
| }); | ||
| } | ||
|
|
@@ -1265,20 +1290,25 @@ private Mono<Msg> doStructuredCall(List<Msg> msgs, Class<?> targetClass, JsonNod | |
| hasTools | ||
| ? model.supportsNativeStructuredOutputWithTools() | ||
| : model.supportsNativeStructuredOutput(); | ||
| Mono<Msg> execution; | ||
| if (useNative) { | ||
| return doNativeStructuredCall(msgs, jsonSchema) | ||
| .onErrorResume( | ||
| e -> { | ||
| log.warn( | ||
| "Native structured output failed ({}) — falling back to" | ||
| + " synthetic tool path", | ||
| e.getMessage() != null | ||
| ? e.getMessage() | ||
| : e.getClass().getSimpleName()); | ||
| return doFallbackStructuredCall(msgs, jsonSchema); | ||
| }); | ||
| execution = | ||
| doNativeStructuredCall(msgs, jsonSchema) | ||
| .onErrorResume( | ||
| e -> { | ||
| log.warn( | ||
| "Native structured output failed ({}) — falling" | ||
| + " back to synthetic tool path", | ||
| e.getMessage() != null | ||
| ? e.getMessage() | ||
| : e.getClass().getSimpleName()); | ||
| return doFallbackStructuredCall(msgs, jsonSchema); | ||
| }); | ||
| } else { | ||
| execution = doFallbackStructuredCall(msgs, jsonSchema); | ||
| } | ||
| return doFallbackStructuredCall(msgs, jsonSchema); | ||
| return Mono.deferContextual( | ||
| cv -> checkpointOnAbnormalTermination(scopeFrom(cv), execution)); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1359,7 +1389,6 @@ private Mono<Msg> doFallbackStructuredCall(List<Msg> msgs, Map<String, Object> j | |
| scope.soTool = createStructuredOutputTool(jsonSchema); | ||
|
|
||
| return scope.doCallInner(msgs) | ||
| .onErrorResume(error -> saveStateAfterCallFailure(scope, error)) | ||
| .flatMap( | ||
| result -> { | ||
| Msg out = result; | ||
|
|
@@ -2124,6 +2153,40 @@ private void synthesizeErrorResultsForPendingToolCalls() { | |
| } | ||
| } | ||
|
|
||
| /** Remove current-call PENDING tool calls that never produced results. */ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Clean implementation: iterating backward from the end of context and bounding by
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Verified as addressed in
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [praise] Clean implementation: iterating backward from the end of context and bounding by
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Verified as addressed in |
||
| private void dropUncommittedToolCallsFromCurrentCall() { | ||
| Set<String> pendingIds = getPendingToolUseIds(); | ||
| if (pendingIds.isEmpty()) { | ||
| return; | ||
| } | ||
| List<Msg> context = state.contextMutable(); | ||
| for (int i = context.size() - 1; i >= loadedContextSize; i--) { | ||
| Msg message = context.get(i); | ||
| if (message.getRole() != MsgRole.ASSISTANT | ||
| || !message.hasContentBlocks(ToolUseBlock.class)) { | ||
| continue; | ||
| } | ||
| List<ContentBlock> retained = | ||
| message.getContent().stream() | ||
| .filter( | ||
| block -> | ||
| !(block instanceof ToolUseBlock toolUse) | ||
| || toolUse.getState() | ||
| != ToolCallState.PENDING | ||
| || !pendingIds.contains(toolUse.getId())) | ||
| .toList(); | ||
| if (retained.size() == message.getContent().size()) { | ||
| return; | ||
| } | ||
| if (retained.isEmpty()) { | ||
| context.remove(i); | ||
| } else { | ||
| context.set(i, message.withContent(retained)); | ||
| } | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| private void publishEvent(AgentEvent event) { | ||
| FluxSink<AgentEvent> sink = eventSink; | ||
| if (sink != null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[praise] Good defensive design: wrapping both
dropUncommittedToolCallsFromCurrentCall()andpersistState()in separate try-catch blocks ensures that a failure in state repair doesn't prevent the best-effort save, and a save failure is properly logged without masking the original error.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Verified as addressed in
1770366: checkpointAbnormalState (lines 519-531) still uses two separate try-catch blocks for repair and save as praised.