Skip to content

fix(windows): preserve binary bytes in anchored file reads - #7077

Closed
allenliang2022 wants to merge 1 commit into
nesquena:masterfrom
allenliang2022:fix/windows-anchored-binary-read
Closed

allenliang2022 wants to merge 1 commit into
nesquena:masterfrom
allenliang2022:fix/windows-anchored-binary-read

Conversation

@allenliang2022

@allenliang2022 allenliang2022 commented Aug 16, 2026 •

Copy link
Copy Markdown
Contributor

Thinking Path

  • Hermes WebUI serves workspace files and immutable media snapshots through an anchored file-descriptor path.
  • On Windows, file descriptors opened without O_BINARY use text semantics.
  • A valid MP4 whose first 0x1A byte occurred at offset 568 was therefore reported and served as 568 bytes even though its real size was 5,248,045 bytes.
  • The non-anchored media path already adds O_BINARY; the shared anchored-open helper did not.
  • This change makes non-directory anchored reads binary-safe while preserving directory-open flags.

What Changed

  • Add the platform-optional O_BINARY flag to non-directory opens in open_anchored_fd().
  • Preserve the existing directory-only flag path.
  • Add a Windows Ctrl-Z regression test using the real anchored snapshot read path.
  • Add platform-independent flag-contract tests so Linux/macOS CI still detects removal of the Windows safety flag.

Why It Matters

Without this flag, arbitrary binary workspace files can be truncated at the first DOS Ctrl-Z byte on Windows. For video snapshots this produces invalid Range lengths and browser playback failures even though the source file and checksum are valid.

Verification

  • Regression test on the unpatched current origin/master: RED, actual_size == 568 instead of 4665.
  • Same test after the fix: GREEN.
  • Focused and neighboring tests: 23 passed, 7 skipped.
  • Full four-shard Windows run: the changed tests and shard 2 were green; shards 0/1/3 reported existing Windows-environment failures. The failure sets were identical across two independent origin/master + 1 commit branches. A clean origin/master control reproduced the failures (or classified the remaining two as one platform skip and one concurrency-only pass). No failure was unique to this branch.
  • python -m py_compile api/workspace.py tests/test_media_message_snapshots.py
  • git diff --check

Risks / Follow-ups

  • The change is limited to read-only non-directory opens. Directory opens still use O_DIRECTORY and do not receive O_BINARY.
  • O_BINARY resolves to zero on platforms that do not define it, preserving existing POSIX behavior.

@greptile-apps

greptile-apps Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes anchored file reads binary-safe on Windows while preserving directory-open behavior.

  • Adds the optional O_BINARY flag to non-directory anchored opens.
  • Adds a real Ctrl-Z snapshot regression test and platform-independent flag-contract tests.

Confidence Score: 5/5

The PR appears safe to merge, with the Windows binary-read correction narrowly scoped and appropriately tested.

The changed flag selection mirrors the existing non-anchored binary-read behavior, does not combine binary and directory flags, and remains a no-op on platforms without O_BINARY.

Important Files Changed

Filename Overview
api/workspace.py Correctly adds O_BINARY only to non-directory opens; POSIX behavior remains unchanged because the optional flag resolves to zero.
tests/test_media_message_snapshots.py Adds focused coverage for Windows Ctrl-Z truncation and verifies that file and directory opens retain distinct flag contracts.

Reviews (1): Last reviewed commit: "fix(windows): open anchored files in bin..." | Re-trigger Greptile

@nesquena-hermes nesquena-hermes added the size:M Medium PR (≤10 files, ≤250 LOC) label Aug 16, 2026
nesquena-hermes added a commit that referenced this pull request Aug 17, 2026
…6871), mobile nav order (#6909) (#7099)

* fix(navigation): preserve mobile action mirror order

Reposition stale extension action mirrors during synchronization without moving already-correct elements, preserving native tab order and keyboard focus.

Assisted-by: Hermes Agent:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-luna
Assisted-by: Claude Code:claude-opus-5

* fix(typography): route native controls through UI font

Assisted-by: Hermes Agent:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-luna
Assisted-by: Claude Code:claude-opus-5

* fix(windows): open anchored files in binary mode

* docs(changelog): note #7077 O_BINARY, #6871 native-control font, #6909 mobile nav order

---------

Co-authored-by: starship-s <45587122+starship-s@users.noreply.github.com>
Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
Co-authored-by: n <a@n>
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in experimental release exp-v0.52.234. Non-directory anchored file opens now set O_BINARY where the platform provides it, so binary workspace files (video/image/archive) no longer truncate at the first 0x1A byte on Windows — Range serving and playback work correctly. Codex SAFE + 88 focused tests + combined batch suite green. Thanks @allenliang2022!

🤖 Released by the release-manager agent.

alai04 pushed a commit to alai04/hermes-webui that referenced this pull request Aug 31, 2026
…l font (nesquena#6871), mobile nav order (nesquena#6909) (nesquena#7099)

* fix(navigation): preserve mobile action mirror order

Reposition stale extension action mirrors during synchronization without moving already-correct elements, preserving native tab order and keyboard focus.

Assisted-by: Hermes Agent:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-luna
Assisted-by: Claude Code:claude-opus-5

* fix(typography): route native controls through UI font

Assisted-by: Hermes Agent:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-luna
Assisted-by: Claude Code:claude-opus-5

* fix(windows): open anchored files in binary mode

* docs(changelog): note nesquena#7077 O_BINARY, nesquena#6871 native-control font, nesquena#6909 mobile nav order

---------

Co-authored-by: starship-s <45587122+starship-s@users.noreply.github.com>
Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
Co-authored-by: n <a@n>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M Medium PR (≤10 files, ≤250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants