Repository: ErikBjare/gptme
File: gptme/tools/save.py (Python)

Code before:
            raise ValueError(
                f"Path traversal detected: {path_display} resolves to {path} "
                f"which is outside current directory {cwd}"
            ) from err

    # Trigger pre-save hooks (file.save.pre)
    if pre_save_msgs := trigger_hook(
        HookType.FILE_SAVE_PRE,
        log=None,
        workspace=None,
        path=path,
        content=content,
    ):
        yield from pre_save_msgs

    # Ensure content ends with newline
    if not content.endswith("\n"):
        content += "\n"

    # Check if file exists and store original content for comparison
    # Note: User has already confirmed via execute_with_confirmation() with diff preview
    overwrite = path.exists()
    original_content = path.read_text() if overwrite else None

    # Check if folder exists
    missing_parent_created = False
    if not path.parent.exists():
        if not confirm(f"Folder {path_display.parent} doesn't exist, create it?"):
            yield Message(
                "system", "Save aborted: user refused to create a missing folder."
            )
            return
        path.parent.mkdir(parents=True)
        missing_parent_created = True

    # Save the file
    with open(path, "w") as f:
        f.write(content)

    # Trigger post-save hooks (file.save.post)
    if post_save_msgs := trigger_hook(
        HookType.FILE_SAVE_POST,
        log=None,
        workspace=None,
        path=path,
        content=content,
        created=not overwrite,
    ):
        yield from post_save_msgs

    # Check if this was an inefficient overwrite (minimal changes)

Review comment (refactor):
But now we are losing this aspect, right? What we maybe want to keep a double-confirm as was the behavior pre-PR? Can we still do it in the new hook-approval design? I think I'd like to see the original thing working first before we simplify/shorten approvals as I requested previously. I am surprised that `confirm(f"File {path_display} exists, overwrite?")` led to the behavior I saw (the "File path exists" in a Preview section and not as the confirm_msg)

Code after:
            raise ValueError(
                f"Path traversal detected: {path_display} resolves to {path} "
                f"which is outside current directory {cwd}"
            ) from err

    # Trigger pre-save hooks (file.save.pre)
    if pre_save_msgs := trigger_hook(
        HookType.FILE_SAVE_PRE,
        log=None,
        workspace=None,
        path=path,
        content=content,
    ):
        yield from pre_save_msgs

    # Ensure content ends with newline
    if not content.endswith("\n"):
        content += "\n"

    # Check if file exists and store original content for comparison
    # Note: User already confirmed via execute_with_confirmation() with diff preview
    overwrite = path.exists()
    original_content = path.read_text() if overwrite else None

    # Check if folder exists
    missing_parent_created = False
    if not path.parent.exists():
        if not confirm(f"Folder {path_display.parent} doesn't exist, create it?"):
            yield Message(
                "system", "Save aborted: user refused to create a missing folder."
            )
            return
        path.parent.mkdir(parents=True)
        missing_parent_created = True

    # Save the file
    with open(path, "w") as f:
        f.write(content)

    # Trigger post-save hooks (file.save.post)
    if post_save_msgs := trigger_hook(
        HookType.FILE_SAVE_POST,
        log=None,
        workspace=None,
        path=path,
        content=content,
        created=not overwrite,
    ):
        yield from post_save_msgs

    # Check if this was an inefficient overwrite (minimal changes)