[Orsted290]

问题与修复

模型同时看到 edit_file 的五组蛇形/驼峰参数、exec 的 command/cmd 等同义字段；执行器按优先级取值，可能静默忽略另一份输入。max_output_tokens 名称还与实际字符截断单位不一致。

• edit_file 的模型 schema 只暴露规范字段；兼容层转换并归一化旧别名，同值合并、异值明确报错，并在任何文件访问前拒绝。
• 保留显式空字符串：new_text="" 仍用于删除；空串与非空串同时传入报冲突，不擅自选取非空值。null/undefined 保持未提供语义。
• exec 只公开 command、cwd、timeout、shell、login、yield_time_ms、max_output_chars；旧命令/目录/超时/等待时间别名继续兼容，异值冲突在命令准备、进程或会话创建前拒绝。
• exec/write_stdin 统一公开 max_output_chars，明确为 UTF-16 码元（JavaScript string.length），不是 token；旧 token 命名字段隐藏并保持历史字符额度含义，不估算 token 换算。
• write_stdin 同步归一化旧别名，在输入、关闭或终止等副作用前检查冲突；保留既有直接调用超时上限兼容。

冲突会作为工具错误回传，调用方可修正后重试；执行器不会自动选择某一份值执行。

改动范围

App/memmy-agent/ 下：

• src/core/agent-runtime/tools/{filesystem,shell,exec-session,parameter-aliases}.ts
• src/templates/agent/tool-contract.md
• tests/core/agent-runtime/tools/{edit-file-aliases,exec-aliases,exec-session-tools}.test.ts

本 PR 不包含定位模式校验顺序的修复，该缺陷由另一条 PR 处理。

迁移与验证

替代原 PR #663，从上游 v1.2.6（8cd095c7）新建分支，仅 cherry-pick 原 PR 的修复提交，不携带 v1.2.4 的其他历史提交。git range-diff 确认迁移前后补丁等价。

• 本分支在新基线上的回归：20 个文件、565 项通过。
• 两项修复在 v1.2.6 上组合验证：21 个文件、669 项通过。
• git diff --check 通过。
• 本轮未重新打包、运行桌面验收或全量类型检查；此前旧基线上的验收不能替代本版本验收。

合并协调

两条新 PR 均独立以 v1.2.6 为基线。修复修改相邻参数入口与工具契约，先合并一条后另一条仍需协调重叠修改；必须同时保留别名归一化与定位校验。已迁移原组合结果并通过上述联合回归。旧 PR 在新 PR 创建后可关闭，避免重复合并。

[origami-chen]

有个单测过不了。
App/memmy-agent/tests/core/agent-runtime/tools/edit-file-validation.test.ts:173,174两行代码改成下面的：
{ occurrence: 1, replace_all: false, replaceAll: false, line_hint: null, lineHint: null },
])("accepts unused selectors and matching aliases: %j", async (options) => {

[Memtensor-AI]

🤖 Open Code Review

Target: PR #675
Task: 39dfdeb1d89ee7b5
Base: v1.2.6
Head: fix-tool-aliases-output-units-v126

🔍 OpenCodeReview found 12 issue(s) in this PR.

---

1. App/memmy-agent/src/core/agent-runtime/tools/filesystem.ts (L809-L810)

This is a private re-implementation of normalizeParameterAliases/castParameterAliases added in parameter-aliases.ts in the same change, and the copies have already diverged:

• null handling: the local version treats an explicit null alias as supplied (legacy !== undefined) and copies it into the canonical slot, while the shared version treats null as unset and deletes the canonical key. { old_text: null, oldText: null } therefore reports "old_text should be string" here but "missing required old_text" through the shared helper.
• the conflict message here omits the Use only ${canonical} hint.

Two copies of the same normalization will keep drifting. Importing the shared helpers keeps the alias tables the single source of truth; EDIT_FILE_ALIASES is structurally compatible with the shared group shape.

💡 Suggested Change

Before:

/** Normalize legacy inputs while rejecting ambiguous values before file access. */
function normalizeEditAliases(params: Record<string, any>): { params: Record<string, any>; error: string | null } {

After:

import { castParameterAliases, normalizeParameterAliases } from "./parameter-aliases.js";

const EDIT_FILE_ALIASES = [
  ["old_text", "oldText"],
  ["new_text", "newText"],
  ["replace_all", "replaceAll"],
  ["line_hint", "lineHint"],
  ["expected_replacements", "expectedReplacements"],
] as const;

---

2. App/memmy-agent/src/core/agent-runtime/tools/filesystem.ts (L969-L973)

Alias normalization now runs three times per call: this.castParams(params) (which already calls normalizeEditAliases), the explicit normalizeEditAliases(params) here, and again inside this.validateParams(params) further down, which also re-checks old_text/new_text that were already null-checked above. Only the first call can actually change anything, so the extra passes add ambiguity about which layer owns validation. Simplifying to castParams + validateParams (and reading params.old_text/new_text directly, as the sibling tools now do) removes the redundant work.

---

3. App/memmy-agent/src/core/agent-runtime/tools/parameter-aliases.ts (L42-L43)

converted is whatever the caller-supplied cast returned, and this function then writes to it (converted[alias] = ...) and lets normalizeParameterAliases delete keys from it. The base Tool.castParams returns the input object itself when the schema type is not "object" (and castObject likewise short-circuits), so a future reuse of this helper can mutate the caller's params in place — deleting alias keys and injecting canonical ones into an object the dispatcher may reuse or log. Copy before mutating so the helper's contract is "never touch the caller's object", which the existing edit_file alias test explicitly asserts.

💡 Suggested Change

Before:

  const converted = cast(params);
  if (!converted || typeof converted !== "object" || Array.isArray(converted)) return converted;

After:

  const castResult = cast(params);
  if (!castResult || typeof castResult !== "object" || Array.isArray(castResult)) return castResult;
  // `cast` may return its input; copy before injecting/deleting alias keys.
  const converted: Record<string, any> = { ...castResult };

---

4. App/memmy-agent/src/core/agent-runtime/tools/parameter-aliases.ts (L9-L11)

The model-facing description hardcodes "Defaults to 10000" while the real default lives in DEFAULT_MAX_OUTPUT_CHARS (exec-session.ts) and ExecTool.DEFAULT_OUTPUT_CHARS (shell.ts). This module cannot import exec-session.ts without creating a cycle, so a future default change silently leaves the schema description stating the wrong behavior. Taking the default as an argument (or exporting a builder) keeps the text derived from the constant that is actually used.

💡 Suggested Change

Before:

export const OUTPUT_LIMIT_DESCRIPTION =
  "Maximum retained command output in UTF-16 code units (JavaScript string length), not tokens. " +
  "Defaults to 10000; truncation notices and status metadata are additional.";

After:

export function outputLimitDescription(defaultChars: number): string {
  return (
    `Maximum retained command output in UTF-16 code units (JavaScript string length), not tokens. ` +
    `Defaults to ${defaultChars}; truncation notices and status metadata are additional.`
  );
}

---

5. App/memmy-agent/src/core/agent-runtime/tools/exec-session.ts (L635-L637)

session_id is required but has no minLength, and nothing here rejects an empty string: {session_id: ""} passes super.validateParams, then execute reads it via params.session_id! and manager.write throws exec session not found: with a blank id. ExecTool gained an equivalent blank check for command in this same PR, so the two tools now validate required strings inconsistently. Add the empty check here (or minLength: 1 on the schema) for parity.

💡 Suggested Change

Before:

    const normalized = normalizeParameterAliases(params, STDIN_ALIASES);
    if (normalized.error) return [normalized.error];
    return super.validateParams(normalized.params);

After:

    const normalized = normalizeParameterAliases(params, STDIN_ALIASES);
    if (normalized.error) return [normalized.error];
    const errors = super.validateParams(normalized.params);
    if (errors.length) return errors;
    return normalized.params.session_id.trim() ? [] : ["session_id must not be blank."];

---

6. App/memmy-agent/src/core/agent-runtime/tools/exec-session.ts (L629-L631)

The castParams/validateParams override pair is now copy-pasted across ExecTool, WriteStdinTool and EditFileTool: the same non-object guard, the same normalize-then-report-error sequence, and the same super.* delegation on the fallback path. Only the alias table differs per tool. Since castParameterAliases/normalizeParameterAliases already live in the shared module, consider exposing a generic validateAliasedParams(params, groups) (returning the error string) or a base Tool hook so the three call sites cannot drift when one tool adds a special case.

---

7. App/memmy-agent/src/core/agent-runtime/tools/shell.ts (L277)

This description hardcodes the default output budget (10 000) even though it is already defined as ExecTool.DEFAULT_OUTPUT_CHARS and is restated with different formatting (")Defaults to 10000") in OUTPUT_LIMIT_DESCRIPTION. Two independent prose copies of a business constant will silently go stale if the constant changes. Interpolate the constant (or drop the number from the description) so there is one source of truth.

💡 Suggested Change

Before:

      "Output defaults to 10 000 UTF-16 code units, controlled by max_output_chars (not tokens); timeout defaults to 60s."

After:

      `Output defaults to ${ExecTool.DEFAULT_OUTPUT_CHARS.toLocaleString("en-US")} UTF-16 code units, controlled by max_output_chars (not tokens); timeout defaults to 60s.`

---

8. App/memmy-agent/src/core/agent-runtime/tools/shell.ts (L539-L541)

The same logical input now behaves differently depending on the entry point: through ToolRegistry.prepareCall an oversized timeout fails with "timeout must be <= 600" (validation runs before execute), while a direct exec() call silently rewrites it to ExecTool.MAX_TIMEOUT. Every other bounded numeric in this schema (yield_time_ms, max_output_chars) now rejects out-of-range values instead of clamping, so timeout is the only one with a silent rewrite — and resolveTimeout already applies Math.min(timeout, MAX_TIMEOUT), so the block mainly duplicates that clamp while hiding the caller's mistake. Consider picking one policy (drop the pre-cap and let resolveTimeout clamp, or keep the cap and drop maximum from the schema) so the two paths agree.

---

9. App/memmy-agent/src/core/agent-runtime/tools/exec-session.ts (L669)

wait_for has no minLength, so { session_id, wait_for: "" } passes validateParams and then falls into the plain-write branch because the empty string is falsy: the caller asked to wait for a target and silently gets a one-poll result with no Wait target not observed marker. This contradicts normalizeParameterAliases' stated rule that empty strings are explicit values (which is why { wait_for: "", waitFor: "ready" } is reported as a conflict). Either reject a blank wait_for in validateParams alongside the other argument checks, or document that empty means "no wait".

---

10. App/memmy-agent/src/core/agent-runtime/tools/parameter-aliases.ts (L18)

normalizeParameterAliases indexes params without the guard its sibling castParameterAliases applies. Called with null/undefined it throws TypeError on params[key]; called with an array it silently builds a garbage {"0": ..., "1": ...} object because { ...[] } spreads indices. Today every caller happens to guard, but this is an exported helper intended for reuse, so a future caller (e.g. a plugin tool) gets a confusing crash. Add the same if (!params || typeof params !== "object" || Array.isArray(params)) guard and return an error/params unchanged.

---

11. App/memmy-agent/src/core/agent-runtime/tools/parameter-aliases.ts (L46)

The cast callback must be the base cast (super.castParams in every current caller). If a caller passes this.castParams instead, this line re-enters castParameterAliases and recurses until the stack overflows, and there is nothing in the signature or docs that says so. Please document the precondition in a JSDoc on castParameterAliases (or take the tool/schema instead of a callback and invoke Tool.prototype.castParams internally) so the trap isn't left for the next tool that adopts aliases.

---

12. App/memmy-agent/src/core/agent-runtime/tools/filesystem.ts (L978)

Now that execute routes through validateParams, which runs super.validateParams against occurrence/line_hint/expected_replacements (type: ["integer", "null"], minimum: 1) before calling editOptionsError, the numeric loop in editOptionsError (value < 1 / !Number.isInteger(value)) can never fire — any value reaching it is already a positive integer or null. That leaves two validation layers where only one is reachable. Either drop the redundant loop so the schema owns numeric bounds, or add a comment stating the schema is authoritative and the loop is defensive for callers that bypass validateParams.

Generated by cloud-assistant via Open Code Review.