Skip to content

Fix attachment de-dup stripping real digits from the filename#214

Open
Osamaali313 wants to merge 1 commit into
andrewyng:mainfrom
Osamaali313:fix-attachment-dedup-filename
Open

Fix attachment de-dup stripping real digits from the filename#214
Osamaali313 wants to merge 1 commit into
andrewyng:mainfrom
Osamaali313:fix-attachment-dedup-filename

Conversation

@Osamaali313

Copy link
Copy Markdown

What

email_download_attachment corrupts the filename when it de-duplicates a name collision. The rename loop:

while target.exists():
    target = (
        scratch.path
        / f"{target.stem.rstrip('-0123456789') or 'attachment'}-{counter}{target.suffix}"
    )
    counter += 1

str.rstrip('-0123456789') removes every trailing digit/dash, including digits that are part of the real name. It was only meant to strip a previously-appended -N suffix before adding the next one.

Why it matters

Downloading two attachments with the same digit-ending name into one session folder mangles the filename:

attachment current expected
invoice_2024.pdf invoice_-1.pdf invoice_2024-1.pdf
IMG_20240115.jpg IMG_-1.jpg IMG_20240115-1.jpg
image001.png (Outlook auto-name) image-1.png image001-1.png
report.pdf (no digits) report-1.pdf report-1.pdf

The wrong name is written to disk and returned as path in the tool result.

How

Strip only a trailing -<number> suffix:

/ f"{re.sub(r'-[0-9]+$', '', target.stem) or 'attachment'}-{counter}{target.suffix}"

re is already imported. Verified this preserves the original digits and still increments correctly on repeated collisions (invoice_2024.pdfinvoice_2024-1.pdfinvoice_2024-2.pdf). Names without a trailing -N are unchanged.

The collision-rename loop in email_download_attachment used
`target.stem.rstrip('-0123456789')`, which strips every trailing digit and
dash, not just a previously-appended `-N` suffix. So saving a second
`invoice_2024.pdf` produced `invoice_-1.pdf`, `IMG_20240115.jpg` became
`IMG_-1.jpg`, and Outlook's `image001.png` became `image-1.png`.

Strip only a trailing `-<number>` suffix with a regex, preserving the
original digits (and still incrementing correctly on repeated collisions).
Copilot AI review requested due to automatic review settings July 26, 2026 20:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants