You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Introduces automated extraction for self-extracting mod executables (SFX), Smart Install Maker packages, DAT/BIG archives, and control bar package normalization within the content acquisition pipeline.
Note
This is a self-contained feature PR carved out of #265 and must be merged ahead of #265.
Motivation
Many legacy Command & Conquer Generals/Zero Hour mods distributed via ModDB and other community hubs are packaged inside SFX exe wrappers (e.g. Smart Install Maker, InnoSetup) or DAT files rather than standard ZIP archives. This PR adds deep inspection and lossless unpacking capabilities.
Changes
Core Interfaces: Added IArchivePayloadProcessor and IControlBarPackageProcessor.
Processors:
Implemented ArchivePayloadProcessor with Smart Install Maker signature scanning, binary payload slicing, and recursive decompression.
Implemented ControlBarPackageProcessor for normalizing control bar UI asset bundles.
DI Registration: Registered processors in ContentPipelineModule.
Tests: Added comprehensive test suites in ArchivePayloadProcessorTests.cs and ControlBarPackageProcessorTests.cs.
Verification
All unit and integration tests passing across Windows, Linux, and macOS.
We reviewed changes in 1f48230...fe12960 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
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.
Status: 0 New Issues Found | Recommendation: Address remaining concern then merge
Overview
Severity
Count
CRITICAL
0
WARNING
0
SUGGESTION
0 (new)
This incremental pass targets commit fe129604 (fix(review): centralize metadata-only BIG check, guard empty signature, use named buffer constant) on top of 9b971bc. Blob comparison between 9b971bc^{tree} and HEAD^{tree} for the three scoped files shows identical blobs, so the commit on HEAD is the same content reviewed previously (rebase-only SHA rewrite).
Changes reviewed (3 files, 0 net changes since previous SHA)
GenHub/GenHub.Core/Constants/IoConstants.cs: unchanged — SignatureScanBufferSize = 8192 still the named constant.
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs: unchanged — empty-signature guard at lines 265-268 and IoConstants.SignatureScanBufferSize at line 273.
GenHub/GenHub/Features/Content/Services/Common/ControlBarPackageProcessor.cs: unchanged — IsMetadataOnlyBig helper at lines 374-378 still delegates from the two prior call sites.
Previously raised issues
File
Prior concern
Status
ControlBarPackageProcessor.cs (file-level)
SUGGESTION: cleanup gate hardcodes exactly two metadata-only file names; future additions silently regress to the prior CRITICAL (id 3860191870)
Still active — unchanged content; tracked under existing comment ID. A HashSet<string> field with StringComparer.OrdinalIgnoreCase plus a unit test pinning the metadata-only set would close the regression gap. Not duplicate-eligible.
All other prior inline findings (DeepSource =~ / == notes, SonarCloud/DeepSource shell findings, SUGGESTIONs on ControlBarPackageProcessorTests, ContentPathPolicy, CatalogConstants, *MetadataKey dead-code, the =~ / == shell notes, the BuildCheck.shTIMEOUT_SECONDS / VERBOSITY / PROJECT / usage() notes, etc.) remain active under their existing comment IDs and are not duplicated here.
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs - 0 new issues
GenHub/GenHub/Features/Content/Services/Common/ControlBarPackageProcessor.cs - 0 new issues; prior centralization SUGGESTION still tracked under id 3860191870
Status: 0 New Issues Found | Recommendation: Address remaining concern then merge
Overview
Severity
Count
CRITICAL
0
WARNING
0
SUGGESTION
0 (new)
This incremental pass targets commit 9b971bc (fix(review): centralize metadata-only BIG predicate, hoist signature scan buffer size) on top of fcdb50d.
GenHub/GenHub.Core/Constants/IoConstants.cs: added SignatureScanBufferSize = 8192 (lines 13-16) as a named, documented constant. Clean, follows docs/dev/constants.md.
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs: added the empty-signature early return at lines 265-268 (if (signature.Length == 0) return -1;) and replaced the magic 8192 at line 273 with IoConstants.SignatureScanBufferSize.
GenHub/GenHub/Features/Content/Services/Common/ControlBarPackageProcessor.cs: extracted the duplicated ControlBarProBaseFileName / ControlBarProLemonBaseFileName equality check into a private static bool IsMetadataOnlyBig(string fileName) helper (lines 374-378); both prior call sites (line 386 EnsureMetadataBigIncludedAsync and line 558 CleanupSourceDirectories) now delegate to it via method-group / lambda.
Previously raised issues resolved in this commit
File
Prior concern
Status
ArchivePayloadProcessor.cs (prior line 272)
SUGGESTION: overlap = signature.Length - 1 had no guard against an empty signature (id 3860191876)
Fixed — new if (signature.Length == 0) return -1; at line 265 makes the underflow path unreachable
ArchivePayloadProcessor.cs (prior file-level)
SUGGESTION: magic 8192 buffer size and implicit buffer.Length > signature.Length invariant should live in GenHub.Core.Constants (id 3860191878)
Fixed — buffer size promoted to IoConstants.SignatureScanBufferSize at line 273
Previously raised issue still active
File
Prior concern
Status
ControlBarPackageProcessor.cs (prior file-level)
SUGGESTION: cleanup gate hardcodes exactly two metadata-only file names; future additions silently regress to the prior CRITICAL (id 3860191870)
Still active — the new IsMetadataOnlyBig helper centralizes the predicate but still hardcodes exactly the same two names; the regression risk moves from two call sites to one helper, which is an improvement but does not eliminate it. A HashSet<string> field with StringComparer.OrdinalIgnoreCase and an accompanying unit test pinning the metadata-only set would close the gap. Not a duplicate-eligible re-post; tracked under the existing comment ID.
All other prior inline findings (DeepSource =~ / == notes, SonarCloud/DeepSource shell findings, SUGGESTIONs on ControlBarPackageProcessorTests, ContentPathPolicy, CatalogConstants, *MetadataKey dead-code, the =~ / == shell notes, the BuildCheck.shTIMEOUT_SECONDS / VERBOSITY / PROJECT / usage() notes, etc.) remain active under their existing comment IDs and are not duplicated here.
Files Reviewed (3 files changed in this commit)
GenHub/GenHub.Core/Constants/IoConstants.cs - 0 issues; properly hoists the magic 8192
GenHub/GenHub/Features/Content/Services/Common/ControlBarPackageProcessor.cs - 0 new issues; predicate extraction improves maintainability, prior centralization SUGGESTION still tracked under id 3860191870
Status: 3 Issues Found | Recommendation: Address before merge
Overview
Severity
Count
CRITICAL
0
WARNING
0
SUGGESTION
3
This incremental pass targets commit fcdb50d (fix(review): resolve SonarCloud and DeepSource findings, gate destructive cleanup) and the intermediate 3638e5d (fix(review): replace [[ == ]] comparisons with case statements for SH-3014), layered on top of 6c44b64.
Lines 62-72: removed the per-archive try/catch (Exception) / throw wrapper around EnsureValidArchivePayload + ExtractSingleArchive + File.Delete. The catch was purely a log-and-rethrow, so removal preserves observable behaviour: exceptions still propagate out of the Task.Run and cancellation is still honored at line 64. The File.Delete at line 70 is no longer in a try block, but the previous version did not protect it either, so no regression.
Lines 263-288: complete rewrite of FindSignatureOffset using IndexOf on a span and a last-overlap-bytes carry. The rewrite resolves the long-standing WARNING: Negative array index in FindSignatureOffset on chunk-boundary partial match (old original_line 294/301) and the CodeRabbit Negative buffer index when a partial signature match crosses a chunk boundary — there is no longer a matchIndex carried across chunks that could underflow. The new code is correct for the realistic case (hardcoded non-empty signatures smaller than the 8192-byte buffer). Three latent edge cases (empty signature precondition, magic 8192 buffer size, implicit buffer.Length > signature.Length invariant) are flagged in new SUGGESTION comments.
Lines 1302-1304: Where(...) now runs before OrderByDescending(d => d.Length). Equivalent set of elements returned (OrderByDescending is stable); no behaviour change for CleanupEmptyDirectories.
GenHub/GenHub/Features/Content/Services/Common/ControlBarPackageProcessor.cs (lines 550-564): CleanupSourceDirectories early-return gate replaced with a substantive hasPackagedContent check that excludes the two metadata-only base filenames (ControlBarProBaseFileName, ControlBarProLemonBaseFileName) from triggering destructive cleanup. This resolves the long-standing CRITICAL: Cleanup gate accepts fallback-only outputs and deletes real content (old original_line 479) for the current two-name metadata set. A new SUGGESTION flags the hardcoded pair as a maintenance hazard: a future third metadata-only BIG (or rename of either constant) silently regresses the gate to its prior CRITICAL state.
scripts/build-check.sh
Line 86: added the * default arm to the TIMEOUT_SECONDScase so the positive-integer check is a complete case/esac block (resolves a DeepSource structural lint). No behaviour change.
Lines 124-130: the PROJECT validation block rewritten as a case with glob ..*|*..*|/* (reject leading .., embedded .., or leading /). Equivalent to the prior [[ == *".."* ]] / [[ == /* ]] form and resolves the SH-3014 DeepSource == lint. Note: ..* and *..* overlap (the latter subsumes the former); harmless but slightly redundant.
Line 100: the named cleanup() + trap cleanup EXIT collapsed into an inline trap '...' EXIT. return 0 correctly dropped (traps don't return values) and exec 9>&- 2>/dev/null || true preserved.
New issues on changed lines
Three SUGGESTION comments posted this pass:
File
Line
Issue
ControlBarPackageProcessor.cs
556
Cleanup gate hardcodes exactly two metadata-only file names; future additions silently regress to the prior CRITICAL
ArchivePayloadProcessor.cs
267
overlap = signature.Length - 1 has no guard against an empty signature
ArchivePayloadProcessor.cs
268
Magic 8192 buffer size and implicit buffer.Length > signature.Length invariant should live in GenHub.Core.Constants
Previously raised issues resolved in this commit
File
Prior concern
Status
ArchivePayloadProcessor.cs (old line 294/301)
WARNING: Negative array index in FindSignatureOffset on chunk-boundary partial match
Fixed — FindSignatureOffset rewritten using Span.IndexOf + last-overlap-bytes carry; matchIndex is gone
ControlBarPackageProcessor.cs (old line 479)
CRITICAL: Cleanup gate accepts fallback-only outputs and deletes real content
Fixed — new hasPackagedContent gate excludes the two metadata-only base filenames; destructive cleanup now only runs when a real content BIG is present
All other prior inline findings remain active and are tracked under their existing comment IDs (DeepSource =~ / == notes, SonarCloud/DeepSource shell findings, Smart Install Maker / cancellation / zip-bomb / off-by-one / O(n²) dedup / magic-number concerns, the SUGGESTION notes on CatalogConstants and *.gib search patterns, the =~ / == shell notes, the BuildCheck.shTIMEOUT_SECONDS / VERBOSITY / PROJECT / usage() notes, etc.).
Files Reviewed (3 files changed in this commit)
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs - 2 new SUGGESTIONs (precondition + constant hoist); 1 prior WARNING resolved by the FindSignatureOffset rewrite
GenHub/GenHub/Features/Content/Services/Common/ControlBarPackageProcessor.cs - 1 new SUGGESTION (metadata-list centralisation); 1 prior CRITICAL resolved by the hasPackagedContent gate
scripts/build-check.sh - 0 new issues (case-glob + inline trap; all prior concerns still active under existing comment IDs)
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs (line 275): int read; -> int read = 0; in FindSignatureOffset. Initializes the local at declaration (resolves the CS-W1022 lint, satisfies the Variables & Declarations rule in AGENTS.md). Behaviour of the while ((read = stream.Read(...)) > 0) loop is unchanged: the read-assignment inside the loop condition overwrites the initializer on the first iteration.
scripts/build-check.sh:
Lines 85-90: the positive-integer TIMEOUT_SECONDS check switched from [[ =~ ^[0-9]+$ ]] || [[ -le 0 ]] to a single case glob (''|*[!0-9]*|0*). Equivalent semantics for the realistic input set: empty, any non-digit, or any value starting with 0 (which would also have been rejected by the prior [[ -le 0 ]] plus implicit octal concerns). No behaviour change for the documented 120 default or any value > 0.
Line 98: the named cleanup() function + trap cleanup EXIT collapsed into an inline trap '...' EXIT. return 0 is correctly dropped (traps don't return values to their caller) and the exec 9>&- 2>/dev/null || true is preserved verbatim.
New issues on changed lines
None. All prior inline findings (CRITICAL Failed promotion permanently deletes payload files at line 1259, CRITICAL Per-variant cleanup destroys sources for every / Cleanup gate accepts fallback-only outputs in ControlBarPackageProcessor, WARNING Negative array index in FindSignatureOffset on chunk-boundary parti..., the Smart Install Maker / cancellation / zip-bomb / off-by-one / O(n^2) dedup / magic-number concerns, the SUGGESTION notes on CatalogConstants and *.gib search patterns, the =~ / == shell notes, etc.) remain active and are tracked under their existing comment IDs. The two-line refactor in FindSignatureOffset and the bash cleanup of the same file do not touch any of those flagged regions, and the change to line 275 specifically removes one of the variables that the active Negative array index WARNING warned about (the read is now guaranteed initialised before any read of the loop body that references it).
Notes on the case ... 0* glob (line 86)
0* rejects 0, 00, 0120, etc. The previous regex + -le 0 check had identical observable behaviour for the same set (-le 0 already caught 0/00; the octal-vs-decimal ambiguity in 0120 was not addressed by either form). The case form is a faithful, slightly more idiomatic rewrite. If the team wants to allow leading-zero positive integers, that should be a deliberate follow-up; the new code is not a regression.
Files Reviewed (2 files changed in this commit)
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs - 0 new issues (line 275 initializer; prior concerns still active)
scripts/build-check.sh - 0 new issues (case-glob + inline trap; all prior concerns resolved)
Status: No New Issues Found | Recommendation: Merge
Overview
Severity
Count
CRITICAL
0
WARNING
0
SUGGESTION
0
This incremental pass targets commit 5392123 (fix(review): resolve deepsource complexity and shell script findings), a follow-up quality refactor on top of bd2c03a.
ParseSmartInstallMakerFileTable loop body extracted into ProcessNextSimCandidate + ValidateAndAddSimRecord. Limit-checks (MaxZipEntryCount, MaxZipUncompressedSizeBytes) and accumulator semantics are preserved bit-for-bit.
GetNonCollidingDestinationPathdo/while -> while refactor; counter starts at 1 and the first candidate path is materialized up-front (equivalent to the previous do-while body).
FilesHaveIdenticalContentwhile ((bytesRead1 = s1.Read(...)) > 0) -> while (true) + explicit break. Behaviour is identical, the bytesRead1 declaration is now scoped inside the loop, and the new shape is the idiom the rest of the file already uses.
scripts/build-check.sh
log_status / log_err helper functions hoisted to the top of the file so they are available to the early usage / validation paths (matches the previous 7b76465 review note about reuse of the helper).
[[ "$PROJECT" =~ \.\. ]] -> [[ "$PROJECT" == *".."* ]] and [[ "$PROJECT" = /* ]] -> [[ "$PROJECT" == /* ]]. Both are [[ ]] glob/pattern matches, supported in bash; the previous regex form was the deeper-source concern.
$(date -Iseconds) extracted into CURRENT_TIME="$(date -u +"%Y-%m-%dT%H:%M:%SZ")" so the timestamp is unambiguous UTC ISO-8601 and the format string cannot be confused with a future refactor of the printf line.
Final exit $EXIT_CODE -> exit "$EXIT_CODE" to keep word-splitting safe under set -u.
New issues on changed lines
None. All prior inline findings (CRITICAL Cleanup gate accepts fallback-only outputs, WARNING Payload processing runs after hash/CAS ingestion, WARNING Any processor failure silently drops a variant manifest, SUGGESTION containment/asymmetry in ContentPathPolicy, the =~ / == DeepSource notes, etc.) remain active and are tracked under their existing comment IDs. The refactor decomposes work but does not change any observable behaviour of the affected helpers.
Notes on the [[ "$PROJECT" == *".."* ]] change (lines 125, 130)
The DeepSource ==-in-[[ ]] notes on these lines are false positives: the shebang is #!/usr/bin/env bash and the script is a bash script (uses [[ ]], set -euo pipefail, printf, bash arrays, flock). Inside [[ ]], == is the documented bash idiom and is fully portable across bash versions, so swapping back to = is a no-op aesthetic change. The new lines are correct.
Files Reviewed (2 files changed in this commit)
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs - 0 new issues (helper extraction; all prior concerns still active)
scripts/build-check.sh - 0 new issues (helper hoist + bash-idiomatic [[ == ]] + quoted timestamp/exit; all prior concerns resolved)
Status: No New Issues Found | Recommendation: Merge
Overview
Severity
Count
CRITICAL
0
WARNING
0
SUGGESTION
0
This incremental pass targets commit bd2c03a (fix(review): resolve sonarcloud quality gate, deepsource shell check, and reduce cognitive complexity), which is a quality-driven refactor of the previously reviewed files.
Previously flagged issues resolved in this commit
File
Prior concern
Status
scripts/build-check.sh (line 40)
Unvalidated TIMEOUT_SECONDS
Fixed — explicit ^[0-9]+$ + > 0 validation at line 73
scripts/build-check.sh (line 44)
Unvalidated VERBOSITY
Fixed — allowlist of MSBuild verbosities at line 65
scripts/build-check.sh (line 109)
PROJECT had no traversal/absolute check
Fixed — .. rejection + absolute-path branch at lines 125–134
None. The refactor decomposes the long ProcessControlBarPackageAsync body into focused helpers (ProcessVariantBigRootAsync, RepackArtAndDataBigsAsync, BuildAndPackArtAndDataBigsAsync, CollectFlatPrebuiltBigs, EnsureMetadataBigIncludedAsync, TryLocateAndCopyMetadataBigAsync, WriteFallbackMetadataBigAsync, CopySourceDirectoriesToPacks, HasControlBarManifestMetadata, HasControlBarFiles), the ArchivePayloadProcessorFindSignatureOffset rewrite, WalkSmartInstallMakerBlocks / DecompressSimTableBlock / DecompressBz2SmartInstallMakerRecord / DecompressDeflateSmartInstallMakerRecord / DecompressRawSmartInstallMakerRecord extraction, and the PromoteDirectoryContents / CleanupEmptyDirectories cleanup, plus 8 new constants in GameContentConstants (DefaultControlBarVariant, ControlBarProBaseFileName, ControlBarProLemonBaseFileName, BigEnDirectoryName, BigDirectoryName, GenToolDirectoryName, WindowDirectoryName). All previously raised concerns on these lines (CRITICAL Cleanup gate accepts fallback-only outputs, SUGGESTION Cancellation token is only honored for image conversion, WARNING Only the first game-specific alias directory is promoted, etc.) remain active and are tracked under their existing comment IDs — no new defects were introduced that warrant a separate inline.
Notes on the partial CRITICAL mitigation
The new if (repackedOutputs.Count == 0) return; early-return in CleanupSourceDirectories (line 553) is a partial mitigation of the long-standing CRITICAL: Cleanup gate accepts fallback-only outputs concern: when WriteFallbackMetadataBigAsync succeeds, the fallback is added to repackedOutputs, so the early return does not fire and the destructive cleanup still runs. This is unchanged behaviour, the existing comment at line 552 already documents it, and no new comment is needed.
Files Reviewed (7 files changed in this commit)
GenHub/GenHub.Core/Constants/GameContentConstants.cs - 0 new issues (8 new constants; reviewed)
GenHub/GenHub.Core/Helpers/ContentPathPolicy.cs - 0 new issues (link-resolution extraction; pre-existing WARNINGs still active)
GenHub/GenHub.Tests/GenHub.Tests.Core/Helpers/ContentPathPolicyTests.cs - 0 new issues (IDisposable resolves prior SUGGESTION)
GenHub/GenHub/Features/Content/Services/Common/ArchivePayloadProcessor.cs - 0 new issues (refactor only; all prior concerns still active)
GenHub/GenHub/Features/Content/Services/Common/ControlBarPackageProcessor.cs - 0 new issues (helper extraction; all prior concerns still active)
scripts/build-check.ps1 - 0 new issues ($args → $buildArgs)
scripts/build-check.sh - 0 new issues (validation block + heredoc; all prior concerns resolved)
Status: 4 Issues Found | Recommendation: Address before merge
Overview
Severity
Count
CRITICAL
0
WARNING
1
SUGGESTION
3
This incremental pass targets the only new file in HEAD 7b76465 (scripts/build-check.sh, mirroring build-check.ps1). The 110 prior review threads already cover the substantive findings in ArchivePayloadProcessor.cs, ControlBarPackageProcessor.cs, GitHubContentProvider.cs, CommunityOutpostManifestFactory.cs, and the Core interfaces.
Issue Details (click to expand)
WARNING
File
Line
Issue
scripts/build-check.sh
40
Unvalidated TIMEOUT_SECONDS accepts negative/non-numeric values that break flock
SUGGESTION
File
Line
Issue
scripts/build-check.sh
44
VERBOSITY forwarded to dotnet without validation against the MSBuild set
scripts/build-check.sh
109
PROJECT concatenated into a path with no traversal/absolute-path check
scripts/build-check.sh
21
usage() hard-codes the comment-block line range (sed -n '2,14p')
Files Reviewed (20 files)
scripts/build-check.sh - 4 issues (new in HEAD)
GenHub/Directory.Packages.props - reviewed (no new issues; trailing-newline loss already flagged)
The reason will be displayed to describe this comment to others. Learn more.
This function is never invoked. Check usage (or ignored if invoked indirectly)
A function was defined but goes out of scope without ever being called. This may indicate a typo in the function name, or that the call is unreachable.
The reason will be displayed to describe this comment to others. Learn more.
Variable `read` is uninitialized
While some variables such as fields can be initialized and be assigned a default value, it is possible for local variables to not be initialized. This however is not a good practice as it is capable of critically altering your program's path, thereby affecting its logic.
It is therefore recommended that you always initialize variables, ideally where they're being declared.
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: Cleanup gate hardcodes exactly two metadata-only file names; future additions silently regress to the prior CRITICAL
The new hasPackagedContent check enumerates ControlBarProBaseFileName and ControlBarProLemonBaseFileName as the exhaustive allow-list of "metadata-only" outputs. If a third metadata-only BIG (e.g. for a new variant or upstream tool) is ever added to WriteFallbackMetadataBigAsync or EnsureMetadataBigIncludedAsync without updating this list, the gate flips back to its prior CRITICAL state and destructive cleanup runs on a fallback-only payload. The same problem would re-emerge if the metadata base filenames are ever renamed but this gate is not updated. Centralize the set (e.g. a private static readonly HashSet<string> MetadataOnlyBigFileNames = new(StringComparer.OrdinalIgnoreCase) { GameContentConstants.ControlBarProBaseFileName, GameContentConstants.ControlBarProLemonBaseFileName }; near the top of the class) and test that the gate stays correct when new metadata outputs are introduced.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION:overlap = signature.Length - 1 has no guard against an empty signature
If a future caller passes an empty byte[], overlap becomes -1, then Math.Min(available, -1) returns -1, and the subsequent buffer.AsSpan(available - buffered, buffered) evaluates to AsSpan(available + 1, -1) which throws ArgumentOutOfRangeException on the negative length. All current call sites pass hardcoded non-empty signatures, so this is a latent precondition violation. Add a Debug.Assert(signature is { Length: > 0 }) (or a documented ArgumentException) so the invariant is explicit and future refactors cannot silently regress it.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: Magic 8192 buffer size and implicit buffer.Length > signature.Length invariant should live in GenHub.Core.Constants
The 8192-byte buffer is a magic number; the new carry-over logic at lines 282-284 implicitly assumes buffer.Length > signature.Length (otherwise overlap >= buffer.Length and the span copy would overrun). Per docs/dev/constants.md, sizes and limits belong in GenHub.Core.Constants. Promote the buffer size to a named constant (e.g. ArchiveConstants.SignatureScanBufferSize) and add an ArgumentOutOfRangeException precondition or unit test that pins the buffer.Length > signature.Length relationship so a future signature larger than the buffer cannot silently corrupt the carry.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
- Replace bash regex comparisons with case globs to satisfy SH-3015
- Inline cleanup into EXIT trap to satisfy SH-2329
- Initialize 'read' variable to satisfy CS-W1022
…e, use named buffer constant
- ControlBarPackageProcessor: extract IsMetadataOnlyBig so the cleanup gate and
metadata detection share one source of truth
- ArchivePayloadProcessor: return early for empty signatures in FindSignatureOffset
- IoConstants: add SignatureScanBufferSize and use it for signature scanning
Reviewed-by: ox-alpha (opencode/x-preview-f-free)
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
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.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Introduces automated extraction for self-extracting mod executables (SFX), Smart Install Maker packages, DAT/BIG archives, and control bar package normalization within the content acquisition pipeline.
Note
This is a self-contained feature PR carved out of #265 and must be merged ahead of #265.
Motivation
Many legacy Command & Conquer Generals/Zero Hour mods distributed via ModDB and other community hubs are packaged inside SFX exe wrappers (e.g. Smart Install Maker, InnoSetup) or DAT files rather than standard ZIP archives. This PR adds deep inspection and lossless unpacking capabilities.
Changes
IArchivePayloadProcessorandIControlBarPackageProcessor.ArchivePayloadProcessorwith Smart Install Maker signature scanning, binary payload slicing, and recursive decompression.ControlBarPackageProcessorfor normalizing control bar UI asset bundles.ContentPipelineModule.ArchivePayloadProcessorTests.csandControlBarPackageProcessorTests.cs.Verification