Skip to content

refactor(io): share settings and document save mechanics - #1628

Merged
eugenioenko merged 4 commits into
mainfrom
refactor/shared-file-save
Nov 15, 2026
Merged

refactor(io): share settings and document save mechanics#1628
eugenioenko merged 4 commits into
mainfrom
refactor/shared-file-save

Conversation

@eugenioenko

@eugenioenko eugenioenko commented Nov 15, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Owner

Settings and document saves duplicated temporary-file write, sync, close, and replacement handling. Share that lifecycle in internal/filewrite, with caller-owned streaming, metadata ordering, symlink handling, retry, and recovery policies. Settings remain replacement-only; document ownership preservation and the in-place recovery fallback remain intact.

Consolidate settings platform helpers into Windows/non-Windows files. Retain the existing extended-path Windows MoveFileEx wrapper: a native concurrent-reader run exhausted retries with os.Rename, so equivalence was not established. Record the remaining Windows document-save differences in ARCHITECTURE.md. This does not prevent concurrent read/modify/write lost updates.

Validation: make test and make lint pass locally; Windows config tests cross-compile. New deterministic tests cover staging failures and cleanup. The native Windows CI lane runs DACL preservation, concurrent readers, long-path creation/replacement, and bounded retry tests (including a real sharing lock) to validate the shared lifecycle with the existing replacement policy.

Closes #1624

Summary by CodeRabbit

  • Bug Fixes
    • File saves are staged before replacing existing files, helping keep the previous version intact if writing fails.
    • Windows settings saves support long paths and retry certain temporary replacement failures.
    • Settings-file access permissions and document ownership are preserved when saving.

@github-actions

github-actions Bot commented Nov 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Binaries

Built from 1ce647478a72. Links expire on Nov 29, 2026.

Try it in the browser (emulated): runs this build in 32-bit Linux emulated inside the page, with no download or sign-in. It is much slower than a native build and is meant for quick checks of behavior and UI, not performance or terminal compatibility. Startup takes up to a minute.

Platform Download
Linux x64 ttt-pr-1628-linux-amd64
macOS Apple Silicon ttt-pr-1628-darwin-arm64
macOS Intel ttt-pr-1628-darwin-amd64
Windows x64 ttt-pr-1628-windows-amd64

Downloads need a GitHub sign-in and arrive as zips. On Linux and macOS, run chmod +x ttt after unzipping. These binaries run this pull request's code, so only run them if you trust it.

@coderabbitai

coderabbitai Bot commented Nov 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

This PR adds shared staged-file operations and uses them for settings and document saves. It consolidates settings helpers into platform-specific files, adds Windows settings tests, updates CI test selection, and documents shared mechanics and caller-specific policies.

Changes: Settings and document save mechanics

Layer / File(s) Summary
Staged-file API and caller boundaries
internal/filewrite/*, ARCHITECTURE.md		 Adds filewrite.Stage, Staged.Commit, and Staged.Discard for same-directory temporary writes, sync, replacement, and cleanup. Tests cover success and failure cases. The architecture document describes the shared mechanics and caller-owned metadata, retry, and recovery policies.
Settings save and platform behavior
internal/config/settings.go, internal/config/settings_other.go, internal/config/settings_windows.go, internal/config/settings_*, .github/workflows/ci.yml		 Settings writes now use filewrite.Stage and replaceSettingsFile. Non-Windows settings operations are consolidated in settings_other.go; Windows access preservation, replacement-friendly reads, long-path handling, and bounded replacement retries are in settings_windows.go. Windows tests cover long paths and retry outcomes, and CI selects the settings and filewrite tests.
Document save integration
internal/core/buffer/io.go		 SaveFile now stages writes through internal/filewrite. It retains the existing ownership-preservation rule and in-place overwrite fallback.

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

Change: Refactor

Possibly related issues

Possibly related PRs

  • eugenioenko/ttt#1541 — This PR moves the atomic settings write mechanics introduced there into internal/filewrite.

Suggested reviewers: lzcwbcgs, danielmrdev

Merge Risk: Minimal · up to 1ce64

This change consolidates how settings and documents are saved to disk while keeping existing behavior. No concrete problems were identified, so it looks ready to merge.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "refactor(io): share settings and document save mechanics" matches the main change. The PR moves temporary-file staging, syncing, closing, and replacement into a new internal/filewrite packa…
Linked Issues check Passed Directly linked issue #1624 has coding requirements. The reviewed head (1ce6474) supports them:

1. Shared mechanics [#1624]: internal/filewrite/filewrite.go defines Stage, Staged.Commit, and Staged.D…
Out of Scope Changes check Passed Each changed area maps to #1624 or supports its implementation:

- internal/filewrite/filewrite.go and filewrite_test.go: the shared staged-replacement primitive and its tests.
- internal/config/setti…
Full details: Title check

Explanation

The title "refactor(io): share settings and document save mechanics" matches the main change. The PR moves temporary-file staging, syncing, closing, and replacement into a new internal/filewrite package, and both settings saves and document saves now use it. The title does not mention the Windows-specific reader, path, and retry changes or the ARCHITECTURE.md update, but it does not need to cover every detail.

Full details: Linked Issues check

Explanation

Directly linked issue #1624 has coding requirements. The reviewed head (1ce6474) supports them:

  1. Shared mechanics [#1624]: internal/filewrite/filewrite.go defines Stage, Staged.Commit, and Staged.Discard. Stage creates an exclusive sibling temp file, runs the caller's write callback, and syncs. Commit closes the file, calls the caller's replace function, and clears the path only on success. internal/config/settings.go and internal/core/buffer/io.go use this API. The package imports only the standard library.
  2. Caller policies stay with callers [#1624]: settings.go keeps serialization, symlink resolution, and replacement-only writes. buffer/io.go keeps streaming, Unix owner/group preservation, and the in-place fallback. The buffer package does not import config. No second full document copy is introduced.
  3. Platform helpers [#1624]: settings helpers are consolidated into settings_windows.go and settings_other.go. The Windows file carries the go:build windows constraint.
  4. Rename wrapper [#1624]: the issue allows os.Rename only if native tests confirm equivalence. The PR retains the extended-path MoveFileEx wrapper (renameSettingsFile and settingsLongPathName in settings_windows.go) and records the reason. This matches the issue's allowance.
  5. Retained settings guarantees [#1624]: settings_windows.go keeps DACL preservation (preserveSettingsFileAccess), the read/rename lock settingsFileMu, and bounded retry (20 attempts, 25 ms) for access-denied and sharing-violation errors. Settings gain no in-place fallback.
  6. Failed replacement [#1624]: Staged.Commit defers Discard, which removes the temp file when replacement fails. TestStagedReplacement covers this path, per the change summary.
  7. Native Windows tests [#1624]: the CI lane selects TestStaged, TestSaveSettings, and TestWindowsSettings. settings_replace_windows_test.go adds long-path and retry tests.
  8. Documentation [#1624]: ARCHITECTURE.md records the remaining Windows document-save differences and states that lost updates are not addressed.

Limitations: I read filewrite.go and settings_windows.go at head. The settings.go output was truncated, so its use of Stage relies on the change summary. The native Windows CI results are not independently verified. The CI change replaces a prior internal/config test selection for concurrent-reader JSON validity; whether that test still runs in the native lane was not confirmed.

Full details: Out of Scope Changes check

Explanation

Each changed area maps to #1624 or supports its implementation:

  • internal/filewrite/filewrite.go and filewrite_test.go: the shared staged-replacement primitive and its tests.
  • internal/config/settings.go: moves settings writes onto filewrite.Stage and Commit.
  • internal/config/settings_windows.go and settings_other.go: consolidated platform helpers. The retained Windows wrapper is justified in the PR description and ARCHITECTURE.md.
  • Deleted settings_access_, settings_read_, settings_rename_*, and settings_windows_path.go: removed or merged into the consolidated files.
  • internal/config/settings_replace_windows_test.go: Windows long-path and retry tests required by the native-test criterion.
  • internal/core/buffer/io.go: document save uses the shared primitive and keeps the ownership fallback.
  • .github/workflows/ci.yml: selects staging, settings, and Windows tests for the native lane. This supports the validation criteria.
  • ARCHITECTURE.md: documents the package boundary and platform differences.

No changed area adds unrelated behavior. The PR does not alter lost-update behavior, and no summarized change does. Limitation: the settings.go and buffer/io.go diffs were not read line by line, so this verdict relies on the whole-PR summary and the head source of filewrite.go and settings_windows.go.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 2b30fbc and 1ce6474.

Files selected for processing: 3

  • internal/config/settings_read_windows.go
  • internal/config/settings_rename_windows.go
  • internal/config/settings_windows.go
Files with no reviewable changes: 2

  • internal/config/settings_read_windows.go
  • internal/config/settings_rename_windows.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

@github-actions

github-actions Bot commented Nov 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Binary size

Comparing 905fab61f20e (base) with 1ce647478a72 (PR). Threshold: 256 KB.

Target Base PR Delta %
linux/amd64 18.70 MB 18.71 MB +4 KB 0.0%
darwin/amd64 19.11 MB 19.11 MB +0 KB 0.0%
darwin/arm64 17.89 MB 17.89 MB +0 KB 0.0%
windows/amd64 19.22 MB 19.22 MB +3 KB 0.0%

@github-actions

github-actions Bot commented Nov 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Editor, diff, and log benchmarks

Comparing 905fab61f20e with 1ce647478a72 on the same runner: fastest of 3 alternating passes at 5x each, -short (2,000-line large file).

Allocated bytes measure total allocation per operation, not peak or retained RAM.

Logs are opened by each side's built ttt binary, 1x per pass, timed from launch until the first frame shows the log; _wrap rows start with word wrap on, and _edit opens a log committed to git, types a character, and moves down 20 lines. Time includes process startup, so the 1 MiB case is mostly startup; peak RSS is the process's maximum resident memory. The 1, 128, and 256 MiB fixtures are generated once outside timing and shared across both checkouts, so reads may use the OS file cache.

Code files open a 64 MiB Go file, this repository's sources repeated, with LSP and the git gutter off; _type also types 20 characters and _enter presses Enter 10 times before quitting.

Editor

Geometric-mean change: time -0.4%, allocated bytes 0.0%, allocations 0.0%.

Click here to see benchmark details
Benchmark Base PR Time change Base allocs PR allocs Allocation change
go/redraw 311 µs 304 µs -2.3% 35 35 0.0%
go/cursor_down 326 µs 323 µs -0.7% 37 37 0.0%
go/page_down 1.6 ms 1.6 ms +0.6% 3015 3015 0.0%
go/wheel_scroll 520 µs 543 µs +4.3% 1507 1507 0.0%
go/type_and_delete 630 µs 607 µs -3.7% 102 102 0.0%
go/open_large_first 10.2 ms 10.3 ms +1.2% 48361 48363 0.0%
go/open_large 2.5 ms 2.6 ms +1.1% 2597 2597 0.0%
go/jump_to_end_large 8.4 ms 8.2 ms -3.2% 11081 11084 0.0%
go/redraw_git_changes_large 372 µs 366 µs -1.6% 87 87 0.0%
go/wheel_scroll_git_changes_large 637 µs 631 µs -1.0% 1570 1571 +0.1%
go/type_and_delete_git_changes_large 783 µs 743 µs -5.1% 217 217 0.0%
go/edit_top_view_end_large 1.3 ms 1.3 ms +5.6% 2814 2814 0.0%
ts/redraw 318 µs 312 µs -1.7% 37 37 0.0%
ts/cursor_down 324 µs 321 µs -0.8% 38 38 0.0%
ts/page_down 4.7 ms 4.7 ms +0.2% 13519 13519 0.0%
ts/wheel_scroll 2.1 ms 2.4 ms +13.6% 12035 12036 0.0%
ts/type_and_delete 660 µs 635 µs -3.8% 114 114 0.0%
ts/open_large_first 35.2 ms 34.8 ms -1.2% 204601 204595 0.0%
ts/open_large 3.6 ms 3.5 ms -1.1% 3870 3869 0.0%
ts/jump_to_end_large 24.1 ms 24.3 ms +1.0% 62315 62314 0.0%
ts/edit_top_view_end_large 1.3 ms 1.3 ms +0.5% 3412 3412 0.0%
cpp/redraw 326 µs 329 µs +1.0% 36 36 0.0%
cpp/cursor_down 348 µs 335 µs -3.7% 38 38 0.0%
cpp/page_down 6.6 ms 6.6 ms -0.1% 7946 7951 +0.1%
cpp/wheel_scroll 1.3 ms 1.4 ms +2.6% 1983 1982 -0.1%
cpp/type_and_delete 661 µs 645 µs -2.4% 114 114 0.0%
cpp/open_large_first 233.4 ms 231.9 ms -0.7% 1386883 1386885 0.0%
cpp/open_large 8.0 ms 8.0 ms 0.0% 5305 5302 -0.1%
cpp/jump_to_end_large 45.9 ms 47.1 ms +2.5% 63500 63506 0.0%
cpp/type_while_catching_up_large 880 µs 866 µs -1.6% 139 139 0.0%
cpp/edit_top_view_end_large 1.5 ms 1.5 ms -2.5% 4347 4347 0.0%
python/redraw 321 µs 314 µs -2.2% 37 37 0.0%
python/cursor_down 324 µs 323 µs -0.4% 38 38 0.0%
python/page_down 2.6 ms 2.3 ms -11.8% 3431 3430 0.0%
python/wheel_scroll 637 µs 638 µs +0.1% 1831 1831 0.0%
python/type_and_delete 677 µs 661 µs -2.4% 113 113 0.0%
python/open_large_first 10.5 ms 10.5 ms 0.0% 45999 45997 0.0%
python/open_large 3.5 ms 3.7 ms +5.8% 3623 3625 +0.1%
python/jump_to_end_large 13.3 ms 13.3 ms +0.4% 12858 12858 0.0%
python/edit_top_view_end_large 1.4 ms 1.3 ms -1.4% 4261 4261 0.0%
Benchmark Base allocated bytes PR allocated bytes Change
go/redraw 1587 B 1596 B +0.6%
go/cursor_down 1659 B 1659 B 0.0%
go/page_down 340790 B 340780 B 0.0%
go/wheel_scroll 111926 B 111926 B 0.0%
go/type_and_delete 4803 B 4803 B 0.0%
go/open_large_first 5.8 MiB 5.8 MiB +0.2%
go/open_large 455435 B 455435 B 0.0%
go/jump_to_end_large 1.5 MiB 1.5 MiB 0.0%
go/redraw_git_changes_large 12675 B 12684 B +0.1%
go/wheel_scroll_git_changes_large 123531 B 123617 B +0.1%
go/type_and_delete_git_changes_large 28792 B 28792 B 0.0%
go/edit_top_view_end_large 134870 B 134873 B 0.0%
ts/redraw 1859 B 1859 B 0.0%
ts/cursor_down 1803 B 1793 B -0.6%
ts/page_down 1.3 MiB 1.3 MiB 0.0%
ts/wheel_scroll 1.0 MiB 1.0 MiB 0.0%
ts/type_and_delete 6590 B 6590 B 0.0%
ts/open_large_first 19.7 MiB 19.6 MiB -0.1%
ts/open_large 518032 B 515552 B -0.5%
ts/jump_to_end_large 6.0 MiB 6.0 MiB 0.0%
ts/edit_top_view_end_large 166972 B 166972 B 0.0%
cpp/redraw 1616 B 1616 B 0.0%
cpp/cursor_down 1688 B 1688 B 0.0%
cpp/page_down 876395 B 883816 B +0.8%
cpp/wheel_scroll 155705 B 155321 B -0.2%
cpp/type_and_delete 5568 B 5568 B 0.0%
cpp/open_large_first 149.5 MiB 149.5 MiB 0.0%
cpp/open_large 726067 B 718790 B -1.0%
cpp/jump_to_end_large 8.5 MiB 8.5 MiB 0.0%
cpp/type_while_catching_up_large 9652 B 9652 B 0.0%
cpp/edit_top_view_end_large 254126 B 254126 B 0.0%
python/redraw 1856 B 1856 B 0.0%
python/cursor_down 1732 B 1732 B 0.0%
python/page_down 282452 B 282281 B -0.1%
python/wheel_scroll 100134 B 100134 B 0.0%
python/type_and_delete 6944 B 6944 B 0.0%
python/open_large_first 5.2 MiB 5.2 MiB -0.1%
python/open_large 516924 B 517014 B 0.0%
python/jump_to_end_large 1.3 MiB 1.3 MiB 0.0%
python/edit_top_view_end_large 210398 B 210398 B 0.0%

Diff views

Geometric-mean change: time -1.5%, allocated bytes 0.0%, allocations 0.0%.

Click here to see benchmark details
Benchmark Base PR Time change Base allocs PR allocs Allocation change
go/split/redraw 342 µs 338 µs -1.2% 43 43 0.0%
go/split/wheel_scroll 726 µs 736 µs +1.3% 2792 2792 0.0%
go/split/type_and_delete 724 µs 711 µs -1.7% 133 133 0.0%
go/split/enter_and_backspace 1.3 ms 1.2 ms -12.3% 2095 2095 0.0%
go/split/page_down 1.9 ms 1.8 ms -4.6% 4236 4234 0.0%
go/split/open_large 5.8 ms 5.6 ms -3.9% 15312 15309 0.0%
go/split/open_large_full_file 5.1 ms 5.0 ms -2.7% 14023 14022 0.0%
go/split/jump_to_end_large 2.5 ms 2.5 ms +1.4% 4996 4996 0.0%
go/split/expand_gap 963 µs 968 µs +0.6% 3255 3254 0.0%
go/unified/redraw 319 µs 313 µs -1.9% 38 38 0.0%
go/unified/wheel_scroll 581 µs 598 µs +2.8% 1528 1528 0.0%
go/unified/type_and_delete 672 µs 676 µs +0.6% 123 123 0.0%
go/unified/page_down 1.7 ms 1.8 ms +1.3% 3018 3019 0.0%
go/unified/open_large 4.1 ms 4.3 ms +2.7% 12776 12780 0.0%
go/unified/open_large_full_file 4.8 ms 4.8 ms -1.5% 12645 12646 0.0%
go/unified/jump_to_end_large 2.3 ms 2.3 ms 0.0% 3276 3275 0.0%
go/unified/expand_gap 807 µs 808 µs +0.2% 1877 1876 -0.1%
go/split_wrap/redraw 346 µs 344 µs -0.6% 135 135 0.0%
go/split_wrap/wheel_scroll 755 µs 715 µs -5.3% 2882 2883 0.0%
go/split_wrap/type_and_delete 1.0 ms 985 µs -3.2% 309 309 0.0%
go/split_wrap/resize 2.0 ms 2.1 ms +4.6% 3879 3879 0.0%
go/unified_wrap/redraw 319 µs 308 µs -3.6% 84 84 0.0%
go/unified_wrap/wheel_scroll 615 µs 582 µs -5.4% 1574 1574 0.0%
go/unified_wrap/resize 1.2 ms 1.2 ms -2.3% 1795 1795 0.0%
go/toggle_split_unified 439 µs 431 µs -2.0% 1608 1608 0.0%
go/switch_diff_and_file_tab 396 µs 387 µs -2.2% 1078 1078 0.0%
go/open_changes_large 6.3 ms 6.2 ms -1.7% 14975 14972 0.0%
commit_detail/open_many_files 43.0 ms 42.4 ms -1.5% 19259 19256 0.0%
commit_detail/wheel_scroll 992 µs 982 µs -0.9% 3072 3072 0.0%
Benchmark Base allocated bytes PR allocated bytes Change
go/split/redraw 2032 B 2032 B 0.0%
go/split/wheel_scroll 177459 B 177510 B 0.0%
go/split/type_and_delete 19097 B 19097 B 0.0%
go/split/enter_and_backspace 162339 B 162345 B 0.0%
go/split/page_down 380433 B 380249 B 0.0%
go/split/open_large 2.3 MiB 2.3 MiB 0.0%
go/split/open_large_full_file 2.8 MiB 2.8 MiB 0.0%
go/split/jump_to_end_large 384686 B 384728 B 0.0%
go/split/expand_gap 562472 B 562369 B 0.0%
go/unified/redraw 1888 B 1897 B +0.5%
go/unified/wheel_scroll 112318 B 112267 B 0.0%
go/unified/type_and_delete 18809 B 18809 B 0.0%
go/unified/page_down 335584 B 335708 B 0.0%
go/unified/open_large 2.0 MiB 2.0 MiB 0.0%
go/unified/open_large_full_file 2.7 MiB 2.7 MiB 0.0%
go/unified/jump_to_end_large 300715 B 300664 B 0.0%
go/unified/expand_gap 411153 B 411092 B 0.0%
go/split_wrap/redraw 2768 B 2768 B 0.0%
go/split_wrap/wheel_scroll 178185 B 178323 B +0.1%
go/split_wrap/type_and_delete 263241 B 263241 B 0.0%
go/split_wrap/resize 2.1 MiB 2.1 MiB 0.0%
go/unified_wrap/redraw 2256 B 2265 B +0.4%
go/unified_wrap/wheel_scroll 112638 B 112689 B 0.0%
go/unified_wrap/resize 1.4 MiB 1.4 MiB 0.0%
go/toggle_split_unified 75670 B 75670 B 0.0%
go/switch_diff_and_file_tab 48675 B 48675 B 0.0%
go/open_changes_large 2.7 MiB 2.7 MiB 0.0%
commit_detail/open_many_files 4.2 MiB 4.2 MiB -0.3%
commit_detail/wheel_scroll 331182 B 331182 B 0.0%

Log files

Geometric-mean change: time +0.3%, peak RSS 0.0%.

Click here to see benchmark details
Benchmark Base PR Time change Base peak RSS PR peak RSS Peak RSS change
1MiB 65.9 ms 66.3 ms +0.6% 26.3 MiB 27.2 MiB +3.4%
128MiB 187.9 ms 187.1 ms -0.4% 171.4 MiB 171.2 MiB -0.1%
256MiB 303.1 ms 304.6 ms +0.5% 315.3 MiB 315.2 MiB 0.0%
1MiB_wrap 76.1 ms 76.1 ms -0.1% 27.6 MiB 27.6 MiB 0.0%
128MiB_wrap 186.4 ms 190.2 ms +2.1% 171.0 MiB 171.2 MiB +0.1%
256MiB_wrap 303.2 ms 301.5 ms -0.6% 315.2 MiB 315.6 MiB +0.1%
128MiB_edit 1878.1 ms 1878.9 ms 0.0% 454.9 MiB 438.8 MiB -3.5%

Code files

Geometric-mean change: time +1.3%, peak RSS +0.1%.

Click here to see benchmark details
Benchmark Base PR Time change Base peak RSS PR peak RSS Peak RSS change
64MiB 368.8 ms 370.5 ms +0.5% 231.4 MiB 231.5 MiB +0.1%
64MiB_type 1584.1 ms 1612.2 ms +1.8% 252.7 MiB 252.9 MiB +0.1%
64MiB_enter 1310.7 ms 1331.3 ms +1.6% 290.5 MiB 291.0 MiB +0.2%

Timing changes are informational because shared CI runners are noisy; a benchmark error still fails the job.

@eugenioenko

eugenioenko commented Nov 15, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai The inconclusive linked-issue check can be verified at these exact call sites:

I am also merging current main and preserving its new settingsFileMu read/rename exclusion inside the consolidated Windows file. Please re-evaluate the linked-issue check using this evidence and the updated head once pushed.

@coderabbitai

coderabbitai Bot commented Nov 15, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

Analysis chain

Script executed:

#!/bin/bash
set -u
echo '=== CHECKED-OUT COMMIT ==='
git log -1 --format='%H%n%ci%n%s'
echo '=== CURRENT PR ==='
gh pr view 1628 --repo eugenioenko/ttt --json headRefOid,baseRefOid,updatedAt,mergeable,statusCheckRollup --jq '{headRefOid,baseRefOid,updatedAt,mergeable,statusCheckRollup: [.statusCheckRollup[] | {name,context,status,conclusion,state,detailsUrl}]}' || true
echo '=== RELEVANT FILES ==='
fd -a '^(settings\.go|settings_windows\.go|settings_other\.go|settings_replace_windows_test\.go|ARCHITECTURE\.md|ci\.yml)$' .
echo '=== SETTINGS OUTLINES ==='
ast-grep outline internal/config/settings.go --items all --match 'writeFileAtomically|settingsFileMu' --view expanded || true
ast-grep outline internal/config/settings_windows.go --items all --view expanded || true
echo '=== SETTINGS CALL CHAIN ==='
rg -n -C 5 'writeFileAtomically|staged\.Commit|settingsFileMu|replaceSettingsFile|renameSettingsFile|settingsLongPathName' internal/config/settings.go internal/config/settings_windows.go internal/config/settings_other.go
echo '=== ARCHITECTURE DOCUMENTATION ==='
cat -n ARCHITECTURE.md | sed -n '55,90p'
echo '=== LINKED WINDOWS JOBS ==='
for job in 132700124707 132701403482; do
  echo "--- job $job ---"
  gh api "repos/eugenioenko/ttt/actions/jobs/$job" --jq '{id,run_id,head_sha,name,status,conclusion,started_at,completed_at,steps: [.steps[] | {name,status,conclusion}]}' || true
done
echo '=== LINKED RUNS ==='
for run in 43677283645 43677705445; do
  echo "--- run $run ---"
  gh api "repos/eugenioenko/ttt/actions/runs/$run" --jq '{id,head_sha,head_branch,event,status,conclusion,created_at,updated_at}' || true
done
echo '=== LINKED ISSUE ==='
gh issue view 1624 --repo eugenioenko/ttt --json title,body,state --jq '{title,state,body}' || true
echo '=== TEST NAMES AND CI SELECTION ==='
rg -n -C 3 'TestWindowsSettings|TestSaveSettings|TestStaged|internal/filewrite|internal/config' .github/workflows/ci.yml internal/config/settings_replace_windows_test.go

Length of output: 19110

Script executed:

#!/bin/bash
set -u
repo=eugenioenko/ttt
old=2b30fbca27f65fe4ff88d0399d4ff1c47b09b324
pr=$(gh api "repos/$repo/pulls/1628") || exit 1
head=$(printf '%s' "$pr" | jq -r '.head.sha')
base=$(printf '%s' "$pr" | jq -r '.base.sha')
printf 'Current head: %s\nCurrent base: %s\n' "$head" "$base"
printf '%s' "$pr" | jq '{updated_at,head_sha:.head.sha,base_sha:.base.sha,mergeable,body}'
echo '=== CURRENT CHECKS ==='
gh pr view 1628 --repo "$repo" --json statusCheckRollup --jq '.statusCheckRollup | map({name,context,status,conclusion,state,detailsUrl})' || true
echo '=== CHANGE SINCE PREVIOUS HEAD ==='
gh api "repos/$repo/compare/$old...$head" --jq '{status,ahead_by,behind_by,commits:[.commits[] | {sha,message:.commit.message}],files:[.files[] | select(.filename | test("^(internal/config/|internal/filewrite/|internal/core/buffer/io.go|ARCHITECTURE.md|.github/workflows/ci.yml)")) | {filename,status,patch}]}' || true
tmp=$(mktemp -d)
fetch_file() {
  local ref="$1" path="$2" dest="$3"
  gh api "repos/$repo/contents/$path?ref=$ref" --jq '.content' | base64 -d > "$dest"
}
echo '=== FETCH CURRENT HEAD FILES ==='
for path in internal/config/settings.go internal/config/settings_windows.go internal/config/settings_other.go internal/config/settings_replace_windows_test.go internal/filewrite/filewrite.go internal/core/buffer/io.go ARCHITECTURE.md .github/workflows/ci.yml; do
  name=$(printf '%s' "$path" | tr '/' '_')
  if fetch_file "$head" "$path" "$tmp/$name"; then echo "Fetched $path"; else echo "FAILED $path"; fi
done
echo '=== HEAD SETTINGS CODE ==='
ast-grep outline "$tmp/internal_config_settings.go" --items all --match 'writeFileAtomically|settingsFileMu' --view expanded || true
ast-grep outline "$tmp/internal_config_settings_windows.go" --items all --view expanded || true
rg -n -C 6 'writeFileAtomically|staged\.Commit|settingsFileMu|replaceSettingsFile|renameSettingsFile|settingsLongPathName' "$tmp/internal_config_settings.go" "$tmp/internal_config_settings_windows.go" "$tmp/internal_config_settings_other.go" || true
echo '=== HEAD ARCHITECTURE ==='
cat -n "$tmp/ARCHITECTURE.md" | sed -n '60,85p'
echo '=== HEAD WINDOWS TESTS AND CI ==='
rg -n -C 3 'TestWindowsSettings|TestSaveSettings|TestStaged|internal/filewrite|internal/config' "$tmp/internal_config_settings_replace_windows_test.go" "$tmp/.github_workflows_ci.yml" || true
echo '=== MAIN MUTEX FILES ==='
for path in internal/config/settings.go internal/config/settings_read_windows.go internal/config/settings_rename_windows.go internal/config/settings_windows.go; do
  name=$(printf '%s' "$path" | tr '/' '_')
  if fetch_file "$base" "$path" "$tmp/base_$name" 2>/dev/null; then
    echo "--- $path at base ---"
    rg -n -C 6 'settingsFileMu|readSettingsFileBytes|renameSettingsFile|replaceSettingsFile' "$tmp/base_$name" || true
  fi
done
echo '=== LINKED WINDOWS LOGS ==='
for pair in '43677283645 132700124707' '43677705445 132701403482'; do
  set -- $pair
  echo "--- run $1 job $2 ---"
  if gh run view "$1" --repo "$repo" --job "$2" --log > "$tmp/log_$2" 2>"$tmp/logerr_$2"; then
    rg -n -C 3 'Test settings replacement on Windows|TestWindowsSettingsReplacementConcurrentReaders|FAIL|PASS|ok[[:space:]]+.*internal/(config|filewrite)|go test -count=1' "$tmp/log_$2" | head -160 || true
  else
    echo 'Log unavailable:'
    cat "$tmp/logerr_$2"
  fi
done

Length of output: 33184

Script executed:

#!/bin/bash
set -u
echo '=== WINDOWS TESTS AT SUCCESSFUL PRIOR HEAD ==='
ast-grep outline internal/config/settings_access_windows_test.go --items all --view expanded || true
ast-grep outline internal/config/settings_replace_windows_test.go --items all --view expanded || true
rg -n -C 3 'func Test|t\.Skip|testing\.Short|LockFile|CreateFile|sharing|SHARING|DACL|dacl' internal/config/settings_access_windows_test.go internal/config/settings_replace_windows_test.go || true
echo '=== CURRENT NATIVE WINDOWS JOB ==='
gh api repos/eugenioenko/ttt/actions/jobs/132715610512 --jq '{id,head_sha,status,conclusion,steps:[.steps[] | {name,status,conclusion}]}' || true
echo '=== CURRENT PR CHECKS ==='
gh pr view 1628 --repo eugenioenko/ttt --json headRefOid,statusCheckRollup --jq '{headRefOid,checks: [.statusCheckRollup[] | select(.name == "native-apps" or .name == "test" or .name == "test-race" or .name == "lint") | {name,status,conclusion,detailsUrl}]}' || true

Length of output: 9431

@eugenioenko The linked-issue reassessment supports retaining the Windows wrapper, but current-head validation is still pending.

  • At head 1ce6474, writeFileAtomically passes renameSettingsFile through replaceSettingsFile. On Windows, that function uses the extended-path MoveFileEx wrapper.
  • The earlier os.Rename run failed in TestWindowsSettingsReplacementConcurrentReaders with access denied. The restored-wrapper run passed the selected staging and settings tests. The failure does not establish that path normalization caused it.
  • The updated head preserves main’s settingsFileMu exclusion between settings reads and renames. ARCHITECTURE.md records the wrapper decision and document-save differences.

The successful Windows run tested the prior head. At the last check, the current-head Windows job and the test, race, and lint jobs were still running. Before merging, confirm that those checks pass for 1ce6474.

Learnings added 

Learnt from: eugenioenko
URL: https://github.com/eugenioenko/ttt/pull/1628

Timestamp: 2026-11-15T05:45:46.828Z
Learning: Within repository "eugenioenko/ttt", only for files matching one of ["internal/config/settings.go"]: In internal/config/settings.go, the Windows settings save chain retains an extended-path MoveFileEx wrapper. A native Windows os.Rename trial exhausted retries in TestWindowsSettingsReplacementConcurrentReaders before the settingsFileMu merge; it did not establish that path normalization caused the failure or prove behavior after the mutex was added. Before replacing the wrapper with os.Rename, require native Windows evidence of equivalent replacement, long-path, and error behavior.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@eugenioenko

eugenioenko commented Nov 15, 2026

Copy link
Copy Markdown
Owner Author

Confirmed current-head validation for 1ce647478a7231cc0cd7d22f7fe0b167ccca3f38: native Windows, tests, race tests, and lint all passed. The other selected CI checks passed as well.

The merge with main preserves its Windows settings reader/rename mutex in the consolidated platform file. GitHub reports the PR mergeable. CodeRabbit's latest review covers this head, has no actionable comments, and all four pre-merge checks now pass, including the linked-issue check.

@eugenioenko
eugenioenko merged commit d5cf5d3 into main Nov 15, 2026
21 checks passed
@eugenioenko
eugenioenko deleted the refactor/shared-file-save branch November 15, 2026 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

refactor(io): consolidate settings and document save mechanics

1 participant

@eugenioenko