Repository navigation
Keep hype render from overwriting its presentation or a half-written manifest - #52
Merged
Merged
Conversation
…manifest render --slide wrote its PNG to any -o, the presentation included, and rendering a whole deck wrote slides.json in place without checking the write, so a full disk left a truncated manifest behind a successful exit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
 [Medium impact] The PR appears safe to merge; no new actionable issues were found. SummaryThis PR protects single-slide renders from overwriting the presentation and makes writes to
Reviews (3) · Last reviewed commit: "Merge master into render-output-safety" · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A hard link to the presentation passed the path check and was overwritten in place. Renaming replaces the link instead, and a failed render no longer leaves a half-written PNG. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keeps both tests that #50 and this branch add at the same place in tests/test_cli.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two ways
hype rendercould leave a user's or an agent's files worse off than it found them.hype render talk.md --slide 1 -o talk.mdwrote the PNG over the presentation, since any-owas taken as the image's path. An agent filling in arguments is exactly who makes that slip. Render now refuses an output that is the presentation itself, through a symlink too, and says so.Rendering every slide wrote
slides.jsonstraight over the old one and ignored whether the write worked, so a full disk left a truncated manifest and still exited 0, and a manifest that couldn't be opened failed with an empty message. It is now written withQSaveFilelike every other file Hype writes, and a failure names the file and the reason. The same function renders the slides for PowerPoint export, which now reports that failure as well.No CI runs here yet: master has no workflow until #48 lands, and #48's needs omacom/omarchy-pkgs#2573. Merged onto #48 with that pull request's dependencies, the tests pass. This and #50 each add a test at the same place in
tests/test_cli.py, so whichever merges second conflicts there and keeps both.🤖 Generated by Opus 5.5 in Claude Code. Reviewed by Codex XHigh.