fix(workspace): materialize and classify executables correctly (backport of #338, #339) - #351
Merged
Merged
Conversation
…nsionless heuristic (#339) * fix(content): classify executables by magic bytes instead of the extensionless heuristic * fix(content): recognise 64-bit fat Mach-O magics and rename name-only classifier overloads * fix(content): preserve execute permission for shebang scripts (cherry picked from commit 4fb9ea1)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
This was referenced Aug 3, 2026
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.
Summary
Backports #338 and #339 into
release/alpha-4.heuristic.
Both bear on whether a prepared workspace produces a launchable entry point,
which is the
A profile can be created and launchedbox still open for bothplatforms in #307.
Why these two
They are the same class of user-facing workspace fix as #346 and #350, both
already backported. The remaining
developmentcommits are deliberately leftout:
NativeClientManifestLaunchTests.cs, a file created byfeat(launching): launch the native BGFX client against retail archives #332, so it is a no-op on this branch.
Backport notes
#339 cherry-picked clean. #338 required one adaptation, worth recording because
a clean textual merge hides it:
On
development, #338 merged before #348, the forward-port of #346. Thisbranch already carries #346, so the cherry-pick applies #338 after it —
reversing the integration order. #348 had amended #338's own tests to coexist
with the workspace-root change, rewriting
Path.Combine(_tempPath, "workspace")toPath.Combine(_tempPath, workspaceId)so the cached workspace path matches the root-derived path and the recreate
branch does not fire.
Without that adaptation the cherry-pick still compiles and reports no conflict in
WorkspaceManager.cs, butPrepareWorkspaceAsync_WhenEntryPointBricked_RepairsAndReusesWorkspacefails.The same two-line adaptation #348 made is folded into the #338 commit here, so
both commits are individually green.
The one genuine conflict was in
WorkspaceManagerReuseTests.cs, where #346'sWhenStorageRootChangestest and #338's two new tests landed adjacently. Allthree are kept; #338's change to that file is purely additive.
Testing
dotnet test GenHub/GenHub.Tests/GenHub.Tests.Core/GenHub.Tests.Core.csproj -c Release#338: 1,478 passed, 0 failed#339(tip): 1,510 passed, 0 faileddotnet build GenHub/GenHub.Linux/GenHub.Linux.csproj -c Release— passedgit diff --check— passedRisks and rollback
release/alpha-4still lacks the blank-root guard fix(workspace): fall back from protected adjacent storage (forward-port of #346) #348 added on top of fix(workspace): fall back from protected adjacent storage #346 inWorkspaceManager. No test exercises it and nothing here depends on it, butthis branch carries a slightly older variant of that fix than
development.classification behavior.
Outstanding
Packaged Windows and Linux validation per #307 still applies; these commits
change workspace materialization, so the
profile can be created and launchedchecks should be re-run against a build containing them.
Backport of #338 and #339
Greptile Summary
This backport makes Unix executable materialization atomic, repairs stale executable modes when reusing workspaces, and centralizes executable classification around file extensions and magic bytes.
Confidence Score: 5/5
The PR appears safe to merge, with no concrete changed-code defect identified.
The executable swap preserves CAS isolation, workspace reuse repairs only contained entry points, unsafe or missing paths fall back to recreation, and changed classifier callers consistently select content-aware or metadata-only classification according to the available file state.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Prepare workspace] --> B{Reusable cached workspace?} B -- No --> C[Materialize files] C --> D[Copy executable to unique temporary path] D --> E[Apply Unix execute mode] E --> F[Atomically replace workspace file] B -- Yes --> G[Resolve contained entry point] G --> H{Exists and executable?} H -- Yes --> I[Reuse workspace] H -- Missing or unsafe --> C H -- Mode missing --> J[Create executable private copy] J --> IReviews (1): Last reviewed commit: "fix(content): classify executables by ma..." | Re-trigger Greptile
Context used (3)