feat(platform): make paths, manifests, and Unix workspaces platform-aware - #328
Merged
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds artifact-variant manifest contracts and fail-closed ingestion, centralizes executable classification, introduces platform-specific game paths and symlink capability detection, and adds Unix hard-link plus atomic executable materialization support with corresponding tests. ChangesManifest and platform workspace changes
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
bobtista
marked this pull request as ready for review
July 29, 2026 19:54
coderabbitai
Bot
added
Bug
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@GenHub/GenHub.Core/Models/Manifest/ContentManifest.cs`:
- Around line 75-84: Ensure ContentManifest.Variants remains non-null when
deserialization supplies a JSON null value by coalescing null in its setter or
rejecting null during deserialization. Preserve the existing empty-list default
and add a regression test covering null Variants through ContentManifestPool and
ManifestIngestionGate.TryAccept.
In `@GenHub/GenHub.Core/Models/Manifest/ManifestVariantResolver.cs`:
- Around line 119-130: The declared entry-point resolution currently returns the
raw declaration instead of the matched manifest path. In
ManifestVariantResolver.cs lines 119-130, retain the matching ManifestFile from
the PathsMatch lookup and pass its RelativePath to
EntryPointResolution.Resolved; in ManifestVariantResolverTests.cs lines 142-156,
assert that normalized separator or case matches resolve to the file’s
RelativePath.
- Around line 115-118: Update the entry-point selection in the variant
resolution flow to avoid falling back to manifest.EntryPoint when Variants is
populated. Use only variant?.EntryPoint for matching variants, while preserving
the manifest entry point behavior for flat manifests without variants.
- Around line 133-160: Update the executable inference logic in the manifest
resolution flow to consider only files where both IsExecutable and
ExecutableFileClassifier.IsLegacyLaunchCandidate(RelativePath) are true. Use
this filtered set for the single-candidate, no-candidate, and ambiguity
outcomes, while leaving explicit EntryPoint resolution unchanged so scripts can
still be selected intentionally.
In
`@GenHub/GenHub.Linux/Infrastructure/DependencyInjection/LinuxServicesModule.cs`:
- Around line 4-12: In LinuxServicesModule.cs, alphabetize the shown using
directives according to the project’s existing convention and remove the
duplicate blank line around the service-registration section near line 46.
Preserve all imports and code behavior while clearing the StyleCop warnings.
In
`@GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/ExecutablePermissionIsolationTests.cs`:
- Around line 110-118: Replace the hand-rolled copy–chmod–move logic in the
affected tests with calls to the production
WorkspaceStrategyBase.EnsureExecutableAsync implementation through a test
strategy. Assert CAS isolation, executable permissions, destination replacement,
and cleanup of any temporary file, covering both referenced test sections
without duplicating the materialization algorithm.
In
`@GenHub/GenHub.Windows/Features/Workspace/WindowsSymlinkCapabilityProvider.cs`:
- Around line 27-34: Replace the Administrator membership check in
CanCreateSymlinks with a cached probe that attempts to create and remove a
temporary symbolic link, returning whether the operation succeeds. Ensure the
probe uses unique temporary paths, cleans up any created artifacts, and returns
false on failure while preserving the cached capability result for strategy
selection.
In `@GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs`:
- Around line 453-464: Ensure the symlink fallback represented by
effectiveStrategy is applied to the actual workspace configuration, not only
logged or used locally. Update the shared workspace-configuration construction
used by gameLauncher and PrepareWorkspaceAsync, or normalize the in-memory
profile before both paths, while preserving explicitly configured strategies and
inherited symlink defaults consistently.
In `@GenHub/GenHub/Features/Manifest/ManifestProvider.cs`:
- Around line 77-80: Centralize the ManifestIngestionGate validation currently
used in ManifestProvider and invoke it on every return path, including cached
manifests and both embedded-manifest flows. Ensure the installation overload
does not return an embedded variant when AddManifestAsync rejects it; rejected
manifests must throw ManifestValidationException with the game client ID and
rejection reason before reaching callers.
In `@GenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.cs`:
- Around line 691-714: Update the exception handling around executable
materialization in the relevant workspace preparation method to rethrow the
original exception after attempting temporary-file cleanup, including
cancellation exceptions. Remove the warning-only success path so failures from
copy, chmod, or move prevent the workspace from being recorded as usable; retain
best-effort cleanup and its debug logging.
In `@GenHub/GenHub/Features/Workspace/UnixFileOperationsService.cs`:
- Around line 66-76: Update CopyFromCasAsync to call CopyFileAsync with
destinationPath and casPath instead of CreateHardLinkAsync, ensuring the method
creates an independent copy of the resolved CAS content while preserving its
existing success and missing-path behavior.
In `@GenHub/GenHub/Features/Workspace/WorkspaceValidator.cs`:
- Around line 350-383: Update HasUnixExecutePermission to validate execute
access for the current process’s effective Unix identity rather than checking
any permission bit directly. Use an effective-access mechanism such as a
platform-specific faccessat wrapper with X_OK and AT_EACCESS, while preserving
the Windows behavior and returning false when the access check fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4c82cc30-68c1-49fb-8439-bf677a3a07f1
📒 Files selected for processing (47)
GenHub/GenHub.Core/Constants/ManifestConstants.csGenHub/GenHub.Core/Interfaces/Workspace/ISymlinkCapabilityProvider.csGenHub/GenHub.Core/Models/Manifest/ArtifactVariant.csGenHub/GenHub.Core/Models/Manifest/ContentManifest.csGenHub/GenHub.Core/Models/Manifest/EntryPointResolution.csGenHub/GenHub.Core/Models/Manifest/ManifestIngestionGate.csGenHub/GenHub.Core/Models/Manifest/ManifestVariantResolver.csGenHub/GenHub.Core/Utilities/ExecutableFileClassifier.csGenHub/GenHub.Linux/Infrastructure/DependencyInjection/LinuxServicesModule.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/GitHubInferenceHelperTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameSettings/GamePathProviderTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestDiscoveryServiceTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/ExecutablePermissionIsolationTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/FileOperationsServiceTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/GameProfileWorkspaceIntegrationTest.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/UnixFileOperationsServiceTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/ApplicationDataPathConventionTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/GameProfileModuleTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/SharedViewModelModuleTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestIngestionGateTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestVariantResolverTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Utilities/ExecutableFileClassifierTests.csGenHub/GenHub.Windows/Features/Workspace/WindowsSymlinkCapabilityProvider.csGenHub/GenHub.Windows/Infrastructure/DependencyInjection/WindowsServicesModule.csGenHub/GenHub/Features/Content/Services/CommunityOutpost/CommunityOutpostDeliverer.csGenHub/GenHub/Features/Content/Services/ContentDiscoverers/GitHubTopicsDiscoverer.csGenHub/GenHub/Features/Content/Services/Helpers/GitHubInferenceHelper.csGenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.csGenHub/GenHub/Features/GameProfiles/ViewModels/FileTreeItem.csGenHub/GenHub/Features/GameSettings/GamePathProviderBase.csGenHub/GenHub/Features/GameSettings/GameSettingsService.csGenHub/GenHub/Features/GameSettings/LinuxGamePathProvider.csGenHub/GenHub/Features/GameSettings/MacOSGamePathProvider.csGenHub/GenHub/Features/GameSettings/WindowsGamePathProvider.csGenHub/GenHub/Features/Manifest/ContentManifestBuilder.csGenHub/GenHub/Features/Manifest/ContentManifestPool.csGenHub/GenHub/Features/Manifest/ManifestDiscoveryService.csGenHub/GenHub/Features/Manifest/ManifestGenerationService.csGenHub/GenHub/Features/Manifest/ManifestProvider.csGenHub/GenHub/Features/Manifest/SteamManifestPatcher.csGenHub/GenHub/Features/Workspace/FileOperationsService.csGenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.csGenHub/GenHub/Features/Workspace/UnixFileOperationsService.csGenHub/GenHub/Features/Workspace/UnixNativeMethods.csGenHub/GenHub/Features/Workspace/UnixSymlinkCapabilityProvider.csGenHub/GenHub/Features/Workspace/WorkspaceValidator.csGenHub/GenHub/Infrastructure/DependencyInjection/ContentPipelineModule.cs
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Incremental Review: This review covers changes from commit Changes Analyzed:
Previous Issues: All 17 issues from the prior review remain unchanged (no files with those issues were modified in this incremental change). Previous Review Summaries (3 snapshots, latest commit 5b1a160)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 5b1a160)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit eafdf81)Status: No Issues Found | Recommendation: Merge Files Reviewed (14 changed files)
Review NotesAll incremental changes from commit 64502bb address the previously identified issues correctly:
No new issues found. All changes are correct and improve the implementation. Previous review (commit 64502bb)Status: 0 New Issues | Recommendation: Address existing issues before merge Overview
Existing Issues (3)This PR has 3 existing inline comments that should be addressed before merge:
Files Reviewed (47 files)
Review NotesThis PR makes substantial improvements to GenHub's platform awareness across Windows and Unix. The changes include:
The architecture is sound and the cross-platform considerations are generally well-handled. No new issues were found beyond the 3 existing comments. Reviewed by glm-4.7 · Input: 43.3K · Output: 4.3K · Cached: 365.7K |
…then-move Copy, set the mode on the copy, then rename over the destination. The previous copy/delete/move/chmod sequence exposed two states: destination missing, and destination present but not executable. The second is unrecoverable because verification never mutates, so the workspace stays broken for every later launch. Closes #316.
… are migrated Deliverers, validators, CAS reference counting and GC all still read ContentManifest.Files, which is empty for a variant manifest. Accepting one would deliver nothing while reporting success and record the wrong blobs in reference counting. Adds a fail-closed gate applied in ManifestDiscoveryService and ManifestProvider, and a manifest format version identifying variant manifests so the rejection is attributable rather than presenting as a parse failure. Temporary: remove as part of the resolved-variant migration (#321). Closes #314.
development builds warning-clean; these were introduced here. Covers using-directive ordering, comment spacing, and a platform guard the analyzer can follow for File.SetUnixFileMode inside the copy lambda.
bobtista
force-pushed
the
feat/cross-platform-fixes
branch
from
July 29, 2026 20:56
64502bb to
eafdf81
Compare
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
coderabbitai
Bot
removed
Bug
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GenHub/GenHub/Features/Manifest/ManifestDiscoveryService.cs (1)
196-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated ingestion-gate check into a shared helper.
The same deserialize →
ManifestIngestionGate.TryAccept→ log-and-skip pattern is duplicated acrossLoadManifestAsync,DiscoverFileSystemManifestsAsync, andDiscoverEmbeddedManifestsAsync, differing only in the log message's context label and the post-acceptance action. Extracting a small helper (e.g.,TryAcceptManifest(manifest, contextLabel, out rejectionReason)that performs theTryAcceptcall and logging) would remove the triplication while keeping the gate applied at all three ingestion points.♻️ Proposed helper (illustrative)
+ private bool TryAcceptManifest(ContentManifest manifest, string context) + { + if (ManifestIngestionGate.TryAccept(manifest, out var rejectionReason)) + { + return true; + } + + logger.LogWarning("Skipping manifest at {Context}: {Reason}", context, rejectionReason); + return false; + }Also applies to: 291-299, 329-337
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@GenHub/GenHub/Features/Manifest/ManifestDiscoveryService.cs` around lines 196 - 215, Extract the repeated ManifestIngestionGate.TryAccept and rejection-warning logic from LoadManifestAsync, DiscoverFileSystemManifestsAsync, and DiscoverEmbeddedManifestsAsync into a shared helper such as TryAcceptManifest. Have the helper accept the manifest and context label, perform the gate check, log rejection with the existing logger, and return the acceptance result; update all three ingestion paths to call it while preserving their distinct post-acceptance actions and context-specific messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceValidatorTests.cs`:
- Around line 254-288: Update
ValidateWorkspaceAsync_OtherOnlyExecuteBit_ReturnsAccessWarning to skip when
UnixNativeMethods.GetEffectiveUserId() == 0, in addition to the existing Windows
guard. Keep the test’s current file-mode setup and assertions unchanged for
non-root Unix identities.
In `@GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs`:
- Around line 169-178: Update the fallback wording in
ProfileLauncherFacade.ResolveSupportedWorkspaceStrategy logging at
GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs:169-178
and the downstream log/notification at
GenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.cs:452-463 to
state that symlinks are unavailable in the current environment. Remove
references implying missing elevation or administrator rights while preserving
the existing fallback behavior.
---
Outside diff comments:
In `@GenHub/GenHub/Features/Manifest/ManifestDiscoveryService.cs`:
- Around line 196-215: Extract the repeated ManifestIngestionGate.TryAccept and
rejection-warning logic from LoadManifestAsync,
DiscoverFileSystemManifestsAsync, and DiscoverEmbeddedManifestsAsync into a
shared helper such as TryAcceptManifest. Have the helper accept the manifest and
context label, perform the gate check, log rejection with the existing logger,
and return the acceptance result; update all three ingestion paths to call it
while preserving their distinct post-acceptance actions and context-specific
messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c05fae93-aabb-497e-a43c-9043752317a5
📒 Files selected for processing (41)
GenHub/GenHub.Core/Constants/ManifestConstants.csGenHub/GenHub.Core/Interfaces/Workspace/ISymlinkCapabilityProvider.csGenHub/GenHub.Core/Models/Manifest/ArtifactVariant.csGenHub/GenHub.Core/Models/Manifest/ContentManifest.csGenHub/GenHub.Core/Models/Manifest/EntryPointResolution.csGenHub/GenHub.Core/Models/Manifest/ManifestIngestionGate.csGenHub/GenHub.Core/Models/Manifest/ManifestVariantResolver.csGenHub/GenHub.Linux/Infrastructure/DependencyInjection/LinuxServicesModule.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/GitHubInferenceHelperTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ContentManifestPoolTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestProviderTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/ExecutablePermissionIsolationTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/FileOperationsServiceTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/GameProfileWorkspaceIntegrationTest.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/UnixFileOperationsServiceTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceStrategyBaseTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Features/Workspace/WorkspaceValidatorTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/ApplicationDataPathConventionTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/GameProfileModuleTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Infrastructure/DependencyInjection/SharedViewModelModuleTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestIngestionGateTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Models/Manifest/ManifestVariantResolverTests.csGenHub/GenHub.Tests/GenHub.Tests.Core/Utilities/ExecutableFileClassifierTests.csGenHub/GenHub.Windows/Features/Workspace/WindowsSymlinkCapabilityProvider.csGenHub/GenHub.Windows/Infrastructure/DependencyInjection/WindowsServicesModule.csGenHub/GenHub/Features/Content/Services/CommunityOutpost/CommunityOutpostDeliverer.csGenHub/GenHub/Features/Content/Services/ContentDiscoverers/GitHubTopicsDiscoverer.csGenHub/GenHub/Features/GameProfiles/Services/ProfileLauncherFacade.csGenHub/GenHub/Features/GameSettings/GameSettingsService.csGenHub/GenHub/Features/Manifest/ContentManifestBuilder.csGenHub/GenHub/Features/Manifest/ContentManifestPool.csGenHub/GenHub/Features/Manifest/ManifestDiscoveryService.csGenHub/GenHub/Features/Manifest/ManifestGenerationService.csGenHub/GenHub/Features/Manifest/ManifestProvider.csGenHub/GenHub/Features/Workspace/FileOperationsService.csGenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.csGenHub/GenHub/Features/Workspace/UnixFileOperationsService.csGenHub/GenHub/Features/Workspace/UnixNativeMethods.csGenHub/GenHub/Features/Workspace/UnixSymlinkCapabilityProvider.csGenHub/GenHub/Features/Workspace/WorkspaceValidator.csGenHub/GenHub/Infrastructure/DependencyInjection/ContentPipelineModule.cs
…only test case The deserialize, gate, log-and-skip sequence was repeated at all three ManifestDiscoveryService ingestion points, differing only in the context label. Collapsed into IsManifestAccepted. The other-only execute-bit test now skips for uid 0. Root bypasses the permission bits, so faccessat reports execute access and the behaviour under test does not exist. geteuid is used rather than the user name, which is wrong under 'sudo -E' and for any uid-0 account named otherwise. GitHub's runners are non-root so this passed in CI, but it fails in a root container.
bobtista
force-pushed
the
feat/cross-platform-fixes
branch
from
July 30, 2026 00:57
eafdf81 to
5b1a160
Compare
…than admin rights The downgrade is driven by a real symlink-creation probe, so the messages no longer claim elevation is required. symlink(2) needs no privilege on Unix, and Windows grants it under Developer Mode, so telling the user to elevate sent them after a fix that would not help. The user-facing notification carried the same claim and is corrected too. Genuine Windows UAC handling for Error 740 is unchanged. Also covers the ingestion gate at the pool, which is the chokepoint every deliverer, resolver and detector reaches. The tests assert that a variant manifest is rejected before any content is stored and before any CAS reference is tracked; mis-tracked references are what corrupts reference counting and garbage collection, so returning a failure alone would not be enough. Both fail if the gate is removed.
Comment on lines
502
to
+504
| } | ||
|
|
||
| await EnsureExecutableAsync(file, targetPath, cancellationToken); |
There was a problem hiding this comment.
When a Unix profile uses FullCopy, HybridCopySymlink, or HardLink, those strategies materialize files through their own loops and bypass ProcessManifestFileAsync, so EnsureExecutableAsync never runs. Executables retain a non-executable mode in copy-based workspaces, while hard-linked executables are not detached from CAS as intended, causing launch failures or shared-inode permission changes.
Knowledge Base Used: Workspace Assembly
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.cs
Line: 502-504
Comment:
**Executable isolation skips strategies**
When a Unix profile uses `FullCopy`, `HybridCopySymlink`, or `HardLink`, those strategies materialize files through their own loops and bypass `ProcessManifestFileAsync`, so `EnsureExecutableAsync` never runs. Executables retain a non-executable mode in copy-based workspaces, while hard-linked executables are not detached from CAS as intended, causing launch failures or shared-inode permission changes.
**Knowledge Base Used:** [Workspace Assembly](https://app.greptile.com/genhub/-/custom-context/knowledge-base/community-outpost/genhub/-/docs/workspace-assembly.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| await CreateHardLinkAsync(destinationPath, casPath, cancellationToken).ConfigureAwait(false); | ||
| return true; | ||
| } | ||
| catch (IOException ex) when (ex is not FileNotFoundException) |
There was a problem hiding this comment.
When link(2) returns EACCES while HardLinkStrategy materializes a CAS file, CreateHardLinkAsync throws UnauthorizedAccessException, which is not caught by this IOException handler. The per-file copy fallback is skipped and workspace preparation aborts instead of producing an independent copy.
Knowledge Base Used: Workspace Assembly
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/Workspace/UnixFileOperationsService.cs
Line: 109
Comment:
**EACCES bypasses copy fallback**
When `link(2)` returns `EACCES` while `HardLinkStrategy` materializes a CAS file, `CreateHardLinkAsync` throws `UnauthorizedAccessException`, which is not caught by this `IOException` handler. The per-file copy fallback is skipped and workspace preparation aborts instead of producing an independent copy.
**Knowledge Base Used:** [Workspace Assembly](https://app.greptile.com/genhub/-/custom-context/knowledge-base/community-outpost/genhub/-/docs/workspace-assembly.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+107
to
+108
| return true; | ||
| } |
There was a problem hiding this comment.
When a launched game or plugin writes to a non-executable content-addressable file in a hard-link workspace, the write changes the same user-writable inode stored under the CAS hash. Another profile can then materialize the modified bytes and receive corrupted or incorrect content. How this was verified: HardLinkStrategy requests real CAS hard links, while copy-on-write isolation applies only to files marked executable and ordinary materialization does not universally revalidate the blob hash.
Knowledge Base Used: Workspace Assembly
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/Workspace/UnixFileOperationsService.cs
Line: 107-108
Comment:
**Workspace writes corrupt shared CAS**
When a launched game or plugin writes to a non-executable content-addressable file in a hard-link workspace, the write changes the same user-writable inode stored under the CAS hash. Another profile can then materialize the modified bytes and receive corrupted or incorrect content. **How this was verified:** `HardLinkStrategy` requests real CAS hard links, while copy-on-write isolation applies only to files marked executable and ordinary materialization does not universally revalidate the blob hash.
**Knowledge Base Used:** [Workspace Assembly](https://app.greptile.com/genhub/-/custom-context/knowledge-base/community-outpost/genhub/-/docs/workspace-assembly.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
bobtista
added a commit
that referenced
this pull request
Jul 30, 2026
#328 made IGamePathProvider a required dependency and #329 added the macOS host, but neither could carry the macOS registrations: they reference types from the first and a module from the second, so they only compile once both are in. Each PR was green in isolation and the merged state is not — GenHub.app now aborts at container build with 'Unable to resolve service for type IGamePathProvider while attempting to activate GameSettingsService', cascading across roughly twenty descriptors including MainViewModel and IGameLauncher. Registers IGamePathProvider, ISymlinkCapabilityProvider and IFileOperationsService for macOS. Only the first breaks startup; without the other two macOS silently copies instead of hard-linking and cannot reach symlink strategies, which are the behaviours #328 set out to fix. Also moves the macOS test run before publish. GenHub.Tests.MacOS was already covered by the job's final 'Run Tests' step, but that sits after Publish and Smoke Test App Launch, so the startup crash killed the job first and the assertion naming the service never ran. MacOSHost_ResolvesEveryRequiredService already asserted this; it was reached too late to be useful. The project is skipped in the later sweep so it runs once.
bobtista
added a commit
that referenced
this pull request
Jul 30, 2026
…331) #328 made IGamePathProvider a required dependency and #329 added the macOS host, but neither could carry the macOS registrations: they reference types from the first and a module from the second, so they only compile once both are in. Each PR was green in isolation and the merged state is not — GenHub.app now aborts at container build with 'Unable to resolve service for type IGamePathProvider while attempting to activate GameSettingsService', cascading across roughly twenty descriptors including MainViewModel and IGameLauncher. Registers IGamePathProvider, ISymlinkCapabilityProvider and IFileOperationsService for macOS. Only the first breaks startup; without the other two macOS silently copies instead of hard-linking and cannot reach symlink strategies, which are the behaviours #328 set out to fix. Also moves the macOS test run before publish. GenHub.Tests.MacOS was already covered by the job's final 'Run Tests' step, but that sits after Publish and Smoke Test App Launch, so the startup crash killed the job first and the assertion naming the service never ran. MacOSHost_ResolvesEveryRequiredService already asserted this; it was reached too late to be useful. The project is skipped in the later sweep so it runs once.
bobtista
added a commit
that referenced
this pull request
Jul 30, 2026
…port zero-exit stderr Validation checked both games' declared roots regardless of which was launching, so a stale Zero Hour path could block a Generals launch that would have worked. It now checks only the launching game's root, taking the game type from the client. A process exiting immediately with code 0 and no spawned child returned a bare message while the stderr drained moments earlier went unreported — the one diagnostic available for that case, and contrary to the point of the capture work in this branch. Test changes: the native-client fixture discovery was duplicated across three classes and is now a shared NativeClientFixture, so they cannot diverge on when to skip; engine-only staging includes .so alongside .dylib, having been macOS-only; and SetUnixFileMode calls are guarded so the platform analyzer can follow them. Also clears every warning in GenHub.Tests.Core, including five that reached development through #328 because I had only ever built the main project.
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
1 participant
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
Make shared GenHub runtime behavior platform-aware across Windows and Unix. This fixes platform-specific settings and data paths, corrects Unix workspace materialization, and introduces deterministic manifest variants needed by native clients.
This PR contains no GenHub.MacOS project changes and can be reviewed independently of the macOS host.
Changes
Testing
dotnet test GenHub/GenHub.Tests/GenHub.Tests.Core/GenHub.Tests.Core.csproj -c Release --no-restoreRisks and rollback
Unix workspace behavior changes: hard-link strategies now create real hard links instead of silently copying. Executable files are detached before permission changes so shared CAS blobs are not mutated.
Existing flat manifests remain supported. No persisted-data migration is required, and the changes are revert-safe.
Related issues
Fixes #314
Fixes #316
Part of #327
Greptile Summary
The PR centralizes platform-aware manifest and workspace behavior.
Confidence Score: 2/5
This PR is not yet safe to merge because most Unix strategies skip executable permission isolation, hard-link permission failures can bypass copy fallback, and writable workspaces can mutate shared CAS content.
The execute-permission fix is only reached by SymlinkOnlyStrategy, EACCES escapes the new hard-link fallback, and intentional hard links leave non-executable CAS objects writable through workspace paths, allowing cross-profile content corruption.
Files Needing Attention: GenHub/GenHub/Features/Workspace/Strategies/WorkspaceStrategyBase.cs, GenHub/GenHub/Features/Workspace/UnixFileOperationsService.cs, and the FullCopy/HybridCopySymlink/HardLink strategy implementations
Security Review
HardLinkStrategy exposes writable CAS inodes to launched workspace processes, allowing workspace writes to alter shared content subsequently consumed by other profiles.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Manifest[Resolved manifest] --> Strategy[Workspace strategy] Strategy -->|FullCopy| Copy[Independent files] Strategy -->|Symlink| Symlink[CAS or installation symlinks] Strategy -->|HardLink| HardLink[Shared CAS inodes] Copy --> Launch[Launch workspace executable] Symlink --> Launch HardLink --> Launch Launch --> Mutation[Game or plugin writes workspace file] Mutation -->|same inode| CAS[Shared CAS blob changes] CAS --> Other[Other profiles materialize modified bytes]Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "fix(launching): attribute the symlink do..." | Re-trigger Greptile
Context used: