🤖 Remove lease concept from file operations #136
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Leases were causing unnecessary errors, forcing models to re-read files repeatedly and draining context. Models often fallback to sed anyway, and this tool is designed to keep changes isolated within worktrees. Changes: - Remove lease parameter from file_edit_replace and file_edit_insert tools - Remove lease from file_read tool results - Remove lease validation logic from all file operations - Update type definitions to remove lease fields - Update and clean up tests to remove lease-related assertions - Remove lease-specific tests that are no longer relevant The file operations now work without lease validation, simplifying the API and reducing context drain.
![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
https://github.com/coder/cmux/blob/16ad58e6a433edeadb20790b7e003f63d8fd175d/src/components/Messages/ToolMessage.tsx#L53
After removing lease from file edit arguments, the type guards still require a lease property. Because the backend no longer provides or expects that field, these guards always return false and every file edit call falls through to GenericToolCall, so users lose the diff-oriented FileEditToolCall UI. This affects both replace and insert guards (the insert check has the same condition a few lines below).
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
ammar-agent
added a commit
that referenced
this pull request
Oct 10, 2025
The type guards for `file_edit_replace` and `file_edit_insert` tools were checking for the presence of `lease` in args, which is no longer part of the tool API after PR #136. Removes the `"lease" in args` checks from: - `isFileEditReplaceTool()` type guard - `isFileEditInsertTool()` type guard This allows the UI components to properly recognize and display these tool calls without the lease parameter.
sreya
pushed a commit
that referenced
this pull request
Nov 15, 2025
## Problem After removing the lease concept from file operations in #136, the type guards in `ToolMessage.tsx` were still checking for the presence of `lease` in the tool arguments. This exposed a deeper architectural issue: **hand-written type guards can drift from type definitions**. ## Root Cause Type guards used manual property checks that TypeScript can't verify match the actual types: ```typescript // ❌ Problem: Can get out of sync with type definition function isFileEditReplaceTool(toolName: string, args: unknown): args is FileEditReplaceToolArgs { return ( toolName === "file_edit_replace" && typeof args === "object" && args !== null && "file_path" in args && "edits" in args && "lease" in args // ← TypeScript doesn't verify this matches FileEditReplaceToolArgs! ); } ``` ## Solution: Schema-Driven Type Guards Use the existing Zod schemas from `toolDefinitions.ts` as the single source of truth: ```typescript // ✅ Solution: Uses schema validation - always in sync import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; function isFileEditReplaceTool(toolName: string, args: unknown): args is FileEditReplaceToolArgs { if (toolName !== "file_edit_replace") return false; return TOOL_DEFINITIONS.file_edit_replace.schema.safeParse(args).success; } ``` ## Benefits 1. **Single source of truth**: Zod schemas define both runtime validation and TypeScript types 2. **Automatic sync**: Schema changes automatically propagate to type guards 3. **More robust**: Validates full structure, not just key presence 4. **Future-proof**: Prevents entire category of drift bugs ## Changes - Import `TOOL_DEFINITIONS` from `toolDefinitions.ts` - Replace all manual type guards with schema-based validation - Apply to all tools: `bash`, `file_read`, `file_edit_replace`, `file_edit_insert`, `propose_plan` - Net result: -33 lines of manual validation code, +13 lines of schema-driven validation ## Related - Depends on #136 (Remove lease concept from file operations) _Generated with `cmux`_
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Leases were leading to many errors and forcing models to re-read files repeatedly, which drains context unnecessarily.
Problem
The lease system added complexity and friction:
Solution
Remove the lease concept entirely:
bash+sedanyway when tool errors occurChanges
leaseparameter fromfile_edit_replaceandfile_edit_insertleasefromfile_readresultsWhy This Works
This tool environment is designed for isolated changes:
bashfor complex file operations if neededGenerated with
cmux