MagicBeanDev/Hookshot

Archived

Watch

Fork 0

fix(build): explain missing prompt stamps and reclaim abandoned staging #2710

Closed

github-sync wants to merge 4 commits from github/pr/2710 into main

github-sync commented

2026-11-12 08:30:54 +00:00

Owner

@mooki000 opened this pull request on GitHub · 2026-11-12 07:53 UTC · original
Merged on GitHub in 141339eb6d

What this is for: answer the two review notes on #2698: give an ordinary first build a clear message, and reclaim staging directories left by interrupted prompt generation. What was checked: Linux source checks and real finish-stage/hard-kill recovery; both Windows source-check shards passed on the fork before the new commit reached this PR. Not tested: a Windows build using game assets, Windows closed-console/power-loss recovery, or very long Windows paths.

Refs #2698.

What changed

A missing stamp now prints prompts: regenerated (no stamp yet). Existing unreadable or malformed stamps keep their diagnostic.
The build considers only .prompts-* directories directly beside its prompt output. Files, entries listed as links or Windows reparse points, the output itself and other neighbours are excluded. Cleanup trouble is logged rather than failing the build.
POSIX: the reviewed descriptor-anchored path stays. The parent and candidate are opened without following links, the candidate is compared by device/inode, the owner is opened relative to that descriptor, and removal uses the parent's descriptor with descriptor-safe rmtree. A live or uncertain owner is kept; a released, initialized owner permits immediate cleanup. Unmarked legacy directories get a 24-hour grace period. Without descriptor-safe removal, cleanup is skipped and says so.
Windows: the rule is now deliberately simpler: only directories at least 24 hours old are eligible, even if they have an owner marker. The sweep never opens a leftover owner file or compares Windows file identities. If Windows refuses removal because a directory/file is locked, in use, read-only or otherwise inaccessible, the directory is kept with the existing prompts: kept staging directory … line and the build continues.
The Windows resolved-parent/identity machinery and its stand-in identity tests are removed. Commit 26edc2c1 is 62 lines smaller overall: 87 added, 149 removed across code, tests and documentation.

Asset generation, input fingerprints, output validation, failed-generation fallback and runtime-derived-file clearing are unchanged.

Review follow-up

Nitnek's parent-swap reproduction was confirmed on 47d9a1d0: the sweep opened and locked an external owner file after the checked directory was replaced by a link. In that interleaving, rmtree then refused the top-level link and external contents remained unchanged, on Python 3.12.0 and 3.12.16. The POSIX fix observes no external owner open or lock in the same regression.

The Windows identity-based attempt in 7ed87ced failed its new cleanup test on the real Windows runner despite passing the Linux stand-ins. Following Joey-Magnus's suggestion, 26edc2c1 removes that Windows approach rather than adding another identity workaround. The replacement tests use real filesystem operations: fresh owned staging is kept, old owned and legacy staging is removed, and removal refusals are nonfatal. Open-file and read-only-file deletion cases run only on real Windows; Linux does not pretend to have those Windows semantics.

Source-check evidence

Latest head: 26edc2c183241c03076b3c893cd9fe9cf6e176b1.

Linux, Python 3.12.16: the focused file reports 39 passed and 2 Windows-only tests skipped. The complete source check reports:
```
472 passed, 0 failed, 20 skipped in 212 s (linux, 8 at once)
```
These checks ran before the commit, on the exact source committed, through core-suppression wrappers. No core dumps were recorded. The POSIX swap, ownership and descriptor-safety checks remain covered.
The commit was then tested on a scratch draft in the fork before this PR received it. In the fork source-check run:
- Python source checks (windows-2022, part 1/2): PASS.
- Python source checks (windows-2022, part 2/2): PASS, including the part that failed on 7ed87ced.
- Linux source-check part 1/2: PASS; part 2/2 was still running at the reported check.
- The fork's Windows builder, Vulkan back ends, SPIR-V fixtures and actionlint checks also passed.
- The fork's AppImage job failed at a download: curl: (7) Couldn't connect to server. This was a fork-only download failure; the upstream AppImage job for this PR passed.

The new commit is now on this PR. Upstream's own run on the new head is the authoritative result and is still in progress; the fork preflight is not a claim that every upstream check has finished.

Real Linux finish-stage evidence

These earlier runs are pinned to 7ed87ced, not relabelled as the latest head; the subsequent change is the Windows age policy and its tests, retaining the POSIX cleanup protections.

2026-11-12; Arch Linux 7.2.15-1-omarchy, Core i9-13900K (32 logical CPUs), Python 3.12.16, all prompt packages present. The real finish-stage statements ran in a fresh reflink copy of an existing built game folder, with real game assets and no drawing/encoder stand-ins. Already-built binaries were reused; compilation, shader preparation and gameplay were not run. Other work was running, including another game and compilers.

Finish-stage case	Time	1-minute load before → after	Printed line
Stamp removed first	81.222 s	33.58 → 22.73	`prompts: regenerated (no stamp yet)`
Unchanged	0.843 s	22.73 → 22.73	`prompts: unchanged, kept`
Old and fresh legacy leftovers	0.877 s	22.73 → 24.35	`prompts: unchanged, kept`
Next build after mid-generation SIGKILL	60.145 s	18.40 → 13.09	`prompts: regenerated (no stamp yet)`

All four stages exited 0 and printed the build's completion line. All 16 prompt originals matched the stamp's recorded sizes and SHA-256 hashes after each stage. The two-day-old legacy directory was removed; the fresh one remained.

The forced real generation was killed after prompts: ps4, the third face, 40.826 seconds into generation. Its live owner lock refused a second lock. SIGKILL left a partial staging directory and no stamp, while the previous originals still matched their saved manifest; the next real finish stage removed that directory and regenerated successfully. The same SOURCE_DATE_EPOCH kept font-header timestamps comparable, and the regenerated output hashes matched the first run.

Limits

Windows cleanup is age-based and path-based, not equivalent to POSIX descriptor anchoring or protection against coordinated malicious renames. A trusted build/output folder is required; someone able to rename its entries during a build can already replace the game itself. A failed removal can leave stale material for a later build; the build does not force access or change file permissions to remove it.

Windows native cleanup fixtures have now run, but no Windows game-asset generation, closed-console/power-loss recovery or very-long-path test is claimed. The real finish runs are Linux build-tool evidence, not gameplay evidence. No generated game files are included.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

> [@mooki000](https://github.com/mooki000) opened this pull request on GitHub · 2026-11-12 07:53 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2710) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/141339eb6de0bbe9ca2ca6c148d79d3b7c751a2d **What this is for:** answer the two review notes on #2698: give an ordinary first build a clear message, and reclaim staging directories left by interrupted prompt generation. **What was checked:** Linux source checks and real finish-stage/hard-kill recovery; both Windows source-check shards passed on the fork before the new commit reached this PR. **Not tested:** a Windows build using game assets, Windows closed-console/power-loss recovery, or very long Windows paths. Refs #2698. ## What changed - A missing stamp now prints `prompts: regenerated (no stamp yet)`. Existing unreadable or malformed stamps keep their diagnostic. - The build considers only `.prompts-*` directories directly beside its prompt output. Files, entries listed as links or Windows reparse points, the output itself and other neighbours are excluded. Cleanup trouble is logged rather than failing the build. - **POSIX:** the reviewed descriptor-anchored path stays. The parent and candidate are opened without following links, the candidate is compared by device/inode, the owner is opened relative to that descriptor, and removal uses the parent's descriptor with descriptor-safe `rmtree`. A live or uncertain owner is kept; a released, initialized owner permits immediate cleanup. Unmarked legacy directories get a 24-hour grace period. Without descriptor-safe removal, cleanup is skipped and says so. - **Windows:** the rule is now deliberately simpler: **only directories at least 24 hours old are eligible**, even if they have an owner marker. The sweep never opens a leftover owner file or compares Windows file identities. If Windows refuses removal because a directory/file is locked, in use, read-only or otherwise inaccessible, the directory is kept with the existing `prompts: kept staging directory …` line and the build continues. - The Windows resolved-parent/identity machinery and its stand-in identity tests are removed. Commit `26edc2c1` is **62 lines smaller overall**: 87 added, 149 removed across code, tests and documentation. Asset generation, input fingerprints, output validation, failed-generation fallback and runtime-derived-file clearing are unchanged. ## Review follow-up [Nitnek's parent-swap reproduction](https://git.magicbean.dev/MagicBeanDev/Hookshot/pulls/2710) was confirmed on `47d9a1d0`: the sweep opened and locked an external owner file after the checked directory was replaced by a link. In that interleaving, `rmtree` then refused the top-level link and external contents remained unchanged, on Python 3.12.0 and 3.12.16. The POSIX fix observes no external owner open or lock in the same regression. The Windows identity-based attempt in `7ed87ced` failed its new cleanup test on the real Windows runner despite passing the Linux stand-ins. Following Joey-Magnus's suggestion, `26edc2c1` removes that Windows approach rather than adding another identity workaround. The replacement tests use real filesystem operations: fresh owned staging is kept, old owned and legacy staging is removed, and removal refusals are nonfatal. Open-file and read-only-file deletion cases run **only on real Windows**; Linux does not pretend to have those Windows semantics. ## Source-check evidence Latest head: `26edc2c183241c03076b3c893cd9fe9cf6e176b1`. - Linux, Python 3.12.16: the focused file reports **39 passed and 2 Windows-only tests skipped**. The complete source check reports: ```text 472 passed, 0 failed, 20 skipped in 212 s (linux, 8 at once) ``` These checks ran before the commit, on the exact source committed, through core-suppression wrappers. No core dumps were recorded. The POSIX swap, ownership and descriptor-safety checks remain covered. - The commit was then tested on a **scratch draft in the fork before this PR received it**. In the [fork source-check run](https://github.com/mooki000/Hookshot/actions/runs/43242986599): - `Python source checks (windows-2022, part 1/2)`: **PASS**. - `Python source checks (windows-2022, part 2/2)`: **PASS**, including the part that failed on `7ed87ced`. - Linux source-check part 1/2: **PASS**; part 2/2 was still running at the reported check. - The fork's Windows builder, Vulkan back ends, SPIR-V fixtures and actionlint checks also passed. - The fork's AppImage job failed at a download: `curl: (7) Couldn't connect to server`. This was a fork-only download failure; the upstream AppImage job for this PR passed. The new commit is now on this PR. **Upstream's own run on the new head is the authoritative result and is still in progress**; the fork preflight is not a claim that every upstream check has finished. ## Real Linux finish-stage evidence These earlier runs are pinned to `7ed87ced`, not relabelled as the latest head; the subsequent change is the Windows age policy and its tests, retaining the POSIX cleanup protections. 2026-11-12; Arch Linux `7.2.15-1-omarchy`, Core i9-13900K (32 logical CPUs), Python 3.12.16, all prompt packages present. The real finish-stage statements ran in a fresh reflink copy of an existing built game folder, with real game assets and no drawing/encoder stand-ins. Already-built binaries were reused; compilation, shader preparation and gameplay were not run. **Other work was running**, including another game and compilers. | Finish-stage case | Time | 1-minute load before → after | Printed line | |---|---:|---:|---| | Stamp removed first | 81.222 s | 33.58 → 22.73 | `prompts: regenerated (no stamp yet)` | | Unchanged | 0.843 s | 22.73 → 22.73 | `prompts: unchanged, kept` | | Old and fresh legacy leftovers | 0.877 s | 22.73 → 24.35 | `prompts: unchanged, kept` | | Next build after mid-generation SIGKILL | 60.145 s | 18.40 → 13.09 | `prompts: regenerated (no stamp yet)` | All four stages exited 0 and printed the build's completion line. All **16 prompt originals matched the stamp's recorded sizes and SHA-256 hashes** after each stage. The two-day-old legacy directory was removed; the fresh one remained. The forced real generation was killed after `prompts: ps4`, the third face, 40.826 seconds into generation. Its live owner lock refused a second lock. SIGKILL left a partial staging directory and no stamp, while the previous originals still matched their saved manifest; the next real finish stage removed that directory and regenerated successfully. The same `SOURCE_DATE_EPOCH` kept font-header timestamps comparable, and the regenerated output hashes matched the first run. ## Limits Windows cleanup is age-based and path-based, not equivalent to POSIX descriptor anchoring or protection against coordinated malicious renames. A trusted build/output folder is required; someone able to rename its entries during a build can already replace the game itself. A failed removal can leave stale material for a later build; the build does not force access or change file permissions to remove it. Windows native cleanup fixtures have now run, but no Windows game-asset generation, closed-console/power-loss recovery or very-long-path test is claimed. The real finish runs are Linux build-tool evidence, not gameplay evidence. No generated game files are included. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]>

github-sync added 1 commit

2026-11-12 08:30:54 +00:00

fix(build): explain missing prompt stamps and reclaim abandoned staging 47d9a1d0d7

Follow up the two review notes on #2698. Treat an absent stamp as a normal first build, and sweep abandoned prompt staging directories using owner locks, with a 24-hour grace period for legacy unmarked directories. Keep uncertain or linked entries and warn without failing the build.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra <[email protected]>

github-sync referenced this pull request

2026-11-12 08:31:27 +00:00

perf(build): keep unchanged button-prompt assets in source builds #2698

github-sync commented

2026-11-12 12:37:32 +00:00

Author

Owner

@Nitnek commented on GitHub · 2026-11-12 12:26 UTC · original

Reproduced parent-directory race in `sweep_prompts`

On head 47d9a1d0d7527c7ea2db7a8dfc57d712e5ffa161, I reproduced a deterministic interleaving that violates the sweeper's documented no-follow invariant:

entry.stat(follow_symlinks=False) checks a real .prompts-* directory.
After that check, move the directory aside and replace its old pathname with a symlink to an owned synthetic external directory containing a regular .owner file.
The unchanged prompt_owner resolves the swapped parent and opens and locks the external .owner inode. A second real descriptor's nonblocking flock is refused while this lock is held and succeeds again after the context exits. O_NOFOLLOW on the leaf does not protect its parent components.
In this Linux probe, the subsequent shutil.rmtree(entry.path) rejects the top-level symlink. No external deletion or content change was observed. This is evidence of an external open/lock, not a claim of demonstrated save loss or a remote exploit.

The ordinary abandoned-directory cleanup and a symlink present before scanning are separate controls and behave as expected. The race probe executes the unchanged public prompt_owner and sweep_prompts function ASTs with standard-library dependencies and synthetic directories; it does not run the complete prompt-asset build or validate Windows locking.

Could the traversal/owner locking/removal be anchored to checked directory handles, with relative no-follow operations and identity checks (or the appropriate Windows equivalent), and a regression covering the parent replacement? Rechecking only the leaf's regular-file status does not close this interval.

AI-assisted review and reproduction: GPT-5.6-terra and GPT-6.1-sol. No game data used.

> [@Nitnek](https://github.com/Nitnek) commented on GitHub · 2026-11-12 12:26 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2710#issuecomment-6823577721) ### Reproduced parent-directory race in `sweep_prompts` On head `47d9a1d0d7527c7ea2db7a8dfc57d712e5ffa161`, I reproduced a deterministic interleaving that violates the sweeper's documented no-follow invariant: 1. `entry.stat(follow_symlinks=False)` checks a real `.prompts-*` directory. 2. After that check, move the directory aside and replace its old pathname with a symlink to an owned synthetic external directory containing a regular `.owner` file. 3. The unchanged `prompt_owner` resolves the swapped parent and **opens and locks the external `.owner` inode**. A second real descriptor's nonblocking `flock` is refused while this lock is held and succeeds again after the context exits. `O_NOFOLLOW` on the leaf does not protect its parent components. 4. In this Linux probe, the subsequent `shutil.rmtree(entry.path)` rejects the top-level symlink. **No external deletion or content change was observed.** This is evidence of an external open/lock, not a claim of demonstrated save loss or a remote exploit. The ordinary abandoned-directory cleanup and a symlink present before scanning are separate controls and behave as expected. The race probe executes the unchanged public `prompt_owner` and `sweep_prompts` function ASTs with standard-library dependencies and synthetic directories; it does not run the complete prompt-asset build or validate Windows locking. Could the traversal/owner locking/removal be anchored to checked directory handles, with relative no-follow operations and identity checks (or the appropriate Windows equivalent), and a regression covering the parent replacement? Rechecking only the leaf's regular-file status does not close this interval. AI-assisted review and reproduction: GPT-5.6-terra and GPT-6.1-sol. No game data used.

github-sync referenced this pull request from a commit

2026-11-12 13:38:09 +00:00

fix(build): anchor prompt staging cleanup to checked directories

github-sync added 2 commits

2026-11-12 13:38:09 +00:00

fix(build): anchor prompt staging cleanup to checked directories 44cb7eb9d7

Reproduce the parent-swap external owner open and lock reported on #2710. Bind POSIX directory checks, owner opens and safe rmtree traversal to directory descriptors. Check the opened owner parent identity on Windows, retain unknown identities, and document the remaining Windows path-race limitations.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra <[email protected]>

fix(build): read Windows staging identities with os.stat 7ed87ceda0

Windows DirEntry.stat does not provide a usable file identity. Use os.stat for the candidate identity, retain DirEntry for the name and directory filter, and report unavailable identities instead of silently skipping cleanup. Cover both owned and aged legacy staging with zero-id Windows entry stand-ins.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra <[email protected]>

github-sync referenced this pull request

2026-11-12 13:40:11 +00:00

docs: a proposal for a faster development loop #2636

github-sync commented

2026-11-12 13:40:26 +00:00

Author

Owner

@mooki000 commented on GitHub · 2026-11-12 13:07 UTC · original

@Nitnek thanks for the precise interleaving. I confirmed your result on 47d9a1d0: the external .owner was opened and locked; a second descriptor could not take its lock until the context closed. rmtree then refused the top-level link, and the external contents were unchanged. That holds on Python 3.12.16 and 3.12.0, the oldest release allowed by our pinned NumPy dependency.

44cb7eb9 fixes the POSIX path: open the parent and candidate directories with no-follow flags, compare the candidate's device/inode, open .owner relative to that checked descriptor, and remove relative to the parent descriptor only with descriptor-safe rmtree. The swap regression now observes no external owner open or lock. With the Windows identity correction in 7ed87ced, all 34 existing tests and seven new tests pass.

Windows uses os.stat for directory file IDs (DirEntry.stat supplies zero IDs), checks the opened owner's resolved parent identity before locking, and checks the candidate again before removal; unknown or changed identity is reported and keeps it. This is still path-based, not equivalent to POSIX: a transient external open before the identity check and later coordinated rename races remain possible. Native Windows execution is untested; stand-ins cover those identity/error paths and successful cleanup of both owned and old legacy directories.

This is a local filesystem race, not evidence of external deletion or a remote exploit. An attacker able to rename entries inside the game folder during a build can already replace the game itself, but the cleanup should not make that trust boundary worse.

The description now has the finding, both commits and four real finish-stage runs on the new head, including the next build after a real mid-generation kill -9.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 13:07 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2710#issuecomment-6824454034) @Nitnek thanks for the precise interleaving. I confirmed your result on `47d9a1d0`: the external `.owner` was opened and locked; a second descriptor could not take its lock until the context closed. `rmtree` then refused the top-level link, and the external contents were unchanged. That holds on Python 3.12.16 and 3.12.0, the oldest release allowed by our pinned NumPy dependency. `44cb7eb9` fixes the POSIX path: open the parent and candidate directories with no-follow flags, compare the candidate's device/inode, open `.owner` relative to that checked descriptor, and remove relative to the parent descriptor only with descriptor-safe `rmtree`. The swap regression now observes no external owner open or lock. With the Windows identity correction in `7ed87ced`, all 34 existing tests and seven new tests pass. Windows uses `os.stat` for directory file IDs (`DirEntry.stat` supplies zero IDs), checks the opened owner's resolved parent identity before locking, and checks the candidate again before removal; unknown or changed identity is reported and keeps it. This is still path-based, not equivalent to POSIX: a transient external open before the identity check and later coordinated rename races remain possible. Native Windows execution is untested; stand-ins cover those identity/error paths and successful cleanup of both owned and old legacy directories. This is a local filesystem race, not evidence of external deletion or a remote exploit. An attacker able to rename entries inside the game folder during a build can already replace the game itself, but the cleanup should not make that trust boundary worse. The description now has the finding, both commits and four real finish-stage runs on the new head, including the next build after a real mid-generation `kill -9`.

github-sync commented

2026-11-12 14:44:31 +00:00

Author

Owner

@Joey-Magnus requested changes on GitHub · 2026-11-12 14:34 UTC · original

The head 7ed87ced fails its own new test on the Windows runner. In "Python source checks (windows-2022, part 2/2)", test_windows_zero_direntry_ids_use_stat_for_owned_and_legacy_cleanup fails at tests/test_prompt_skip.py:385: "owned Windows staging must not silently skip on DirEntry's zero ids". So on a real Windows file system the owned staging directory is still kept. Merged into current main on Linux, all 41 tests in test_prompt_skip.py pass. The failure is specific to the Windows path, and Windows is where most builds run and the one platform you note as untested natively. The job log should show which "prompts: kept staging directory ... (reason)" line it printed. My guess is the identity check against prompt_owner_parent or os.stat, or rmtree meeting a handle that is still open.

On scope: this is about 280 lines of descriptor and identity handling to reclaim a temp folder that an interrupted build leaves behind. The POSIX half answers Nitnek's swap race well. If the Windows half stays hard to get right, consider limiting Windows to the 24-hour age rule and leaving locked or unknown directories alone. The new imports are all stdlib, so prompt_key() needs no change.

> [@Joey-Magnus](https://github.com/Joey-Magnus) requested changes on GitHub · 2026-11-12 14:34 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2710#pullrequestreview-5876237436) The head 7ed87ced fails its own new test on the Windows runner. In "Python source checks (windows-2022, part 2/2)", test_windows_zero_direntry_ids_use_stat_for_owned_and_legacy_cleanup fails at tests/test_prompt_skip.py:385: "owned Windows staging must not silently skip on DirEntry's zero ids". So on a real Windows file system the owned staging directory is still kept. Merged into current main on Linux, all 41 tests in test_prompt_skip.py pass. The failure is specific to the Windows path, and Windows is where most builds run and the one platform you note as untested natively. The job log should show which "prompts: kept staging directory ... (reason)" line it printed. My guess is the identity check against prompt_owner_parent or os.stat, or rmtree meeting a handle that is still open. On scope: this is about 280 lines of descriptor and identity handling to reclaim a temp folder that an interrupted build leaves behind. The POSIX half answers Nitnek's swap race well. If the Windows half stays hard to get right, consider limiting Windows to the 24-hour age rule and leaving locked or unknown directories alone. The new imports are all stdlib, so prompt_key() needs no change.

github-sync commented

2026-11-12 14:44:31 +00:00

Author

Owner

@mooki000 commented on GitHub · 2026-11-12 14:38 UTC · original

Thanks, and sorry for the red Windows job: I pushed that commit without waiting for its result. I am taking your suggestion: on Windows only the 24-hour age rule, a locked or unremovable directory left alone, and the Windows identity code removed, so the change gets smaller and the POSIX half stays as reviewed. This time the Windows job runs on my fork before the pull request sees the commit.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 14:38 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2710#issuecomment-6826614186) Thanks, and sorry for the red Windows job: I pushed that commit without waiting for its result. I am taking your suggestion: on Windows only the 24-hour age rule, a locked or unremovable directory left alone, and the Windows identity code removed, so the change gets smaller and the POSIX half stays as reviewed. This time the Windows job runs on my fork before the pull request sees the commit.

github-sync referenced this pull request from a commit

2026-11-12 15:48:59 +00:00

fix(build): use age-only prompt staging cleanup on Windows

github-sync added 1 commit

2026-11-12 15:48:59 +00:00

fix(build): use age-only prompt staging cleanup on Windows 26edc2c183

Take the review suggestion on #2710: Windows attempts cleanup only after 24 hours and logs OS removal refusals without failing the build. Remove leftover-owner identity lookup and its stand-in tests; retain the reviewed POSIX descriptor and owner-lock path. Test the portable age rule on either OS, with real Windows-only open-file and read-only-file cases.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra <[email protected]>

github-sync commented

2026-11-12 15:52:08 +00:00

Author

Owner

@mooki000 commented on GitHub · 2026-11-12 15:16 UTC · original

Thanks, I took your suggestion in 26edc2c1. On Windows only the 24-hour age rule is left; a directory Windows refuses to remove (locked, in use, read-only) is kept with the existing line and the build goes on. The Windows owner-identity code is gone and the POSIX half is as you reviewed it. The commit removes 149 lines and adds 87, 62 fewer overall; the Windows test cases that need real open-file and read-only behaviour now run only on Windows.

Both windows-2022 parts passed on a run on my fork before the commit reached this pull request. This pull request's own run on 26edc2c1: the Windows part that failed before (part 2/2) passes; part 1/2 was still running when this was written. The description is updated for the simpler rule and says which result ran where.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 15:16 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2710#issuecomment-6827563732) Thanks, I took your suggestion in `26edc2c1`. On Windows only the 24-hour age rule is left; a directory Windows refuses to remove (locked, in use, read-only) is kept with the existing line and the build goes on. The Windows owner-identity code is gone and the POSIX half is as you reviewed it. The commit removes 149 lines and adds 87, **62 fewer overall**; the Windows test cases that need real open-file and read-only behaviour now run only on Windows. Both `windows-2022` parts passed on [a run on my fork](https://github.com/mooki000/Hookshot/actions/runs/43242986599) before the commit reached this pull request. This pull request's own run on `26edc2c1`: the Windows part that failed before (part 2/2) passes; part 1/2 was still running when this was written. The description is updated for the simpler rule and says which result ran where.

github-sync referenced this pull request from a commit

2026-11-12 17:57:24 +00:00

fix(build): explain missing prompt stamps and reclaim abandoned staging (#2710)

github-sync closed this pull request

2026-11-12 17:58:43 +00:00

github-sync commented

2026-11-12 17:58:44 +00:00

Author

Owner

@hckrman101 approved on GitHub · 2026-11-12 17:23 UTC · original

Thanks, this addresses Joey-Magnus's CHANGES_REQUESTED at 7ed87ce: the red Windows test and the identity machinery behind it are gone, and Windows now only uses the 24-hour age rule, which is what he suggested. Windows part 2/2 and everything else is green on 26edc2c.

What I checked:

Live builds are protected. On POSIX the sweeper takes a non-blocking flock on .owner and only deletes when the lock is free and the owner wrote the ready byte. The builder creates the owner file, locks it and only then writes the ready byte, so a sweeper that sneaks in between sees "ownership not established" and keeps the directory. Legacy unmarked directories get 24 h. test_live_owner_is_kept_even_when_old and test_generation_holds_its_owner_lock cover a sweep during a running build.
Nitnek's parent-swap race is closed by the descriptor-anchored open, inode compare and dir_fd removal, with a regression test.
Windows deletes only .prompts-* directories at least a day old and keeps anything the OS refuses to remove. A running build's staging is fresh, so it is not touched; a locked or open file makes rmtree fail and the directory is kept. The Windows-only open-file and read-only tests ran on the real runner.
Scope of deletion is limited to non-link .prompts-* directories next to the prompt output, never the output itself. Staging holds only generated prompt files, nothing leaves the PC.
The new message (prompts: regenerated (no stamp yet)) is accurate and the docs paragraph describes the sweep correctly.

Nits, none blocking:

A staging directory Windows cannot remove will print "kept staging directory" on every build until someone deletes it by hand. The message could say that, so the line is actionable.
no stamp yet is also what you see after an interrupted build or a manually deleted stamp, so "yet" reads a little odd; fine as is.
The Windows age is judged from the directory's mtime, which does not change when files inside are written. Only a build suspended for more than a day could be affected, and an open file would still block the removal.

The in-game side does not apply here: this is build-tool behaviour, and the real finish-stage runs in the description cover it. Good to merge from my side.

> [@hckrman101](https://github.com/hckrman101) approved on GitHub · 2026-11-12 17:23 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2710#pullrequestreview-5878976314) Thanks, this addresses Joey-Magnus's CHANGES_REQUESTED at 7ed87ce: the red Windows test and the identity machinery behind it are gone, and Windows now only uses the 24-hour age rule, which is what he suggested. Windows part 2/2 and everything else is green on 26edc2c. What I checked: - **Live builds are protected.** On POSIX the sweeper takes a non-blocking flock on `.owner` and only deletes when the lock is free and the owner wrote the ready byte. The builder creates the owner file, locks it and only then writes the ready byte, so a sweeper that sneaks in between sees "ownership not established" and keeps the directory. Legacy unmarked directories get 24 h. `test_live_owner_is_kept_even_when_old` and `test_generation_holds_its_owner_lock` cover a sweep during a running build. - **Nitnek's parent-swap race** is closed by the descriptor-anchored open, inode compare and `dir_fd` removal, with a regression test. - **Windows** deletes only `.prompts-*` directories at least a day old and keeps anything the OS refuses to remove. A running build's staging is fresh, so it is not touched; a locked or open file makes `rmtree` fail and the directory is kept. The Windows-only open-file and read-only tests ran on the real runner. - **Scope of deletion** is limited to non-link `.prompts-*` directories next to the prompt output, never the output itself. Staging holds only generated prompt files, nothing leaves the PC. - **The new message** (`prompts: regenerated (no stamp yet)`) is accurate and the docs paragraph describes the sweep correctly. Nits, none blocking: - A staging directory Windows cannot remove will print "kept staging directory" on every build until someone deletes it by hand. The message could say that, so the line is actionable. - `no stamp yet` is also what you see after an interrupted build or a manually deleted stamp, so "yet" reads a little odd; fine as is. - The Windows age is judged from the directory's mtime, which does not change when files inside are written. Only a build suspended for more than a day could be affected, and an open file would still block the removal. The in-game side does not apply here: this is build-tool behaviour, and the real finish-stage runs in the description cover it. Good to merge from my side.

github-sync referenced this pull request

2026-11-12 17:59:57 +00:00

feat(menu): a Quality preset "Auto (this PC)" that picks settings from the hardware and tunes them in play #2850

Commenting is not possible because the repository is archived.

No reviewers