Browse Source

Merge pull request #449 from alvinunreal/fix/apply-patch-readonly-output

fix: handle readonly apply_patch args
Alvin 3 months ago
parent
commit
85de4c65f4
2 changed files with 82 additions and 2 deletions
  1. 56 0
      src/hooks/apply-patch/hook.test.ts
  2. 26 2
      src/hooks/apply-patch/index.ts

+ 56 - 0
src/hooks/apply-patch/hook.test.ts

@@ -228,6 +228,62 @@ PATCH`,
     );
   });
 
+  test('rewrites by replacing readonly args instead of mutating them', async () => {
+    const root = await createTempDir('apply-patch-hook-');
+    await writeFixture(root, 'sample.txt', 'prefix\nstale-value\nsuffix\n');
+    const hook = createHook();
+    const patchText = `*** Begin Patch
+*** Update File: sample.txt
+@@
+ prefix
+-old-value
++new-value
+ suffix
+*** End Patch`;
+    const args = Object.freeze({ patchText });
+    const output = { args };
+
+    await hook['tool.execute.before'](
+      { tool: 'apply_patch', directory: root },
+      output,
+    );
+
+    expect(output.args).not.toBe(args);
+    expect(output.args.patchText).toContain('-stale-value');
+    expect(output.args.patchText).toContain('+new-value');
+  });
+
+  test('fails open when output args cannot be replaced', async () => {
+    const root = await createTempDir('apply-patch-hook-');
+    await writeFixture(root, 'sample.txt', 'prefix\nstale-value\nsuffix\n');
+    const hook = createHook();
+    const patchText = `*** Begin Patch
+*** Update File: sample.txt
+@@
+ prefix
+-old-value
++new-value
+ suffix
+*** End Patch`;
+    const args = Object.freeze({ patchText });
+    const output = {} as { args?: typeof args };
+    Object.defineProperty(output, 'args', {
+      configurable: false,
+      enumerable: true,
+      get: () => args,
+    });
+
+    await expect(
+      hook['tool.execute.before'](
+        { tool: 'apply_patch', directory: root },
+        output,
+      ),
+    ).resolves.toBeUndefined();
+
+    expect(output.args).toBe(args);
+    expect(output.args?.patchText).toBe(patchText);
+  });
+
   test('does not alter new_lines during rewrite', async () => {
     const root = await createTempDir('apply-patch-hook-');
     await writeFixture(

+ 26 - 2
src/hooks/apply-patch/index.ts

@@ -27,6 +27,22 @@ interface ToolExecuteBeforeOutput {
   };
 }
 
+function replacePatchArgs(
+  output: ToolExecuteBeforeOutput,
+  args: NonNullable<ToolExecuteBeforeOutput['args']>,
+  patchText: string,
+): boolean {
+  const nextArgs = { ...args, patchText };
+
+  try {
+    output.args = nextArgs;
+  } catch {
+    return false;
+  }
+
+  return output.args?.patchText === patchText;
+}
+
 export function createApplyPatchHook(ctx: PluginInput) {
   function logHookStatus(
     state:
@@ -68,8 +84,16 @@ export function createApplyPatchHook(ctx: PluginInput) {
         );
 
         if (result.changed) {
-          args.patchText = result.patchText;
-          logHookStatus('rewrite');
+          if (replacePatchArgs(output, args, result.patchText)) {
+            logHookStatus('rewrite');
+          } else {
+            logHookStatus('skipped', {
+              reason: 'readonly output args',
+              failOpen: true,
+              rescueOptions: APPLY_PATCH_RESCUE_OPTIONS,
+              rewriteStage: 'before-native',
+            });
+          }
           return;
         }