Skip to content

feat: parse web_sphere_mq TSH message#11484

Merged
taloric merged 1 commit intomainfrom
add-mq-parse
Mar 20, 2026
Merged

feat: parse web_sphere_mq TSH message#11484
taloric merged 1 commit intomainfrom
add-mq-parse

Conversation

@lzf575
Copy link
Copy Markdown
Contributor

@lzf575 lzf575 commented Mar 19, 2026

This PR is for:

@lzf575 lzf575 enabled auto-merge (rebase) March 19, 2026 08:30
@lzf575 lzf575 force-pushed the add-mq-parse branch 2 times, most recently from ea3b978 to 3621b83 Compare March 19, 2026 08:31
@lzf575
Copy link
Copy Markdown
Contributor Author

lzf575 commented Mar 19, 2026

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 19, 2026

Claude finished @lzf575's task in 13m 16s —— View job


- Event: PR comment on #11484 requesting `@claude review`. - Request type: code review only; no implementation requested. - Reviewed the diff against `origin/main` and the touched config/parser paths. - No `CLAUDE.md` file was present in this checkout. - Result: I found 2 issues worth addressing before merge. - Validation note: I did not run `cargo check` in this review run because shell execution required additional approval.

Todo

  • Review repository instructions and PR context
  • Inspect the PR diff and changed files
  • Analyze the changes for correctness and risks
  • Post the review summary to the PR comment

Review

I found 2 issues:

  1. The new config knob is exposed under the wrong key, so users cannot configure it correctly

    server/agent_config/template.yaml:5916 adds decompress_enabled_, but the agent only deserializes decompress_enabled in agent/src/config/config.rs:1835-1845. The same typo is also documented in server/agent_config/README.md:8300-8318 and server/agent_config/README-CH.md:8117-8135.

    This is not just a docs typo: the server-side template/validator uses the template key verbatim when building and validating config paths, and it only strips the _comment suffix (server/agent_config/template_json.go:156-163, server/agent_config/template_json.go:776-781). That means a user-provided decompress_enabled key will be rejected as unexpected, while decompress_enabled_ is what gets propagated. Since the Rust config struct has no alias for that key, this option is effectively not configurable.

    Fix this →

  2. skip_frame is checked but never consumed, so the flag can leak into later frames

    In agent/src/flow_generator/protocol_logs/mq/web_sphere_mq.rs:323-325, the wrapper skips output when self.parser.skip_frame is true, but it never clears that flag afterward. Because the flag lives on the parser instance itself (agent/crates/enterprise-utils/src/lib.rs:334-338), one skipped frame can suppress later frames handled by the same parser instance unless the EE parser implementation resets the flag on every call.

    This wrapper should consume/reset the flag itself, e.g. with std::mem::take(&mut self.parser.skip_frame), so the state is guaranteed to be per-frame.

    Fix this →

Validation

  • Not run in this review: cargo check / tests, because shell execution needed additional approval in this run.

@lzf575
Copy link
Copy Markdown
Contributor Author

lzf575 commented Mar 19, 2026

@claude review in 中文

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 19, 2026

Claude finished @lzf575's task in 5m 29s —— View job


- 事件类型:PR 评论触发(GENERAL_COMMENT),触发词为 `@claude`。 - 请求类型:代码审查;触发评论明确要求“review in 中文”,因此仅输出中文 review,不实现代码修改。 - 关键上下文:PR 分支为 `add-mq-parse -> main`;按要求以 `origin/main` 作为审查基线。仓库中未发现 `CLAUDE.md` 文件。 - 已核对的重点文件:`agent/src/flow_generator/protocol_logs/mq/web_sphere_mq.rs`、`agent/crates/enterprise-utils/src/lib.rs`、`agent/src/config/config.rs`、`agent/src/config/handler.rs`、`server/agent_config/template.yaml`、`server/agent_config/README.md`、`server/agent_config/README-CH.md`、`server/agent_config/template_json.go`。 - 结论:我确认有 1 个需要在合并前修复的问题;此前英文 review 里提到的配置键名问题,在当前代码中已不存在。 - 限制说明:本次仅做静态审查,未运行 `cargo check` / 测试。

Todo

  • Review repository instructions and PR context
  • Inspect the PR diff and relevant files
  • Analyze the changes and draft Chinese review feedback
  • Update the GitHub comment with the final review

Review

我复核了这次改动和相关配置路径,目前确认有 1 个需要修复的问题

  1. skip_frame 是粘性状态,但这里读取后没有消费,可能导致后续帧被误跳过

    agent/src/flow_generator/protocol_logs/mq/web_sphere_mq.rs:323-325,当前逻辑是:

    if self.parser.skip_frame {
        pos += parsed_size;
        continue;
    }

    skip_frame 保存在解析器实例本身上(agent/crates/enterprise-utils/src/lib.rs:333-338),而不是局部变量;我在仓库里也没有看到别处对这个字段做复位。这样一来,只要某一帧把 skip_frame 置成 true,后续由同一个 WebSphereMqParser 实例处理的帧也会持续命中这个分支,被静默跳过。

    这会让 “跳过当前帧” 变成 “持续跳过后续帧”,属于状态泄漏。更稳妥的写法是让 wrapper 自己消费这个标志,例如使用 std::mem::take(&mut self.parser.skip_frame),保证它是严格的 per-frame 行为。

    Fix this →

补充说明:我也复查了此前英文 review 提到的 decompress_enabled 配置键问题。当前 server/agent_config/template.yaml:5916server/agent_config/README.md:8300-8318server/agent_config/README-CH.md:8117-8135 以及 Rust 配置结构 agent/src/config/config.rs:1835-1845 / agent/src/config/handler.rs:763-777 使用的都是 decompress_enabled,所以这个问题在当前分支代码里已经看不到了。

Validation

  • 本次 review 未运行 cargo check / 自动化测试;结论基于静态代码审查。

@lzf575 lzf575 marked this pull request as draft March 19, 2026 10:42
auto-merge was automatically disabled March 19, 2026 10:42

Pull request was converted to draft

@lzf575 lzf575 marked this pull request as ready for review March 19, 2026 11:26
@lzf575
Copy link
Copy Markdown
Contributor Author

lzf575 commented Mar 19, 2026

@claude review 用中文

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 19, 2026

Claude finished @lzf575's task in 5m 24s —— View job


- 事件类型:PR 评论触发(GENERAL_COMMENT),触发词为 `@claude`,用户要求“review 用中文”。 - 请求类型:代码审查,不实现修改。 - 已检查的重点路径:`agent/src/flow_generator/protocol_logs/mq/web_sphere_mq.rs`、`agent/crates/enterprise-utils/src/lib.rs`、`agent/src/config/config.rs`、`agent/src/config/handler.rs`、`server/agent_config/template.yaml`、`server/agent_config/README.md`、`server/agent_config/README-CH.md`。 - 仓库说明:当前 checkout 中未找到 `CLAUDE.md`。 - 结论:我确认有 1 个需要在合并前修复的问题;此前提到的 `decompress_enabled` 配置键名问题,在当前分支代码里已经对齐。 - 验证范围:本次为静态代码审查,未运行 `cargo check` / 自动化测试。

Todo

  • Read repository instructions and PR context
  • Inspect the touched files and relevant WebSphere MQ paths
  • Analyze correctness and risks
  • Post the final review in Chinese

Review

我复核了这次 PR 当前分支里的 WebSphere MQ 相关改动,当前确认有 1 个需要修复的问题

  1. skip_frame 是解析器实例上的状态,但 wrapper 读取后没有消费,可能把“跳过当前帧”扩大成“持续跳过后续帧”

    agent/src/flow_generator/protocol_logs/mq/web_sphere_mq.rs:325-327,当前逻辑是:

    if self.parser.skip_frame {
        pos += parsed_size;
        continue;
    }

    skip_frame 挂在解析器实例本身上(agent/crates/enterprise-utils/src/lib.rs:333-338),不是局部变量。我在当前仓库可见代码里也没有看到其它地方会在这里之后主动清掉这个标志。这样一来,只要某一帧把 skip_frame 设成 true,后续仍由同一个 WebSphereMqParser 实例处理的帧就可能持续命中这个分支,被静默跳过。

    更稳妥的做法是让 wrapper 自己消费这个 flag,例如改成 if std::mem::take(&mut self.parser.skip_frame) { ... },或在 continue 前明确复位,确保它只作用于当前帧。

    Fix this →

补充说明:我也复查了此前提到的 decompress_enabled 配置键名。当前 server/agent_config/template.yaml:5916server/agent_config/README.md:8300-8329server/agent_config/README-CH.md:8117-8145agent/src/config/config.rs:1835-1846agent/src/config/handler.rs:763-784 使用的都是 decompress_enabled,这一项在当前分支代码里看起来已经一致,不再是问题。

Validation

  • 本次 review 基于静态代码审查。
  • 未运行 cargo check / 自动化测试。

@lzf575
Copy link
Copy Markdown
Contributor Author

lzf575 commented Mar 20, 2026

@claude review 用中文

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 20, 2026

Claude finished @lzf575's task in 4m 57s —— View job


分析

a. 事件类型:PR 评论触发(GENERAL_COMMENT),触发词为 `@claude`,用户要求“review 用中文”。 b. 请求类型:代码审查,不实现修改。 c. 关键上下文:PR 分支为 `add-mq-parse -> main`;按要求应以 `origin/main` 为审查基线。当前会话里 shell 相关 git diff / git log 需要额外审批,因此本次主要基于已提供的 PR 上下文与对变更文件的静态阅读完成审查。仓库中未找到 `CLAUDE.md`。 d. 已检查的重点文件:`agent/src/flow_generator/protocol_logs/mq/web_sphere_mq.rs`、`agent/crates/enterprise-utils/src/lib.rs`、`agent/src/config/config.rs`、`agent/src/config/handler.rs`、`server/agent_config/template.yaml`、`server/agent_config/README.md`、`server/agent_config/README-CH.md`。 e. 主要任务:确认 WebSphere MQ 解析路径中的状态管理、配置项透传和文档/模板键名是否一致,并检查新增逻辑是否会引入持久化状态泄漏。由于是 review 请求,本次不做代码修改;由于 shell 未获批,本次未运行 `cargo check` / 自动化测试。 f. 结论:当前确认有 1 个需要在合并前修复的问题;此前提到的 `decompress_enabled` 配置键名问题,在当前分支代码里已经对齐,不再构成问题。

Todo

  • Review repository instructions and PR context
  • Inspect the PR diff and relevant files
  • Analyze the changes for correctness and risks
  • Update the GitHub comment with the final Chinese review

Review

我复核了这次 PR 当前分支里的 WebSphere MQ 相关改动,当前确认有 1 个需要修复的问题

  1. skip_frame 是解析器实例上的状态,但 wrapper 读取后没有消费,可能把“跳过当前帧”扩大成“持续跳过后续帧”

    agent/src/flow_generator/protocol_logs/mq/web_sphere_mq.rs:325-327,当前逻辑是:

    if self.parser.skip_frame {
        pos += parsed_size;
        continue;
    }

    skip_frame 挂在解析器实例本身上(agent/crates/enterprise-utils/src/lib.rs:333-338),不是局部变量。我在当前仓库可见代码里也没有看到别处会在这里之后主动清掉这个标志;仓库内对 skip_frame 的读取也只看到这一处。这样一来,只要某一帧把 skip_frame 设成 true,后续仍由同一个 WebSphereMqParser 实例处理的帧就可能持续命中这个分支,被静默跳过。

    这会让“跳过当前帧”的语义变成“持续跳过后续帧”,属于典型的状态泄漏。更稳妥的做法是让 wrapper 自己消费这个 flag,例如改成 if std::mem::take(&mut self.parser.skip_frame) { ... },或在 continue 前明确复位,确保它只作用于当前帧。

    Fix this →

补充说明:我也复查了此前提到的 decompress_enabled 配置键名。当前 server/agent_config/template.yaml:5916server/agent_config/README.md:8300-8329server/agent_config/README-CH.md:8117-8145agent/src/config/config.rs:1835-1846agent/src/config/handler.rs:763-784 使用的都是 decompress_enabled,这一项在当前分支代码里看起来已经一致,不再是问题。

Validation

  • 本次 review 基于静态代码审查。
  • 未运行 git diff origin/main...HEADcargo check 或自动化测试;当前会话里相关 shell 命令需要额外审批。

@taloric taloric merged commit 8be2c9a into main Mar 20, 2026
10 of 11 checks passed
@taloric taloric deleted the add-mq-parse branch March 20, 2026 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants