diff --git a/docs/opx.md b/docs/opx.md new file mode 100644 index 0000000..6a76bcf --- /dev/null +++ b/docs/opx.md @@ -0,0 +1,293 @@ +# OPX v1: Overwrite Patch XML — Product Requirements Document (PRD) + +## Summary + +OPX is an original XML-based change protocol for Overwrite that LLMs produce and the extension applies to the workspace. + +Operations supported: + +- Create files +- Patch specific regions of files (search-and-replace) +- Replace entire files +- Remove files +- Move/rename files + +## Goals + +- Replace the existing immediately with a new, original protocol. +- Keep the format minimal and highly reliable for LLMs to generate. +- Map OPX operations directly onto our existing apply engine with no behavior changes beyond parsing. + +## Non‑Goals + +- No phased rollout or legacy compatibility. +- No telemetry changes. +- No new runtime dependencies. + +--- + +## Protocol Specification + +### Top-level + +- One or more `` elements may appear at the top level. +- Optionally, edits may be wrapped in a single `...` container. The container is ignored by the parser. + +### Edit element + +Element: `` +- Required attributes: + - `file`: path to the target file (prefer workspace-relative) + - `op`: operation type (see below) +- Optional attributes: + - `root`: VS Code workspace root folder name for multi-root workspaces + +Supported `op` values and semantics: +- `new` — create a new file. Requires a `` child. +- `patch` — search-and-replace a region. Requires both `` and `` children. +- `replace` — replace entire file contents. Requires a `` child. +- `remove` — delete file. No children required. +- `move` — rename/move file. Requires a `` child. + +### Children of `` + +- `` (optional): A brief sentence describing the change for this edit. +- `` (for `op="patch"`): + - Optional attribute: `occurrence="first|last|N"` to disambiguate repeated matches. + - Contains a literal block delimited by `<<<` (start) and `>>>` (end) on their own lines. +- `` (for `op="new"`, `op="patch"`, `op="replace"`): + - Contains a literal block delimited by `<<<` and `>>>` on their own lines. +- `` (for `op="move"`): + - Self-closing element with a required `file` attribute specifying destination path. + +### Literal content blocks + +- Inside `` and ``, the payload must be wrapped between lines containing only `<<<` and `>>>`. +- The parser takes all text strictly between those markers. Surrounding whitespace/newlines around markers are trimmed. +- Parser auto-heal: to mitigate Markdown/chat copy issues, if a line inside ``/`` contains only `<` or `<<`, it is treated as `<<<`; if it contains only `>` or `>>`, it is treated as `>>>`. This repair happens before extraction and affects only full-line markers. + +### Path rules + +- Prefer workspace-relative paths (e.g., `src/lib/logger.ts`). +- `file://` URIs and absolute paths are tolerated but not required. +- Do not reference paths outside the workspace. + +### Examples + +New file +```xml + + Create utility module + +<<< +export function titleCase(s: string): string { + return s.split(/\s+/).map(w => w ? w[0]!.toUpperCase() + w.slice(1) : w).join(' ') +} +>>> + + +``` + +Patch region +```xml + + Add timeout and error logging + +<<< +export async function fetchUser(id: string) { + const res = await fetch(`/api/users/${id}`); + if (!res.ok) throw new Error(`Request failed: ${res.status}`); + return res.json(); +} +>>> + + +<<< +async function withTimeout(p: Promise, ms = 10000): Promise { + const t = new Promise((_, r) => setTimeout(() => r(new Error('Request timed out')), ms)); + return Promise.race([p, t]); +} + +export async function fetchUser(id: string) { + try { + const res = await withTimeout(fetch(`/api/users/${id}`), 10000); + if (!res.ok) throw new Error(`Request failed: ${res.status}`); + return res.json(); + } catch (err) { + console.error('[api] fetchUser error', err); + throw err; + } +} +>>> + + +``` + +Replace entire file +```xml + + +<<< +export interface AppConfig { + apiBaseUrl: string; + enableTelemetry: boolean; + maxConcurrentJobs: number; +} + +export const config: AppConfig = { + apiBaseUrl: process.env.API_BASE_URL || 'http://localhost:3000', + enableTelemetry: process.env.TELEMETRY === '1', + maxConcurrentJobs: Number(process.env.MAX_JOBS || 4), +}; +>>> + + +``` + +Remove file +```xml + +``` + +Move / rename file +```xml + + + +``` + +--- + +## Architecture Changes (Immediate) + +All changes below are applied at once to switch the system fully to OPX v1. + +### Parser — OPX only + +File: [src/utils/xml-parser.ts](file:///Users/minhthanh/Work/Side/overwrite/src/utils/xml-parser.ts) + +- Remove legacy parsing and markers; accept only OPX tags. +- New parse path: + - Accept either top-level `` elements or a single `` wrapper containing `` elements. + - For each ``: + - Read attributes `file`, `op`, optional `root`. + - `op -> action` mapping: + - `new` → `create` + - `patch` → `modify` + - `replace` → `rewrite` + - `remove` → `delete` + - `move` → `rename` + - For `patch`: extract `` (with optional `occurrence`) and `` blocks. + - For `new|replace`: extract `` block. + - For `move`: read `` into `newPath`. + - Literal extraction uses `<<<`/`>>>` only. +- Return the same `ParseResult` and `FileAction[]` types used by the apply engine. + +### Preprocessor (Apply tab) + +File: [src/webview-ui/src/components/apply-tab/preprocess.ts](file:///Users/minhthanh/Work/Side/overwrite/src/webview-ui/src/components/apply-tab/preprocess.ts) + +- Extend normalization to `` and `` tags (lowercase keys, normalize quotes). +- Remove legacy tag handling (``, ``, etc.). +- Lint rules: + - `` must include `file` and `op`. + - `op="patch"` requires both `` and ``. + - `op="move"` requires ``. + +### Apply Pipeline (no changes) + +File: [src/providers/file-explorer/file-action-handler.ts](file:///Users/minhthanh/Work/Side/overwrite/src/providers/file-explorer/file-action-handler.ts) + +- No behavior changes required. The parser still emits `action` in `{create|modify|rewrite|delete|rename}` and the same `changes`/`newPath` fields. + +### Prompt Text + +File: [src/prompts/xml-instruction.ts](file:///Users/minhthanh/Work/Side/overwrite/src/prompts/xml-instruction.ts) + +- Replace the entire instruction constant with OPX-only wording and examples. +- Remove references to legacy format. + +--- + +## DX + +- OPX is simpler to read and write: + - One element per edit (``), with an explicit `op` attribute. + - Clear, distinct delimiters `<<<` / `>>>` for literal content. + - Optional `` enables short, per-edit intent + +--- + +## Testing Plan (Immediate) + +Parser unit tests (targeted): +- Valid cases: `new`, `patch`, `replace`, `remove`, `move`. +- Occurrence handling: `first`, `last`, numeric `N`, ambiguous without `occurrence` (error). +- Marker parsing: mixed whitespace around `<<<`/`>>>`. +- Error cases: missing attributes, missing required children, multiple ``/``. + +Webview tests (Apply tab): +- Lint/normalization for ``/`` tags. +- Preview renders correct summary and per-row apply functions. + +Manual smoke: +- Dev playground at http://localhost:5173/. +- Paste each example from this document and verify preview/apply success. + +--- + +## Risks & Mitigations + +- LLM adherence to the new format: + - Keep instruction text concise and provide 3–4 OPX examples. + - Use stable tags/attributes and simple markers. +- Content delimiter collision: + - `<<<`/`>>>` are uncommon; if conflicts appear, OPX v1.1 can add an optional custom `marker` attribute. +- Markdown copy truncation of markers: + - The parser auto-heals full-line `<`/`<<`/`>`/`>>` into `<<<`/`>>>` inside ``/`` blocks. + - Guidance: ask models to emit OPX inside a single fenced block (e.g., ```xml … ```), and copy from within the fence. + +--- + +## Acceptance Criteria + +- Legacy input is no longer accepted; OPX-only responses are parsed and applied. +- All five operations (`new`, `patch`, `replace`, `remove`, `move`) work end-to-end via Apply tab. +- Instruction text is replaced with OPX-only content. +- Unit tests for OPX pass; Apply tab previews and applies OPX responses successfully. + +--- + +## Implementation Checklist + +1) Parser +- [x] Remove legacy parsing path and `===` marker support. +- [x] Implement OPX-only parsing with `<<<`/`>>>` markers. +- [x] Map `op` → internal actions. + +2) Preprocessor +- [x] Normalize/lint `` and ``. +- [x] Remove legacy tag handling and lints. + +3) Prompt text +- [x] Replace instruction in [src/prompts/xml-instruction.ts](file:///Users/minhthanh/Work/Side/overwrite/src/prompts/xml-instruction.ts) with OPX. + +4) Tests +- [x] Add OPX parser tests. +- [x] Update/Add webview tests for OPX samples. + +5) Manual smoke +- [ ] Validate examples from this PRD in the dev playground. + +--- + +## Release Checklist (OPX v1) + +- [x] Replace prompt instructions with OPX-only (src/prompts/xml-instruction.ts) +- [x] Parser OPX-only (src/utils/xml-parser.ts) and preprocessor normalization (src/webview-ui/.../preprocess.ts) +- [x] Backend tests green: `pnpm test` +- [x] Webview tests green: `pnpm -C src/webview-ui test --run` +- [ ] Optional manual smoke in dev playground (http://localhost:5173/) +- [ ] Bump version in package.json if releasing +- [ ] Package: `pnpm package` (or `pnpm vscode:package`) + diff --git a/src/prompts/xml-instruction.ts b/src/prompts/xml-instruction.ts index ad1a4c2..138ed88 100644 --- a/src/prompts/xml-instruction.ts +++ b/src/prompts/xml-instruction.ts @@ -1,486 +1,129 @@ -export const XML_FORMATTING_INSTRUCTIONS: string = ` - -## Role -- You are a **code editing assistant**: Your primary function is to accurately apply code changes to a user's repository based on their requests. You fulfill these requests using a specific XML-based protocol. Provide complete instructions or code lines when replying with xml formatting. Adherence to the protocol is paramount for successful code modification. - -## Capabilities - -- **Create new files:** Generate files that do not currently exist. -- **Rewrite entire files:** Replace the complete content of an existing file. -- **Perform partial modifications:** Search for specific code blocks and replace them with new content. -- **Delete existing files:** Remove files from the repository. -- **Rename or move files:** Change the path of an existing file. - -**Core Principle:** Avoid placeholders like \`...\` or \`// existing code here\` or comments indicating where code should go. Always provide the complete, literal code required for the change. - -## Paths & Workspace Roots - -- Prefer workspace-relative paths, e.g., \`path="src/llm.py"\`. -- In multi-root workspaces, include the workspace folder name as \`root\`, e.g., \`\`. -- Alternatively, you may prefix the path with the root name like \`my-workspace:src/app.ts\`. -- Absolute paths and \`file://\` URIs are accepted but not required. -- Never reference files outside the workspace. - -## Tools & Actions (File Operations) - -1. **\`create\`**: Creates a new file at the specified \`path\`. Requires the full file content within \`\`. Fails if the file already exists. -2. **\`rewrite\`**: Replaces the *entire* content of an existing file at \`path\`. Requires the new full file content within \`\`. Use for significant refactoring or when \`modify\` becomes too complex. -3. **\`modify\`**: Performs a search-and-replace within an existing file at \`path\`. Requires *exact* matching \`\` block and the \`\` block that will replace it. Ideal for targeted changes. If the \`\` block might match multiple places, include \`first|last|N\` to disambiguate. -4. **\`delete\`**: Removes the file at the specified \`path\`. Requires an empty \`\` block. -5. **\`rename\`**: Moves or renames the file at \`path\`. Requires a \`\` tag *instead* of \`\`. The content of the file remains the same during the rename operation itself. - -### **Format to Follow: Diff Protocol** - - -Clearly and concisely describe your step-by-step approach or reasoning for the changes you are about to make. Explain *why* you chose specific actions (e.g., "Using modify for a small change," "Using rewrite due to significant structural changes"). - - - - - - - - Brief but clear explanation of this specific change within the file. - - -=== -// Exactly matching lines to find, including all whitespace and comments. -// Must be unique enough to avoid ambiguity. -=== - - - - -=== -// The new code content. -// For modify: This replaces the block entirely. -// For create/rewrite: This is the full file content. -// For delete: This block must be empty. -=== - - - - - - Description of the second distinct change in the same file. - -=== -// Unique search block for the second change. -=== - - -=== -// Content for the second change. -=== - - - - - - - - - -#### Tools Demonstration Summary - -1. \`\` – Full file content in \`\`. -2. \`\` – Empty \`\`. -3. \`\` – Requires \`\` + \`\` for partial edits. Can have multiple \`\` blocks for the *same file*. -4. \`\` – Entire new file content in \`\`. No \`\` needed. Only *one* \`\` block allowed per rewrite. -5. \`\` – Uses \`\` tag; no \`\`, \`\`, or \`\`. - -## Format Guidelines & Best Practices - -1. **Plan First**: Always start with a \`\` block. Explain your strategy. -2. **File Tag**: Use \`\`. The \`action\` must be one of the five tools (\`create\`, \`rewrite\`, \`modify\`, \`delete\`, \`rename\`). Prefer workspace-relative paths. In multi-root workspaces, include \`root\` (VS Code workspace folder name) or prefix the path like \`rootName:relative/path\`. -3. **Change Tag**: Inside \`\` (except for \`rename\`), use \`\`. Always include a clear \`\`. -4. **\`modify\` - Search Precision**: - - The \`\` block MUST *exactly* match the current code in the file, including indentation, spacing, line breaks, and comments. - - The \`\` block should be unique within the file to avoid ambiguous matches. Aim for at least 3-5 lines of context if possible, unless the target is inherently unique (like a specific import). Avoid overly generic searches (e.g., just \`}\` or \`});\`). - - The *entire* \`\` block is replaced by the *entire* \`\` block. -5. **\`modify\` - Content**: The \`\` block should contain the code as it should appear *after* the replacement. If you are only adding lines, include the original lines from the \`\` block that you want to keep, plus the new lines, within the \`\`. Maintain correct indentation relative to the surrounding code. -6. **\`modify\` - Multiple Changes**: To make multiple, distinct changes in the *same file*, use multiple \`\` blocks within the *same* \`\` tag. Ensure each \`\` block targets a unique section. -7. **\`rewrite\` vs. \`modify\`**: Use \`rewrite\` for substantial changes (e.g., changing class structure, rewriting most functions). Use \`modify\` for targeted additions, deletions, or alterations. If a file requires many (>3-4) complex \`modify\` operations, \`rewrite\` might be simpler and less error-prone. -8. **\`rewrite\` Content**: Provide the *complete* new file content in the \`\` block. Only *one* \`\` block is allowed per \`rewrite\`. -9. **\`create\` Content**: Provide the *complete* file content for the new file. Include necessary imports, exports, class/function definitions, etc. -10. **\`delete\` Content**: The \`\` block *must* be empty (\`=== ===\`). -11. **\`rename\` Action**: Use *only* the \`\` tag inside the \`\` tag. Do **not** include \`\`, \`\`, or \`\`. -12. **\`rename\` Constraints**: - - **Do not** attempt to \`modify\` or \`rewrite\` the file (using either the old or new path) in the *same* response as the \`rename\`. Perform the rename first, and subsequent edits in a later request if necessary. - - **Do not** reference the *old* path again in subsequent operations within the same response. - - **Do not** \`create\` a file at the \`new path\` in the same response. The \`rename\` action handles creating the file at the new path with the old content. - - Ensure the \`new path\` does not already exist. - - Rename a given file at most *once* per response. -13. **Imports**: When creating files or adding code via \`modify\`/\`rewrite\`, ensure necessary \`import\` statements are included at the top of the file or added appropriately. -14. **Atomicity**: Each XML response should represent a complete, self-contained set of changes that leaves the codebase in a valid state (though potentially incomplete with respect to the overall user goal, which might require multiple steps). -15. **No Placeholders**: Never use comments like \`// ... rest of function\` or \`...\` within \`\`. Provide the full, literal code. -16. **Syntax**: Ensure the code provided in \`\` is syntactically correct TypeScript. -17. **No Markdown Fences**: Do not wrap the XML response itself in Markdown code fences (\`\`\` ... \`\`\`). Return raw XML only. - -## Code Examples (TypeScript) - - -### Example: \`modify\` - Add Email Property (Simple Replace) - - -Add an optional email property to the \`User\` interface using \`modify\`. - -\`\`\`XML - - - Add optional email property to User interface - -=== -export interface User { - id: string; - name: string; +export const XML_FORMATTING_INSTRUCTIONS = ` + +# Role +You produce OPX (Overwrite Patch XML) that precisely describes file edits to apply to the current workspace. + +# What you can do +- Create files +- Patch specific regions of files (search-and-replace) +- Replace entire files +- Remove files +- Move/rename files + +# OPX at a glance +- One per file operation. Optionally wrap multiple edits in a single ... container. +- Attributes on : + - file="path/to/file" (required) + - op="new|patch|replace|remove|move" (required) + - root="workspaceRootName" (optional for multi-root workspaces) +- Optional per edit to briefly explain intent. +- For literal payloads, wrap code between lines containing only <<< and >>>. + +# Operations +1) op="new" (Create file) + - Children: <<< ... >>> + +2) op="patch" (Search-and-replace a region) + - Children: <<< ... >>> + <<< ... >>> + +3) op="replace" (Replace entire file) + - Children: <<< ... >>> + +4) op="remove" (Delete file) + - Self-closing is allowed, or an empty body. + +5) op="move" (Rename/move file) + - Children: + +# Path rules +- Prefer workspace-relative paths (e.g., src/lib/logger.ts). +- file:// URIs and absolute paths are tolerated. +- Do not reference paths outside the workspace. + +# Examples + + + + Create a string utilities module + +<<< +export function titleCase(s: string): string { + return s.split(/\s+/).map(w => (w ? w[0]!.toUpperCase() + w.slice(1) : w)).join(' '); } -=== - - -=== -export interface User { - id: string; - name: string; - email?: string; // Added optional email +>>> + + + + + + Add timeout and error logging + +<<< +export async function fetchUser(id: string) { + const res = await fetch(\`/api/users/\${id}\`); + if (!res.ok) throw new Error(\`Request failed: \${res.status}\`); + return res.json(); } -=== - - - -\`\`\` - ------ - -### Example: \`modify\` - Add Method and Update Constructor (Multiple Changes) - - -Modify the \`UserService\` class. First, add a \`getUserByEmail\` method. Second, update the constructor to accept an optional logger. This requires two \`\` blocks within the same \`\`. - -\`\`\`XML - - - Add getUserByEmail method to UserService - -=== - } - - // Method to get user by ID - public getUserById(id: string): User | undefined { - return this.users.find(user => user.id === id); - } - +>>> + + +<<< +async function withTimeout(p: Promise, ms = 10000): Promise { + const t = new Promise((_, r) => setTimeout(() => r(new Error('Request timed out')), ms)); + return Promise.race([p, t]); } -=== - - - -=== - } - - // Method to get user by ID - public getUserById(id: string): User | undefined { - return this.users.find(user => user.id === id); - } - - // New method to get user by email - public getUserByEmail(email: string): User | undefined { - this.logger?.log(\`Searching for user with email: \${email}\`); - return this.users.find(user => user.email === email); - } +export async function fetchUser(id: string) { + try { + const res = await withTimeout(fetch(\`/api/users/\${id}\`), 10000); + if (!res.ok) throw new Error(\`Request failed: \${res.status}\`); + return res.json(); + } catch (err) { + console.error('[api] fetchUser error', err); + throw err; + } } -=== - - - - - - Update constructor to accept optional logger - -=== -import { User } from '../interfaces/User'; - -export class UserService { - private users: User[] = []; - - constructor() { - // Initial setup maybe - } - -=== - - -=== - -import { User } from '../interfaces/User'; - -// Define a simple Logger interface (assuming it exists elsewhere or is simple) -interface Logger { - log(message: string): void; +>>> + + + + + + +<<< +export interface AppConfig { + apiBaseUrl: string; + enableTelemetry: boolean; + maxConcurrentJobs: number; } -export class UserService { - private users: User[] = []; - private logger?: Logger; // Added optional logger - - constructor(logger?: Logger) { // Updated constructor signature - this.logger = logger; // Assign logger - this.logger?.log('UserService initialized'); - // Initial setup maybe - } -=== - - - - -\`\`\` - ------ - -### Example: \`modify\` - Removing Lines (Empty Content) - - -Remove a deprecated configuration setting from \`config.ts\` using \`modify\` with an empty content block. - -\`\`\`XML - - - Remove deprecated LEGACY_API_ENDPOINT setting - -=== -export const API_ENDPOINT = "/api/v2"; -export const TIMEOUT_MS = 5000; -export const LEGACY_API_ENDPOINT = "/api/v1"; // Deprecated -export const MAX_RETRIES = 3; -=== - - -=== -export const API_ENDPOINT = "/api/v2"; -export const TIMEOUT_MS = 5000; -// LEGACY_API_ENDPOINT removed -export const MAX_RETRIES = 3; -=== - - - -\`\`\` - -*Note: A slightly safer way to remove a single line might be to search for the line plus its surrounding lines and provide the surrounding lines back in the content, omitting the target line.* - ------ - -### Example: \`rewrite\` - Refactor User Class - - -Rewrite the \`User.ts\` file from a simple interface to a class with a constructor and a method, as the structure is changing significantly. - -\`\`\`XML - - - Rewrite User from interface to class with constructor and validation method - -=== -import { v4 as uuidv4 } from 'uuid'; // Assuming uuid is installed - -export class User { - public readonly id: string; - public name: string; - public email: string; - public createdAt: Date; - - constructor(name: string, email: string) { - if (!this.isValidEmail(email)) { - throw new Error(\`Invalid email format: \${email}\`); - } - if (!name || name.trim().length === 0) { - throw new Error('User name cannot be empty.'); - } - this.id = uuidv4(); - this.name = name; - this.email = email; - this.createdAt = new Date(); - } - - private isValidEmail(email: string): boolean { - // Basic email validation regex - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; - return emailRegex.test(email); - } - - public updateName(newName: string): void { - if (!newName || newName.trim().length === 0) { - console.warn('Attempted to update user name with an empty value.'); - return; - } - this.name = newName; - console.log(\`User \${this.id} name updated to \${this.name}\`); - } - -} -=== - - - - -\`\`\` - ------ - -### Example: \`create\` - New Utility Module - - -Create a new utility file \`stringUtils.ts\` with functions for capitalizing and reversing strings. - -\`\`\`XML - - - Create string utility functions module - -=== -/** - * Capitalizes the first letter of a string. - * @param str The input string. - * @returns The capitalized string, or an empty string if input is null/empty. - */ -export function capitalize(str: string | null | undefined): string { - if (!str) { - return ''; - } - return str.charAt(0).toUpperCase() + str.slice(1); -} - -/** - -- Reverses a string. -- @param str The input string. -- @returns The reversed string, or an empty string if input is null/empty. - */ -export function reverseString(str: string | null | undefined): string { - if (!str) { - return ''; - } - return str.split('').reverse().join(''); -} -=== - - - - -\`\`\` - ------ - -### Example: \`delete\` - Remove Obsolete File - - -Delete the obsolete \`legacyDataMapper.ts\` file as it's no longer needed. - -\`\`\`XML - - - Remove obsolete legacy data mapper file - -=== -=== - - - -\`\`\` - ------ - -### Example: \`rename\` - Rename Service File - - -Rename the \`AuthService.ts\` file to \`AuthenticationService.ts\` for better clarity. - -\`\`\`XML - - - -\`\`\` - ------ - -### Example: Negative Example - Mismatched Search (\`modify\`) - - -Demonstrate a failed \`modify\` due to a mismatched search block (extra whitespace). - -\`\`\`XML - - - - - - - - FAIL: Search block has incorrect whitespace - -=== -export class Config { - static readonly MAX_USERS = 100; // Search has extra space before static -} -=== - - -=== -export class Config { - static readonly MAX_USERS = 200; // Intended change -} -=== - - - - -\`\`\` - ------ - -### Example: Negative Example - Trying to \`modify\` after \`rename\` (Same Response) - - -Demonstrate the invalid operation of trying to modify a file immediately after renaming it within the same response. This should be done in two separate steps/responses. - -\`\`\`XML - - - - - - - - FAIL: Attempting to modify file immediately after rename in the same response - -=== -// Some existing code in the service -export class OldService { -=== - - -=== -// Some existing code in the service -export class NewService { // Trying to update class name -=== - - - - -\`\`\` - -## Final Notes & Cautions - -1. **Exactness is Key**: For \`modify\`, the \`\` block must be *identical* to the code in the file. Double-check whitespace, comments, and line endings. -2. **Uniqueness**: Ensure \`\` blocks are specific enough to match only the intended code section. -3. **Completeness**: Always provide full code lines in \`\`, never partial snippets or placeholders. -4. **Indentation**: Maintain correct indentation within \`\` blocks relative to where the code will be placed. -5. **\`rename\` Isolation**: Never combine \`rename\` with \`modify\` or \`rewrite\` on the same logical file (old or new path) in a single response. -6. **XML Validity**: Ensure your response strictly follows the XML structure defined here. Do not add extra elements or attributes. -7. **No CDATA**: Never wrap XML content in \`\` tags. The system expects raw XML. -8. **One Task Per Response**: While a response can contain multiple \`\` operations, focus on fulfilling a discrete part of the user's request reliably. Complex tasks might require multiple responses. -9. **Error Checking**: Before finalizing the response, mentally review the changes. Do they make sense? Is the syntax correct? Will the code be in a valid state? - -**IMPORTANT**: YOUR ABILITY TO EDIT CODE DEPENDS ENTIRELY ON FOLLOWING THIS XML PROTOCOL CORRECTLY. ERRORS IN FORMATTING WILL LEAD TO FAILED OPERATIONS. -` +export const config: AppConfig = { + apiBaseUrl: process.env.API_BASE_URL || 'http://localhost:3000', + enableTelemetry: process.env.TELEMETRY === '1', + maxConcurrentJobs: Number(process.env.MAX_JOBS || 4), +}; +>>> + + + + + + + + + + + +# Guidance for reliable patches +- Make unique: include enough surrounding lines so it matches exactly once. +- The entire region is replaced by the entire payload. +- If a match may occur multiple times, set occurrence="first|last|N" on . +- Preserve indentation to fit the surrounding code. + +# Validity +- Emit syntactically correct code for each file type. +- Avoid CDATA; write raw XML as shown. +- Do not mix move with other operations for the same file in one edit. + +` diff --git a/src/test/suite/opx/file-action-handler.opx.test.ts b/src/test/suite/opx/file-action-handler.opx.test.ts new file mode 100644 index 0000000..5514129 --- /dev/null +++ b/src/test/suite/opx/file-action-handler.opx.test.ts @@ -0,0 +1,144 @@ +import * as assert from 'node:assert' +import * as os from 'node:os' +import * as path from 'node:path' +import { after, before, describe, it } from 'mocha' +import * as vscode from 'vscode' +import { applyFileActions } from '../../../providers/file-explorer/file-action-handler' +import { parseXmlResponse } from '../../../utils/xml-parser' + +const baseDir = path.join(os.tmpdir(), 'overwrite-opx-tests') + +async function writeFile(absRel: string, content: string): Promise { + const abs = path.isAbsolute(absRel) ? absRel : path.join(baseDir, absRel) + const uri = vscode.Uri.file(abs) + await vscode.workspace.fs.createDirectory(vscode.Uri.file(path.dirname(abs))) + await vscode.workspace.fs.writeFile(uri, Buffer.from(content, 'utf8')) + return uri +} + +async function readFile(absRel: string): Promise { + const abs = path.isAbsolute(absRel) ? absRel : path.join(baseDir, absRel) + const uri = vscode.Uri.file(abs) + const buf = await vscode.workspace.fs.readFile(uri) + return Buffer.from(buf).toString('utf8') +} + +async function removePath(absRel: string): Promise { + const abs = path.isAbsolute(absRel) ? absRel : path.join(baseDir, absRel) + const uri = vscode.Uri.file(abs) + try { + await vscode.workspace.fs.delete(uri, { recursive: true }) + } catch {} +} + +describe('applyFileActions with OPX', () => { + before(async () => { + await vscode.workspace.fs.createDirectory(vscode.Uri.file(baseDir)) + }) + + after(async () => { + await removePath(baseDir) + }) + + it('creates a file (op=new)', async () => { + const target = path.join(baseDir, 'newfile.ts') + const xml = ` + + +<<< +export const HELLO = 'world'; +>>> + +` + const { fileActions, errors } = parseXmlResponse(xml) + assert.deepStrictEqual(errors, []) + + const results = await applyFileActions(fileActions) + assert.strictEqual(results[0]?.success, true) + const text = await readFile(target) + assert.ok(text.includes("HELLO = 'world'")) + }) + + it('patches a CRLF document with LF search and occurrence=last', async () => { + const target = path.join(baseDir, 'crlf.ts') + await writeFile(target, 'a\r\nconst x = 1;\r\nconst x = 1;\r\nz\r\n') + + const xml = ` + + +<<< +const x = 1; +>>> + + +<<< +const x = 2; +>>> + +` + const { fileActions, errors } = parseXmlResponse(xml) + assert.deepStrictEqual(errors, []) + + const results = await applyFileActions(fileActions) + assert.strictEqual(results[0]?.success, true) + + const text = await readFile(target) + assert.ok(text.includes('const x = 2;')) + // Ensure first occurrence still present + assert.ok(text.indexOf('const x = 2;') > text.indexOf('const x = 1;')) + }) + + it('renames a file (op=move)', async () => { + const from = path.join(baseDir, 'move-from.ts') + const to = path.join(baseDir, 'move-to.ts') + await writeFile(from, 'A') + + const xml = ` + + +` + const { fileActions, errors } = parseXmlResponse(xml) + assert.deepStrictEqual(errors, []) + const results = await applyFileActions(fileActions) + assert.strictEqual(results[0]?.success, true) + + const text = await readFile(to) + assert.strictEqual(text, 'A') + }) + + it('deletes a file (op=remove)', async () => { + const target = path.join(baseDir, 'del.ts') + await writeFile(target, 'DEL') + + const xml = `` + const { fileActions, errors } = parseXmlResponse(xml) + assert.deepStrictEqual(errors, []) + const results = await applyFileActions(fileActions) + assert.strictEqual(results[0]?.success, true) + + let exists = true + try { + await readFile(target) + } catch { + exists = false + } + assert.strictEqual(exists, false) + }) + + it('rewrite fails when file does not exist', async () => { + const target = path.join(baseDir, 'nope.ts') + const xml = ` + + +<<< +A +>>> + +` + const { fileActions, errors } = parseXmlResponse(xml) + assert.deepStrictEqual(errors, []) + const results = await applyFileActions(fileActions) + assert.strictEqual(results[0]?.success, false) + assert.match(results[0]!.message, /does not exist|cannot rewrite/i) + }) +}) diff --git a/src/test/suite/opx/xml-parser.test.ts b/src/test/suite/opx/xml-parser.test.ts new file mode 100644 index 0000000..286ef13 --- /dev/null +++ b/src/test/suite/opx/xml-parser.test.ts @@ -0,0 +1,221 @@ +import * as assert from 'node:assert' +import { describe, it } from 'mocha' +import { parseXmlResponse } from '../../../utils/xml-parser' + +describe('OPX Parser (parseXmlResponse)', () => { + it('parses op=new with content', () => { + const xml = ` + + Create strings util + +<<< +export const A = 1; +>>> + +` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + assert.strictEqual(res.fileActions.length, 1) + const a = res.fileActions[0]! + assert.strictEqual(a.action, 'create') + assert.strictEqual(a.path, 'src/utils/strings.ts') + assert.strictEqual( + (a.changes?.[0]?.content ?? '').trim(), + 'export const A = 1;', + ) + }) + + it('parses op=patch with occurrence and maps correctly', () => { + const xml = ` + + Patch region + +<<< +const x = 1; +>>> + + +<<< +const x = 2; +>>> + +` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + const f = res.fileActions[0]! + assert.strictEqual(f.action, 'modify') + assert.strictEqual(f.changes?.[0]?.occurrence, 2) + assert.strictEqual((f.changes?.[0]?.search ?? '').trim(), 'const x = 1;') + assert.strictEqual((f.changes?.[0]?.content ?? '').trim(), 'const x = 2;') + }) + + it('parses op=replace into rewrite', () => { + const xml = ` + + +<<< +export default 1; +>>> + +` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + const f = res.fileActions[0]! + assert.strictEqual(f.action, 'rewrite') + assert.strictEqual( + (f.changes?.[0]?.content ?? '').trim(), + 'export default 1;', + ) + }) + + it('parses op=remove (self-closing)', () => { + const xml = `` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + const f = res.fileActions[0]! + assert.strictEqual(f.action, 'delete') + assert.strictEqual(f.path, 'tests/legacy/user-auth.spec.ts') + }) + + it('parses op=move with ', () => { + const xml = ` + + +` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + const f = res.fileActions[0]! + assert.strictEqual(f.action, 'rename') + assert.strictEqual(f.newPath, 'src/lib/feature-flags.ts') + }) + + it('errors when edit missing file/op', () => { + const xml = `<<<\nA\n>>> ` + const res = parseXmlResponse(xml) + assert.ok(res.errors.length >= 1) + assert.strictEqual(res.fileActions.length, 0) + }) + + it('errors when patch missing or ', () => { + const xml1 = `<<<\nA\n>>> ` + const xml2 = `<<<\nA\n>>> ` + const r1 = parseXmlResponse(xml1) + const r2 = parseXmlResponse(xml2) + assert.ok(r1.errors.length >= 1) + assert.ok(r2.errors.length >= 1) + }) + + it('supports wrapper with multiple edits', () => { + const xml = ` + + <<<\nA\n>>> + +` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + assert.strictEqual(res.fileActions.length, 2) + assert.strictEqual(res.fileActions[0]!.action, 'create') + assert.strictEqual(res.fileActions[1]!.action, 'delete') + }) + + it('sanitizes code fences and leading chatter', () => { + const xml = + '```xml\n<<<\nA\n>>> \n```' + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + assert.strictEqual(res.fileActions[0]!.action, 'create') + }) + + it('rejects legacy format', () => { + const xml = `===\nA\n===` + const res = parseXmlResponse(xml) + // Our OPX-only parser surfaces a generic error when no is present + assert.ok(res.errors.length >= 1) + assert.strictEqual(res.fileActions.length, 0) + }) + + it('errors on unknown op value', () => { + const xml = `` + const res = parseXmlResponse(xml) + assert.ok(res.errors[0]?.includes('unknown op')) + assert.strictEqual(res.fileActions.length, 0) + }) + + it('errors when move has no or missing file attr', () => { + const xml1 = `` + const xml2 = `` + const xml3 = `` + for (const xml of [xml1, xml2, xml3]) { + const res = parseXmlResponse(xml) + assert.ok(res.errors[0]?.includes('Missing markers are empty/whitespace', () => { + const xml = ` + + +<<< + \n \n\t +>>> + + +<<< +OK +>>> + +` + const res = parseXmlResponse(xml) + assert.ok(res.errors[0]?.includes('Empty or missing marker block')) + }) + + it('auto-heals truncated markers "<"/">" into OPX markers inside /', () => { + const xml = ` + + +< +export const metadata: Metadata = { title: "A", description: "B" }; +>>> + + +<< +export const metadata: Metadata = { title: "X", description: "Y" }; +> + +` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + const f = res.fileActions[0]! + assert.strictEqual(f.action, 'modify') + assert.ok((f.changes?.[0]?.search || '').includes('title: "A"')) + assert.ok((f.changes?.[0]?.content || '').includes('title: "X"')) + }) + + it('ignores invalid occurrence value (no error, undefined occurrence)', () => { + const xml = ` + + +<<< +AAA +>>> + + +<<< +BBB +>>> + +` + const res = parseXmlResponse(xml) + assert.deepStrictEqual(res.errors, []) + const f = res.fileActions[0]! + assert.strictEqual(f.action, 'modify') + assert.strictEqual(f.changes?.[0]?.occurrence, undefined) + }) + + it('errors when no tags are present', () => { + const res = parseXmlResponse('') + assert.ok(res.errors[0]?.includes('No ')) + assert.strictEqual(res.fileActions.length, 0) + }) +}) diff --git a/src/utils/xml-parser.ts b/src/utils/xml-parser.ts index 64eda0f..e444906 100644 --- a/src/utils/xml-parser.ts +++ b/src/utils/xml-parser.ts @@ -1,4 +1,4 @@ -// Define interfaces for the parsed XML structure +// Define interfaces for the parsed XML structure (public API remains the same) export interface FileAction { path: string action: 'create' | 'rewrite' | 'modify' | 'delete' | 'rename' @@ -15,197 +15,246 @@ interface ChangeBlock { } interface ParseResult { + // OPX does not define a plan; keep for tolerance with existing callers plan?: string fileActions: FileAction[] errors: string[] } /** - * Parses an XML-formatted LLM response to extract file actions. - * @param xmlContent The XML content string from the LLM. - * @returns Structured representation of the file actions to perform. + * Parses OPX (Overwrite Patch XML) responses and returns unified FileAction[] + * OPX-only: accepts (optionally wrapped in ...) + * + * op mapping: + * - new -> create (requires ) + * - patch -> modify (requires and ) + * - replace -> rewrite (requires ) + * - remove -> delete (no children) + * - move -> rename (requires ) */ export function parseXmlResponse(xmlContent: string): ParseResult { - const result: ParseResult = { - fileActions: [], - errors: [], - } + const result: ParseResult = { fileActions: [], errors: [] } try { - // 1) Sanitize copy-pasted chat text (trim fences, leading/trailing chatter) const cleaned = sanitizeResponse(xmlContent) - // Extract plan - const planMatch = cleaned.match(/<\s*Plan\s*>([\s\S]*?)<\/\s*Plan\s*>/i) - if (planMatch?.[1]) { - result.plan = planMatch[1].trim() + if (!cleaned) { + return { fileActions: [], errors: ['Empty input'] } } - // Extract file actions - const fileRegex = /]*)>([\s\S]*?)<\/\s*file\s*>/gi - let fileMatch: RegExpExecArray | null - let fileIndex = 0 + // 1) Collect and ... with document order preserved + const edits: Array<{ + index: number + attrs: Record + body: string | null + }> = [] + + // Self-closing edits (e.g., op="remove") + const selfClosingRegex = /<\s*edit\b([^>]*)\/>/gi + for (const m of cleaned.matchAll(selfClosingRegex)) { + edits.push({ + index: m.index ?? 0, + attrs: parseAttributes(m[1] ?? ''), + body: null, + }) + } + + // Paired edits + const pairedRegex = /<\s*edit\b([^>]*)>([\s\S]*?)<\s*\/\s*edit\s*>/gi + for (const m of cleaned.matchAll(pairedRegex)) { + edits.push({ + index: m.index ?? 0, + attrs: parseAttributes(m[1] ?? ''), + body: m[2] ?? '', + }) + } + + edits.sort((a, b) => a.index - b.index) + + if (edits.length === 0) { + return { + fileActions: [], + errors: ['No elements found (expecting OPX)'], + } + } - // biome-ignore lint/suspicious/noAssignInExpressions: - while ((fileMatch = fileRegex.exec(cleaned)) !== null) { - const [, rawAttrs, fileContent] = fileMatch - const attrs = parseAttributes(rawAttrs) - const pathAttr = attrs.path - const actionAttr = attrs.action + let idx = 0 + for (const e of edits) { + idx += 1 + const file = e.attrs.file + const op = (e.attrs.op || '').toLowerCase() + const root = e.attrs.root - if (!pathAttr || !actionAttr) { - const trimmedAttrs = rawAttrs.trim().slice(0, 120) + if (!file || !op) { + const trimmed = Object.entries(e.attrs) + .map(([k, v]) => `${k}="${v}"`) + .join(' ') result.errors.push( - `Missing required attribute: path or action (at #${ - fileIndex + 1 - }, attrs="${trimmedAttrs}")`, + `Edit #${idx}: missing required attribute(s): ${!file ? 'file ' : ''}${ + !op ? 'op' : '' + }. attrs="${trimmed}"`, ) - fileIndex++ continue } - const fileAction: FileAction = { - path: pathAttr, - action: actionAttr as FileAction['action'], - changes: [], - root: attrs.root, + const action = mapOpToAction(op) + if (!action) { + result.errors.push(`Edit #${idx}: unknown op="${op}"`) + continue } - // Handle rename action - if (actionAttr === 'rename') { - const newPathMatch = fileContent.match(//i) - if (newPathMatch?.[1]) { - fileAction.newPath = newPathMatch[1] - } else { - result.errors.push( - `Missing element for rename action on: ${pathAttr}`, - ) - } - } else { - // Extract change blocks - const changeRegex = /<\s*change\s*>([\s\S]*?)<\/\s*change\s*>/gi - let changeMatch: RegExpExecArray | null - - // biome-ignore lint/suspicious/noAssignInExpressions: - while ((changeMatch = changeRegex.exec(fileContent)) !== null) { - const changeContent = changeMatch[1] - - // Extract description - const descMatch = changeContent.match( - /<\s*description\s*>([\s\S]*?)<\/\s*description\s*>/i, - ) - const description = descMatch - ? descMatch[1].trim() - : 'No description provided' - - // Extract search block for modify - let search: string | undefined - if (actionAttr === 'modify') { - const searchMatch = changeContent.match( - /<\s*search\s*>([\s\S]*?)<\/\s*search\s*>/i, - ) - if (searchMatch?.[1]) { - search = extractContentBetweenMarkers(searchMatch[1]) + const fileAction: FileAction = { path: file, action, root, changes: [] } + + try { + switch (op) { + case 'move': { + // inside body + const body = e.body || '' + const toMatch = body.match(/<\s*to\b([^>]*)\/>/i) + const toAttrs = parseAttributes(toMatch?.[1] ?? '') + const newPath = toAttrs.file + if (!newPath) { + throw new Error('Missing for move') } + fileAction.newPath = newPath + break } - - // Extract content - let content = '' - const contentMatch = changeContent.match( - /<\s*content\s*>([\s\S]*?)<\/\s*content\s*>/i, - ) - if (contentMatch?.[1]) { - content = extractContentBetweenMarkers(contentMatch[1]) || '' + case 'remove': { + // nothing else needed + break } - - // Optional occurrence disambiguator - let occurrence: ChangeBlock['occurrence'] - const occMatch = changeContent.match( - /<\s*occurrence\s*>([\s\S]*?)<\/\s*occurrence\s*>/i, - ) - if (occMatch?.[1]) { - const occRaw = occMatch[1].trim().toLowerCase() - if (occRaw === 'first' || occRaw === 'last') { - occurrence = occRaw - } else { - const n = Number.parseInt(occRaw, 10) + case 'new': + case 'replace': { + const body = e.body || '' + const putMatch = body.match( + /<\s*put\s*>([\s\S]*?)<\s*\/\s*put\s*>/i, + ) + if (!putMatch) throw new Error('Missing block') + const content = + extractBetweenMarkers(putMatch[1] ?? '', '<<<', '>>>') ?? '' + fileAction.changes!.push({ + description: + extractWhy(body) ?? + (op === 'new' ? 'Create file' : 'Replace file'), + content, + }) + break + } + case 'patch': { + const body = e.body || '' + const findMatch = body.match( + /<\s*find\b([^>]*)>([\s\S]*?)<\s*\/\s*find\s*>/i, + ) + const putMatch = body.match( + /<\s*put\s*>([\s\S]*?)<\s*\/\s*put\s*>/i, + ) + if (!findMatch || !putMatch) + throw new Error('Missing or for patch') + const findAttrs = parseAttributes(findMatch[1] ?? '') + const occurrenceRaw = (findAttrs.occurrence || '').toLowerCase() + let occurrence: ChangeBlock['occurrence'] + if (occurrenceRaw === 'first' || occurrenceRaw === 'last') + occurrence = occurrenceRaw + else if (occurrenceRaw) { + const n = Number.parseInt(occurrenceRaw, 10) if (!Number.isNaN(n) && n > 0) occurrence = n } + const search = extractBetweenMarkers( + findMatch[2] ?? '', + '<<<', + '>>>', + ) + const content = + extractBetweenMarkers(putMatch[1] ?? '', '<<<', '>>>') ?? '' + if (!search) + throw new Error('Empty or missing marker block in ') + fileAction.changes!.push({ + description: extractWhy(body) ?? 'Patch region', + search, + content, + occurrence, + }) + break } - - fileAction.changes!.push({ - description, - search, - content, - occurrence, - }) } - } - result.fileActions.push(fileAction) - fileIndex++ + result.fileActions.push(fileAction) + } catch (err) { + result.errors.push( + `Edit #${idx} (${file}): ${err instanceof Error ? err.message : String(err)}`, + ) + } } } catch (error) { - result.errors.push(`Failed to parse XML: ${error}`) + result.errors.push(`Failed to parse OPX: ${error}`) } return result } +/** Extract simple description from if present */ +function extractWhy(body: string): string | undefined { + const m = body.match(/<\s*why\s*>([\s\S]*?)<\s*\/\s*why\s*>/i) + return m?.[1]?.trim() +} + /** - * Extracts content between === markers in the LLM response. - * @param text The raw text containing === markers. - * @returns The extracted content or undefined if not found. + * Extracts content between custom markers like <<< and >>>, trimming outer whitespace. */ -function extractContentBetweenMarkers(text: string): string | undefined { - // Be liberal: accept markers with or without surrounding newlines/whitespace. - // We look for the first '===' and the last '===' and trim whitespace around them. - const s = text.trim() - const first = s.indexOf('===') +function extractBetweenMarkers( + text: string, + start: string, + end: string, +): string | undefined { + // Auto-heal common markdown/chat truncation of marker lines inside literal blocks. + // If a line contains only "<" or "<<", treat it as the opening marker "<<<". + // If a line contains only ">" or ">>", treat it as the closing marker ">>>". + let s = text.trim() + // Repair before searching for markers; operate on full-line matches only. + s = s + .replace(/^[ \t]*<\s*$/gm, '<<<') + .replace(/^[ \t]*<<\s*$/gm, '<<<') + .replace(/^[ \t]*>\s*$/gm, '>>>') + .replace(/^[ \t]*>>\s*$/gm, '>>>') + + const first = s.indexOf(start) if (first === -1) return undefined - const last = s.lastIndexOf('===') + const last = s.lastIndexOf(end) if (last === -1 || last <= first) return undefined - // Start after the first marker and skip optional whitespace/newlines - let start = first + 3 - while (start < s.length && /[ \t\r\n]/.test(s[start]!)) start++ - // End before the last marker, trim trailing whitespace/newlines - const end = last - let endTrim = end - 1 - while (endTrim >= 0 && /[ \t\r\n]/.test(s[endTrim]!)) endTrim-- - if (endTrim < start) return '' - return s.slice(start, endTrim + 1) + let startIdx = first + start.length + while (startIdx < s.length && /[ \t\r\n]/.test(s[startIdx]!)) startIdx++ + let endIdx = last - 1 + while (endIdx >= 0 && /[ \t\r\n]/.test(s[endIdx]!)) endIdx-- + if (endIdx < startIdx) return '' + return s.slice(startIdx, endIdx + 1) } /** * Strips leading/trailing noise: code fences, chat preambles/epilogues. - * Keeps the slice from the first to the last . + * Keeps the slice from the first to the last . */ function sanitizeResponse(raw: string): string { let s = raw.trim() + if (!s) return '' // Remove triple backtick fences if present - if (s.startsWith('```')) { - // Drop opening fence line - s = s.replace(/^```[\w-]*\s*\n?/, '') - } - if (s.endsWith('```')) { - s = s.replace(/\n?```\s*$/, '') - } - // Find useful XML region - const startIdxOptions = [s.indexOf('..., keep inner slice; else start at first + const opxStart = s.indexOf(' i >= 0, ) as number[] const startIdx = startIdxOptions.length ? Math.min(...startIdxOptions) : -1 if (startIdx >= 0) s = s.slice(startIdx) // Determine end by the last closing tag of interest - const lastCloseFile = s.lastIndexOf('') - const lastClosePlan1 = s.lastIndexOf('') - const lastClosePlan2 = s.lastIndexOf('') - const lastClose = Math.max(lastCloseFile, lastClosePlan1, lastClosePlan2) + const lastCloseEdit = s.lastIndexOf('') + const lastCloseOpx = s.lastIndexOf('') + const lastClose = Math.max(lastCloseEdit, lastCloseOpx) if (lastClose > -1) { - // Include the closing tag - const end = - lastClose + - (s.slice(lastClose).toLowerCase().startsWith('') ? 7 : 7) + const isOpx = s.slice(lastClose).toLowerCase().startsWith('') + const end = lastClose + (isOpx ? 6 : 7) s = s.slice(0, end) } return s.trim() @@ -213,15 +262,34 @@ function sanitizeResponse(raw: string): string { /** * Parses attributes from a tag attribute string into a key-value map. + * Accepts both double and single quotes and lowercases keys. */ function parseAttributes(attrString: string): Record { const attrs: Record = {} - // Matches key="value" pairs, ignoring order and whitespace - const regex = /(\w+)\s*=\s*"([^"]*)"/g + const regex = /(\w+)\s*=\s*(?:"([^"]*)"|'([^']*)')/g let match: RegExpExecArray | null // biome-ignore lint/suspicious/noAssignInExpressions: iterative regex exec while ((match = regex.exec(attrString)) !== null) { - attrs[match[1]] = match[2] + const key = match[1].toLowerCase() + const val = match[2] !== undefined ? match[2] : (match[3] ?? '') + attrs[key] = val } return attrs } + +function mapOpToAction(op: string): FileAction['action'] | null { + switch (op) { + case 'new': + return 'create' + case 'patch': + return 'modify' + case 'replace': + return 'rewrite' + case 'remove': + return 'delete' + case 'move': + return 'rename' + default: + return null + } +} diff --git a/src/webview-ui/src/components/apply-tab/__tests__/apply-opx-flow.test.tsx b/src/webview-ui/src/components/apply-tab/__tests__/apply-opx-flow.test.tsx new file mode 100644 index 0000000..8a96ab3 --- /dev/null +++ b/src/webview-ui/src/components/apply-tab/__tests__/apply-opx-flow.test.tsx @@ -0,0 +1,112 @@ +import { fireEvent, render, screen } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import ApplyTab from '../index' + +// Mock VS Code API +const postMessageSpy = vi.fn() +vi.mock('../../../utils/vscode', () => ({ + getVsCodeApi: () => ({ + postMessage: postMessageSpy, + getState: () => ({}), + setState: () => undefined, + }), +})) + +// Reuse simple child mocks from existing suite to get a textarea and buttons +vi.mock('../response-textarea', () => ({ + default: ({ + responseText, + onTextChange, + }: { + responseText: string + onTextChange: (event: React.SyntheticEvent) => void + }) => ( +