Skip to content

feat(manifest): modernize ManifestGenerationService with authoritative CSV catalog resolution - #430

Open
undead2146 wants to merge 3 commits into
developmentfrom
feat/csv-manifest-authority
Open

feat(manifest): modernize ManifestGenerationService with authoritative CSV catalog resolution#430
undead2146 wants to merge 3 commits into
developmentfrom
feat/csv-manifest-authority

Conversation

@undead2146

Copy link
Copy Markdown
Member

Summary

Modernizes ManifestGenerationService to use the authoritative CSV game installation catalog (Generals-1.08.csv and ZeroHour-1.04.csv) rather than ad-hoc disk directory file discovery. Embeds the CSV registries and index metadata directly into GenHub.Core for offline fallback, enforces pristine vanilla manifests by excluding untracked non-vanilla files, and implements multi-language filtering.

Motivation

As outlined in the CSV suite plan (Phase 1), base game installation manifests must be generated strictly from authoritative file registry records with exact SHA256 hashes, sizes, and requirement flags to ensure deterministic CAS workspace materialization and eliminate corrupt/modded file contamination in base game manifests.

Changes

  • Core Assets: Embedded Generals-1.08.csv, ZeroHour-1.04.csv, and index.json directly into GenHub.Core as assembly resources for zero-latency, offline authoritative catalog loading.
  • Content Manifest Builder: Updated IContentManifestBuilder and ContentManifestBuilder (AddGameInstallationFileAsync) to accept optional precomputed hash, size, and isRequired flags, allowing fast, authoritative record population.
  • Manifest Generation: Refactored ManifestGenerationService.AddGameFilesToManifest to:
    • Query CsvResolver or embedded/local CSV fallbacks for authoritative records matching game type, version, and language.
    • Perform cross-platform case-insensitive path resolution (FindFileCaseInsensitive).
    • Calculate SHA256 hashes and classify executable permissions accurately.
    • Support backup file (.ghbak / .bak) resolution when original binaries are replaced.
    • Exclude extra non-vanilla files from base installation manifests to keep base manifests completely clean.
  • Multi-Language: Added language parameter overloads and normalized language matching (ContentSearchQuery.NormalizeLanguage) across manifests.
  • Tests: Added comprehensive unit tests in ManifestGenerationServiceTests covering CSV authority, multi-language filtering, backup file handling, and non-vanilla file exclusion.

Verification

  • Targeted test suite passing (all 2,419 tests in GenHub.Tests.Core passed)
  • Clean build without new warnings or errors
  • Tested multi-language filtering and non-vanilla file exclusion

Created with Claude 3.7 Sonnet via Antigravity

…e CSV catalog resolution

- Embed GameInstallationFilesRegistry CSV and index catalog as assembly resources in GenHub.Core
- Implement authoritative CSV catalog resolution in ManifestGenerationService supporting Generals 1.08 and Zero Hour 1.04
- Enhance IContentManifestBuilder and ContentManifestBuilder to accept precomputed hashes, sizes, and requirement flags
- Add multi-language detection, normalization, and filtering to game installation manifest generation
- Implement cross-platform case-insensitive path resolution and pristine vanilla manifest enforcement (excluding untracked non-vanilla mod files)
- Add comprehensive unit tests covering CSV authority, multi-language filtering, backup file handling, and non-vanilla file exclusion
@qodo-code-review

Copy link
Copy Markdown

ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

  • 🔍 Trigger review

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

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

@deepsource-io

deepsource-io Bot commented Aug 30, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 19678f2...be24d50 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
C# Aug 30, 2026 7:30p.m. Review ↗
JavaScript Aug 30, 2026 7:30p.m. Review ↗
Shell Aug 30, 2026 7:30p.m. Review ↗
Secrets Aug 30, 2026 7:30p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

Comment on lines +668 to +670
var version = !string.IsNullOrWhiteSpace(manifestVersion) && manifestVersion != "0"
? manifestVersion
: (gameType == GameType.Generals ? ManifestConstants.GeneralsManifestVersion : ManifestConstants.ZeroHourManifestVersion);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ternary expression used is too complex


The ternary operator ?: evaluates a boolean expression and returns the result of one of the two expressions, depending on whether the expression evaluates to true or false. While the ternary operator may be particularly useful in avoiding simple if statements, it can, however, affect the readability when nested. Therefore, it is recommended that you avoid nesting such operators.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Generate pristine manifests from authoritative CSV catalogs

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Builds installation manifests only from authoritative, versioned CSV catalog entries.
• Filters localized assets and resolves case differences, backups, hashes, sizes, and permissions.
• Embeds offline catalogs and tests pristine Generals and Zero Hour manifest generation.
Diagram

graph TD
  A["Manifest Request"] --> B["Generation Service"] --> C["Language Detection"]
  B --> D["Catalog Resolver"]
  D -. fallback .-> E["Embedded Registries"]
  C --> F["File Matching"]
  D --> F
  E --> F
  F --> G["Manifest Builder"] --> H["Pristine Manifest"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Remote resolver only
  • ➕ Avoids shipping duplicate catalog data
  • ➕ Allows catalog corrections without rebuilding the application
  • ➖ Prevents deterministic offline generation
  • ➖ Adds network availability and latency to a local workflow
2. Build-time generated registry
  • ➕ Eliminates runtime CSV parsing
  • ➕ Can validate catalog schema and checksums during compilation
  • ➖ Requires a generation pipeline and generated model maintenance
  • ➖ Makes catalog inspection and replacement less direct

Recommendation: Keep the layered resolver-first approach with embedded CSV fallback: it preserves remotely managed authority when available while guaranteeing offline operation. A build-time generated registry is worth revisiting only if runtime parsing or catalog validation becomes measurable operational risk.

Files changed (9) +933 / -193

Enhancement (4) +388 / -191
IContentManifestBuilder.csAccept authoritative file metadata in manifest builder API +11/-1

Accept authoritative file metadata in manifest builder API

• Extends game installation file addition with optional precomputed hash, size, and required-state values.

GenHub/GenHub.Core/Interfaces/Manifest/IContentManifestBuilder.cs

IManifestGenerationService.csExpose language-aware installation manifest generation +14/-2

Expose language-aware installation manifest generation

• Adds an optional language argument to both string and integer version overloads, preserving automatic detection when omitted.

GenHub/GenHub.Core/Interfaces/Manifest/IManifestGenerationService.cs

ContentManifestBuilder.csPropagate supplied file metadata into manifests +41/-10

Propagate supplied file metadata into manifests

• Stores caller-provided hash, size, and required flags while computing missing metadata from local files. Game installation files now receive hashes consistently rather than only for executables.

GenHub/GenHub/Features/Manifest/ContentManifestBuilder.cs

ManifestGenerationService.csReplace directory discovery with authoritative catalog resolution +322/-178

Replace directory discovery with authoritative catalog resolution

• Generates manifests from resolver or embedded/local CSV entries filtered by game, version, and normalized language. It resolves paths case-insensitively, prefers .ghbak or .bak sources, classifies executable permissions, hashes matched files, and excludes untracked installation content.

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs

Tests (1) +162 / -1
ManifestGenerationServiceTests.csTest authoritative and pristine manifest behavior +162/-1

Test authoritative and pristine manifest behavior

• Covers Generals and Zero Hour catalog resolution, metadata population, language filtering, non-vanilla exclusion, and legacy backup selection.

GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Manifest/ManifestGenerationServiceTests.cs

Other (4) +383 / -1
Generals-1.08.csvAdd authoritative Generals 1.08 file registry +165/-0

Add authoritative Generals 1.08 file registry

• Adds the versioned Generals installation catalog with canonical paths, sizes, checksums, language scopes, and required flags used to constrain manifest contents.

GenHub/GenHub.Core/Assets/Registries/Generals-1.08.csv

ZeroHour-1.04.csvAdd authoritative Zero Hour 1.04 file registry +176/-0

Add authoritative Zero Hour 1.04 file registry

• Adds the versioned Zero Hour installation catalog with canonical file metadata and language classifications for deterministic manifest selection.

GenHub/GenHub.Core/Assets/Registries/ZeroHour-1.04.csv

index.jsonIndex embedded game installation registries +39/-0

Index embedded game installation registries

• Defines registry versions, source URLs, supported languages, file counts, checksums, and activation metadata for the packaged catalogs.

GenHub/GenHub.Core/Assets/Registries/index.json

GenHub.Core.csprojEmbed CSV registries and index metadata +3/-1

Embed CSV registries and index metadata

• Packages registry CSV and JSON assets into GenHub.Core so authoritative catalog fallback works without repository or network access.

GenHub/GenHub.Core/GenHub.Core.csproj

@qodo-code-review

qodo-code-review Bot commented Aug 30, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Required files are omitted 🐞 Bug ≡ Correctness
Description
When an authoritative entry marked required is absent, generation only emits a debug message and
skips the entry. The caller then builds and registers a manifest with no record of that required
file, preventing downstream validation from reporting the missing installation dependency.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R705-712]

+                    if (entry.IsRequired)
+                    {
+                        logger.LogDebug(
+                            "Required vanilla file missing from installation: {RelativePath}",
+                            entry.RelativePath);
+                    }
+
+                    continue;
Relevance

●●● Strong

Required catalog entries are silently omitted, allowing incomplete manifests to be registered as
successful.

PR-#423

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The generation loop continues after logging a missing required entry, and only found entries are
added. GameInstallationService immediately builds and stores the resulting manifest without a
required-entry completeness check; validators can only inspect files present in that manifest.

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[702-727]
GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[632-646]
GenHub/GenHub/Features/Content/Services/ContentValidator.cs[141-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Missing required catalog entries are silently omitted from the generated manifest.

## Issue Context
Treat a missing required source as manifest-generation failure, or preserve the authoritative required entry in a form downstream validation can check. Do not return a successful incomplete builder.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[702-712]
- GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[632-646]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Modified files become authoritative ✓ Resolved 🐞 Bug ≡ Correctness
Description
Manifest generation ignores each catalog entry's authoritative Sha256 and Size, instead hashing
and sizing the selected local or backup file and recording those values in the manifest. A corrupted
or modded tracked file can therefore become the trusted baseline and pass later content validation,
defeating pristine authoritative catalog validation.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R715-727]

+                var sourcePath = ResolveSourcePathWithBackup(resolvedFilePath, entry.RelativePath);
+                var isExecutable = ExecutableFileClassifier.RequiresExecutePermission(entry.RelativePath, sourcePath);
+                var fileInfo = new FileInfo(sourcePath);
+                var localHash = await hashProvider.ComputeFileHashAsync(sourcePath);
+
+                await builder.AddGameInstallationFileAsync(
+                    entry.RelativePath,
+                    sourcePath,
+                    isExecutable,
+                    permissions: null,
+                    hash: localHash,
+                    size: fileInfo.Length,
+                    isRequired: entry.IsRequired);
Relevance

●●● Strong

Manifest authority requires catalog metadata; hashing local bytes defeats pristine validation and
contradicts CSV authority.

PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The catalog model exposes authoritative Size and Sha256 fields, but the generation loop computes
localHash and uses FileInfo.Length from the selected local or backup file, then passes those
values to the builder. The builder's precomputed-metadata path persists them without comparison to
the catalog, and ContentValidator later checks installation content against that locally derived
manifest hash, allowing the same modified bytes to validate successfully.

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[715-727]
GenHub/GenHub.Core/Models/Content/CsvCatalogEntry.cs[16-32]
GenHub/GenHub/Features/Manifest/ContentManifestBuilder.cs[912-933]
GenHub/GenHub/Features/Content/Services/ContentValidator.cs[152-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description

`AddGameFilesToManifest` currently replaces the authoritative CSV catalog SHA-256 and size with metadata calculated from the selected local installation or backup file. This allows modified tracked files to define the expected baseline in newly generated base-installation manifests and pass later validation.

## Issue Context

`CsvCatalogEntry` already provides authoritative `Sha256` and `Size` values, and the builder accepts precomputed metadata for this flow. Compare the selected source, including backups, against the catalog metadata; local bytes may be hashed to determine whether a file should be omitted or reported as modified, but only entries satisfying the authoritative metadata should be emitted, and the catalog values—not locally calculated values—should be stored in the manifest.

## Fix Focus Areas

- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[715-727]
- GenHub/GenHub/Features/Manifest/ContentManifestBuilder.cs[912-933]
- GenHub/GenHub.Core/Models/Content/CsvCatalogEntry.cs[16-32]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Advertised languages lack records 🐞 Bug ≡ Correctness
Description
The embedded index advertises DE, FR, ES, IT, KO, PL, PT-BR, ZH-CN, and ZH-TW, but both new
registries contain only All and EN rows. Automatic detection or selection of an advertised
non-English language therefore retains only language-neutral files, omits required localized
archives and movies, and still returns a successful but incomplete manifest that cannot materialize
the detected language's base game.
Code

GenHub/GenHub.Core/Assets/Registries/index.json[13]

+      "languages": ["All", "EN", "DE", "FR", "ES", "IT", "KO", "PL", "PT-BR", "ZH-CN", "ZH-TW"],
Relevance

●●● Strong

Advertised non-English languages have no matching catalog records, causing demonstrably incomplete
manifests.

PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both index entries advertise eleven languages, while the added CSV rows use only All and EN.
MatchesLanguage retains an entry only when it is language-neutral (All) or exactly matches the
normalized requested language, so a DE, FR, or other advertised non-English request excludes every
EN-localized asset without finding a corresponding localized replacement; LanguageDetector can
nevertheless return those advertised codes based on language-specific files or directories.

GenHub/GenHub.Core/Assets/Registries/index.json[7-16]
GenHub/GenHub.Core/Assets/Registries/index.json[23-32]
GenHub/GenHub.Core/Assets/Registries/Generals-1.08.csv[1-6]
GenHub/GenHub.Core/Assets/Registries/ZeroHour-1.04.csv[1-6]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[551-565]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[523-565]
GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[31-45]
GenHub/GenHub.Core/Features/GameInstallations/LanguageDetector.cs[57-99]
GenHub/GenHub.Core/Assets/Registries/index.json[11-29]
GenHub/GenHub.Core/Assets/Registries/Generals-1.08.csv[1-165]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Language filtering is enabled for detected or selected non-English installations, but the embedded registries contain no records for most languages advertised by the index. Filtering consequently retains only `All` entries and excludes required language-specific files, producing an incomplete manifest.

## Issue Context
The index declares support for DE, FR, ES, IT, KO, PL, PT-BR, ZH-CN, and ZH-TW in addition to the registry's available language values, and `LanguageDetector` can detect such codes from game directories and archives. Add authoritative language-specific rows for every advertised language or restrict each index language list so unsupported languages are neither advertised nor accepted; manifest generation should also reject a requested language when the selected registry does not support it.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[523-565]
- GenHub/GenHub.Core/Assets/Registries/index.json[7-16]
- GenHub/GenHub.Core/Assets/Registries/index.json[23-32]
- GenHub/GenHub.Core/Assets/Registries/Generals-1.08.csv[1-165]
- GenHub/GenHub.Core/Assets/Registries/ZeroHour-1.04.csv[1-176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (3)
4. Resolver parses index as CSV ✓ Resolved 🐞 Bug ≡ Correctness
Description
The service sets SourceUrl to the JSON index and stores the actual CSV filename only in csvUrl
resolver metadata, while CsvResolver loads and parses SourceUrl directly without consuming that
metadata. The production resolver path therefore attempts to parse index.json as CSV, fails, and
silently bypasses the selected remote/current catalog in favor of the embedded fallback.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R1019-1026]

+                    SourceUrl = CsvConstants.DefaultIndexFileUrl,
+                    ResolverId = CsvConstants.ResolverId,
+                    ResolverMetadata =
+                    {
+                        [CsvConstants.GameTypeMetadataKey] = gameTypeStr,
+                        [CsvConstants.VersionMetadataKey] = version,
+                        [CsvConstants.LanguageMetadataKey] = language,
+                        [CsvConstants.CsvUrlMetadataKey] = csvFileName,
Relevance

●●● Strong

Resolver metadata names the CSV, but SourceUrl remains the JSON index, causing the production
resolver path to parse the wrong format.

PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed code assigns DefaultIndexFileUrl to SourceUrl and stores only the CSV filename under
CsvUrlMetadataKey. CsvResolver passes discoveredItem.SourceUrl to LoadCsvContentAsync and
parses the returned content as CsvCatalogEntry records, while its metadata readers never consume
csvUrl; the manifest service then catches the resolver failure and falls back.

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1012-1030]
GenHub/GenHub.Core/Constants/CsvConstants.cs[11-14]
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[51-70]
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[140-155]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1052-1059]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GetAuthoritativeEntriesAsync` in `ManifestGenerationService` gives `CsvResolver` the registry index URL instead of the selected CSV resource URL. Because the resolver downloads and parses `ContentSearchResult.SourceUrl` as CSV, it cannot resolve this request and falls back instead of using the selected remote catalog.

## Issue Context
The CSV filename is currently stored only under resolver metadata, but `CsvResolver` does not consume `CsvUrlMetadataKey` while loading content. Resolve the selected registry URL from the index and assign the actual absolute CSV URL to `SourceUrl`, or explicitly update `CsvResolver` to resolve and load the metadata URL before parsing.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1012-1030]
- GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[51-70]
- GenHub/GenHub.Core/Constants/CsvConstants.cs[41-44]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Catalog paths escape installation ✓ Resolved 🐞 Bug ⛨ Security
Description
Fallback CSV entries are accepted after only a non-empty path check, and FindFileCaseInsensitive
combines rooted or traversal paths directly with the installation path. A modified local fallback
catalog can therefore read, hash, and place an arbitrary file outside the game directory into the
generated manifest.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[702]

+                var resolvedFilePath = FindFileCaseInsensitive(installationPath, entry.RelativePath);
Relevance

●●● Strong

Exact recent precedent accepts rejecting rooted and traversal paths in CSV catalog entries.

PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The fallback filter validates only that RelativePath is non-empty plus game/language matching.
FindFileCaseInsensitive then feeds Path.Combine the record path and the generation loop hashes and
adds the resolved file. CsvResolver already has an IsUnsafeRelativePath guard, demonstrating that
equivalent validation is required on the newly added fallback path.

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[523-548]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[571-608]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[702-727]
GenHub/GenHub/Features/Content/Services/ContentResolvers/CsvResolver.cs[158-198]
PR-#425

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Fallback catalog relative paths are not validated or constrained to the game installation root before filesystem access.

## Issue Context
Reject rooted paths and traversal components, normalize the combined path, and verify it remains beneath the normalized installation root. Apply the same validation to every fallback record before adding it to the authoritative path set or manifest.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[571-608]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[693-702]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1081-1085]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Requested version is ignored ✓ Resolved 🐞 Bug ≡ Correctness
Description
Both resolver and embedded fallback loading select a hard-coded Generals 1.08 or Zero Hour 1.04
catalog solely from gameType, ignoring the detected or explicitly requested version. As a
result, requests for any other version produce a manifest labeled with the caller-supplied version
but populated from the wrong release registry.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R1070-1071]

+        var gameTypeStr = gameType == GameType.ZeroHour ? CsvConstants.ZeroHourGameType : CsvConstants.GeneralsGameType;
+        var csvFileName = gameType == GameType.ZeroHour ? "ZeroHour-1.04.csv" : "Generals-1.08.csv";
Relevance

●●● Strong

Hard-coding registry filenames by game type ignores the requested version and creates an incorrect
manifest baseline.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The public path forwards the supplied version to GetAuthoritativeEntriesAsync, and the search
result and resulting manifest identity retain that version, but both resolver and fallback methods
derive csvFileName only from gameType. GameInstallationService passes its base client's
detected version through this public API, making the mismatch reachable for non-default
installations.

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[663-679]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1004-1005]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1065-1071]
GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[1009-1026]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[999-1017]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1065-1084]
GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[594-610]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The requested manifest version is accepted and retained in the manifest identity, but it is not used to select or validate the authoritative catalog. Versions other than the hard-coded Generals 1.08 and Zero Hour 1.04 therefore receive files from the wrong registry while remaining labeled with the requested version.

## Issue Context
Game-installation callers pass the detected base-client version through to manifest generation. Resolve the catalog through index metadata using both game type and exact version; if no exact active registry or supported catalog exists, return no authoritative entries with a clear warning or otherwise fail generation instead of silently substituting another version.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[663-679]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[999-1005]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1065-1071]
- GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[1009-1026]
- GenHub/GenHub.Core/Assets/Registries/index.json[5-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

7. File counting swallows failures ✓ Resolved 📘 Rule violation ≡ Correctness
Description
CountExtraNonVanillaFiles catches every failure and returns 0, so recursive enumeration,
permission, and I/O errors are silently reported as no extra files. This hides an inaccurate
cleanliness result instead of handling specific expected exceptions.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R637-639]

+        catch
        {
-            logger.LogError(ex, "Error adding game files to manifest");
+            return 0;
Relevance

●●● Strong

Bare catch returning zero hides filesystem failures and produces an inaccurate cleanliness result.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001350 forbids suppressing generic exceptions in non-boundary helpers. This private helper
recursively enumerates files and its newly added bare catch returns zero without logging or
rethrowing.

Rule 3001350: Avoid catching generic Exception except at explicit process boundaries
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[614-640]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[732-738]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CountExtraNonVanillaFiles` uses a bare catch and converts every failure into a successful count of zero.

## Issue Context
Catch only expected filesystem exceptions and make the degraded result explicit through logging or failure propagation; do not suppress unrelated exceptions.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[614-640]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Manifest errors return partial results ✓ Resolved 📘 Rule violation ≡ Correctness
Description
AddGameFilesToManifest catches generic Exception, logs it, and returns normally—and also returns
normally when no catalog entries exist—so failures during language detection, catalog loading,
filesystem access, hashing, or insertion are reported as successful generation.
CreateGameInstallationManifestAsync can consequently return an empty or partially populated
manifest that callers may build and register in the manifest pool.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R740-743]

        catch (Exception ex)
        {
-            logger.LogWarning(ex, "Failed to recursively add files from {DirectoryPath}", directoryPath);
+            logger.LogError(ex, "Error adding authoritative game files to manifest for {GameType}", gameType);
        }
Relevance

●●● Strong

Generic catch-and-continue can persist partial state; recent history favors propagating failures and
preventing partial registration.

PR-#385
PR-#423

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001350 permits generic catches in non-boundary code only when the failure is rethrown, but
this private helper catches all exceptions, logs them, and returns without rethrowing; it also
returns when zero catalog entries are found. Its public caller then logs creation success and
returns the manifest builder, after which GameInstallationService builds the manifest and adds it
to the manifest pool, demonstrating how failed generation can be persisted as an empty or partial
success.

Rule 3001350: Avoid catching generic Exception except at explicit process boundaries
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[740-743]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[103-112]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[679-688]
GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[632-646]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description

`AddGameFilesToManifest` suppresses all exceptions and returns normally when no catalog entries are available, allowing authoritative manifest generation to report an empty or partially populated manifest as successful.

## Issue Context

The helper is not a process boundary and must not suppress unexpected failures. Handle only concrete, expected exceptions where recovery is valid; otherwise propagate failures to `CreateGameInstallationManifestAsync`, or represent predictable domain failures with the project's result type or an explicit failed result. Ensure callers never build or register an empty or partial manifest after generation has failed.

## Fix Focus Areas

- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[652-743]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[103-112]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[679-688]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[740-743]
- GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[632-646]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Registry paths remain hardcoded ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The fallback logic embeds the docs/GameInstallationFilesRegistry layout directly in service code
and duplicates authoritative CSV filenames elsewhere in the same class. These operational path
segments and filenames should be centralized and reused through constants.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R1102-1104]

+                Path.Combine(AppContext.BaseDirectory, "docs", "GameInstallationFilesRegistry", csvFileName),
+                Path.Combine(Directory.GetCurrentDirectory(), "docs", "GameInstallationFilesRegistry", csvFileName),
+                Path.Combine(AppContext.BaseDirectory, csvFileName),
Relevance

●●● Strong

Recent reviews accepted removing hardcoded mappings and centralizing duplicated business
identifiers.

PR-#423
PR-#417

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001297 requires business-meaningful filesystem paths and filenames to come from centralized
constants. The service directly embeds and duplicates the CSV names plus the development registry
directory layout.

Rule 3001297: Avoid inline hardcoded paths and use centralized constants
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1004-1005]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1070-1078]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1097-1105]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Authoritative CSV filenames and fallback filesystem path segments are hardcoded and duplicated in manifest service logic.

## Issue Context
Define the registry filenames, embedded resource naming, and development fallback path segments in an appropriate centralized constants class, then compose filesystem paths with `Path.Combine` using those constants.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1004-1005]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1070-1078]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1097-1105]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (4)
10. Manifest generation ignores cancellation 📘 Rule violation ☼ Reliability
Description
The changed manifest-generation API performs language detection, CSV resolution, per-file hashing,
and file insertion without accepting or propagating a CancellationToken. Callers therefore cannot
cancel this long-running I/O workflow even though downstream detector, resolver, and hash APIs
support cancellation.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R77-78]

+        string? manifestVersion = null,
+        string? language = null)
Relevance

●●● Strong

Recent repository precedent accepts adding cancellation propagation through long-running
asynchronous workflows.

PR-#278
PR-#385

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001311 requires long-running hashing and I/O methods to accept and propagate cancellation. The
changed signature has no token, while the workflow calls token-aware
ILanguageDetector.DetectAsync, CsvResolver.ResolveAsync, and
IFileHashProvider.ComputeFileHashAsync without passing one.

Rule 3001311: Long-running I/O methods must accept and propagate CancellationToken
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[73-104]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[663-679]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[715-727]
GenHub/GenHub.Core/Features/GameInstallations/ILanguageDetector.cs[9-17]
GenHub/GenHub.Core/Interfaces/Common/IFileHashProvider.cs[6-14]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The game-installation manifest generation API does not accept or propagate a `CancellationToken` through its long-running I/O operations.

## Issue Context
Add a token to both public overloads and their interface declarations, then forward it through language detection, CSV resolution, hashing, and builder operations. Extend internal and builder APIs where necessary rather than dropping the caller's token.

## Fix Focus Areas
- GenHub/GenHub.Core/Interfaces/Manifest/IManifestGenerationService.cs[20-41]
- GenHub/GenHub.Core/Interfaces/Manifest/IContentManifestBuilder.cs[174-181]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[73-142]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[652-727]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[999-1030]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. Disk CSV catch returns empty ✓ Resolved 📘 Rule violation ≡ Correctness
Description
Local CSV fallback catches every Exception and then returns an empty catalog, causing read or
parse failures to look like no authoritative records exist. This suppresses unexpected failures in a
non-boundary service helper.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R1123-1125]

        catch (Exception ex)
        {
-            logger.LogError(ex, "Failed to add files from CSV for {GameType}", gameType);
-            return false;
+            logger.LogWarning(ex, "Failed to load authoritative CSV from local disk for {GameType}", gameType);
Relevance

●● Moderate

Returning an empty catalog after broad failure is risky, but historical evidence is mixed for
best-effort fallback behavior.

PR-#425
PR-#305

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001350 forbids generic catches that merely log and continue outside a process boundary. The
catch is followed by return [], and the caller handles that as an empty catalog rather than a load
failure.

Rule 3001350: Avoid catching generic Exception except at explicit process boundaries
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1097-1128]
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[681-688]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Local CSV loading suppresses all failures and converts them into an empty authoritative catalog.

## Issue Context
Handle only specific recoverable filesystem exceptions. Propagate malformed data and unexpected failures, or return an explicit failure result so callers do not confuse errors with an empty catalog.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1097-1128]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[681-688]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Embedded CSV catch masks defects ✓ Resolved 📘 Rule violation ≡ Correctness
Description
Embedded-resource loading catches generic Exception, logs a warning, and continues to disk
fallback. CSV parse and programming failures are therefore suppressed alongside legitimate
resource-access failures.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R1092-1095]

+        catch (Exception ex)
+        {
+            logger.LogWarning(ex, "Failed to load authoritative CSV from embedded resource for {GameType}", gameType);
+        }
Relevance

●● Moderate

Generic catch concerns are plausible, but fallback-oriented exception handling has no close decisive
precedent here.

PR-#250
PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001350 prohibits generic catch-and-continue logic in service helpers. The cited code catches
failures from resource access and CSV parsing indiscriminately, then continues to another source.

Rule 3001350: Avoid catching generic Exception except at explicit process boundaries
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1073-1098]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Embedded CSV loading catches and suppresses every exception before attempting local-disk fallback.

## Issue Context
Catch only concrete resource or I/O failures for which fallback is valid. Let malformed CSV and unexpected failures propagate or become explicit failure results.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1073-1095]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


13. Resolver catch hides unexpected failures ✓ Resolved 📘 Rule violation ≡ Correctness
Description
The CSV resolver path catches every Exception and treats it as an ordinary signal to use fallback
data. This can mask programming or configuration defects instead of limiting fallback behavior to
specific recoverable resolver failures.
Code

GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[R1052-1055]

+            catch (Exception ex)
            {
-                HasHeaderRecord = true,
-            };
-            using var csv = new CsvReader(reader, config);
+                logger.LogWarning(ex, "Failed to resolve CSV catalog via resolver for {GameType} ({Language})", gameType, language);
+            }
Relevance

●● Moderate

Generic fallback handling may be intentional recovery, but catching every exception can conceal
unexpected resolver defects.

PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 3001350 disallows generic catch-and-continue behavior outside an explicit process boundary.
After the generic catch logs, this service helper unconditionally proceeds to fallback loading.

Rule 3001350: Avoid catching generic Exception except at explicit process boundaries
GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1030-1059]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CSV resolver fallback catches generic `Exception` and continues, suppressing unexpected failures.

## Issue Context
Identify concrete recoverable resolver exceptions that should trigger offline fallback. Allow all other failures to propagate or return an explicit failure result.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[1007-1059]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 29 rules
Review mode: 🧠 Deep: This is a broad, behavior-changing manifest/catalog refactor spanning core logic, interfaces, embedded data, language filtering, backup resolution, hashing, and multiple independent code paths, making subtle defects plausibly dense enough for redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +77 to +78
string? manifestVersion = null,
string? language = null)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Manifest generation ignores cancellation 📘 Rule violation ☼ Reliability

The changed manifest-generation API performs language detection, CSV resolution, per-file hashing,
and file insertion without accepting or propagating a CancellationToken. Callers therefore cannot
cancel this long-running I/O workflow even though downstream detector, resolver, and hash APIs
support cancellation.
Agent Prompt
## Issue description
The game-installation manifest generation API does not accept or propagate a `CancellationToken` through its long-running I/O operations.

## Issue Context
Add a token to both public overloads and their interface declarations, then forward it through language detection, CSV resolution, hashing, and builder operations. Extend internal and builder APIs where necessary rather than dropping the caller's token.

## Fix Focus Areas
- GenHub/GenHub.Core/Interfaces/Manifest/IManifestGenerationService.cs[20-41]
- GenHub/GenHub.Core/Interfaces/Manifest/IContentManifestBuilder.cs[174-181]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[73-142]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[652-727]
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[999-1030]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs Outdated
Comment thread GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs Outdated
Comment thread GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs Outdated
Comment thread GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs Outdated
Comment thread GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs Outdated
Comment on lines +705 to +712
if (entry.IsRequired)
{
logger.LogDebug(
"Required vanilla file missing from installation: {RelativePath}",
entry.RelativePath);
}

continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

10. Required files are omitted 🐞 Bug ≡ Correctness

When an authoritative entry marked required is absent, generation only emits a debug message and
skips the entry. The caller then builds and registers a manifest with no record of that required
file, preventing downstream validation from reporting the missing installation dependency.
Agent Prompt
## Issue description
Missing required catalog entries are silently omitted from the generated manifest.

## Issue Context
Treat a missing required source as manifest-generation failure, or preserve the authoritative required entry in a form downstream validation can check. Do not return a successful incomplete builder.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[702-712]
- GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs[632-646]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

"url": "https://raw.githubusercontent.com/community-outpost/GenHub/main/docs/GameInstallationFilesRegistry/Generals-1.08.csv",
"fileCount": 164,
"totalSizeBytes": 48166,
"languages": ["All", "EN", "DE", "FR", "ES", "IT", "KO", "PL", "PT-BR", "ZH-CN", "ZH-TW"],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

11. Advertised languages lack records 🐞 Bug ≡ Correctness

The embedded index advertises DE, FR, ES, IT, KO, PL, PT-BR, ZH-CN, and ZH-TW, but both new
registries contain only All and EN rows. Automatic detection or selection of an advertised
non-English language therefore retains only language-neutral files, omits required localized
archives and movies, and still returns a successful but incomplete manifest that cannot materialize
the detected language's base game.
Agent Prompt
## Issue description
Language filtering is enabled for detected or selected non-English installations, but the embedded registries contain no records for most languages advertised by the index. Filtering consequently retains only `All` entries and excludes required language-specific files, producing an incomplete manifest.

## Issue Context
The index declares support for DE, FR, ES, IT, KO, PL, PT-BR, ZH-CN, and ZH-TW in addition to the registry's available language values, and `LanguageDetector` can detect such codes from game directories and archives. Add authoritative language-specific rows for every advertised language or restrict each index language list so unsupported languages are neither advertised nor accepted; manifest generation should also reject a requested language when the selected registry does not support it.

## Fix Focus Areas
- GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs[523-565]
- GenHub/GenHub.Core/Assets/Registries/index.json[7-16]
- GenHub/GenHub.Core/Assets/Registries/index.json[23-32]
- GenHub/GenHub.Core/Assets/Registries/Generals-1.08.csv[1-165]
- GenHub/GenHub.Core/Assets/Registries/ZeroHour-1.04.csv[1-176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs Outdated
Comment thread GenHub/GenHub/Features/Manifest/ManifestGenerationService.cs Outdated
…resolution

- Centralize CSV registry filenames, resource namespace, and URLs in CsvConstants
- Add path traversal protection and directory boundary verification in FindFileCaseInsensitive
- Use authoritative SHA256 hashes and file sizes from CSV catalog entries
- Map catalog filename dynamically from game type and version
- Handle specific exceptions (IOException, UnauthorizedAccessException, CsvHelperException) instead of generic catches
- Simplify ternary expressions and update unit tests to verify authoritative hashes
Comment on lines 686 to 689
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
{
logger.LogError(ex, "Error adding game files to manifest");
return 0;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`Exception` caught is very generic


The exception caught is generic and defeats the purpose of exception handling. Each type of exception provides an insight into what exactly went wrong and provides scenario-specific ways for graceful recovery. While it is easy to recover from some exceptions, a small subset of them make the recovery very difficult, usually because the conditions are not suitable for the program to continue executing. It is therefore suggested that you switch to a better approach of exception handling and recovery.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant