Repository navigation
fix(chat): preserve raw Copilot tool arguments - #2024
Conversation
Refs #3 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
String argument handling server/copilot.ts, server/copilot.test.ts, changelog.d/copilot-raw-tool-input.md |
Assistant tool-request validation accepts string arguments. addTool parses valid JSON strings for summaries and uses an empty object when parsing fails, while storing string input verbatim. Tests cover display limits, output references, errors, and final-answer retention. The changelog records the fix. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Possibly related issues
- devswha/ovis#1574: This change supports displaying tool activity in Copilot transcripts, but does not implement the full native transcript adapter.
Possibly related PRs
- devswha/ovis#1956: Both changes affect Copilot tool-output references; this PR’s tests check that output references remain retrievable.
- devswha/ovis#1652: Both changes extend Copilot transcript parsing for tool requests and results.
Merge Risk: 
cc842
-
String tool arguments retain bounded raw input for display, with no identified issue blocking merge after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: preserving raw Copilot tool arguments in chat. Description check The description includes the required Change and Validation sections. It explains the behavior, preserved constraints, related issue, test coverage, known unrelated failures, and unrun environments. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Verified at cc842f8, rebased locally onto current main (7c0f34c) without conflicts.
No findings: call-ID/name validation, unsupported-container rejection and offset output retrieval are unchanged. Ready for the merge pass. Thanks, @YuuuXie. |
|
Thanks @YuuuXie. The merge-pass gate found nothing in the code. Raw-input bounds, malformed input, text rendering, root/subagent filtering and signed output retrieval all hold, and the unit suite passes on top of main (7431/0). Both new tests fail without the change. One small request: please add the contributor credit to |
|
Correction from the merge pass: no changelog change is needed after all. Per changelog.d/README.md, a maintainer adds |
|
Merge pass review report. Overall: PASSED. This outside contribution from @YuuuXie keeps Copilot's raw tool arguments. The maintainer approved merging it once the gate passed, and the gate (omo-native-gate-reviewer, gpt-6-astra) passed it. What it does: Copilot can store tool arguments as raw strings, including invalid JSON. Ovis now keeps a bounded copy of that raw input for display, and parses it best-effort only to build summaries, the same way the Hermes and Codex readers do. Gate verification:
A maintainer adds the changelog credit when the release folds the fragments (changelog.d/README.md). |
## Release 0.4.19 The release PR, made by the steps in [docs/development.md#releasing](docs/development.md#releasing), on main 9d76cca (#2078): - **Version:** `version` is 0.4.19 in `package.json` and `herdr-plugin.toml`. - **Changelog:** `bun scripts/changelog-fold.ts 0.4.19` moved the `changelog.d` fragments of 54 PRs into CHANGELOG.md, with PR links, and deleted them. - Outside contributors are credited: #1955 @YuuuXie, #1957 @Noobear, #1984 @bonecool999, #2003 @philipprass-prog, #2016 @kyop21, #2024 @YuuuXie, #2025 @YuuuXie, #2027 @jpcarranza94, #2030 @phlips5437, #2031 @yaaayaaa, #2034 @jpcarranza94, #2035 @nahwan-kim, #2041 @odfalik, #2045 @mattmarcin, #2052 @jpcarranza94, #2057 @kyop21, #2061 @YuuuXie. - **Release summaries:** `release-summaries.json` has the 0.4.19 entry in en, ko, ja and zh, with the same lists in each: 5 new, 5 improved, 5 fixed. Every English line is at most 70 characters. - **Remote bundle:** `REMOTE_BUNDLE_VERSION` goes from 37 to 38 in `shared/machines.ts`, because the remote bundle's inputs changed since `remote-v37`. Once this merges, `remote-v38` is pushed on the merge commit, and the Remote PC bundles workflow has to finish before the Release run. `bun scripts/release-guards.ts` reports: "Release 0.4.19: remote bundle, telemetry Worker and release metadata are in order." No telemetry Worker deploy is needed. `bun run check fast` passes locally. **Not checked on a real Windows PC:** #1989 (Git for Windows' OpenSSH for remote PCs) and #2016 (Antigravity chat on Windows through the Restart Manager) ship without their real-device Windows checks, because the maintainer's Windows test PC is off. The owner accepted releasing without them. Both will be checked on the real PC when it is back. The owner approved this release.
Change
Copilot can persist tool arguments as raw strings, including invalid JSON inside an otherwise valid event. Preserve bounded raw input for display and best-effort parse it only for summaries, following the existing Hermes/Codex readers.
Keep call-ID/name validation, root/subagent filtering, request/result association and signed offset-based output retrieval unchanged. Arrays and other unsupported assistant-request containers remain rejected.
Tracks YuuuXie#3. Independent branch from upstream main
e04cb9f9; no personal deployment tooling.Validation
bun run check fast: workflow syntax, generated files, typecheck and build passed. Unit suite: 7,286 passed, 3 failed, 24 skipped; failures were unchanged Codex/composer timing bounds and the installer's 19-second lower bound, not Copilot regressions.Full browser/integration lanes and native Windows were not run locally for this parser-only fix. Upstream CI and maintainer merge remain required.