feat: add native mergiraf tool - #38
Conversation
There was a problem hiding this comment.
Sorry @trotsky1997, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Review request note: I attempted to add GitHub reviewer |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c859af190
ℹ️ 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".
| let ours = required_path(¶ms.ours, "ours", "merge")?; | ||
| let theirs = required_path(¶ms.theirs, "theirs", "merge")?; | ||
|
|
||
| let mut args = vec!["merge".to_string(), base, ours, theirs]; |
There was a problem hiding this comment.
Pass --git so merge updates the ours file
For real mergiraf merge calls this argv omits the merge-driver flag, so the command only prints the merge result to stdout and leaves the ours path unchanged. I checked the upstream CLI semantics: the documented Git driver form is mergiraf merge --git %O %A %B, and the implementation writes back to the left/ours file only in the git branch; otherwise it prints the result. In any workflow expecting this native tool to resolve the working-tree file in place, the tool can return ok: true while the conflicted file remains unmodified.
Useful? React with 👍 / 👎.
| if rendered.len() <= max_output_bytes || best == 0 { | ||
| return Ok(rendered); |
There was a problem hiding this comment.
Enforce max_output_bytes for oversized envelopes
When max_output_bytes is smaller than the fixed JSON envelope, or when stderr/command metadata alone exceed the budget, this branch returns the rendered JSON even though it is still over the requested limit. For example, max_output_bytes: 1 or a very long stderr/cwd still produces an over-budget response despite truncated: true, which violates the tool contract that callers can bound returned output size.
Useful? React with 👍 / 👎.
| Ok(vec![ | ||
| "solve".to_string(), | ||
| required_path(¶ms.file, "file", "solve")?, |
There was a problem hiding this comment.
Pass the language override through solve
When solve is used on a conflict file whose extension does not identify the language, upstream mergiraf needs the same --language/-L override that it supports for merge; I checked the upstream CLI, where the shared MergeOrSolveArgs is flattened into both Merge and Solve, and upstream tests run mergiraf solve --language=json. This wrapper accepts language but ignores it for solve, so a caller-supplied override still results in a failed solve for otherwise resolvable files.
Useful? React with 👍 / 👎.
| if params.allow_parse_errors { | ||
| args.push("--allow-parse-errors".to_string()); |
There was a problem hiding this comment.
Don't emit unsupported --allow-parse-errors
When allow_parse_errors is true, the wrapper appends a flag that the current mergiraf CLI does not define; I checked the upstream merge/shared option definitions and they include --debug, --compact, marker length, --language, merge-specific flags like --git, but no --allow-parse-errors. With a real binary this makes the documented option fail argument parsing before any merge runs.
Useful? React with 👍 / 👎.
Adds the full spec-driven planning artifacts for the mergiraf native tool change: proposal, design, specs (mergiraf-tool + tools delta), and tasks. Tracked in GitHub issue #36. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
4c859af to
a9ecc3f
Compare
Summary
Validation
Refs #36
Remaining OpenSpec tasks in this change: documentation tasks 3.1-3.3 (README, spec/tools.md, ra.toml/init template docs).