Repository navigation
Conversation
Models trained on other agent/file-reading tools frequently emit `offset`/limit` where Read declares line_offset/n_lines, which fails strict schema validation and discards the entire turn. Add an optional argAliases declaration on ExecutableTool and apply it in preflightToolCall before runtime validation, mapping the well-known aliases for Read. An alias only fills a canonical argument when it is absent, so an explicit canonical value always wins; the alias key is always removed so it can no longer trip additionalProperties.
🦋 Changeset detectedLatest commit: 291a1c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 498ef80a30
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /** | ||
| * Rewrites well-known argument aliases (e.g. `offset` -> `line_offset`) to the |
There was a problem hiding this comment.
The new JSDoc here, the inline comment at lines 52–53, toolContract.ts lines 97–103, and the test comment at test/tool/tool-args-parse.test.ts:50 are all inside packages/agent-core-v2/{src,test}. scripts/check-no-comments.mjs scans exactly these trees, so pnpm lint will fail until all four comments are removed rather than merely moving the one here.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
| readonly argAliases: Readonly<Record<string, string>> = { | ||
| offset: 'line_offset', | ||
| limit: 'n_lines', |
There was a problem hiding this comment.
Accepting these aliases changes shipped CLI-visible Read behavior: calls that previously returned Invalid args now run. The diff contains no .changeset file, so this user-useful bug fix will be absent from the next release changelog; run the required changeset workflow and add a confirmed patch entry for @moonshot-ai/kimi-code.
AGENTS.md reference: AGENTS.md:L90-L91
Useful? React with 👍 / 👎.
Summary
Closes #4559.
Models trained on other agent/file-reading tools frequently emit
offset/limit, but theReadtool's schema declaresline_offset/n_lines. BecausetoInputJsonSchemasetsadditionalProperties: falseon every object node, a single stray alias key causesInvalid args for tool "Read": must NOT have additional property 'offset'and the whole turn is discarded.Root cause
closeObjectNodes(ininput-schema.ts) hard-codesadditionalProperties: false, and the same schema object is used both as the LLM-facing tool description and the runtime validator invalidateExecutableToolArgs. Strict rejection is intentional (seeinput-schema.test.ts), so this PR does not relax it.Changes
toolContract.ts— add an optionalargAliasesdeclaration toExecutableTool(Readonly<Record<string, string>>, alias → canonical name).tool-args-parse.ts— addapplyToolArgAliases, a pure helper that rewrites alias keys to canonical names before validation.toolExecutorService.ts— applyapplyToolArgAliases(tool.argAliases, parsedArgs.data)inpreflightToolCallbeforevalidateExecutableToolArgs.readTool.ts— declareargAliases = { offset: 'line_offset', limit: 'n_lines' }.tool-args-parse.test.ts— new unit tests covering mapping, canonical-wins precedence, alias-key removal, non-mutation, and non-object passthrough.Design notes
additionalPropertieseven when both alias and canonical are present.additionalProperties: genuine typo arguments are still rejected, preserving the existinginput-schema.test.tscontract.Glob,Grep) that face the same prior.Verification
Local environment is Node 22 while the repo requires Node >=24.15, so the unit tests could not be executed locally against the toolchain; the
applyToolArgAliaseslogic was verified with a standalone Node script for all cases. CI will run the fullvitest+ typecheck.