M3/M4 full-issue closure wave: onboarding, ops hardening, release strategy#48
M3/M4 full-issue closure wave: onboarding, ops hardening, release strategy#48Prekzursil merged 3 commits intomainfrom
Conversation
|
Unable to trigger custom agent "Code Reviewer". You have run out of credits 😔 |
github-actions
bot
added
area:app
|
Reviewer automation could not request a non-author reviewer for this PR.
Next steps:
|
📝 WalkthroughWalkthroughAdds mod onboarding, calibration, telemetry, support-bundle export, transactional profile install/rollback, schema validators and CI release automation, plus UI/ViewModel wiring, data models, tests, and documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant MainVM as MainViewModel
participant ModOnboard as ModOnboardingService
participant ProfileRepo as IProfileRepository
participant FS as FileSystem
User->>MainVM: Scaffold Draft Profile(request)
MainVM->>ModOnboard: ScaffoldDraftProfileAsync(request)
ModOnboard->>ProfileRepo: Resolve base profile
ProfileRepo-->>ModOnboard: base profile JSON
ModOnboard->>ModOnboard: Infer workshop IDs & path hints
ModOnboard->>FS: Write draft profile JSON
FS-->>ModOnboard: output path
ModOnboard-->>MainVM: ModOnboardingResult(profileId, hints)
MainVM-->>User: Update UI (OnboardingSummary)
sequenceDiagram
participant User as User
participant MainVM as MainViewModel
participant ProfileSvc as GitHubProfileUpdateService
participant FS as FileSystem
User->>MainVM: Install Profile Update(profileId)
MainVM->>ProfileSvc: InstallProfileTransactionalAsync(profileId)
ProfileSvc->>ProfileSvc: Fetch manifest & download ZIP
ProfileSvc->>ProfileSvc: Validate SHA256 & extract
ProfileSvc->>FS: Create timestamped backup
FS-->>ProfileSvc: backup path
ProfileSvc->>FS: Write new profile JSON
ProfileSvc->>FS: Write install receipt
ProfileSvc-->>MainVM: ProfileInstallResult(success, paths)
User->>MainVM: Rollback Profile
MainVM->>ProfileSvc: RollbackLastInstallAsync(profileId)
ProfileSvc->>FS: Locate latest backup
FS-->>ProfileSvc: backup path
ProfileSvc->>FS: Restore backup, write rollback receipt
ProfileSvc-->>MainVM: ProfileRollbackResult(restored, paths)
MainVM-->>User: Display rollback status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||||||||||||||||||||
Codacy's Analysis Summary69 new issues (≤ 0 issue) Review Pull Request in Codacy →
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||||
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b10f239328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| generatedAtUtc = DateTimeOffset.UtcNow, | ||
| profileId = request.ProfileId, | ||
| notes = request.Notes, | ||
| includedFiles = included.OrderBy(x => x, StringComparer.OrdinalIgnoreCase).ToArray(), |
There was a problem hiding this comment.
ExportAsync builds manifestPayload from includedFiles before manifest.json is appended to included, so every emitted manifest omits itself from the inventory even though the bundle later contains it. This breaks the support-bundle contract introduced here (validate-support-bundle-manifest.ps1 -Strict requires manifest.json in includedFiles) and makes produced manifests fail strict validation.
Useful? React with 👍 / 👎.
| ExportCalibrationArtifactCommand = new AsyncCommand(ExportCalibrationArtifactAsync, () => !string.IsNullOrWhiteSpace(SelectedProfileId)); | ||
| BuildCompatibilityReportCommand = new AsyncCommand(BuildCompatibilityReportAsync, () => !string.IsNullOrWhiteSpace(SelectedProfileId)); |
There was a problem hiding this comment.
The onboarding commands are gated on SelectedProfileId being non-empty, but the new onboarding export/report methods are written to fall back to OnboardingDraftProfileId; this guard makes that fallback unreachable and causes Mod Compatibility Studio actions to run against the currently selected manifest profile instead of the just-scaffolded draft profile. In the documented onboarding flow, that produces calibration/report evidence for the wrong profile.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (28)
docs/release-notes-template.md (1)
15-21: Add an explicit hash comparison to the verification snippet.The current snippet prints the computed hash and the stored hash separately, requiring the reader to compare them visually. An explicit comparison reduces the chance of human error during release verification.
📋 Suggested verification snippet
## Verification - Validate checksum before use: ```powershell Get-FileHash .\SwfocTrainer-portable.zip -Algorithm SHA256 Get-Content .\SwfocTrainer-portable.zip.sha256 +# Compare — must match: +$computed = (Get-FileHash .\SwfocTrainer-portable.zip -Algorithm SHA256).Hash +$expected = (Get-Content .\SwfocTrainer-portable.zip.sha256).Trim().Split(' ')[0].ToUpper() +if ($computed -ne $expected) { throw "Checksum mismatch: $computed vs $expected" } +Write-Host "Checksum OK: $computed"</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/release-notes-template.mdaround lines 15 - 21, The verification
snippet currently prints hashes separately; update the block that uses
Get-FileHash and Get-Content to compute and compare the values programmatically
by assigning the computed hash to a variable (e.g., $computed = (Get-FileHash
.\SwfocTrainer-portable.zip -Algorithm SHA256).Hash), read and normalize the
stored value into another variable (e.g., $expected from Get-Content and
Trim()/Split()), then perform an explicit comparison (if ($computed -ne
$expected) { throw "Checksum mismatch..." }) and emit a clear success message
(e.g., Write-Host "Checksum OK: $computed") so users don’t have to compare
values manually; modify the snippet containing Get-FileHash and Get-Content
accordingly.</details> </blockquote></details> <details> <summary>tools/fixtures/save-corpus/base_swfoc_steam_v1.json (1)</summary><blockquote> `1-5`: **Consider schema-representative `syntheticByteLength` values across fixtures.** Profile naming is correct and complies with guidelines. However, all four corpus fixtures share the same `syntheticByteLength: 300000`. If `SaveCorpusRoundTripTests` feeds this as a typed payload through the per-schema codec (rather than raw byte passthrough), uniform sizes reduce the chance of catching schema-specific framing bugs (e.g., fixed-size headers, minimum record counts, or alignment constraints that differ between SWEAW, SWFOC, AOTR, and ROE formats). <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@tools/fixtures/save-corpus/base_swfoc_steam_v1.jsonaround lines 1 - 5, The
fixture for schemaId "base_swfoc_steam_v1" uses a generic syntheticByteLength
(300000) shared across multiple corpus fixtures; update this JSON to use a
schema-representative syntheticByteLength that reflects SWFOC framing/record
expectations (e.g., a value exercising any fixed headers, minimum record counts,
or alignment constraints unique to SWFOC) and ensure the other fixtures for
SWEAW, AOTR, and ROE use different, distinct lengths so SaveCorpusRoundTripTests
exercise schema-specific sizes and catch framing bugs.</details> </blockquote></details> <details> <summary>.github/workflows/visual-audit.yml (1)</summary><blockquote> `27-28`: **Scope `APPLITOOLS_API_KEY` to the step that needs it, not the whole job.** The env context pattern (`env.APPLITOOLS_API_KEY != ''`) in the `if:` condition is correct and more reliable than checking `secrets.*` directly in a step condition. However, declaring the secret at job level exposes it to every step's process environment (Checkout, path resolution, visual-compare, artifact upload) when only the informational note step uses it. The informational step itself is also a stub — it only prints a message and never actually reads the key value. Scoping the env declaration to the one step that references it reduces the blast radius if any other step in this job is ever replaced with an untrusted action: <details> <summary>🔒 Proposed scoping change</summary> ```diff - env: - APPLITOOLS_API_KEY: ${{ secrets.APPLITOOLS_API_KEY }} steps: - name: Checkout uses: actions/checkout@v4 # ... other steps ... - name: Applitools availability note - if: ${{ env.APPLITOOLS_API_KEY != '' }} + if: ${{ secrets.APPLITOOLS_API_KEY != '' }} + env: + APPLITOOLS_API_KEY: ${{ secrets.APPLITOOLS_API_KEY }} shell: pwsh run: | Write-Host "APPLITOOLS_API_KEY is configured. Visual pack can be uploaded for external review."If a real Applitools upload step is added later, apply the same step-level pattern there.
Also applies to: 61-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/visual-audit.yml around lines 27 - 28, Move the APPLITOOLS_API_KEY environment variable out of the job-level env block and declare it only on the specific step(s) that require it (e.g., the informational "note" step that checks env.APPLITOOLS_API_KEY and any real Applitools upload step you add later); keep using the condition pattern env.APPLITOOLS_API_KEY != '' in those step-level if: checks so the secret is only injected into the process environment of the step(s) that actually reference it and not exposed to other steps like checkout, path resolution, visual-compare, or artifact upload.tools/fixtures/save-corpus/manifest.json (1)
1-9: Consider adding a$schemareference for tooling-backed validation.All four fixture paths are present in the repo and profile names (
base,aotr,roe) comply with guidelines. Adding a$schemapointer would let the CI schema-smoke checks that already exist in this PR validate the manifest structure automatically, consistent with "Script outputs must be stable and schema-backed where practical."🔧 Suggested addition
{ + "$schema": "../schemas/save-corpus-manifest.schema.json", "schemaVersion": "1.0", "fixtures": [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/fixtures/save-corpus/manifest.json` around lines 1 - 9, Add a $schema pointer to the manifest JSON so CI/schema-backed tooling can validate it: update the manifest object (where "schemaVersion" and "fixtures" are defined in manifest.json) to include a top-level "$schema" property referencing the canonical JSON Schema for these fixtures; ensure the $schema value matches the repo/tooling schema used by other manifests so the CI smoke checks validate this file automatically.tools/schemas/support-bundle-manifest.schema.json (1)
35-40:warningsitems allow empty strings — inconsistent withincludedFiles.
includedFilesitems enforce"minLength": 1(Line 32), butwarningsitems have no such constraint. If empty-string warning entries should be rejected for consistency, add the same constraint.♻️ Proposed fix
"warnings": { "type": "array", "items": { - "type": "string" + "type": "string", + "minLength": 1 } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/schemas/support-bundle-manifest.schema.json` around lines 35 - 40, The schema's "warnings" array currently allows empty strings whereas "includedFiles" enforces non-empty entries; update the "warnings" definition (the "warnings" property and its "items" schema) to include "minLength": 1 so each warning string must be non-empty, matching the behavior of "includedFiles".src/SwfocTrainer.Core/Models/SupportBundleModels.cs (1)
15-20: ClarifyBundlePath/ManifestPathsemantics whenSucceeded = false.Both properties are non-nullable
string, but on a failed export they will presumably be empty or meaningless. Callers checking onlyBundlePathwithout first testingSucceededmay act on stale or empty paths. Consider making themstring?to make the failure contract explicit, or at minimum add XML doc comments documenting the convention.♻️ Option A — nullable paths on failure
public sealed record SupportBundleResult( bool Succeeded, - string BundlePath, - string ManifestPath, + string? BundlePath, + string? ManifestPath, IReadOnlyList<string> IncludedFiles, IReadOnlyList<string> Warnings);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Models/SupportBundleModels.cs` around lines 15 - 20, The SupportBundleResult record's BundlePath and ManifestPath should explicitly allow null to signal meaningless values when Succeeded is false: change SupportBundleResult to use string? for BundlePath and ManifestPath (i.e., SupportBundleResult(... string? BundlePath, string? ManifestPath ...)) and update any callers that construct or read SupportBundleResult to handle null (check Succeeded before using paths or null-coalesce appropriately); alternatively, if you prefer not to change types, add XML doc comments on SupportBundleResult and its BundlePath/ManifestPath properties clearly stating they are undefined when Succeeded is false—referencing the SupportBundleResult type and its BundlePath, ManifestPath and Succeeded members so reviewers can find the change.tests/SwfocTrainer.Tests/Saves/SaveCorpusRoundTripTests.cs (1)
18-64: Consider[Theory]+[MemberData]for per-fixture test isolation.The current single
[Fact]iterates all fixtures in a loop. If fixture N fails, xUnit reports one failing test with no information about fixtures N+1..M. Using[Theory]gives a separate result per fixture and makes failures easier to triage.♻️ Sketch of parameterised approach
+ public static IEnumerable<object[]> FixtureNames() + { + var root = TestPaths.FindRepoRoot(); + var manifestPath = Path.Combine(root, "tools", "fixtures", "save-corpus", "manifest.json"); + if (!File.Exists(manifestPath)) yield break; + var manifest = JsonSerializer.Deserialize<SaveCorpusManifest>( + File.ReadAllText(manifestPath), JsonOptions); + foreach (var name in manifest?.Fixtures ?? []) + yield return [name]; + } + + [Theory] + [MemberData(nameof(FixtureNames))] - [Fact] - public async Task Codec_ShouldRoundTripForShippedSchemaCorpusFixtures() + public async Task Codec_ShouldRoundTripForShippedSchemaCorpusFixtures(string fixtureName) { var root = TestPaths.FindRepoRoot(); var fixtureDir = Path.Combine(root, "tools", "fixtures", "save-corpus"); - var manifestPath = Path.Combine(fixtureDir, "manifest.json"); - File.Exists(manifestPath).Should().BeTrue(...); - var manifest = JsonSerializer.Deserialize<SaveCorpusManifest>(...); - ... - foreach (var fixtureName in manifest.Fixtures) - { var fixturePath = Path.Combine(fixtureDir, fixtureName); // ... rest of per-fixture logic unchanged ... - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/SwfocTrainer.Tests/Saves/SaveCorpusRoundTripTests.cs` around lines 18 - 64, Replace the single-loop Fact with a parameterized Theory so each fixture runs as its own test: add a static MemberData provider (e.g., a public static IEnumerable<object[]> FixtureManifests or GetFixtureNames method) that reads the manifest.json (using SaveCorpusManifest / manifest.Fixtures) and yields each fixture name, change the test attribute on Codec_ShouldRoundTripForShippedSchemaCorpusFixtures to [Theory] and [MemberData(nameof(FixtureManifests))], change the test signature to accept a single string fixtureName and remove the foreach loop, and keep the existing per-fixture temp file creation and BinarySaveCodec (and SaveOptions) logic inside the body so failures report per-fixture.src/SwfocTrainer.Profiles/Services/GitHubProfileUpdateService.cs (3)
146-157: Downloaded zip is not cleaned up on SHA mismatch or subsequent failures.When the SHA check fails (or extraction/validation fails later), the downloaded zip at
zipPathremains on disk. Over time, repeated failed installs will accumulate orphaned files in the download cache. Consider deleting the zip in failure paths or adding a general cleanup in afinallyblock.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Profiles/Services/GitHubProfileUpdateService.cs` around lines 146 - 157, The SHA-mismatch and other failure paths in GitHubProfileUpdateService (where ComputeSha256 is called and a ProfileInstallResult with Succeeded: false is returned) leave the downloaded zip at zipPath on disk; update the method to ensure zipPath is deleted on all error/early-return paths—either delete the file before returning in the SHA mismatch branch and other failure branches (extraction/validation) or wrap the download/extract logic in a try/finally that removes zipPath in the finally block; reference ComputeSha256, zipPath, and the ProfileInstallResult return sites when making the change.
322-347: Receipt timestamps may drift from backup timestamps.
DateTimeOffset.UtcNowis called independently in both the install flow (Line 228 for backup naming) and inWriteInstallReceiptAsync(Line 332 and Line 337). The receiptgeneratedAtUtcand receipt filename timestamp may differ from the backup filename timestamp by seconds. This is cosmetic but could cause confusion when correlating receipts to backups. Consider capturing the timestamp once at the top of the transaction and threading it through.Also applies to: 349-370
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Profiles/Services/GitHubProfileUpdateService.cs` around lines 322 - 347, The receipt timestamp is generated separately inside WriteInstallReceiptAsync (and the similar method around the other receipt code), causing potential drift versus the backup filename timestamp; fix this by generating a single DateTimeOffset timestamp at the start of the install/uninstall transaction and pass that timestamp into WriteInstallReceiptAsync (and the corresponding receipt-writing method) as an extra parameter (e.g., generatedAtUtc) so both the receipt payload property generatedAtUtc and the receipt filename use the exact same timestamp value.
236-248:File.Copyto destination is not atomic — preferFile.Movewith overwrite.The temp-file pattern here copies source→tmp then tmp→destination via
File.Copy. On Windows/NTFS,File.Copyis not atomic and can leave a partially-written destination on crash.File.Move(src, dst, overwrite: true)(available since .NET Core 3.0) performs a rename which is much closer to atomic on the same volume.♻️ Proposed fix
var tempInstallPath = $"{destination}.tmp"; File.Copy(targetProfileJson, tempInstallPath, overwrite: true); - try - { - File.Copy(tempInstallPath, destination, overwrite: true); - } - finally - { - if (File.Exists(tempInstallPath)) - { - File.Delete(tempInstallPath); - } - } + File.Move(tempInstallPath, destination, overwrite: true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Profiles/Services/GitHubProfileUpdateService.cs` around lines 236 - 248, Replace the non-atomic File.Copy(tempInstallPath, destination, overwrite: true) with File.Move(tempInstallPath, destination, overwrite: true) so the tmp→destination step uses an atomic rename; keep the initial File.Copy(targetProfileJson, tempInstallPath, overwrite: true) to create tempInstallPath, then in the try use File.Move(tempInstallPath, destination, overwrite: true) and retain the existing finally cleanup that deletes tempInstallPath if it still exists (ensuring operations occur on the same volume).src/SwfocTrainer.Core/Services/TrainerOrchestrator.cs (1)
97-98: Consider wrapping telemetry recording defensively.
RecordActioncurrently cannot throw in practice (the implementation guards against nullactionIdand uses simple dictionary operations), but telemetry should never break the primary execution path. A baretry-catchwould make this guarantee explicit and protect against future changes to the telemetry service.🛡️ Defensive wrapping
- _telemetry.RecordAction(actionId, result.AddressSource, result.Succeeded); + try + { + _telemetry.RecordAction(actionId, result.AddressSource, result.Succeeded); + } + catch + { + // Telemetry must never fail the primary action path. + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Services/TrainerOrchestrator.cs` around lines 97 - 98, Wrap the telemetry call defensively so telemetry failures cannot affect the main flow: surround the call to _telemetry.RecordAction(actionId, result.AddressSource, result.Succeeded) in a try-catch inside TrainerOrchestrator (the method containing this return), swallow or log the exception (use the existing logger, e.g. _logger?.LogWarning/LogError) and ensure the original result is still returned; do not change return behavior or throw from the catch block.src/SwfocTrainer.Core/Contracts/IProfileUpdateService.cs (1)
9-13: Consider retiring or wrappingInstallProfileAsyncto align return types.
InstallProfileAsyncreturns a bareTask<string>while the new transactional method returns a structuredTask<ProfileInstallResult>. If the old method is kept only for backward compatibility, a deprecation note ([Obsolete]) would clarify intent. If both are long-lived, the inconsistency in result type will create confusion at call sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Contracts/IProfileUpdateService.cs` around lines 9 - 13, The InstallProfileAsync method returns a raw Task<string> while the newer InstallProfileTransactionalAsync returns Task<ProfileInstallResult>, creating an inconsistent API; either mark InstallProfileAsync with [Obsolete] and update callers to use InstallProfileTransactionalAsync, or change InstallProfileAsync to return Task<ProfileInstallResult> (or implement a wrapper that calls InstallProfileTransactionalAsync and maps/returns the string-friendly value) so both methods produce consistent result types; update the interface IProfileUpdateService accordingly and adjust implementations of InstallProfileAsync, InstallProfileTransactionalAsync, and any callers to use the new/obsolete contract.tests/SwfocTrainer.Tests/Profiles/ModOnboardingServiceTests.cs (1)
11-12: Consider adding negative-path coverage forInvalidDataExceptioncases.
ModOnboardingService.ScaffoldDraftProfileAsyncthrowsInvalidDataExceptionfor emptyDraftProfileId, null/emptyDisplayName, and missingLaunchSamples. A few[Theory]/[InlineData]cases covering these would harden the contract, especially since the service docs call these out explicitly.♻️ Sketch for additional negative-path tests
+ [Theory] + [InlineData("", "My Mod", "base_swfoc")] // empty DraftProfileId + [InlineData(" ", "My Mod", "base_swfoc")] // whitespace-only + [InlineData("valid_id", "", "base_swfoc")] // empty DisplayName + public async Task ScaffoldDraftProfileAsync_InvalidInput_ShouldThrow( + string draftId, string displayName, string baseId) + { + // minimal setup omitted for brevity — reuse tempRoot pattern + var request = new ModOnboardingRequest( + DraftProfileId: draftId, + DisplayName: displayName, + BaseProfileId: baseId, + LaunchSamples: Array.Empty<ModLaunchSample>(), // also covers empty samples + ProfileAliases: Array.Empty<string>(), + NamespaceRoot: "custom"); + + var act = async () => await service.ScaffoldDraftProfileAsync(request); + await act.Should().ThrowAsync<InvalidDataException>(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/SwfocTrainer.Tests/Profiles/ModOnboardingServiceTests.cs` around lines 11 - 12, Add negative-path tests for ModOnboardingService.ScaffoldDraftProfileAsync by creating a new [Theory] in ModOnboardingServiceTests (or ModOnboardingServiceTests.cs) with [InlineData] rows that cover empty DraftProfileId, null and empty DisplayName, and missing/empty LaunchSamples; for each input construct the request payload that triggers the invalid condition and assert Assert.ThrowsAsync<InvalidDataException>(() => service.ScaffoldDraftProfileAsync(...)) to verify the method throws the expected exception for those cases.docs/RELEASE_RUNBOOK.md (1)
37-37: Document or reference the GitHub tag protection configuration backing this immutability claim."Tags for published releases are immutable by policy" is a correctness guarantee, but it only holds if tag protection rules are enforced in the GitHub repository settings. Consider linking to the repository's branch/tag protection configuration or noting the required setting (e.g., "Protected tags:
v*") so operators can verify or restore the enforcement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/RELEASE_RUNBOOK.md` at line 37, Update the sentence "Tags for published releases are immutable by policy" to cite or link the concrete GitHub tag protection settings that enforce this (or add a short note describing the required protection), e.g., reference the repository's tag protection rules or include the required rule pattern and options (for example "Protected tags: v*; prevent deletions and force-pushes"), and add a pointer or checklist so operators can verify/restore the enforcement in the repo settings.tests/SwfocTrainer.Tests/Profiles/ProfileUpdateServiceTransactionalTests.cs (1)
55-70:HttpClientwrapping the stub handler is not disposed.The
HttpClientat line 60 and theStubHttpMessageHandlerare bothIDisposablebut never disposed. In a test context this won't cause failures, but if the service holds onto the client, it could delay handler finalization.💡 Suggested cleanup
- var handler = new StubHttpMessageHandler(new Dictionary<string, (string ContentType, byte[] Body)> + using var handler = new StubHttpMessageHandler(new Dictionary<string, (string ContentType, byte[] Body)> { ["https://example.invalid/manifest.json"] = ("application/json", Encoding.UTF8.GetBytes(manifestJson)), [$"https://example.invalid/{profileId}.zip"] = ("application/zip", zipBytes) }); - var client = new HttpClient(handler); + using var client = new HttpClient(handler);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/SwfocTrainer.Tests/Profiles/ProfileUpdateServiceTransactionalTests.cs` around lines 55 - 70, The test creates a StubHttpMessageHandler and an HttpClient (used to construct GitHubProfileUpdateService) but never disposes them; update the test to dispose both by scoping the StubHttpMessageHandler and HttpClient in using blocks or a try/finally that calls Dispose on the handler and client after creating service (reference StubHttpMessageHandler, new HttpClient(...), and GitHubProfileUpdateService) so the handler and HttpClient are properly cleaned up when the test completes.src/SwfocTrainer.App/MainWindow.xaml (1)
437-438: Consider using a ComboBox for Base Profile to constrain input to supported profiles.The
OnboardingBaseProfileIdfield is a free-textTextBox, allowing users to type arbitrary values. Since the base profile should be one of the supported profiles (base_sweaw,base_swfoc,aotr_*,roe_*), aComboBoxbound to the existingProfilescollection (or a filtered subset) would reduce user error and keep profile compatibility explicit.💡 Suggested change
- <TextBox Grid.Column="1" Text="{Binding OnboardingBaseProfileId}" Margin="0,0,0,6"/> + <ComboBox Grid.Column="1" ItemsSource="{Binding Profiles}" SelectedItem="{Binding OnboardingBaseProfileId}" IsEditable="True" Margin="0,0,0,6"/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.App/MainWindow.xaml` around lines 437 - 438, The OnboardingBaseProfileId is currently a free-text TextBox; replace it with a non-editable ComboBox bound to the existing Profiles collection (or a filtered subset of supported profiles) so users can only pick valid profiles. Bind the ComboBox's SelectedValue (or SelectedItem) to OnboardingBaseProfileId, set DisplayMemberPath to the profile display property and SelectedValuePath to the profile id property, and ensure the DataContext exposes Profiles (or the filtered collection) used by the ComboBox; make the ComboBox non-editable to prevent arbitrary text entry.src/SwfocTrainer.Core/Models/ProfileUpdateModels.cs (1)
6-13: Clean result record design with reason codes.The records are well-structured. One observation:
InstalledPathandRestoredPathare non-nullable, which means callers must supply a value even on failure paths (e.g.,Succeeded = false). If the service always knows the intended target path, this is fine. Otherwise, consider making these nullable to avoid empty-string sentinels on failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Models/ProfileUpdateModels.cs` around lines 6 - 13, The ProfileInstallResult record currently requires InstalledPath (and similar non-nullable path properties) even on failure paths; change InstalledPath (and any other path properties that may be unknown on failure) to nullable (string?) so callers don’t need to pass empty-string sentinels when Succeeded is false; update the record declaration (ProfileInstallResult) to make InstalledPath nullable and audit related records/constructors that create ProfileInstallResult to handle null path values appropriately.src/SwfocTrainer.Core/Contracts/IModCalibrationService.cs (1)
12-17:BuildCompatibilityReportAsyncalso lacks an XML comment and has a wide parameter list.Same XML doc warning applies here. Additionally, the method takes 5 parameters — this is manageable given three are optional with defaults, but if more are added later, consider grouping into a request object.
💡 Suggested fix (XML doc)
+ /// <summary> + /// Builds a compatibility report evaluating the given profile against current session and catalog state. + /// </summary> Task<ModCompatibilityReport> BuildCompatibilityReportAsync(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Contracts/IModCalibrationService.cs` around lines 12 - 17, Add an XML documentation comment for the BuildCompatibilityReportAsync method on IModCalibrationService describing its purpose, parameters (TrainerProfile profile, AttachSession? session, DependencyValidationResult? dependencyValidation, IReadOnlyDictionary<string,IReadOnlyList<string>>? catalog, CancellationToken cancellationToken) and return value (Task<ModCompatibilityReport>), and then refactor the signature by introducing a single request/DTO (e.g., ModCompatibilityRequest) that encapsulates profile, session, dependencyValidation, and catalog to replace the multiple parameters in BuildCompatibilityReportAsync while keeping CancellationToken separate; update the interface method to accept the new request type and update any implementations accordingly.tools/validate-calibration-artifact.ps1 (2)
22-49:Add-ErrorandRequire-Fieldare duplicated across both validation scripts.Both
validate-support-bundle-manifest.ps1andvalidate-calibration-artifact.ps1define identicalAdd-ErrorandRequire-Fieldfunctions. Consider extracting them into a shared helper script (e.g.,tools/validation-helpers.ps1) and dot-sourcing it from both scripts to reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-calibration-artifact.ps1` around lines 22 - 49, The Add-Error and Require-Field functions are duplicated in both validation scripts; extract these helpers into a shared script (e.g., tools/validation-helpers.ps1) that defines Add-Error and Require-Field, then replace the duplicated definitions in validate-calibration-artifact.ps1 and validate-support-bundle-manifest.ps1 with a dot-source import of that helper. Ensure the shared script exports the same function names (Add-Error, Require-Field) and that any script-scoped variables (like $script:errors) remain accessible or are refactored so both callers work after dot-sourcing.
51-61: Redundant required-field checks: schema-driven loop + hardcoded list.Lines 51–53 iterate
$schema.requiredto validate fields, then Lines 59–61 re-validate a hardcoded list that likely overlaps with the schema. Ifschema.requiredalready coversgeneratedAtUtc,profileId,moduleFingerprint, andcandidates, the second loop is pure duplication. If they differ, the intent should be documented.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-calibration-artifact.ps1` around lines 51 - 61, The script currently performs duplicate required-field validation: it iterates $schema.required with Require-Field and then re-checks a hardcoded list ("generatedAtUtc","profileId","moduleFingerprint","candidates"). Either remove the hardcoded loop (rely on $schema.required) or explicitly merge the lists and deduplicate before calling Require-Field; for example, replace the second loop with building a combined list like ($schema.required + @(...)) | Select-Object -Unique and iterate that, and add a short comment explaining why extra hardcoded fields are included if they differ from the schema; keep the existing schemaVersion check intact.tools/validate-support-bundle-manifest.ps1 (1)
88-96:Write-Hostusage is acceptable for this CI script, but considerWrite-Informationfor broader compatibility.PSScriptAnalyzer flags
Write-Hostbecause it can't be captured or redirected. For a CI validation script that targets colored terminal output, this is a pragmatic choice. If you ever need to consume this script's output programmatically (e.g., piping to a log aggregator), switching toWrite-Informationwith-InformationAction Continuewould help. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-support-bundle-manifest.ps1` around lines 88 - 96, The script currently uses Write-Host in the exit and success paths (the block that checks $errors.Count and the final Write-Host "support bundle manifest validation passed: $ManifestPath"), which PSScriptAnalyzer flags because output can't be redirected; replace those Write-Host calls with Write-Information (and add -InformationAction Continue where appropriate) so messages can be captured programmatically while preserving colored output for interactive terminals, and keep the existing $errors iteration and exit 1 behavior the same.src/SwfocTrainer.Core/Contracts/ITelemetrySnapshotService.cs (1)
8-17: Clean contract — consider a failure reason parameter for richer diagnostics.The interface is well-structured and the method signatures are clear. One thing to consider:
RecordActioncaptures success/failure but doesn't carry a failure reason code. If downstream diagnostics or drift analysis need to distinguish why actions fail (e.g.,"signature_missing","address_stale"), adding an optionalstring? reasonCode = nullparameter would make snapshots more actionable without breaking existing callers.This is non-blocking given that the current counters already provide useful aggregate signals. Based on learnings: "Record actionable reason codes in diagnostics for runtime operations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Contracts/ITelemetrySnapshotService.cs` around lines 8 - 17, Add an optional failure reason parameter to the telemetry contract so callers can record diagnostic codes: modify the ITelemetrySnapshotService interface method RecordAction to accept an additional optional string? reasonCode = null (i.e., RecordAction(string actionId, AddressSource source, bool succeeded, string? reasonCode = null)), and update all implementing classes/methods that implement RecordAction and any internal places that construct telemetry entries or snapshots (e.g., snapshot creation logic that reads recorded actions) to capture and store the reasonCode so it is surfaced in TelemetrySnapshot and ExportSnapshotAsync outputs.src/SwfocTrainer.Profiles/Services/ModOnboardingService.cs (2)
206-226: Hardcoded blocklist inInferPathHints— consider making it configurable or a static field.The inline string literal list on Line 215 (
"steamapps","workshop","content", etc.) is easy to miss during maintenance. Moving it to aprivate static readonly HashSet<string>would improve readability and make additions more discoverable. Also, as more mods are onboarded, the blocklist may need to grow — an externalized configuration could help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Profiles/Services/ModOnboardingService.cs` around lines 206 - 226, The hardcoded blocklist in InferPathHints should be extracted from the inline token check into a single reusable static field or configurable option: create a private static readonly HashSet<string> (e.g., s_pathHintBlocklist) on the ModOnboardingService class and replace the inline pattern (the token is "steamapps" or "workshop" or ...) with a membership check like s_pathHintBlocklist.Contains(token); optionally make it configurable by wiring the set from options or configuration (e.g., IOptions or constructor parameter) so the list can be extended without code changes and update any unit tests that reference InferPathHints accordingly.
113-150: Duplicated normalization logic acrossNormalizeNamespace,NormalizeProfileId,InferAliases, andTokenizeHintInput.The pattern of
.Trim().ToLowerInvariant().Select(ch => char.IsLetterOrDigit(ch) ? ch : '_').ToArray()followed by.Trim('_')appears in at least four places (Lines 120–125, 132–137, 262–267, 283–288). Extracting a sharedSanitizeIdentifier(string input)helper would reduce duplication and ensure consistent normalization rules.♻️ Proposed helper extraction
+ private static string SanitizeIdentifier(string input) + { + var sanitized = new string(input + .Trim() + .ToLowerInvariant() + .Select(ch => char.IsLetterOrDigit(ch) ? ch : '_') + .ToArray()) + .Trim('_'); + return sanitized; + } + private static string NormalizeNamespace(string? namespaceRoot) { if (string.IsNullOrWhiteSpace(namespaceRoot)) { return "custom"; } - var sanitized = new string(namespaceRoot - .Trim() - .ToLowerInvariant() - .Select(ch => char.IsLetterOrDigit(ch) ? ch : '_') - .ToArray()) - .Trim('_'); - + var sanitized = SanitizeIdentifier(namespaceRoot); return string.IsNullOrWhiteSpace(sanitized) ? "custom" : sanitized; }Apply the same pattern to
NormalizeProfileIdandInferAliases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Profiles/Services/ModOnboardingService.cs` around lines 113 - 150, The normalization logic is duplicated across NormalizeNamespace, NormalizeProfileId, InferAliases, and TokenizeHintInput; extract a single helper method (e.g., SanitizeIdentifier(string? input)) that performs the shared steps: null/whitespace handling, Trim(), ToLowerInvariant(), replace non-alphanumeric chars with '_', then Trim('_') and return the sanitized string; then update NormalizeNamespace to call SanitizeIdentifier and return "custom" when the helper yields null/empty, update NormalizeProfileId to call SanitizeIdentifier, keep the existing InvalidDataException if sanitized is empty, and preserve the logic that ensures the returned id starts with "custom_" (adding it only if not present), and replace the same inline logic in InferAliases and TokenizeHintInput to call SanitizeIdentifier so all normalization is centralized and consistent.tests/SwfocTrainer.Tests/Core/SupportBundleServiceTests.cs (1)
49-56: Consider validating manifest content, not just file existence.The test asserts that the bundle and manifest files exist and that the ZIP contains expected entry names, but doesn't validate the manifest JSON structure (e.g.,
schemaVersion,includedFilesentries,profileId). Given the manifest consistency bug identified inSupportBundleService.cs, a content assertion here would have caught it. Adding a quick deserialization + field check would strengthen this test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/SwfocTrainer.Tests/Core/SupportBundleServiceTests.cs` around lines 49 - 56, Extend the test to open and deserialize the manifest file at result.ManifestPath and assert its important fields (e.g., schemaVersion, includedFiles array contents, and profileId) rather than only checking file existence; for example, read File.ReadAllText(result.ManifestPath), parse with System.Text.Json/JsonDocument or Newtonsoft.Json into a dynamic/typed object, then assert schemaVersion equals the expected value, includedFiles contains the expected entries (like "runtime-snapshot.json" / telemetry / runs), and profileId is present and matches the profile used when creating the bundle (this will surface the manifest consistency bug in SupportBundleService.cs).src/SwfocTrainer.Core/Services/TelemetrySnapshotService.cs (1)
44-73:UnresolvedRatesemantics may be surprising — consider a doc comment.Line 57 defines "unresolved" as actions that appear in failures but never in successes. This means an action that succeeded once and then failed 100 times is considered "resolved" (not counted in UnresolvedRate). This is a valid definition but may surprise consumers expecting it to reflect recent/current state. A brief doc comment on the
TelemetrySnapshot.UnresolvedRateproperty explaining this would help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Services/TelemetrySnapshotService.cs` around lines 44 - 73, The UnresolvedRate calculation in CreateSnapshot (unresolvedCount/unresolvedRate) treats an action as "unresolved" only if it appears in failure counters but never in success counters; update the TelemetrySnapshot.UnresolvedRate property with a brief XML doc comment that states this exact semantics (i.e., only-actions seen solely in failures are counted, so an action that has any success entries is excluded even if it has many failures), and optionally note that this is a lifetime-snapshot measure rather than a recent/current-state indicator so consumers are not surprised; place the comment on the TelemetrySnapshot.UnresolvedRate property to make the behavior clear.src/SwfocTrainer.Core/Services/ModCalibrationService.cs (2)
220-227:SanitizeFileTokencan return an empty string, producing a malformed artifact filename.A
ProfileIdconsisting entirely of special characters (e.g.,"---") sanitizes to"", yieldingcalibration--{timestamp}.json. Add a fallback:🛠️ Proposed fix
private static string SanitizeFileToken(string value) { - return new string(value + var result = new string(value .ToLowerInvariant() .Select(ch => char.IsLetterOrDigit(ch) ? ch : '_') .ToArray()) .Trim('_'); + return string.IsNullOrEmpty(result) ? "unknown" : result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Services/ModCalibrationService.cs` around lines 220 - 227, SanitizeFileToken can return an empty string (e.g., ProfileId of "---"), producing malformed filenames; update the SanitizeFileToken method so after sanitizing it checks for empty or all-underscores and returns a safe fallback (e.g., "unknown" or a deterministic fallback based on the original input) instead of "", ensuring callers that build filenames (like those that produce "calibration-{token}-{timestamp}.json") always get a non-empty token; implement this check and return the fallback string when the sanitized result is empty.
103-109:cancellationTokenis accepted but never observed.Since the method body is entirely synchronous today the token is unused. Either suppress the parameter or document the intent:
🛠️ Proposed fix
public Task<ModCompatibilityReport> BuildCompatibilityReportAsync( TrainerProfile profile, AttachSession? session, DependencyValidationResult? dependencyValidation = null, IReadOnlyDictionary<string, IReadOnlyList<string>>? catalog = null, - CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default) // reserved for future async evaluationAlternatively, add
_ = cancellationToken;to make the omission explicit until the method gains real async work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SwfocTrainer.Core/Services/ModCalibrationService.cs` around lines 103 - 109, The BuildCompatibilityReportAsync method accepts a CancellationToken parameter that is never observed; update the method to either explicitly acknowledge the unused token (e.g., add a single line like "_ = cancellationToken;" at the top of BuildCompatibilityReportAsync) or remove the parameter if cancellation is not intended, or document why it is currently unused—choose one approach and apply it to the BuildCompatibilityReportAsync signature/body so the unused parameter is no longer silent.
| $args = @( | ||
| "release", "create", $tag, $zipPath, $checksumPath, | ||
| "--title", "SWFOC Trainer $tag", | ||
| "--notes-file", $notesPath, | ||
| "--verify-tag" | ||
| ) | ||
|
|
||
| $manualPrerelease = "${{ inputs.prerelease }}" -eq "true" | ||
| if ($manualPrerelease -and "${{ github.ref_type }}" -ne "tag") { | ||
| $args += "--prerelease" | ||
| } | ||
|
|
||
| gh @args |
There was a problem hiding this comment.
$args is a PowerShell automatic variable — rename to avoid shadowing.
$args is a reserved automatic variable in PowerShell. Assigning to it shadows the built-in and can produce subtle, hard-to-diagnose issues. Rename it (e.g., $releaseArgs).
🐛 Proposed fix
- $args = @(
+ $releaseArgs = @(
"release", "create", $tag, $zipPath, $checksumPath,
"--title", "SWFOC Trainer $tag",
"--notes-file", $notesPath,
"--verify-tag"
)
$manualPrerelease = "${{ inputs.prerelease }}" -eq "true"
if ($manualPrerelease -and "${{ github.ref_type }}" -ne "tag") {
- $args += "--prerelease"
+ $releaseArgs += "--prerelease"
}
- gh `@args`
+ gh `@releaseArgs`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $args = @( | |
| "release", "create", $tag, $zipPath, $checksumPath, | |
| "--title", "SWFOC Trainer $tag", | |
| "--notes-file", $notesPath, | |
| "--verify-tag" | |
| ) | |
| $manualPrerelease = "${{ inputs.prerelease }}" -eq "true" | |
| if ($manualPrerelease -and "${{ github.ref_type }}" -ne "tag") { | |
| $args += "--prerelease" | |
| } | |
| gh @args | |
| $releaseArgs = @( | |
| "release", "create", $tag, $zipPath, $checksumPath, | |
| "--title", "SWFOC Trainer $tag", | |
| "--notes-file", $notesPath, | |
| "--verify-tag" | |
| ) | |
| $manualPrerelease = "${{ inputs.prerelease }}" -eq "true" | |
| if ($manualPrerelease -and "${{ github.ref_type }}" -ne "tag") { | |
| $releaseArgs += "--prerelease" | |
| } | |
| gh `@releaseArgs` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-portable.yml around lines 96 - 108, The script is
shadowing PowerShell's automatic $args variable; rename the array variable (for
example to $releaseArgs) and update all references: change the declaration that
builds "release", "create", $tag, $zipPath, $checksumPath, "--title",
"--notes-file", "--verify-tag" to the new name, replace the $args +=
"--prerelease" usage (and any other $args manipulations) with $releaseArgs, and
invoke the GitHub CLI with gh `@releaseArgs` instead of gh `@args` so the built-in
automatic variable is no longer overwritten.
|
|
||
| ## Calibration Artifacts | ||
| Artifacts are written to: | ||
| - `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` (default app path) |
There was a problem hiding this comment.
Double backslash renders literally in Markdown inline code spans.
In a backtick inline code span, backslashes are literal — \\ renders as \\, not \. The path will display with double backslashes in the rendered runbook.
📝 Proposed fix
-- `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` (default app path)
+- `%LOCALAPPDATA%\SwfocTrainer\support\calibration` (default app path)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` (default app path) | |
| - `%LOCALAPPDATA%\SwfocTrainer\support\calibration` (default app path) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/MOD_ONBOARDING_RUNBOOK.md` at line 27, The inline code span currently
contains `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` which renders with
doubled backslashes; update the runbook so the displayed path uses single
backslashes by replacing that inline code with
`%LOCALAPPDATA%\SwfocTrainer\support\calibration` or convert the path into a
fenced code block (triple backticks) containing
%LOCALAPPDATA%\SwfocTrainer\support\calibration to ensure it renders with single
backslashes; locate the literal
`%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` string in the document and
make the replacement.
| var launchLines = OnboardingLaunchSample | ||
| .Split(Environment.NewLine, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries) | ||
| .ToArray(); |
There was a problem hiding this comment.
Environment.NewLine-only split drops \n-terminated input (e.g., clipboard pastes from terminal/VS Code).
WPF's TextBox emits \r\n for typed newlines, but pasted text often uses bare \n. With Environment.NewLine as the sole separator, a multi-line paste would be treated as one launch sample.
🛠️ Proposed fix
-var launchLines = OnboardingLaunchSample
- .Split(Environment.NewLine, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries)
- .ToArray();
+var launchLines = OnboardingLaunchSample
+ .Split(new[] { "\r\n", "\r", "\n" }, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries)
+ .ToArray();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var launchLines = OnboardingLaunchSample | |
| .Split(Environment.NewLine, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries) | |
| .ToArray(); | |
| var launchLines = OnboardingLaunchSample | |
| .Split(new[] { "\r\n", "\r", "\n" }, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries) | |
| .ToArray(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SwfocTrainer.App/ViewModels/MainViewModel.cs` around lines 1221 - 1223,
The split using Environment.NewLine on OnboardingLaunchSample (producing
launchLines) misses inputs that use bare '\n' (common when pasting), so change
the Split call to use explicit line separators like new[] { "\r\n", "\n", "\r" }
with StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries so
both CRLF and LF-only (and stray CR) line endings are handled and each pasted
line becomes a separate launchLines entry.
| private async Task BuildCompatibilityReportAsync() | ||
| { | ||
| var profileId = SelectedProfileId ?? OnboardingDraftProfileId; | ||
| var profile = await _profiles.ResolveInheritedProfileAsync(profileId); | ||
| var report = await _modCalibration.BuildCompatibilityReportAsync( | ||
| profile, | ||
| _runtime.CurrentSession); | ||
|
|
||
| ModCompatibilityRows.Clear(); | ||
| foreach (var action in report.Actions) | ||
| { | ||
| ModCompatibilityRows.Add($"{action.ActionId} | {action.State} | {action.ReasonCode} | {action.Confidence:0.00}"); | ||
| } | ||
|
|
||
| ModCompatibilitySummary = $"promotionReady={report.PromotionReady} dependency={report.DependencyStatus} unresolvedCritical={report.UnresolvedCriticalSymbols}"; | ||
| Status = $"Compatibility report generated for {profileId}"; | ||
| } |
There was a problem hiding this comment.
Compatibility report omits catalog data, producing reliability states inconsistent with the reliability panel.
RefreshActionReliabilityAsync (line 1742) loads the catalog before scoring, but BuildCompatibilityReportAsync calls the service without it. Since _actionReliability.Evaluate uses the catalog to score catalog-gated actions, omitting it can flip action states between Healthy and Unavailable, leading operators to an incorrect PromotionReady verdict.
🛠️ Proposed fix
private async Task BuildCompatibilityReportAsync()
{
var profileId = SelectedProfileId ?? OnboardingDraftProfileId;
var profile = await _profiles.ResolveInheritedProfileAsync(profileId);
+
+ IReadOnlyDictionary<string, IReadOnlyList<string>>? catalog = null;
+ try
+ {
+ catalog = await _catalog.LoadCatalogAsync(profileId);
+ }
+ catch
+ {
+ // Catalog is optional for reliability scoring.
+ }
+
var report = await _modCalibration.BuildCompatibilityReportAsync(
profile,
- _runtime.CurrentSession);
+ _runtime.CurrentSession,
+ catalog: catalog);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async Task BuildCompatibilityReportAsync() | |
| { | |
| var profileId = SelectedProfileId ?? OnboardingDraftProfileId; | |
| var profile = await _profiles.ResolveInheritedProfileAsync(profileId); | |
| var report = await _modCalibration.BuildCompatibilityReportAsync( | |
| profile, | |
| _runtime.CurrentSession); | |
| ModCompatibilityRows.Clear(); | |
| foreach (var action in report.Actions) | |
| { | |
| ModCompatibilityRows.Add($"{action.ActionId} | {action.State} | {action.ReasonCode} | {action.Confidence:0.00}"); | |
| } | |
| ModCompatibilitySummary = $"promotionReady={report.PromotionReady} dependency={report.DependencyStatus} unresolvedCritical={report.UnresolvedCriticalSymbols}"; | |
| Status = $"Compatibility report generated for {profileId}"; | |
| } | |
| private async Task BuildCompatibilityReportAsync() | |
| { | |
| var profileId = SelectedProfileId ?? OnboardingDraftProfileId; | |
| var profile = await _profiles.ResolveInheritedProfileAsync(profileId); | |
| IReadOnlyDictionary<string, IReadOnlyList<string>>? catalog = null; | |
| try | |
| { | |
| catalog = await _catalog.LoadCatalogAsync(profileId); | |
| } | |
| catch | |
| { | |
| // Catalog is optional for reliability scoring. | |
| } | |
| var report = await _modCalibration.BuildCompatibilityReportAsync( | |
| profile, | |
| _runtime.CurrentSession, | |
| catalog: catalog); | |
| ModCompatibilityRows.Clear(); | |
| foreach (var action in report.Actions) | |
| { | |
| ModCompatibilityRows.Add($"{action.ActionId} | {action.State} | {action.ReasonCode} | {action.Confidence:0.00}"); | |
| } | |
| ModCompatibilitySummary = $"promotionReady={report.PromotionReady} dependency={report.DependencyStatus} unresolvedCritical={report.UnresolvedCriticalSymbols}"; | |
| Status = $"Compatibility report generated for {profileId}"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SwfocTrainer.App/ViewModels/MainViewModel.cs` around lines 1265 - 1281,
BuildCompatibilityReportAsync calls
_modCalibration.BuildCompatibilityReportAsync without first loading the catalog,
so catalog-gated actions are scored inconsistently versus
RefreshActionReliabilityAsync which loads the catalog; before calling
_modCalibration.BuildCompatibilityReportAsync in BuildCompatibilityReportAsync,
load the catalog into the action reliability component (the same call/sequence
used in RefreshActionReliabilityAsync, e.g. await
_actionReliability.LoadCatalogAsync(...) using _runtime.CurrentSession/catalog)
so that _actionReliability.Evaluate has the catalog available and
report.PromotionReady and action states match the reliability panel.
| private async Task ExportTelemetrySnapshotAsync() | ||
| { | ||
| var telemetryDir = Path.Combine(SupportBundleOutputDirectory, "telemetry"); | ||
| Directory.CreateDirectory(telemetryDir); | ||
| var path = await _telemetry.ExportSnapshotAsync(telemetryDir); | ||
| OpsArtifactSummary = path; | ||
| Status = $"Telemetry snapshot exported: {path}"; | ||
| } |
There was a problem hiding this comment.
ExportTelemetrySnapshotAsync always reports success — inconsistent with ExportSupportBundleAsync.
ExportSupportBundleAsync guards on result.Succeeded; this method blindly formats path into a success message even when the path is null/empty. Add a failure check consistent with the bundle export pattern:
🛠️ Proposed fix
private async Task ExportTelemetrySnapshotAsync()
{
var telemetryDir = Path.Combine(SupportBundleOutputDirectory, "telemetry");
Directory.CreateDirectory(telemetryDir);
var path = await _telemetry.ExportSnapshotAsync(telemetryDir);
OpsArtifactSummary = path;
- Status = $"Telemetry snapshot exported: {path}";
+ Status = !string.IsNullOrWhiteSpace(path)
+ ? $"Telemetry snapshot exported: {path}"
+ : "Telemetry snapshot export failed.";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async Task ExportTelemetrySnapshotAsync() | |
| { | |
| var telemetryDir = Path.Combine(SupportBundleOutputDirectory, "telemetry"); | |
| Directory.CreateDirectory(telemetryDir); | |
| var path = await _telemetry.ExportSnapshotAsync(telemetryDir); | |
| OpsArtifactSummary = path; | |
| Status = $"Telemetry snapshot exported: {path}"; | |
| } | |
| private async Task ExportTelemetrySnapshotAsync() | |
| { | |
| var telemetryDir = Path.Combine(SupportBundleOutputDirectory, "telemetry"); | |
| Directory.CreateDirectory(telemetryDir); | |
| var path = await _telemetry.ExportSnapshotAsync(telemetryDir); | |
| OpsArtifactSummary = path; | |
| Status = !string.IsNullOrWhiteSpace(path) | |
| ? $"Telemetry snapshot exported: {path}" | |
| : "Telemetry snapshot export failed."; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SwfocTrainer.App/ViewModels/MainViewModel.cs` around lines 1296 - 1303,
ExportTelemetrySnapshotAsync currently always sets OpsArtifactSummary and a
success Status even when ExportSnapshotAsync fails/returns null or empty; change
it to mirror the ExportSupportBundleAsync pattern by checking the returned path
(e.g., if string.IsNullOrEmpty(path) or a returned result.Succeeded equivalent).
If the export failed, set Status to a clear failure message and avoid assigning
OpsArtifactSummary (or clear it), otherwise assign OpsArtifactSummary = path and
set the success Status. Use the existing method name
ExportTelemetrySnapshotAsync and the variables path, OpsArtifactSummary and
Status to locate and implement the conditional handling.
| [PSCustomObject]@{ | ||
| FullPath = $_.FullName | ||
| RelativePath = $_.FullName.Substring((Resolve-Path $Root).Path.Length).TrimStart('\\', '/') | ||
| RelativePath = ($_.FullName.Substring((Resolve-Path $Root).Path.Length) -replace '^[\\/]+', '') |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for test files, fixtures, or CI references to compare-visual-pack
echo "=== Searching for compare-visual-pack references outside the script itself ==="
rg -i "compare-visual-pack" --type yaml --type powershell -g "!tools/compare-visual-pack.ps1" 2>/dev/null || echo "No matches in yaml/ps1"
echo -e "\n=== Checking GitHub workflows ==="
find .github -type f \( -name "*.yml" -o -name "*.yaml" \) -exec grep -l "compare-visual-pack" {} \; 2>/dev/null || echo "No matches in workflows"
echo -e "\n=== Checking test directories ==="
find tests -type f \( -name "*.ps1" -o -name "*.js" -o -name "*.ts" \) -exec grep -l "compare-visual-pack" {} \; 2>/dev/null || echo "No matches in tests"
echo -e "\n=== Listing all PowerShell test files ==="
find tests -type f -name "*.ps1" 2>/dev/null | head -20 || echo "No test.ps1 files found"
echo -e "\n=== Checking tools directory structure ==="
ls -la tools/ 2>/dev/null | head -20Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 1745
🏁 Script executed:
#!/bin/bash
# Verify the content and structure of compare-visual-pack.ps1
echo "=== compare-visual-pack.ps1 context (lines 20-40) ==="
sed -n '20,40p' tools/compare-visual-pack.ps1
echo -e "\n=== Looking for test invocations or fixtures ==="
find . -type f \( -name "*.md" -o -name "*.ps1" -o -name "*.yml" \) -exec grep -l "compare.visual" {} \; 2>/dev/null || echo "No matches"Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 909
🏁 Script executed:
#!/bin/bash
# Check if there's a test script or smoke test framework in the repo
echo "=== Looking for test runners or fixture frameworks ==="
find . -path ./node_modules -prune -o -type f \( -name "*test*.ps1" -o -name "*smoke*.ps1" -o -name "*fixture*.ps1" \) -print 2>/dev/null | head -20
echo -e "\n=== Checking for CI test runners ==="
find .github -type f -name "*.yml" -o -name "*.yaml" | xargs grep -l "test\|Test" 2>/dev/null | head -5Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 325
🏁 Script executed:
#!/bin/bash
# Inspect the visual-audit.yml workflow to understand how compare-visual-pack.ps1 is invoked
echo "=== visual-audit.yml workflow content ==="
cat .github/workflows/visual-audit.yml
echo -e "\n=== Checking VISUAL_AUDIT_RUNBOOK.md for details ==="
head -60 ./docs/VISUAL_AUDIT_RUNBOOK.mdRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 3497
🏁 Script executed:
#!/bin/bash
# Check if this is a new script or an existing one in the git history
echo "=== Git history of compare-visual-pack.ps1 ==="
git log --oneline -n 20 -- tools/compare-visual-pack.ps1 2>/dev/null || echo "No git history available"
echo -e "\n=== Checking compare-visual-pack.ps1 for any inline test/validation logic ==="
cat tools/compare-visual-pack.ps1 | grep -i "test\|fixture\|validation\|example" || echo "No inline test comments found"
echo -e "\n=== Full compare-visual-pack.ps1 to understand context ==="
cat tools/compare-visual-pack.ps1Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 3821
🏁 Script executed:
#!/bin/bash
# Verify if tools/AGENTS.md or any documentation specifies what "deterministic fixture or smoke command" means for this project
echo "=== Checking tools/AGENTS.md for guidance definition ==="
cat tools/AGENTS.md
echo -e "\n=== Checking if there's a fixtures directory structure in tools/ ==="
ls -la tools/fixtures/ 2>/dev/null || echo "No fixtures/ directory in tools/"
echo -e "\n=== Checking if any tools have documented smoke/test invocations ==="
head -30 tools/*.ps1 | grep -E "^(#|param|\.SYNOPSIS|\.EXAMPLE)" | head -30Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 1767
🏁 Script executed:
#!/bin/bash
# Check what's in the visual fixtures directory
echo "=== Contents of tools/fixtures/visual/ ==="
ls -la tools/fixtures/visual/
echo -e "\n=== Checking for any compare-visual-pack test or fixture references ==="
find tools/fixtures -type f -exec grep -l "compare" {} \;
echo -e "\n=== Checking other tools for how they document smoke commands ==="
grep -r "Example\|example\|smoke\|test" tools/*.ps1 2>/dev/null | head -20Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 2028
Add deterministic fixture or smoke command for compare-visual-pack.ps1 script changes.
Per project coding guidelines ("Every tooling script change must include a deterministic fixture or smoke command"), this new script requires a standalone test fixture or documented invocation command. While visual-audit.yml provides CI integration, this does not fulfill the requirement for a portable, reproducible fixture that developers can execute locally.
Add a fixture under tools/fixtures/visual/ with sample baseline and candidate images, or document an explicit smoke command showing how to verify the script's behavior independently (e.g., instructions in a comment or separate fixture invocation script).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/compare-visual-pack.ps1` at line 28, Add a deterministic smoke fixture
or invocation for the new tools/compare-visual-pack.ps1 script: create a
tools/fixtures/visual/ directory containing a minimal, checked-in baseline and
candidate image pair plus a README or small script that demonstrates the exact
PowerShell invocation (e.g., calling compare-visual-pack.ps1 with those files
and expected exit code), or alternatively add a clear commented "smoke command"
example at the top of compare-visual-pack.ps1 showing the concrete invocation
and expected output; ensure the fixture names and invocation match the script's
parameter names so developers can run the test locally without CI (reference:
compare-visual-pack.ps1 and visual-audit.yml).
| "generatedAtUtc": { | ||
| "type": "string", | ||
| "format": "date-time" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
JSON Schema 2020-12 format annotation vs format assertion validation default behavior
💡 Result:
In JSON Schema 2020-12, format was split into two vocabularies with different semantics:
-
format-annotation:formatis annotation-only (it communicates intent/semantics but does not make validation fail). This vocabulary is required by the default 2020-12 meta-schema, so this is the default behavior. (json-schema.org)- Implementations may offer an option to also validate formats here, but that option must be disabled by default (i.e., default = do not assert on
format). (json-schema.org)
- Implementations may offer an option to also validate formats here, but that option must be disabled by default (i.e., default = do not assert on
-
format-assertion:formatis a true assertion keyword (invalid formats must fail validation). This vocabulary is optional in the spec, and when a schema/dialect declares it as required (via$vocabulary: { …: true }), an implementation must perform format validation for the defined formats (or refuse to process the schema if it can’t). (json-schema.org)
So, by default in 2020-12, format should not cause validation failures unless you (a) use a dialect/meta-schema that enables format-assertion, or (b) explicitly turn on an implementation-specific “validate formats” option (which must default to off). (json-schema.org)
Citations:
- 1: https://json-schema.org/draft/2020-12/release-notes?utm_source=openai
- 2: https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-00?utm_source=openai
- 3: https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-00?utm_source=openai
- 4: https://json-schema.org/draft/2020-12/release-notes?utm_source=openai
🌐 Web query:
NJsonSchema JsonSchema.Net format "date-time" validation default 2020-12
💡 Result:
In both NJsonSchema and JsonSchema.Net, "format": "date-time" is intended to validate an RFC 3339 date-time string (i.e., a full date and time, with a T separator and a timezone like Z or an offset). "2020-12" is just year-month, so it will fail "date-time" validation. Valid examples look like 2002-10-02T15:00:00Z. [1], [2]
If you want to keep a value like "2020-12":
- It is not a
"date-time"(or even a"date";"date"expectsYYYY-MM-DD). [1] - Use a custom constraint instead, e.g. a regex:
"pattern": "^\\d{4}-\\d{2}$", or register a custom format validator (library-specific). [3]
Also: “2020-12” is commonly seen as the JSON Schema draft name (Draft 2020-12), which is unrelated to the "date-time" string format. [4]
Sources
[1] learnjsonschema.com – format (2020-12) / date-time listed under 2020-12 validation
[2] W3C validator docs – RFC 3339 date-time explanation + valid examples
[3] JsonSchema.Net docs – format validation is configurable and supports date-time
[4] JsonSchema.Net docs – supported drafts include Draft 2020-12
🌐 Web query:
JsonSchema.Net .NET PowerShell format validation does it enforce date-time
💡 Result:
JsonSchema.Net can enforce "format": "date-time" (it has a built-in date-time format validator), but it does not necessarily enforce it by default because format is treated as an annotation unless you opt into “format-as-assertion” behavior. (docs.json-everything.net)
How to make it enforce date-time
Enable format validation in evaluation options:
- Current docs: set
EvaluationOptions.RequireFormatValidation = truewhen callingJsonSchema.Evaluate(). (docs.json-everything.net) - Release note (older naming / change): a library option named
ValidationOptions.ValidateFormatwas added so you can configure whetherformatis validated, and the default behavior was aligned soformatis not validated by default. (docs.json-everything.net)
So, in PowerShell/.NET: if you’re just evaluating a schema without enabling that option, "format": "date-time" may not be enforced; once enabled, it will validate (using the library’s built-in date-time checker). (docs.json-everything.net)
Citations:
- 1: https://docs.json-everything.net/schema/basics/
- 2: https://docs.json-everything.net/schema/basics/
- 3: https://docs.json-everything.net/rn-json-schema/?utm_source=openai
- 4: https://docs.json-everything.net/schema/basics/
🏁 Script executed:
cat tools/schemas/support-bundle-manifest.schema.json | head -30Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 766
format: "date-time" does not enforce validation under JSON Schema 2020-12 without explicit opt-in.
In JSON Schema 2020-12, the "format-annotation" vocabulary — used in the default meta-schema — treats format as an annotation only, not an assertion. By default, this keyword does not perform validation. JsonSchema.Net, commonly used in .NET/PowerShell tooling, aligns with this specification: it requires explicit configuration (e.g., EvaluationOptions.RequireFormatValidation = true) to validate format constraints, and this validation is disabled by default.
Without a pattern constraint or explicit validator enablement, an invalid string such as "not-a-date" will pass schema validation against this field. To ensure robust, interoperable validation across tools and implementations, combine format with a pattern constraint:
🛡️ Proposed fix
"generatedAtUtc": {
"type": "string",
- "format": "date-time"
+ "format": "date-time",
+ "pattern": "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(\\.\\d+)?(Z|[+-]\\d{2}:\\d{2})$"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "generatedAtUtc": { | |
| "type": "string", | |
| "format": "date-time" | |
| }, | |
| "generatedAtUtc": { | |
| "type": "string", | |
| "format": "date-time", | |
| "pattern": "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(\\.\\d+)?(Z|[+-]\\d{2}:\\d{2})$" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/schemas/support-bundle-manifest.schema.json` around lines 18 - 21, The
"generatedAtUtc" property currently uses "format": "date-time" which is only an
annotation under JSON Schema 2020-12 and won't enforce validation by default;
update the "generatedAtUtc" schema (the generatedAtUtc object) to include a
validating "pattern" regex that enforces an RFC3339/ISO8601 datetime (e.g. a
regex covering YYYY-MM-DDTHH:MM:SS with optional fractional seconds and a Z or
±HH:MM offset) so tools that don't enable format validation still reject invalid
strings; keep "type": "string" and retain "format": "date-time" as documentation
while adding the pattern constraint to ensure interoperability.
| for ($i = 0; $i -lt $candidates.Count; $i++) { | ||
| $candidate = $candidates[$i] | ||
| foreach ($required in @("symbol", "source", "healthStatus", "confidence")) { | ||
| Require-Field -Object $candidate -Field $required | ||
| } | ||
|
|
||
| $confidence = [double]$candidate.confidence | ||
| if ($confidence -lt 0 -or $confidence -gt 1) { | ||
| Add-Error "candidates[$i].confidence must be between 0 and 1" | ||
| } | ||
| } |
There was a problem hiding this comment.
Unsafe cast [double]$candidate.confidence can throw an unhandled exception.
If confidence is a non-numeric value (e.g., "high" or $null), the [double] cast on Line 74 will throw a terminating error before reaching the range check. Since $ErrorActionPreference = "Stop", this terminates the entire script with a raw PowerShell error rather than a clean validation message.
🛡️ Proposed defensive cast
- $confidence = [double]$candidate.confidence
- if ($confidence -lt 0 -or $confidence -gt 1) {
- Add-Error "candidates[$i].confidence must be between 0 and 1"
- }
+ $rawConfidence = $candidate.confidence
+ $confidence = 0.0
+ if ($null -eq $rawConfidence -or -not [double]::TryParse([string]$rawConfidence, [ref]$confidence)) {
+ Add-Error "candidates[$i].confidence must be a number"
+ } elseif ($confidence -lt 0 -or $confidence -gt 1) {
+ Add-Error "candidates[$i].confidence must be between 0 and 1"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for ($i = 0; $i -lt $candidates.Count; $i++) { | |
| $candidate = $candidates[$i] | |
| foreach ($required in @("symbol", "source", "healthStatus", "confidence")) { | |
| Require-Field -Object $candidate -Field $required | |
| } | |
| $confidence = [double]$candidate.confidence | |
| if ($confidence -lt 0 -or $confidence -gt 1) { | |
| Add-Error "candidates[$i].confidence must be between 0 and 1" | |
| } | |
| } | |
| for ($i = 0; $i -lt $candidates.Count; $i++) { | |
| $candidate = $candidates[$i] | |
| foreach ($required in @("symbol", "source", "healthStatus", "confidence")) { | |
| Require-Field -Object $candidate -Field $required | |
| } | |
| $rawConfidence = $candidate.confidence | |
| $confidence = 0.0 | |
| if ($null -eq $rawConfidence -or -not [double]::TryParse([string]$rawConfidence, [ref]$confidence)) { | |
| Add-Error "candidates[$i].confidence must be a number" | |
| } elseif ($confidence -lt 0 -or $confidence -gt 1) { | |
| Add-Error "candidates[$i].confidence must be between 0 and 1" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/validate-calibration-artifact.ps1` around lines 68 - 78, The cast
[double]$candidate.confidence can throw for non-numeric values; change the logic
in the candidates loop (where Require-Field is called and Add-Error is used) to
defensively attempt conversion (e.g., use the -as operator or TryParse
semantics) so that non-numeric or null confidences produce a controlled
Add-Error message (like "candidates[$i].confidence must be a number between 0
and 1") instead of letting a terminating exception bubble up, then only perform
the range check when the conversion succeeds.
There was a problem hiding this comment.
Strict mode check for "manifest.json" will always fail with current SupportBundleService output.
As noted in the review of SupportBundleService.cs, the manifest's includedFiles is serialized before "manifest.json" is appended to the list. This means the strict-mode check on Line 78 will always report a failure. Once the service-side ordering is fixed (adding "manifest.json" before serialization), this check will work correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/validate-support-bundle-manifest.ps1` around lines 77 - 86, The
strict-mode manifest check currently requires "manifest.json" be present in
$included but SupportBundleService serializes includedFiles before appending
manifest.json, causing a false failure; modify the strict-mode block (the
$Strict conditional that inspects $included and calls Add-Error) to stop
requiring "manifest.json" in $included (remove or disable the Add-Error call
that checks for "manifest.json") and retain the runtime-snapshot check, or
alternatively gate the manifest.json requirement behind a feature flag/boolean
so it only runs once the service-side ordering is fixed.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
| - Release packages include checksum files for integrity verification. | ||
| - Do not consume unsigned/unverified artifacts from non-release channels for production usage. | ||
|
|
||
| ## Rollback Procedure |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Rollback Procedure | |
| ## Rollback Procedure | |
| pwsh ./tools/validate-calibration-artifact.ps1 -ArtifactPath <artifact.json> -SchemaPath tools/schemas/calibration-artifact.schema.json -Strict | ||
| ``` | ||
|
|
||
| ## Compatibility Report Interpretation |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Compatibility Report Interpretation | |
| ## Compatibility Report Interpretation | |
|
|
||
| ## Dry-Run Evidence Template | ||
| Use this in issue/PR comments: | ||
| - Workflow run URL |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - Workflow run URL | |
| - Workflow run URL |
| - `localPathHints` from `MODPATH=...` and process/path tokens | ||
| - `profileAliases` from draft id + display name (+ optional aliases) | ||
|
|
||
| ## Calibration Artifacts |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Calibration Artifacts | |
| ## Calibration Artifacts | |
| - `publish_release=false`: package + artifact only (no GitHub Release publish). | ||
| - `publish_release=true`: publish/update release for `tag_name`. | ||
|
|
||
| ## Artifact Verification |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Artifact Verification | |
| ## Artifact Verification | |
| ## Goal | ||
| Generate a draft custom profile, export calibration evidence, and compute a promotion readiness report. | ||
|
|
||
| ## UI Flow (Profiles & Updates tab) |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## UI Flow (Profiles & Updates tab) | |
| ## UI Flow (Profiles & Updates tab) | |
| - Runtime actions are profile-gated and may be unavailable when dependency markers are unresolved. | ||
| - Some features remain behind reliability gates to prevent unsafe writes. | ||
|
|
||
| ## Verification |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Verification | |
| ## Verification | |
| 5. Click `Export Calibration Artifact`. | ||
| 6. Click `Build Compatibility Report`. | ||
|
|
||
| ## Draft Profile Output |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Draft Profile Output | |
| ## Draft Profile Output | |
| - Some features remain behind reliability gates to prevent unsafe writes. | ||
|
|
||
| ## Verification | ||
| - Validate checksum before use: |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - Validate checksum before use: | |
| - Validate checksum before use: |
| ## Highlights | ||
| - Summary of key changes in this release. | ||
|
|
||
| ## Included Artifacts |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Included Artifacts | |
| ## Included Artifacts | |
| ``` | ||
|
|
||
| This triggers `.github/workflows/release-portable.yml` to: | ||
| - build release binaries, |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - build release binaries, | |
| - build release binaries, |
| - `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` (default app path) | ||
|
|
||
| Schema contract: | ||
| - `tools/schemas/calibration-artifact.schema.json` |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `tools/schemas/calibration-artifact.schema.json` | |
| - `tools/schemas/calibration-artifact.schema.json` |
| The computed hash must match the `.sha256` file. | ||
|
|
||
| ## Security Controls | ||
| - Tags for published releases are immutable by policy. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - Tags for published releases are immutable by policy. | |
| - Tags for published releases are immutable by policy. |
|
|
||
| ### Manual dispatch | ||
| Use workflow dispatch for dry runs or controlled publish. | ||
| - `publish_release=false`: package + artifact only (no GitHub Release publish). |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `publish_release=false`: package + artifact only (no GitHub Release publish). | |
| - `publish_release=false`: package + artifact only (no GitHub Release publish). |
| - Summary of key changes in this release. | ||
|
|
||
| ## Included Artifacts | ||
| - `SwfocTrainer-portable.zip` |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `SwfocTrainer-portable.zip` | |
| - `SwfocTrainer-portable.zip` |
|
|
||
| ## Calibration Artifacts | ||
| Artifacts are written to: | ||
| - `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` (default app path) |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` (default app path) | |
| - `%LOCALAPPDATA%\\SwfocTrainer\\support\\calibration` (default app path) |
|
|
||
| ## Compatibility Report Interpretation | ||
| Promotion is blocked when any of these are true: | ||
| - dependency status is `HardFail` |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - dependency status is `HardFail` | |
| - dependency status is `HardFail` |
| - `profiles/custom/profiles/<draftProfileId>.json` by default. | ||
|
|
||
| The scaffold process infers: | ||
| - `requiredWorkshopIds` from `STEAMMOD=...` |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `requiredWorkshopIds` from `STEAMMOD=...` | |
| - `requiredWorkshopIds` from `STEAMMOD=...` |
| - `SwfocTrainer-portable.zip.sha256` | ||
|
|
||
| ## Known Limitations | ||
| - Live profile validation still requires local SWFOC sessions on Windows. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - Live profile validation still requires local SWFOC sessions on Windows. | |
| - Live profile validation still requires local SWFOC sessions on Windows. |
| @@ -0,0 +1,52 @@ | |||
| # Release Runbook | |||
|
|
|||
| ## Channel Decision | |||
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Channel Decision | |
| ## Channel Decision | |
| - Do not consume unsigned/unverified artifacts from non-release channels for production usage. | ||
|
|
||
| ## Rollback Procedure | ||
| 1. Identify the bad release tag and the last known-good tag. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| 1. Identify the bad release tag and the last known-good tag. | |
| 1. Identify the bad release tag and the last known-good tag. |
|
|
||
| ## Draft Profile Output | ||
| Draft profile files are emitted under: | ||
| - `profiles/custom/profiles/<draftProfileId>.json` by default. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - `profiles/custom/profiles/<draftProfileId>.json` by default. | |
| - `profiles/custom/profiles/<draftProfileId>.json` by default. |
| - `SwfocTrainer-portable.zip` | ||
| - `SwfocTrainer-portable.zip.sha256` | ||
|
|
||
| ## Known Limitations |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Known Limitations | |
| ## Known Limitations | |
| # SWFOC Trainer {{TAG}} | ||
|
|
||
| ## Highlights | ||
| - Summary of key changes in this release. |
There was a problem hiding this comment.
Codacy has a fix for the issue: Lists should be surrounded by blank lines
| - Summary of key changes in this release. | |
| - Summary of key changes in this release. |
| Get-Content .\SwfocTrainer-portable.zip.sha256 | ||
| ``` | ||
|
|
||
| ## Rollback |
There was a problem hiding this comment.
Codacy has a fix for the issue: Expected: 1; Actual: 0; Below
| ## Rollback | |
| ## Rollback | |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c3ac6c0f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : session.Symbols.Symbols | ||
| .Where(x => criticalSymbols.Contains(x.Key)) | ||
| .Count(x => x.Value.HealthStatus == SymbolHealthStatus.Unresolved); |
There was a problem hiding this comment.
In BuildCompatibilityReportAsync, unresolved critical symbols are computed only from symbols that exist in session.Symbols.Symbols, so any symbol listed in profile.Metadata["criticalSymbols"] but absent from the session map is treated as resolved. In that case promotionReady can be set to true even though a required critical symbol was never resolved, which undermines the promotion gate for custom mod profiles.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
tools/validate-calibration-artifact.ps1 (2)
3-3:$SchemaPathdefault is CWD-relative — same fragility as the other two validator scripts. Replace withJoin-Path $PSScriptRoot "schemas/calibration-artifact.schema.json".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-calibration-artifact.ps1` at line 3, The default $SchemaPath in tools/validate-calibration-artifact.ps1 is CWD-relative and fragile; change its initialization to build an absolute path using $PSScriptRoot and Join-Path (e.g., set $SchemaPath by calling Join-Path $PSScriptRoot "schemas/calibration-artifact.schema.json") so the script reliably locates the calibration-artifact.schema.json regardless of the current working directory.
23-33: Lines 31–33 duplicate the required-field loop already performed on Lines 23–25.If
generatedAtUtc,profileId,moduleFingerprint, andcandidatesare all listed in$schema.required(the likely intent),Require-ValidationFieldis called twice for each, producing duplicate error messages for any missing field. Remove the explicit hardcoded list if the schema already covers it, or add a brief comment explaining why it's intentionally redundant (e.g., as a compile-time schema-drift guard).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-calibration-artifact.ps1` around lines 23 - 33, The second foreach loop re-invokes Require-ValidationField for the same properties already iterated from $schema.required, causing duplicate error messages; either remove the explicit foreach (@("generatedAtUtc", "profileId", "moduleFingerprint", "candidates")) entirely or, if you intend redundancy as a defensive guard, keep it but add a concise comment explaining that it is intentionally duplicated to protect against schema drift; update the code around Require-ValidationField, $schema.required, and Add-ValidationError accordingly so each required field is validated exactly once or clearly documented if duplicated.tools/validation-helpers.ps1 (2)
16-16:Require-is not an approved PowerShell verb — rename to avoid PSScriptAnalyzer noise.PSScriptAnalyzer flags
Require-ValidationFieldwithPSUseApprovedVerbs. While this is a dot-sourced helper (not a module), the warning still surfaces in any PSScriptAnalyzer pass and CI linting. Consider renaming toAssert-ValidationFieldorConfirm-ValidationField, both of which are approved verbs and semantically accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validation-helpers.ps1` at line 16, The function name Require-ValidationField uses an unapproved PowerShell verb; rename the function to an approved verb (e.g., Assert-ValidationField or Confirm-ValidationField) and update all references: change the function declaration Require-ValidationField to Assert-ValidationField, update any callers, dot-sourcing imports, unit tests and documentation to the new name, and remove/suppress any PSUseApprovedVerbs ignores; run PSScriptAnalyzer to verify the PSUseApprovedVerbs warning is resolved.
51-60: ReplaceWrite-Hostwith redirectable output cmdlets.PSScriptAnalyzer flags all three
Write-Hostcalls (PSAvoidUsingWriteHost). In non-interactive CI hosts,Write-Hostoutput bypasses the standard streams and cannot be captured or redirected cleanly. Per the coding guideline that script outputs must be stable, preferWrite-Errorfor the failure path (which writes to stream 2 and is visible in CI logs) andWrite-InformationorWrite-Host-equivalent-safe output for the success message.♻️ Proposed refactor
if ($Errors.Count -gt 0) { - Write-Host "$Label validation failed:" -ForegroundColor Red + Write-Error "$Label validation failed:" foreach ($err in $Errors) { - Write-Host " - $err" -ForegroundColor Red + Write-Error " - $err" } exit 1 } - Write-Host "$Label validation passed: $Path" + Write-Information "$Label validation passed: $Path" -InformationAction Continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validation-helpers.ps1` around lines 51 - 60, Replace the three Write-Host calls so output is redirectable: on failure when $Errors.Count -gt 0, call Write-Error to emit the overall "$Label validation failed:" message and write each error with Write-Error (or use Write-Error for the combined message and include $err details) instead of Write-Host; on success replace the final Write-Host "$Label validation passed: $Path" with a redirectable stream such as Write-Output or Write-Information so CI can capture it. Update the blocks that reference $Errors, $Label, and $Path accordingly and preserve the same message text and exit 1 behavior for the failure path.tools/validate-repro-bundle.ps1 (1)
3-3:$SchemaPathdefault is CWD-relative — same issue as invalidate-support-bundle-manifest.ps1. UseJoin-Path $PSScriptRoot "schemas/repro-bundle.schema.json"to be consistent with the dot-source pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-repro-bundle.ps1` at line 3, The default $SchemaPath is currently CWD-relative; change its initialization so it builds an absolute path using the script directory instead of the working directory — use $PSScriptRoot combined with the schemas folder (via Join-Path) to point to "schemas/repro-bundle.schema.json" so the variable $SchemaPath always resolves correctly when the script is dot-sourced or run from another CWD.tools/validate-support-bundle-manifest.ps1 (1)
3-3:$SchemaPathdefault is CWD-relative, not$PSScriptRoot-relative.
"tools/schemas/support-bundle-manifest.schema.json"resolves relative to the current working directory. When the script is invoked from withintools/(or any directory other than the repo root),Test-Pathfails immediately. The dot-source at Line 9 already uses$PSScriptRootcorrectly — the default$SchemaPathshould follow the same pattern.♻️ Proposed fix
- [string]$SchemaPath = "tools/schemas/support-bundle-manifest.schema.json", + [string]$SchemaPath = (Join-Path $PSScriptRoot "schemas/support-bundle-manifest.schema.json"),The same issue exists in
validate-repro-bundle.ps1(Line 3) andvalidate-calibration-artifact.ps1(Line 3).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-support-bundle-manifest.ps1` at line 3, The default $SchemaPath is CWD-relative which breaks Test-Path when the script is run from a different working directory; change the $SchemaPath default to be $PSScriptRoot-relative (use Join-Path or string-expand $PSScriptRoot with the schemas/support-bundle-manifest.schema.json relative path) so it resolves correctly regardless of CWD, and apply the same fix to the $SchemaPath defaults in validate-repro-bundle.ps1 and validate-calibration-artifact.ps1; keep the rest of the logic (including the dot-sourcing that already uses $PSScriptRoot) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/validate-calibration-artifact.ps1`:
- Around line 52-56: Replace the unsafe guard that directly accesses
$artifact.process with a property-existence check using PSObject.Properties so
the code does not reference a missing property; e.g. change the if condition to
check $artifact.PSObject.Properties['process'] (or use
$artifact.PSObject.Properties.Name -contains 'process') and then run the same
foreach calling Require-ValidationField -Object $artifact.process -Field
$required -Errors $errors -Prefix "process".
In `@tools/validate-repro-bundle.ps1`:
- Around line 69-79: The code currently accesses nested properties
($bundle.launchContext, $bundle.runtimeMode, $bundle.diagnostics) directly under
Set-StrictMode which throws PropertyNotFoundException if those keys are missing;
change the calls so you first safely extract each nested object using the
PSObject.Properties/null-check pattern (e.g.,
$bundle.PSObject.Properties['launchContext']?.Value) or test for $null before
passing to Require-ValidationField, and ensure Require-ValidationField is
invoked only when the extracted value is non-null (or pass $null explicitly if
AllowNull is intended) so validation errors are collected instead of the script
terminating; update the call sites that reference Require-ValidationField
accordingly.
In `@tools/validate-support-bundle-manifest.ps1`:
- Around line 42-47: Avoid accessing $manifest.warnings directly when the key
may be missing and prevent @($null) producing a one-element null array: first
check whether the manifest actually contains the "warnings" property and that it
is not $null (e.g. if ($manifest.PSObject.Properties.Name -contains 'warnings'
-and $manifest.warnings -ne $null) { $warnings = @($manifest.warnings) } else {
$warnings = @() }), then iterate $warnings and call Add-ValidationError as
before; this ensures no PropertyNotFoundException under Set-StrictMode and
avoids the false-positive "warnings[0] must not be null".
In `@tools/validation-helpers.ps1`:
- Around line 7-14: All three functions (Add-ValidationError,
Require-ValidationField, Write-ValidationResult) currently accept $Errors
without marking it mandatory; under Set-StrictMode this can yield a null and
cause a NullReferenceException on .Add()/.Count. Update the parameter
declarations for $Errors in each function to include [Parameter(Mandatory =
$true)] while keeping the existing type ([System.Collections.ArrayList]) so
callers get a clear missing-parameter error instead of a runtime null
dereference.
---
Duplicate comments:
In `@tools/validate-calibration-artifact.ps1`:
- Around line 46-49: Replace the unsafe cast of $candidate.confidence with a
TryParse guard: attempt to parse $candidate.confidence into a [double] (using
[double]::TryParse with a [ref] variable for $confidence) and if parsing fails
call Add-ValidationError -Errors $errors -Message "candidates[$i].confidence
must be a numeric value between 0 and 1"; only when TryParse returns true
perform the existing range check (if ($confidence -lt 0 -or $confidence -gt 1)
...). This ensures non-numeric/null values produce a validation error instead of
a terminating cast exception.
In `@tools/validate-support-bundle-manifest.ps1`:
- Around line 50-52: The strict-mode check currently errors if $included does
not contain "manifest.json", but SupportBundleService appends "manifest.json"
after serializing includedFiles so this will always fail; update the validation
in tools/validate-support-bundle-manifest.ps1 to stop requiring "manifest.json"
to be present in $included (either remove the Add-ValidationError call or change
the condition to ignore "manifest.json" by evaluating $includedFiltered =
$included | Where-Object { $_ -ne "manifest.json" } and perform the strict-mode
existence checks against $includedFiltered), leaving the Add-ValidationError
invocation only for genuine missing entries; keep references to the same symbols
($included and Add-ValidationError) so the change is localized.
---
Nitpick comments:
In `@tools/validate-calibration-artifact.ps1`:
- Line 3: The default $SchemaPath in tools/validate-calibration-artifact.ps1 is
CWD-relative and fragile; change its initialization to build an absolute path
using $PSScriptRoot and Join-Path (e.g., set $SchemaPath by calling Join-Path
$PSScriptRoot "schemas/calibration-artifact.schema.json") so the script reliably
locates the calibration-artifact.schema.json regardless of the current working
directory.
- Around line 23-33: The second foreach loop re-invokes Require-ValidationField
for the same properties already iterated from $schema.required, causing
duplicate error messages; either remove the explicit foreach
(@("generatedAtUtc", "profileId", "moduleFingerprint", "candidates")) entirely
or, if you intend redundancy as a defensive guard, keep it but add a concise
comment explaining that it is intentionally duplicated to protect against schema
drift; update the code around Require-ValidationField, $schema.required, and
Add-ValidationError accordingly so each required field is validated exactly once
or clearly documented if duplicated.
In `@tools/validate-repro-bundle.ps1`:
- Line 3: The default $SchemaPath is currently CWD-relative; change its
initialization so it builds an absolute path using the script directory instead
of the working directory — use $PSScriptRoot combined with the schemas folder
(via Join-Path) to point to "schemas/repro-bundle.schema.json" so the variable
$SchemaPath always resolves correctly when the script is dot-sourced or run from
another CWD.
In `@tools/validate-support-bundle-manifest.ps1`:
- Line 3: The default $SchemaPath is CWD-relative which breaks Test-Path when
the script is run from a different working directory; change the $SchemaPath
default to be $PSScriptRoot-relative (use Join-Path or string-expand
$PSScriptRoot with the schemas/support-bundle-manifest.schema.json relative
path) so it resolves correctly regardless of CWD, and apply the same fix to the
$SchemaPath defaults in validate-repro-bundle.ps1 and
validate-calibration-artifact.ps1; keep the rest of the logic (including the
dot-sourcing that already uses $PSScriptRoot) unchanged.
In `@tools/validation-helpers.ps1`:
- Line 16: The function name Require-ValidationField uses an unapproved
PowerShell verb; rename the function to an approved verb (e.g.,
Assert-ValidationField or Confirm-ValidationField) and update all references:
change the function declaration Require-ValidationField to
Assert-ValidationField, update any callers, dot-sourcing imports, unit tests and
documentation to the new name, and remove/suppress any PSUseApprovedVerbs
ignores; run PSScriptAnalyzer to verify the PSUseApprovedVerbs warning is
resolved.
- Around line 51-60: Replace the three Write-Host calls so output is
redirectable: on failure when $Errors.Count -gt 0, call Write-Error to emit the
overall "$Label validation failed:" message and write each error with
Write-Error (or use Write-Error for the combined message and include $err
details) instead of Write-Host; on success replace the final Write-Host "$Label
validation passed: $Path" with a redirectable stream such as Write-Output or
Write-Information so CI can capture it. Update the blocks that reference
$Errors, $Label, and $Path accordingly and preserve the same message text and
exit 1 behavior for the failure path.
| if ($null -ne $artifact.process) { | ||
| foreach ($required in @("pid", "name", "path", "commandLineAvailable", "launchKind", "launchReasonCode")) { | ||
| Require-ValidationField -Object $artifact.process -Field $required -Errors $errors -Prefix "process" | ||
| } | ||
| } |
There was a problem hiding this comment.
$null -ne $artifact.process guard itself throws under strict mode when process is absent.
The intent of Line 52 is to make process optional, but the guard expression $artifact.process directly accesses the property. With version 2.0 strict mode, PowerShell returns an error if you reference uninitialized variables or non-existent properties. If process is not present as a JSON key at all, this line throws PropertyNotFoundException before the guard comparison is evaluated. Use the safe PSObject.Properties intrinsic for the check:
🛡️ Proposed fix
-if ($null -ne $artifact.process) {
- foreach ($required in @("pid", "name", "path", "commandLineAvailable", "launchKind", "launchReasonCode")) {
- Require-ValidationField -Object $artifact.process -Field $required -Errors $errors -Prefix "process"
- }
-}
+$processProp = $artifact.PSObject.Properties['process']
+if ($null -ne $processProp -and $null -ne $processProp.Value) {
+ foreach ($required in @("pid", "name", "path", "commandLineAvailable", "launchKind", "launchReasonCode")) {
+ Require-ValidationField -Object $processProp.Value -Field $required -Errors $errors -Prefix "process"
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ($null -ne $artifact.process) { | |
| foreach ($required in @("pid", "name", "path", "commandLineAvailable", "launchKind", "launchReasonCode")) { | |
| Require-ValidationField -Object $artifact.process -Field $required -Errors $errors -Prefix "process" | |
| } | |
| } | |
| $processProp = $artifact.PSObject.Properties['process'] | |
| if ($null -ne $processProp -and $null -ne $processProp.Value) { | |
| foreach ($required in @("pid", "name", "path", "commandLineAvailable", "launchKind", "launchReasonCode")) { | |
| Require-ValidationField -Object $processProp.Value -Field $required -Errors $errors -Prefix "process" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/validate-calibration-artifact.ps1` around lines 52 - 56, Replace the
unsafe guard that directly accesses $artifact.process with a property-existence
check using PSObject.Properties so the code does not reference a missing
property; e.g. change the if condition to check
$artifact.PSObject.Properties['process'] (or use
$artifact.PSObject.Properties.Name -contains 'process') and then run the same
foreach calling Require-ValidationField -Object $artifact.process -Field
$required -Errors $errors -Prefix "process".
| foreach ($required in @("profileId", "reasonCode", "confidence", "launchKind")) { | ||
| Require-Field -Object $bundle.launchContext -Field $required -AllowNull | ||
| Require-ValidationField -Object $bundle.launchContext -Field $required -Errors $errors -AllowNull | ||
| } | ||
|
|
||
| foreach ($required in @("hint", "effective", "reasonCode")) { | ||
| Require-Field -Object $bundle.runtimeMode -Field $required | ||
| Require-ValidationField -Object $bundle.runtimeMode -Field $required -Errors $errors | ||
| } | ||
|
|
||
| foreach ($required in @("dependencyState", "helperReadiness", "symbolHealthSummary")) { | ||
| Require-Field -Object $bundle.diagnostics -Field $required | ||
| Require-ValidationField -Object $bundle.diagnostics -Field $required -Errors $errors | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check PowerShell version targets in CI workflows
rg -n "pwsh|powershell" .github/workflows/ -A 2 -B 2Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 5685
🏁 Script executed:
# Examine the actual script and Set-StrictMode setting
cat -n tools/validate-repro-bundle.ps1 | head -100Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 4536
🏁 Script executed:
# Find validation-helpers.ps1 and check Require-ValidationField implementation
find . -name "validation-helpers.ps1" -type fRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 99
🏁 Script executed:
# Search for any old Require-Field usage or definition
rg -n "Require-Field" tools/ --type ps1 -A 3Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 96
🏁 Script executed:
# Read validation-helpers.ps1 to see Require-ValidationField implementation
cat -n tools/validation-helpers.ps1Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 2307
🏁 Script executed:
# Search for old Require-Field code (without type filter)
rg -n "Require-Field" tools/Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 534
🏁 Script executed:
# Check the fixture file to see if launchContext, runtimeMode, diagnostics are always present
cat -n tools/fixtures/repro_bundle_sample.jsonRepository: Prekzursil/SWFOC-Mod-Menu
Length of output: 1537
🏁 Script executed:
# Check if there's any guard before lines 69-79, and see the old Require-Field implementation
sed -n '28,50p' tools/validate-save-patch-pack.ps1Repository: Prekzursil/SWFOC-Mod-Menu
Length of output: 616
Accessing absent nested objects directly throws PropertyNotFoundException under strict mode.
Lines 70, 74, and 78 each pass $bundle.launchContext, $bundle.runtimeMode, and $bundle.diagnostics directly as the -Object parameter to Require-ValidationField. Under Set-StrictMode -Version Latest (line 7), if any of these nested objects are absent from the JSON bundle, the property access itself throws a terminating PropertyNotFoundException at the call site — before Require-ValidationField is ever invoked — bypassing validation error collection and crashing the script.
The old Require-Field implementation (in tools/validate-save-patch-pack.ps1) had an explicit null guard (if ($null -eq $Object)), which the new Require-ValidationField lacks. The fixture file (tools/fixtures/repro_bundle_sample.json) contains all these nested fields, so the CI smoke test does not expose this latent bug; it will only surface when a malformed bundle is validated.
Use the safe PSObject.Properties pattern to extract nested objects before passing them to validation:
🛡️ Proposed fix for lines 69–79
-foreach ($required in @("profileId", "reasonCode", "confidence", "launchKind")) {
- Require-ValidationField -Object $bundle.launchContext -Field $required -Errors $errors -AllowNull
-}
-
-foreach ($required in @("hint", "effective", "reasonCode")) {
- Require-ValidationField -Object $bundle.runtimeMode -Field $required -Errors $errors
-}
-
-foreach ($required in @("dependencyState", "helperReadiness", "symbolHealthSummary")) {
- Require-ValidationField -Object $bundle.diagnostics -Field $required -Errors $errors
-}
+$launchContext = $bundle.PSObject.Properties['launchContext']?.Value
+foreach ($required in @("profileId", "reasonCode", "confidence", "launchKind")) {
+ Require-ValidationField -Object $launchContext -Field $required -Errors $errors -AllowNull
+}
+
+$runtimeMode = $bundle.PSObject.Properties['runtimeMode']?.Value
+foreach ($required in @("hint", "effective", "reasonCode")) {
+ Require-ValidationField -Object $runtimeMode -Field $required -Errors $errors
+}
+
+$diagnostics = $bundle.PSObject.Properties['diagnostics']?.Value
+foreach ($required in @("dependencyState", "helperReadiness", "symbolHealthSummary")) {
+ Require-ValidationField -Object $diagnostics -Field $required -Errors $errors
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($required in @("profileId", "reasonCode", "confidence", "launchKind")) { | |
| Require-Field -Object $bundle.launchContext -Field $required -AllowNull | |
| Require-ValidationField -Object $bundle.launchContext -Field $required -Errors $errors -AllowNull | |
| } | |
| foreach ($required in @("hint", "effective", "reasonCode")) { | |
| Require-Field -Object $bundle.runtimeMode -Field $required | |
| Require-ValidationField -Object $bundle.runtimeMode -Field $required -Errors $errors | |
| } | |
| foreach ($required in @("dependencyState", "helperReadiness", "symbolHealthSummary")) { | |
| Require-Field -Object $bundle.diagnostics -Field $required | |
| Require-ValidationField -Object $bundle.diagnostics -Field $required -Errors $errors | |
| } | |
| foreach ($required in @("profileId", "reasonCode", "confidence", "launchKind")) { | |
| Require-ValidationField -Object $bundle.launchContext -Field $required -Errors $errors -AllowNull | |
| } | |
| $launchContext = $bundle.PSObject.Properties['launchContext']?.Value | |
| foreach ($required in @("profileId", "reasonCode", "confidence", "launchKind")) { | |
| Require-ValidationField -Object $launchContext -Field $required -Errors $errors -AllowNull | |
| } | |
| $runtimeMode = $bundle.PSObject.Properties['runtimeMode']?.Value | |
| foreach ($required in @("hint", "effective", "reasonCode")) { | |
| Require-ValidationField -Object $runtimeMode -Field $required -Errors $errors | |
| } | |
| $diagnostics = $bundle.PSObject.Properties['diagnostics']?.Value | |
| foreach ($required in @("dependencyState", "helperReadiness", "symbolHealthSummary")) { | |
| Require-ValidationField -Object $diagnostics -Field $required -Errors $errors | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/validate-repro-bundle.ps1` around lines 69 - 79, The code currently
accesses nested properties ($bundle.launchContext, $bundle.runtimeMode,
$bundle.diagnostics) directly under Set-StrictMode which throws
PropertyNotFoundException if those keys are missing; change the calls so you
first safely extract each nested object using the PSObject.Properties/null-check
pattern (e.g., $bundle.PSObject.Properties['launchContext']?.Value) or test for
$null before passing to Require-ValidationField, and ensure
Require-ValidationField is invoked only when the extracted value is non-null (or
pass $null explicitly if AllowNull is intended) so validation errors are
collected instead of the script terminating; update the call sites that
reference Require-ValidationField accordingly.
| $warnings = @($manifest.warnings) | ||
| for ($i = 0; $i -lt $warnings.Count; $i++) { | ||
| if ($null -eq $warnings[$i]) { | ||
| Add-ValidationError -Errors $errors -Message "warnings[$i] must not be null" | ||
| } | ||
| } |
There was a problem hiding this comment.
@($manifest.warnings) produces a false positive when warnings is absent or null.
@($null) in PowerShell creates a one-element array containing $null, not an empty array. If warnings is an optional field that is absent from the JSON (or explicitly null), the loop iterates once and emits warnings[0] must not be null — a spurious error. Additionally, under Set-StrictMode -Version Latest, accessing $manifest.warnings when warnings is not a JSON key at all throws a PropertyNotFoundException before the array wrapping even occurs.
🛡️ Proposed fix
-$warnings = @($manifest.warnings)
-for ($i = 0; $i -lt $warnings.Count; $i++) {
- if ($null -eq $warnings[$i]) {
- Add-ValidationError -Errors $errors -Message "warnings[$i] must not be null"
- }
-}
+$warningsProp = $manifest.PSObject.Properties['warnings']
+if ($null -ne $warningsProp -and $null -ne $warningsProp.Value) {
+ $warnings = @($warningsProp.Value)
+ for ($i = 0; $i -lt $warnings.Count; $i++) {
+ if ($null -eq $warnings[$i]) {
+ Add-ValidationError -Errors $errors -Message "warnings[$i] must not be null"
+ }
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $warnings = @($manifest.warnings) | |
| for ($i = 0; $i -lt $warnings.Count; $i++) { | |
| if ($null -eq $warnings[$i]) { | |
| Add-ValidationError -Errors $errors -Message "warnings[$i] must not be null" | |
| } | |
| } | |
| $warningsProp = $manifest.PSObject.Properties['warnings'] | |
| if ($null -ne $warningsProp -and $null -ne $warningsProp.Value) { | |
| $warnings = @($warningsProp.Value) | |
| for ($i = 0; $i -lt $warnings.Count; $i++) { | |
| if ($null -eq $warnings[$i]) { | |
| Add-ValidationError -Errors $errors -Message "warnings[$i] must not be null" | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/validate-support-bundle-manifest.ps1` around lines 42 - 47, Avoid
accessing $manifest.warnings directly when the key may be missing and prevent
@($null) producing a one-element null array: first check whether the manifest
actually contains the "warnings" property and that it is not $null (e.g. if
($manifest.PSObject.Properties.Name -contains 'warnings' -and $manifest.warnings
-ne $null) { $warnings = @($manifest.warnings) } else { $warnings = @() }), then
iterate $warnings and call Add-ValidationError as before; this ensures no
PropertyNotFoundException under Set-StrictMode and avoids the false-positive
"warnings[0] must not be null".
| function Add-ValidationError { | ||
| param( | ||
| [Parameter()][System.Collections.ArrayList]$Errors, | ||
| [Parameter(Mandatory = $true)][string]$Message | ||
| ) | ||
|
|
||
| [void]$Errors.Add($Message) | ||
| } |
There was a problem hiding this comment.
$Errors should be Mandatory = $true across all three consumer functions.
All three functions — Add-ValidationError (Line 9), Require-ValidationField (Line 20), and Write-ValidationResult (Line 46) — declare $Errors without [Parameter(Mandatory = $true)]. Under Set-StrictMode -Version Latest, a caller that accidentally omits -Errors gets $null, and the first .Add() or .Count access becomes a terminating NullReferenceException rather than a clear parameter-missing error.
🛡️ Proposed fix (apply to all three functions)
function Add-ValidationError {
param(
- [Parameter()][System.Collections.ArrayList]$Errors,
+ [Parameter(Mandatory = $true)][System.Collections.ArrayList]$Errors,
[Parameter(Mandatory = $true)][string]$Message
) function Require-ValidationField {
param(
[Parameter(Mandatory = $true)][object]$Object,
[Parameter(Mandatory = $true)][string]$Field,
- [Parameter()][System.Collections.ArrayList]$Errors,
+ [Parameter(Mandatory = $true)][System.Collections.ArrayList]$Errors, function Write-ValidationResult {
param(
- [Parameter()][System.Collections.ArrayList]$Errors,
+ [Parameter(Mandatory = $true)][System.Collections.ArrayList]$Errors,Also applies to: 16-23, 44-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/validation-helpers.ps1` around lines 7 - 14, All three functions
(Add-ValidationError, Require-ValidationField, Write-ValidationResult) currently
accept $Errors without marking it mandatory; under Set-StrictMode this can yield
a null and cause a NullReferenceException on .Add()/.Count. Update the parameter
declarations for $Errors in each function to include [Parameter(Mandatory =
$true)] while keeping the existing type ([System.Collections.ArrayList]) so
callers get a clear missing-parameter error instead of a runtime null
dereference.
1 participant
User description
Summary
Implements the post-cutoff full-issue closure wave for deterministic and ops/product surfaces:
SaveCorpusRoundTripTests+ tracked fixtures)Affected Profiles
base_sweawbase_swfocaotr_1397421866_swfocroe_3447786229_swfoccustom_*(M3 onboarding drafts)Reliability Evidence
Repro bundle JSON: justified skip (deterministic/tooling/docs/release workflow changes; no live runtime behavior change claimed in this PR)
Classification: justified skip
Launch reason code(s): n/a (live evidence remains tracked in #19/#34)
Deterministic verification run in this branch:
dotnet build SwfocTrainer.sln -c Release --no-restoredotnet test tests/SwfocTrainer.Tests/SwfocTrainer.Tests.csproj -c Release --no-build --filter "FullyQualifiedName!~SwfocTrainer.Tests.Profiles.Live&FullyQualifiedName!~RuntimeAttachSmokeTests"dotnet test tests/SwfocTrainer.Tests/SwfocTrainer.Tests.csproj -c Release --no-build --filter "FullyQualifiedName~LaunchContext"dotnet test tests/SwfocTrainer.Tests/SwfocTrainer.Tests.csproj -c Release --no-build --filter "FullyQualifiedName~ModDependencyValidator"powershell.exe -NoProfile -ExecutionPolicy Bypass -File tools/validate-policy-contracts.ps1powershell.exe -NoProfile -ExecutionPolicy Bypass -File tools/validate-repro-bundle.ps1 -BundlePath tools/fixtures/repro_bundle_sample.json -SchemaPath tools/schemas/repro-bundle.schema.json -Strictpowershell.exe -NoProfile -ExecutionPolicy Bypass -File tools/validate-save-patch-pack.ps1 -PatchPackPath tools/fixtures/save_patch_pack_sample.json -SchemaPath tools/schemas/save-patch-pack.schema.json -Strictpowershell.exe -NoProfile -ExecutionPolicy Bypass -File tools/validate-calibration-artifact.ps1 -ArtifactPath tools/fixtures/calibration_artifact_sample.json -SchemaPath tools/schemas/calibration-artifact.schema.json -Strictpowershell.exe -NoProfile -ExecutionPolicy Bypass -File tools/validate-support-bundle-manifest.ps1 -ManifestPath tools/fixtures/support_bundle_manifest_sample.json -SchemaPath tools/schemas/support-bundle-manifest.schema.json -Strictpowershell.exe -NoProfile -ExecutionPolicy Bypass -File tools/compare-visual-pack.ps1 -BaselineDir tools/fixtures/visual/baseline -CandidateDir tools/fixtures/visual/candidate -OutputPath TestResults/visual-compare-smoke.jsonpython3 tools/detect-launch-context.py --from-process-json tools/fixtures/launch_context_cases.json --profile-root profiles/default --pretty > TestResults/launch_context_results_phase2.jsonIssue Mapping
Closes #8
Closes #9
Closes #10
Closes #29
Closes #39
Closes #40
Closes #41
Closes #42
Closes #43
Closes #44
Refs #6
Refs #7
Refs #19
Refs #34
PR Type
Enhancement, Tests, Documentation
Description
M3/M4 Full-Issue Closure Wave: Onboarding, Ops Hardening, and Release Strategy
Core Feature Implementations:
M3 Mod Compatibility Studio: Custom mod profile scaffolding from launch samples with Steam Workshop ID and local path inference; calibration artifact export with symbol candidate tracking; compatibility report generation with promotion readiness blocking
M4 Transactional Profile Updates: Comprehensive profile installation with backup creation, SHA256 verification, and rollback support via timestamped backups and install receipts
M4 Support Bundle & Telemetry: Zipped diagnostics export aggregating logs, calibration artifacts, repro bundles, runtime snapshots, and telemetry metrics; thread-safe action recording with failure/fallback/unresolved rate computation
M2 Save Corpus Coverage: Round-trip codec validation for all shipped save schemas (base SWEAW, base SWFOC, AOTR, ROE) using tracked deterministic fixtures
Ops & Release Hardening:
GitHub Releases Workflow: Portable zip artifact generation with SHA256 checksum verification; tag-driven and manual dispatch support; release notes templating
Schema & Contract Validation: New JSON schemas for calibration artifacts and support bundle manifests; policy contract enforcement; CI schema smoke tests
SonarCloud Conditional Execution: Graceful skip when token unavailable; visual audit environment variable configuration
UI & Service Integration:
MainViewModel Expansion: Four new service dependencies; nine new properties for onboarding state; five async command handlers for scaffolding, export, and reporting
MainWindow Layout Redesign: "Mod Compatibility Studio" group with base/draft profile fields; "Ops Hardening" group with bundle/telemetry exports; three-row Profiles & Updates tab
DI Container Registration: All M3/M4 service implementations registered in App.xaml.cs
Documentation & Tracking:
Release runbook with artifact verification and rollback procedures
Release notes template with highlights and known limitations
Profile format documentation for M3 onboarding and custom namespaces
Calibration evidence payload standardization
Save Lab runbook enhancements for cross-profile compatibility and fault tolerance
TODO.md completion tracking for M2/M3/M4 milestones
Bug Fixes:
Diagram Walkthrough
File Walkthrough
23 files
MainViewModel.cs
M3/M4 UI integration with onboarding and ops servicessrc/SwfocTrainer.App/ViewModels/MainViewModel.cs
IModOnboardingService,IModCalibrationService,ISupportBundleService,ITelemetrySnapshotService) to support M3/M4 featuresprofile, draft profile, display name, launch samples, etc.)
exporting calibration artifacts, building compatibility reports, and
exporting support/telemetry bundles
ModCompatibilityRowsobservable collection and updatedInstallUpdateAsyncto use transactional install result withreceipt/backup tracking
GitHubProfileUpdateService.cs
Transactional profile updates with rollback supportsrc/SwfocTrainer.Profiles/Services/GitHubProfileUpdateService.cs
InstallProfileAsyncto wrap newInstallProfileTransactionalAsyncmethod for backward compatibilityInstallProfileTransactionalAsyncwith comprehensive errorhandling, returning structured
ProfileInstallResultwithsuccess/failure details, backup paths, and install receipts
RollbackLastInstallAsyncmethod to restore profiles fromtimestamped backups with rollback receipt generation
WriteInstallReceiptAsyncandWriteRollbackReceiptAsyncto track installation/rollback operationswith JSON metadata
ModOnboardingService.cs
Custom mod profile scaffolding from launch samplessrc/SwfocTrainer.Profiles/Services/ModOnboardingService.cs
ScaffoldDraftProfileAsyncto generate custom mod profilesfrom launch samples with base profile inheritance
STEAMMOD=markers and local path hintsfrom
MODPATH=tokens and process pathswrites draft profiles to custom namespace directories
workshop/path hints
SupportBundleService.cs
Support bundle export with diagnostics aggregationsrc/SwfocTrainer.Core/Services/SupportBundleService.cs
ExportAsyncto create zipped support bundles containinglogs, calibration artifacts, recent repro bundles, runtime snapshots,
and telemetry
LocalAppData directories
health summary when session is attached
warnings for missing directories
ModCalibrationService.cs
Calibration artifact export and compatibility reportingsrc/SwfocTrainer.Core/Services/ModCalibrationService.cs
ExportCalibrationArtifactAsyncto write symbol candidateartifacts with module fingerprint, process metadata, and health status
BuildCompatibilityReportAsyncto compute promotionreadiness by evaluating action reliability, dependency validation, and
critical symbol resolution
are unresolved, or actions are unavailable
compatibility notes
TelemetrySnapshotService.cs
Telemetry snapshot service for runtime diagnosticssrc/SwfocTrainer.Core/Services/TelemetrySnapshotService.cs
and address source tracking
CreateSnapshotto compute aggregated metrics (failure rate,fallback rate, unresolved rate)
ExportSnapshotAsyncto serialize telemetry to JSON withtimestamp
Resetmethod for clearing countersModOnboardingModels.cs
Mod onboarding and calibration data modelssrc/SwfocTrainer.Core/Models/ModOnboardingModels.cs
ModLaunchSamplerecord for process/command-line captureModOnboardingRequestandModOnboardingResultcontracts forprofile scaffolding
ModCalibrationArtifactRequestandModCalibrationArtifactResultfor artifact export
CalibrationCandidaterecord for symbol entries in artifactsTrainerOrchestrator.cs
Telemetry integration into action orchestrationsrc/SwfocTrainer.Core/Services/TrainerOrchestrator.cs
ITelemetrySnapshotServicedependency with defaultinstantiation
to telemetry service after each action
CompatibilityReportModels.cs
Compatibility report data modelssrc/SwfocTrainer.Core/Models/CompatibilityReportModels.cs
ModActionCompatibilityrecord for per-action reliability stateModCompatibilityReportrecord with promotion readinessverdict, dependency status, and action compatibility list
IModCalibrationService.cs
Mod calibration service contractsrc/SwfocTrainer.Core/Contracts/IModCalibrationService.cs
report generation
advanced analysis
ProfileUpdateModels.cs
Profile update operation result modelssrc/SwfocTrainer.Core/Models/ProfileUpdateModels.cs
ProfileInstallResultrecord with success flag, paths, message,and reason code
ProfileRollbackResultrecord with restoration status andbackup tracking
SupportBundleModels.cs
Support bundle request/result modelssrc/SwfocTrainer.Core/Models/SupportBundleModels.cs
SupportBundleRequestrecord with output directory, profile ID,notes, and max recent runs
SupportBundleResultrecord with bundle path, manifest path,included files, and warnings
IProfileUpdateService.cs
Profile update service contract expansionsrc/SwfocTrainer.Core/Contracts/IProfileUpdateService.cs
InstallProfileTransactionalAsyncmethod returning structuredProfileInstallResultRollbackLastInstallAsyncmethod returningProfileRollbackResultTelemetrySnapshotModels.cs
Telemetry snapshot data modelsrc/SwfocTrainer.Core/Models/TelemetrySnapshotModels.cs
TelemetrySnapshotrecord with action success/failure counters,address source counters, and computed rates
ITelemetrySnapshotService.cs
Telemetry snapshot service contractsrc/SwfocTrainer.Core/Contracts/ITelemetrySnapshotService.cs
to JSON, and resetting counters
IModOnboardingService.cs
Mod onboarding service contractsrc/SwfocTrainer.Core/Contracts/IModOnboardingService.cs
launch samples
ISupportBundleService.cs
Support bundle service contractsrc/SwfocTrainer.Core/Contracts/ISupportBundleService.cs
runtime snapshots
validate-calibration-artifact.ps1
Calibration artifact schema validation scripttools/validate-calibration-artifact.ps1
checks
confidence (0-1 range)
validate-support-bundle-manifest.ps1
Support bundle manifest schema validation scripttools/validate-support-bundle-manifest.ps1
field checks
included files
MainWindow.xaml
M3/M4 UI layout for onboarding and ops featuressrc/SwfocTrainer.App/MainWindow.xaml
draft profile ID, display name, and launch samples
build compatibility report
buttons
feed and onboarding/ops summary panels
release-portable.yml
GitHub Release publishing with checksum verification.github/workflows/release-portable.yml
prerelease flag
tag-triggered vs manual dispatch
release with notes template
calibration-artifact.schema.json
Calibration artifact JSON schema definitiontools/schemas/calibration-artifact.schema.json
(schemaVersion, generatedAtUtc, profileId, moduleFingerprint,
candidates)
confidence (0-1) fields
commandLineAvailable, launchKind, launchReasonCode
sonarcloud.yml
SonarCloud workflow conditional execution.github/workflows/sonarcloud.yml
when token is not configured
if: ${{steps.sonar-token.outputs.enabled == 'true' }}13 files
ProfileUpdateServiceTransactionalTests.cs
Transactional profile update and rollback teststests/SwfocTrainer.Tests/Profiles/ProfileUpdateServiceTransactionalTests.cs
verification
timestamped backups
previous version
ModCalibrationServiceTests.cs
Mod calibration service unit teststests/SwfocTrainer.Tests/Profiles/ModCalibrationServiceTests.cs
are unresolved
fingerprint generation
ModOnboardingServiceTests.cs
Mod onboarding service unit teststests/SwfocTrainer.Tests/Profiles/ModOnboardingServiceTests.cs
and path hint inference
generation
metadata with inferred hints
SupportBundleServiceTests.cs
Support bundle service unit teststests/SwfocTrainer.Tests/Core/SupportBundleServiceTests.cs
files
session
SaveCorpusRoundTripTests.cs
Save corpus round-trip validation teststests/SwfocTrainer.Tests/Saves/SaveCorpusRoundTripTests.cs
tracked corpus fixtures
tools/fixtures/save-corpusand validateseach schema
TelemetrySnapshotServiceTests.cs
Telemetry snapshot service unit teststests/SwfocTrainer.Tests/Core/TelemetrySnapshotServiceTests.cs
aggregation
unresolved)
ci.yml
CI pipeline schema validation expansion.github/workflows/ci.yml
with strict mode
support_bundle_manifest_sample.json
Support bundle manifest test fixturetools/fixtures/support_bundle_manifest_sample.json
and included files
manifest.json
Save corpus manifest for deterministic testingtools/fixtures/save-corpus/manifest.json
aotr_1397421866_swfoc_v1.json
AOTR schema corpus fixturetools/fixtures/save-corpus/aotr_1397421866_swfoc_v1.json
roe_3447786229_swfoc_v1.json
ROE schema corpus fixturetools/fixtures/save-corpus/roe_3447786229_swfoc_v1.json
base_swfoc_steam_v1.json
Base SWFOC schema corpus fixturetools/fixtures/save-corpus/base_swfoc_steam_v1.json
base_sweaw_steam_v1.json
Base SWEAW schema corpus fixturetools/fixtures/save-corpus/base_sweaw_steam_v1.json
4 files
App.xaml.cs
Service registration for M3/M4 featuressrc/SwfocTrainer.App/App.xaml.cs
IModCalibrationService,ITelemetrySnapshotService,ISupportBundleService,IModOnboardingServicevalidate-policy-contracts.ps1
Policy contract updates for M3/M4 artifactstools/validate-policy-contracts.ps1
.github/workflows/release-portable.ymlRELEASE_RUNBOOK.md, release-notes-template.md)
support-bundle-manifest.schema.json)
gh release commands)
support-bundle-manifest.schema.json
Support bundle manifest schema definitiontools/schemas/support-bundle-manifest.schema.json
schemaVersion,generatedAtUtc,includedFiles,warningsprofileIdandnotesvisual-audit.yml
Visual audit workflow environment variable configuration.github/workflows/visual-audit.yml
APPLITOOLS_API_KEYenvironment variable configuration fromsecrets
direct secret reference
1 files
compare-visual-pack.ps1
Visual pack comparison path normalization fixtools/compare-visual-pack.ps1
separators using regex replacement
7 files
TODO.md
Execution tracking for M2/M3/M4 completionTODO.md
evidence
and validation evidence
evidence
RELEASE_RUNBOOK.md
Release workflow and distribution channel documentationdocs/RELEASE_RUNBOOK.md
workflows for GitHub Releases
templates
PROFILE_FORMAT.md
Profile format documentation for M3 onboardingdocs/PROFILE_FORMAT.md
onboardingNotesmetadata fieldcustom_prefixsupport bundle manifest)
SAVE_LAB_RUNBOOK.md
Save Lab runbook enhancement and safety documentationdocs/SAVE_LAB_RUNBOOK.md
SYMBOL_CALIBRATION_TODO.md
Calibration evidence payload standardizationdocs/SYMBOL_CALIBRATION_TODO.md
runId,classification,profileId, launchrecommendation, runtime mode
release-notes-template.md
Release notes template for GitHub Releasesdocs/release-notes-template.md
and known limitations
EXTERNAL_TOOLS_SETUP.md
External tools setup for GitHub Releasesdocs/EXTERNAL_TOOLS_SETUP.md
3 files
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests