Skip to content

engine: a name up to 255 bytes is written, and a longer one is refused everywhere - #137

Merged
donislawdev merged 1 commit into
mainfrom
engine/long-names
Sep 24, 2026
Merged

donislawdev merged 1 commit into
mainfrom
engine/long-names

Conversation

@donislawdev

@donislawdev donislawdev commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

What changes

A file name from 238 to 255 bytes long is written. Every file is written under a temporary name beside it and renamed once it is whole. That name was the file's name with eighteen bytes after it, so a name from 238 bytes up was never written on any system, while every file system stores it. The same held for a manifest named that long and for tfg recipe fmt -w on a recipe file named that long.

core.SiblingName now makes every such name. It keeps the plain join whenever it fits in 255 bytes, so nothing changes for any name the tool could write before. A longer one is cut to whole characters and tagged with sixteen hex digits of the SHA-256 of the whole name, so two long names that begin alike still get two temporary names. The marker stays last, so what an interrupted run leaves behind is still reported as a leftover.

A file name longer than 255 bytes is refused before anything is written, on every system. Linux (ext4) stores at most 255 bytes in a name, while NTFS and APFS count UTF-16 units and characters. A name of 200 CJK characters (604 bytes) used to be written on Windows and macOS and fail on Linux with exit code 8 and no reason. It now ends with exit code 3 everywhere, and the refusal gives the length and the limit.

The rules a file name has to pass moved from engine.go into engine/filename.go, because the new rule took engine.go past its length ceiling. The ceiling went down to 407 rather than up.

How it was checked

  • Six guards in internal/guard/longnames_test.go, each asserting that it is in the state it asks about. Nine mutations, all caught. Ten existing mutation entries repointed at filename.go, all caught.
  • One recipe with 50 unusual file names, run on Windows 11, Linux (Docker, ext4) and macOS 27 (APFS): 50 of 50 written byte for byte on each system (before: 49, 47, 49), and tfg verify passes. The 604-byte name is refused with exit 3 on all three.
  • Byte stability (D11) is untouched. Generated bytes and hashes do not depend on the temporary name.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Names from 238 to 255 bytes can now be written, including manifest names and inputs to tfg recipe fmt -w.
    • Temporary files created while saving or replacing long-named files no longer exceed the supported name limit.
  • Breaking Changes

    • Names longer than 255 bytes are refused on all systems before any files are written. The error reports the name’s byte length and the maximum allowed.

…d everywhere

Every file is written under a temporary name beside it and renamed once it
is whole. That name was the file's name with eighteen bytes after it, so a
name from 238 bytes up was never written on any system, while every file
system stores it. A sibling name now keeps the plain join whenever it fits,
and a longer name is cut to whole characters and tagged with a digest of the
whole name, so two long names that begin alike still get two. The manifest
and recipe fmt -w write their own siblings the same way.

A name over 255 bytes was written on Windows and macOS, which count
characters, and could not be on Linux, which counts bytes. It is now refused
before anything is written, on every system, with its length and the limit.

The rules a file name has to pass moved from engine.go into filename.go,
which the length rule took past its ceiling.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 84c3816b-281d-426e-8723-b2a42f982519

📥 Commits

Reviewing files that changed from the base of the PR and between d1ed853 and 189eff8.

📒 Files selected for processing (10)
  • CHANGELOG.md
  • internal/core/limits.go
  • internal/core/replace.go
  • internal/core/sibling.go
  • internal/engine/engine.go
  • internal/engine/filename.go
  • internal/engine/preflight.go
  • internal/guard/codeshape_test.go
  • internal/guard/longnames_test.go
  • internal/manifest/manifest.go
💤 Files with no reviewable changes (1)
  • internal/engine/engine.go

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (15)
  • GitHub Check: review new dependencies
  • GitHub Check: linters
  • GitHub Check: bill of materials
  • GitHub Check: test on windows-latest
  • GitHub Check: known vulnerabilities
  • GitHub Check: coverage gate
  • GitHub Check: test on macos-latest
  • GitHub Check: test on ubuntu-latest
  • GitHub Check: reference tools actually installed
  • GitHub Check: import table of the window binary
  • GitHub Check: staticcheck
  • GitHub Check: semgrep
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (actions)
🧰 Additional context used
📓 Path-based instructions (13)
Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
Verify tests check real behavior and would fail if the implementation were broken.

⚙️ CodeRabbit configuration file

Files:

  • internal/guard/codeshape_test.go
  • internal/guard/longnames_test.go
These are end-user desktop applications.

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
Performance is a known weak spot of these projects.

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
Applies only to code that builds or styles a GUI.

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
User-facing changelog.

⚙️ CodeRabbit configuration file

Files:

  • CHANGELOG.md
Domain: test file generator (Go; `tfg` CLI and `tfg-gui` Fyne window over one engine).

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
SECURITY, HIGH PRIORITY.

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
These apps are QA/developer tools.

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
Go code.

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
Check that documentation matches the actual code in this PR: commands, flags, config keys, file paths, build steps and examples must exist.

⚙️ CodeRabbit configuration file

Files:

  • CHANGELOG.md
All code in this repository is written by an AI coding agent (Claude Code).

⚙️ CodeRabbit configuration file

Files:

  • internal/engine/preflight.go
  • internal/core/limits.go
  • CHANGELOG.md
  • internal/guard/codeshape_test.go
  • internal/core/replace.go
  • internal/manifest/manifest.go
  • internal/engine/filename.go
  • internal/core/sibling.go
  • internal/guard/longnames_test.go
Source excerpt: **Words a user reads are English, with a flat hyphen and no semicolons.**

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Files:

  • CHANGELOG.md
🔇 Additional comments (9)
internal/core/limits.go (1)

94-96: LGTM!

Also applies to: 129-130

internal/guard/longnames_test.go (1)

1-195: LGTM!

internal/engine/filename.go (1)

1-198: LGTM!

internal/guard/codeshape_test.go (1)

53-56: LGTM!

CHANGELOG.md (1)

19-25: LGTM!

Also applies to: 510-515

internal/engine/preflight.go (1)

192-197: LGTM!

internal/core/replace.go (1)

55-57: LGTM!

internal/manifest/manifest.go (1)

855-858: LGTM!

internal/core/sibling.go (1)

46-53: 🎯 Functional Correctness

The concern is refuted. All marker-based temporary names use the sibling helpers, and the write path shares the same tempPathFor implementation as preflight. No remaining plain join was found.


📝 Walkthrough

Walkthrough

The change adds a cross-platform 255-byte filename limit, retains recipe filename validation in a separate file, and shortens temporary sibling names when needed. Generation, replacement, and manifest-writing paths use the bounded names. Tests cover long names, refusal of overlong names, and temporary-file handling.

Changes

Filename handling

Layer / File(s) Summary
Bounded sibling-name rules
internal/core/sibling.go, internal/core/limits.go, internal/guard/longnames_test.go
Sibling-name helpers preserve the requested suffix and keep names within 255 bytes. Long names are shortened at a UTF-8 character boundary and include a digest tag. Tests check distinct shortened names and suffixes.
Recipe filename validation
internal/engine/engine.go, internal/engine/filename.go, internal/guard/longnames_test.go, internal/guard/codeshape_test.go, CHANGELOG.md
Filename validation now resides in filename.go and rejects names over 255 bytes, alongside the existing invalid-name checks. Tests check refusal before output is written and acceptance of a 255-byte name.
Temporary sibling paths
internal/engine/preflight.go, internal/core/replace.go, internal/manifest/manifest.go, internal/guard/longnames_test.go, CHANGELOG.md
Generation, replacement, and manifest writing use bounded sibling paths. Tests cover long-name generation, manifest writing, verification leftovers, and in-place recipe replacement.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: bug

Merge Risk: ⚪ Minimal · up to 189ef

No actionable issue remains; the change is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 14
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: names up to 255 bytes are written, and longer names are refused everywhere. It is specific, user-relevant, and 81 characters long.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Tests For Changed Behavior ✅ Passed The PR changes runtime behavior and adds coverage in internal/guard/longnames_test.go. The tests cover 255-byte names, UTF-8-safe shortened siblings, distinct sibling names, leftover reporting, over…
No Secrets Or Debug Leftovers ✅ Passed The pull request changes only source, tests, and CHANGELOG.md. It adds no CLAUDE.md, CLAUDE.local.md, AGENTS.md, .claude/, or .env files. Addition-only scans found no credentials, private URLs, local …
No Hardcoded Ui Styling ✅ Passed The pull request changes only Markdown and Go files. The authoritative diff contains no XAML, Slint, Fyne, Tkinter, or WPF UI code, and no UI styling or reusable-control changes.
No Obvious Performance Problems ✅ Passed No clear performance problem is introduced. SiblingName performs one SHA-256 calculation only for names exceeding the temporary-name limit, and normal validated names are at most 255 bytes. The oper…
Desktop Robustness ✅ Passed PASS: The PR changes temporary sibling-name construction and filename validation. ReplaceFile, manifest saving, and generated-file writes still use temporary files followed by Sync/close and `os.R…
Safe File Parsing ✅ Passed No safe-file-parsing issue is introduced. The PR adds core.SiblingName and core.SiblingPath for temporary sibling paths, moves checkFileName to internal/engine/filename.go, and changes `Manife…
System Changes Are Reversible ✅ Passed PASS: The PR changes temporary filename generation, recipe file replacement, manifest saving, and filename validation. The diff adds no network filter, proxy, firewall, system-time, process-hook/injec…
Clear User-Facing Text ✅ Passed The PR adds a long-name error that states the generated name, its byte length, the 255-byte limit, and actions to fix it. The changelog uses the same terms and explains the behavior clearly. No vague …
No Resource Leaks ✅ Passed No resource leak is introduced. The executable changes add only pure path/name computation in internal/core/sibling.go and filename validation in internal/engine/filename.go. The changed write pat…
Scope, Duplication And Docs ✅ Passed The PR scope is consistent with its title and description. The changed code only implements the 255-byte filename limit and safe temporary sibling naming, updates the manifest and in-place replacement…

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the bug Something isn't working label Sep 24, 2026
@donislawdev
donislawdev merged commit 4198c43 into main Sep 24, 2026
20 checks passed
@donislawdev
donislawdev deleted the engine/long-names branch September 24, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant