Skip to content

Fix local Markdown image uploads for absolute and home paths - #79

Merged
ma2za merged 2 commits into
ma2za:mainfrom
wosheesh:fix/local-markdown-images
Sep 27, 2026
Merged

ma2za merged 2 commits into
ma2za:mainfrom
wosheesh:fix/local-markdown-images

Conversation

@wosheesh

Copy link
Copy Markdown
Contributor

Markdown images such as ![animation](/absolute/path/animation.gif) lose their leading slash before Api.get_image() checks the filesystem. The path is then treated as relative to the server's working directory. Upload exceptions are swallowed, leaving a broken local path in the saved draft.

Preserve existing absolute paths, expand ~, and decode Markdown-escaped filenames before upload. Keep the legacy leading-slash fallback only when the absolute file is absent and the corresponding relative file exists. Missing files, upload failures, and invalid upload responses now raise an error naming the file. HTTP(S) and protocol-relative URLs remain unchanged, as do sources rendered without an API client. PNG, JPEG, GIF, and WebP continue through the existing upload implementation without conversion.

Validation:

  • 25 offline regression cases with mocked Api.get_image, including absolute paths for all four formats, home paths, spaces/Unicode/percent signs, missing files, directories, absolute-vs-relative precedence, upload failures, invalid responses, remote URLs, and rendering without an API.
  • Full offline suite on this branch: python -m pytest -q -m "not live" --strict-markers — 225 passed, 8 live tests deselected.
  • Live MCP round trip using the same renderer: created a new disposable draft with an absolute animated GIF path and publish=False, prepublish=False, send=False; re-fetched it and verified the stored https://substack-post-media.s3.amazonaws.com/...gif source; deleted only the disposable draft. Nothing published and no email sent.
  • Pre-commit passes on all changed files. Running it repository-wide also detects pre-existing formatting issues in tests/substack/test_widget_preservation.py; that unrelated file is intentionally excluded from this PR.

This branch contains only the image fix, its regression tests, and documentation/changelog updates. It does not include fork-specific MCP editing or throttling changes.

@ma2za ma2za left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Request changes: the new local-image test module is not portable to Windows. On Windows, 23 tests pass and two fail: est_home_path_expanded sets HOME, but pathlib.expanduser() uses USERPROFILE; and est_absolute_file_takes_priority_over_relative slices a drive-qualified absolute path with [1:], producing an invalid :\... path. Please make both fixtures platform-aware and run the suite on Windows (or add Windows CI) before approval. The implementation itself did successfully upload Windows absolute paths in my review.

@ma2za ma2za left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Follow-up review: fixed Windows portability in the new local-image tests. The updated branch passes 24 focused tests with one platform-specific skip and 224 offline tests overall. Production image-path handling was verified with a real Windows absolute path upload.

@ma2za
ma2za merged commit b5f9b91 into ma2za:main Sep 27, 2026
7 checks passed
ma2za added a commit that referenced this pull request Sep 27, 2026
Fix local Markdown image uploads for absolute and home paths
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