Skip to content

fix: Infinite loop increases maximum loop count limit #4088

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.

Sign up for GitHub

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

Merged
shaohuzhang1 merged 1 commit into from
Sep 23, 2025
Merged

fix: Infinite loop increases maximum loop count limit #4088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions apps/application/flow/step_node/loop_node/impl/base_loop_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
from application.flow.tools import Reasoning
from application.models import ChatRecord
from common.handle.impl.response.loop_to_response import LoopToResponse
from maxkb.const import CONFIG
from django.utils.translation import gettext as _

max_loop_count = (CONFIG.get("MAX_LOOP_COUNT") or 1000)


def _is_interrupt_exec(node, node_variable: Dict, workflow_variable: Dict):
Expand Down Expand Up @@ -127,6 +131,7 @@ def loop(workflow_manage_new_instance, node: INode, generate_loop):
current_index = node.context.get("current_index") or 0
node_params = node.node_params
start_node_id = node_params.get('child_node', {}).get('runtime_node_id')
loop_type = node_params.get('loop_type')
start_node_data = None
chat_record = None
child_node = None
Expand All @@ -138,6 +143,8 @@ def loop(workflow_manage_new_instance, node: INode, generate_loop):
details=loop_node_data[current_index])

for item, index in generate_loop(current_index):
if 0 < max_loop_count <= index - current_index and loop_type == 'LOOP':
raise Exception(_('Exceeding the maximum number of cycles'))
"""
指定次数循环
@return:
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some suggestions to improve the provided code:

  1. Add import typing at the top of your file, especially if it's using type annotations.

  2. Consider using f-strings instead of .format() for string interpolation in Python 3.6+.

  3. If you're using Django templates, use gettext_lazy instead of gettext.

  4. Make sure that the CONFIG.get("MAX_LOOP_COUNT") handles any exceptions gracefully (e.g., returning a default value or raising an informative exception).

  5. If node_params.get('child_node') is always non-empty, consider checking for None explicitly before accessing attributes like runtime_node_id.

  6. It might be beneficial to extract logic into separate functions with descriptive names, such as _find_next_child, _check_max_iter, etc.

These improvements will make the code cleaner and easier to maintain while being efficient.

Expand Down
1 change: 1 addition & 0 deletions installer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ ENV MAXKB_VERSION="${DOCKER_IMAGE_TAG} (build at ${BUILD_AT}, commit: ${GITHUB_C
MAXKB_LOCAL_MODEL_HOST=127.0.0.1 \
MAXKB_LOCAL_MODEL_PORT=11636 \
MAXKB_LOCAL_MODEL_PROTOCOL=http \
MAXKB_MAX_LOOP_COUNT=1000 \
PIP_TARGET=/opt/maxkb/python-packages


Expand Down
4 changes: 2 additions & 2 deletions ui/src/workflow/common/validate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ export class WorkFlowInstance {
const base_node_list = this.nodes.filter((item) => item.id === WorkflowType.Base)
return base_node_list[0]
}
extis_break_node() {
return this.nodes.some((item) => item.id === WorkflowType.LoopBreakNode)
exist_break_node() {
return this.nodes.some((item) => item.type === WorkflowType.LoopBreakNode)
}
/**
* 校验工作流
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code has a minor issue with the exist_break_node method. The comparison operation is incorrect; it should compare item.type rather than item.id. Here’s the corrected version of the method:

@@ -84,7 +84,7 @@ export class WorkFlowInstance {
     const base_node_list = this.nodes.filter((item) => item.id === WorkflowType.Base);
     return base_node_list[0];
   }
-  extis_break_node() {
+  exist_break_node() {
     return this.nodes.some((item) => item.type === WorkflowType.LoopBreakNode);
   }

This change ensures that the correct property (type) is checked when looking for nodes to determine if there is a loop break node present in the workflow instance.

No other irregularities or major issues were identified in the rest of the code. If you have any further questions or need additional assistance, feel free to ask!

Expand Down
2 changes: 1 addition & 1 deletion ui/src/workflow/nodes/loop-body-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const validate = () => {
const loop_node = props.nodeModel.graphModel.getNodeModelById(loop_node_id)
try {
workflow.is_loop_valid()
if (loop_node.properties.node_data.loop_type == 'LOOP' && !workflow.extis_break_node()) {
if (loop_node.properties.node_data.loop_type == 'LOOP' && !workflow.exist_break_node()) {
return Promise.reject({
node: loop_node,
errMessage: t('views.applicationWorkflow.validate.loopNodeBreakNodeRequired'),
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code you provided has a few issues:

  1. The variable exist_break_node should be capitalized as it is likely defined elsewhere and used with capitalization.

  2. In the string 'views.applicationWorkflow.validate.loopNodeBreakNodeRequired', there might be incorrect characters that need to be corrected, such as single quotes instead of double ones if they are meant to be displayed directly in the user interface.

Here's an optimized version of the function assuming those corrections are made:

const validate = () => {
  const loop_node = props.nodeModel.graphModel.getNodeModelById(loop_node_id);
  try {
    workflow.is_loop_valid();
    // Check if the loop type is 'LOOP' and ensure a break node exists
    if (loop_node.properties.node_data?.loop_type === 'Loop' && !workflow.exists_break_node()) {
      return Promise.reject({ 
        node: loop_node,
        errMessage: 'loops must have at least one exit point'
      });
    }
  } catch (error) {
    console.error(error.message); // Log errors for debugging purposes
    throw error; // Re-throw the error if validation fails
  }
};

This version includes:

  • Corrected spelling and casing.
  • Added proper object destructuring syntax for better readability.
  • Included a comment explaining the purpose of the condition.
  • Added logging for thrown errors.
  • Ensured that the code is wrapped in a promise reject call only if the conditions fail.

Expand Down
Loading