diff --git a/AGENTS.md b/AGENTS.md index 5fcad0a1..7c1b9aed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -258,8 +258,11 @@ Widescreen-only pages are added dynamically after the base install when needed. - `DownloadsExplainPage` - background downloads continue while the wizard advances - `InstallStartPage` - - review page before the real install begins - + - Environment readiness summary (`InstallStartReadiness`); Next blocked on critical failures + - Optional **Resume previous install** / **Start over** when `install_session.json` has incomplete selected work +- `InstallingPage` + - Honest success / failed / cancelled states; Resume/Retry continues remaining mods (not pristine undo) + - Review/progress page before Next is allowed after a successful install ## Full-build workflow expectation For `KOTOR1_Full.toml` / `KOTOR2_Full.toml` tests: diff --git a/docs/brainstorms/2026-07-25-trust-first-install-experience-requirements.md b/docs/brainstorms/2026-07-25-trust-first-install-experience-requirements.md new file mode 100644 index 00000000..c4c1e219 --- /dev/null +++ b/docs/brainstorms/2026-07-25-trust-first-install-experience-requirements.md @@ -0,0 +1,156 @@ +--- +title: "Trust-first install experience" +date: 2026-07-25 +topic: trust-first-install-experience +strategy_track: player install trust +related: docs/knowledgebase/install-lifecycle.md +--- + +# Trust-first install experience + +## Summary + +Make long guided installs feel safe to start and safe to continue: catch bad environments before install begins, show honest progress while it runs, and make resume/retry after failure an obvious player action. Extend existing checkpoint/resume behavior rather than shipping a new full-folder rollback product. + +## Problem Frame + +KOTOR full builds are long, order-sensitive, and opaque. Across mod managers, the highest-friction failures are bad paths/permissions at start and mid-install breaks with no clear recovery. ModSync already validates, checkpoints, and can resume components under the hood, but players often experience the loop as “start, fail, guess, restart.” Industry patterns for guided installers (Wabbajack-class) prioritize preflight, transparent progress, and resumability over open-ended conflict tooling. + +## Key Decisions + +- **Trust over manager parity.** Optimize for curated build install trust, not Vortex/MO2 general-manager features (per-mod conflict UI, arbitrary mix-and-match). +- **Extend checkpoint/resume, don’t replace it.** Resume already happens automatically inside the install service when session state is valid; the product gap is wizard discovery and failure UX, not new recovery machinery. +- **Three recovery concepts stay distinct.** Component-session resume (skip completed work) is the MVP; the Git checkpoint browser (roll back to a commit) is existing post-hoc tooling; CAS pristine restore stays deferred. Copy must not blur them. +- **Players first in this pass.** Author publish/share and agent/CLI parity improvements stay adjacent tracks, not this MVP. +- **Evidence assumption.** Framing rests on strategy, install-lifecycle docs, and cross-manager best-practice research — not a single named player incident captured in this brainstorm. + +## Actors + +- A1. Player installing a curated instruction-file build through the install wizard +- A2. Maintainer/agent running the same install path headlessly (CLI) — must not regress; player-facing clarity is the primary design target + +## Requirements + +**Install-start readiness** + +- R1. Before a real install starts, the player sees an install-start readiness summary covering detectable start blockers — at minimum path/drive safety and required tools such as HoloPatcher. Reuse existing detection rather than building a second validation pipeline; add new probes only for failure modes validation cannot catch. +- R2. Failed readiness items use plain-language remediation, and the player cannot advance into install while blockers remain. +- R3. The readiness summary is a compact pass/fail surface on install start, pointing back to validation detail for validation-owned blockers rather than duplicating the full report. + +**Progress** + +- R4. During install, the player always sees which component is active, overall progress through the selected set, and whether the run is healthy, waiting, or failed. +- R5. Long installs communicate that interruption is recoverable when resume is available; progress does not imply “start over from zero” after a non-fatal stop. + +**Recovery** + +- R6. When an install fails or stops mid-run, the player is offered a clear next action: resume/retry from the last successfully completed component when session state allows it. +- R7. Resume/retry stays available when the player reopens the install wizard for the same instruction file and directories, not only while the failed run's window is still open. +- R8. On failure the player sees what completed versus what remains; when resume is unavailable the UI says so, and recovery copy never implies one-click restoration of the pre-install game folder. + +**Non-regression** + +- R9. Do not break existing headless recoverability: resume-by-session must keep working for CLI installs. Wizard trust UX is the delivery target, and CLI behavior changes only where required to keep current recoverability honest. + +## Key Flows + +- F1. Clean start + - **Trigger:** Player reaches install start with a valid selection and directories set. + - **Actors:** A1 + - **Steps:** Readiness check runs → blockers cleared → player confirms → install begins with live component progress. + - **Outcome:** Install proceeds only when readiness is known. + +- F2. Mid-install failure with resume + - **Trigger:** A component fails after earlier components succeeded and checkpoint/resume state is intact. + - **Actors:** A1 + - **Steps:** Failure surfaces with completed/remaining summary → player chooses resume/retry → install continues from the next incomplete selected component. + - **Outcome:** Player does not reinstall already-completed components by default. + +- F2b. Recovery after reopening + - **Trigger:** Player closes the wizard after a failure and returns later with session state intact. + - **Actors:** A1 + - **Steps:** Wizard detects resumable session for the same instruction file and directories → offers resume → install continues from the next incomplete component. + - **Outcome:** Walking away does not cost the completed work. + +- F3. Failure without resume + - **Trigger:** Install stops and resume is unavailable or invalid. + - **Actors:** A1 + - **Steps:** Failure surfaces completed/remaining truthfully → player is directed to validate/fix root cause or restart with eyes open — not promised a full undo. + - **Outcome:** No false recovery promise. + +## Acceptance Examples + +- AE1. Covers R1, R2 + - **Given:** Game path is on a protected/invalid location ModSync can detect, or HoloPatcher is missing on Linux. + - **When:** Player attempts to start install. + - **Then:** Readiness blocks with a remediation message and install does not begin until fixed. + +- AE2. Covers R4, R6 + - **Given:** Components 1–10 of 40 completed, then component 11 fails. + - **When:** The player returns to recover in the same session. + - **Then:** UI offers resume/retry from after the last successful component and shows that 1–10 are done. + +- AE3. Covers R7 + - **Given:** The player closes the wizard after a failed run and reopens it against the same instruction file and directories. + - **When:** The install flow is re-entered. + - **Then:** Resume/retry is offered instead of a silent restart from the first component. + +- AE4. Covers R8 + - **Given:** Session state is missing or invalid after a failure. + - **When:** Failure UI is shown. + - **Then:** Copy states resume is unavailable and does not claim one-click restoration of the pre-install game folder. + +## Success Criteria + +- A player can tell, before clicking install, whether the environment is ready and what to fix if not. +- After a mid-build failure with intact resume state, the default recovery path is continue — not “wipe and start over.” +- Failure copy never claims full game-directory rollback for this MVP. +- Existing headless resume-by-session behavior remains usable for agents after the wizard trust work ships. + +## Scope Boundaries + +**In scope** + +- Wizard-facing readiness summary, progress clarity, and resume/retry affordances for classic guided installs +- Reuse of existing validation detectors; install-start readiness is a gate plus summary, not a second validation product +- Rewriting existing install and completion copy that currently implies rollback the product does not deliver +- Non-regression of headless resume + +**Deferred for later** + +- One-action pristine game-folder rollback / CAS pre-image restore for in-place patcher overwrites +- Unsafe operator overrides for readiness blockers +- Redesigning the checkpoint management dialog as the primary player recovery surface +- Managed-vs-classic DryRun staging parity as a trust narrative +- Author publish/share, structured JSON validation reports, profile CLI CRUD +- FOMOD image/advanced-conditional polish beyond existing fail-closed gate + +**Outside this product's identity** + +- Becoming a general multi-game mod manager with user-driven conflict resolution as the core loop +- Embedding an MCP/headless API inside the desktop app (CLI/scripts remain the agent path) + +## Dependencies / Assumptions + +- Checkpoint/resume via the install coordinator remains the recovery foundation to extend. Verified: session/resume state exists in `src/ModSync.Core/Installation/` and is consumed by `src/ModSync.Core/Services/InstallationService.cs`, while the GUI exposes resume only for downloads — the wizard-facing recovery surface is the gap. +- Validation pipeline and FOMOD configure-before-install gate stay prerequisites to install start, and already cover much of the environment detection the readiness summary will surface. +- The installing page currently cannot be re-entered after it marks a run complete, so wizard flow changes are a prerequisite for the reopen-and-resume behavior. +- Industry pattern priority for guided installers: preflight + resumable progress over open-ended conflict tooling. +- No firsthand player incident was captured in this brainstorm; priority is inferred from strategy metrics (full-build pass rate) and external mod-manager pain patterns. + +## Outstanding Questions + +**Deferred to Planning** + +- Which readiness signals are re-surfaced from validation versus genuinely new probes (disk space and protected-path policy are the likely net-new candidates) +- How the wizard detects and offers a resumable session on re-entry, given the installing page is currently one-shot and treats any exit as completion +- Whether the documented-but-unread `--no-checkpoint` CLI option is wired up or removed, since it is a no-op today +- Exact progress presentation on the installing page versus download-status surfaces + +## Sources / Research + +- `STRATEGY.md` — player-primary hire; full-build pass rate metric +- `docs/knowledgebase/install-lifecycle.md` — wizard order, checkpoints, resume +- `docs/knowledgebase/product-vision.md` — vision vs current state +- `docs/plans/vortex-mo2-feature-parity-living-plan.md` — parity residuals (rollback/CAS deferred) +- External patterns (2024–2026): Wabbajack/Collections-style guided installers prioritize preflight, manifest transparency, and resumable long installs; Vortex/MO2 communities cite deployment/path failures and missing rollback as top trust failures diff --git a/docs/knowledgebase/core-cli-reference.md b/docs/knowledgebase/core-cli-reference.md index 589b537f..e59ed8eb 100644 --- a/docs/knowledgebase/core-cli-reference.md +++ b/docs/knowledgebase/core-cli-reference.md @@ -77,7 +77,7 @@ Install selected mods from an instruction file. | `--concurrent` | No | Parallel downloads | | `-y` / `--yes` | No | Auto-confirm prompts | | `--skip-validation` | No | Skip pre-install checks (not recommended) | -| `--no-checkpoint` | No | Disable checkpointing | +| `--no-checkpoint` | No | Disable Git checkpoint commits during install; `install_session.json` resume still works | | `--best-effort` | No | Continue on missing sources and mod failures; implies `-y`; without Nexus key, **deselects Nexus-only mods** | | `--continue-on-missing-sources` | No | Partial install when archives missing | | `--continue-on-mod-failure` | No | Continue after per-mod failure | diff --git a/docs/knowledgebase/gui-validation-surfaces.md b/docs/knowledgebase/gui-validation-surfaces.md index acdc03db..9f236e60 100644 --- a/docs/knowledgebase/gui-validation-surfaces.md +++ b/docs/knowledgebase/gui-validation-surfaces.md @@ -23,6 +23,7 @@ How the Avalonia app presents `InstallationValidationPipeline` results after PRs | Surface | Entry | Pipeline preset | Result UI | |---------|--------|-----------------|-----------| | Install wizard **ValidatePage** | `ValidateAsync` / **Run Validation** | `WizardFull` | In-page log, progress, summary badges; `ApplyPipelineResultToWizardUi` | +| Install wizard **InstallStartPage** | navigate-to / Next gate | `InstallStartReadiness` | Compact Environment readiness status; points back to Validate for full detail | | Legacy **Getting Started → Validate** | `MainWindow.ValidateButton_Click` | `WizardFull` | Progress dialog log + `ValidationDialog` mod issue list | | **ValidationService.AnalyzeValidationFailures** | Async helper (pre-check / failure analysis) | `WizardFull` | Populates `Dialogs.ValidationIssue` + optional `systemIssues` strings | | Core CLI **validate** / **install** pre-check | `ModBuildConverter` | `WizardFull` (install) | stdout / exit code — no GUI mapper | diff --git a/docs/knowledgebase/install-lifecycle.md b/docs/knowledgebase/install-lifecycle.md index 4433e30d..87006fc7 100644 --- a/docs/knowledgebase/install-lifecycle.md +++ b/docs/knowledgebase/install-lifecycle.md @@ -78,7 +78,9 @@ flowchart TD `[REPO]` **`InstallCoordinator`** owns **`GitCheckpointService`** for the KOTOR destination directory. After each successful component install, a checkpoint commit may be created; failures to checkpoint are logged as warnings and do not abort the install. -CLI: **`--no-checkpoint`** disables the checkpoint system (`ModBuildConverter` install options). Default is checkpoints **enabled**. +CLI: **`--no-checkpoint`** disables **Git** checkpoint commits (`enableGitCheckpoints: false` on `InstallationService` / `InstallCoordinator`). Session resume via `install_session.json` remains available. Default is Git checkpoints **enabled**. + +**InstallStartPage** runs a fast Environment readiness check (`ValidationPipelineOptions.InstallStartReadiness`) and may offer **Resume previous install** vs **Start over** when an incomplete session exists for the destination. **InstallingPage** surfaces healthy/failed/cancelled states and in-session Resume/Retry; it does not promise full-folder pristine rollback. ## Continue-on-failure flags diff --git a/docs/knowledgebase/validation-pipeline.md b/docs/knowledgebase/validation-pipeline.md index 4bdb9b18..7775b770 100644 --- a/docs/knowledgebase/validation-pipeline.md +++ b/docs/knowledgebase/validation-pipeline.md @@ -50,6 +50,7 @@ Details: [managed-deployment.md](managed-deployment.md#validation-vs-managed-ins | Preset | FullValidation | DryRun | DryRunOnly | UseFileSelection | Typical caller | |--------|----------------|--------|------------|------------------|----------------| | `WizardFull` | yes | yes | no | yes | Install wizard **ValidatePage**; legacy Validate; `install` pre-check | +| `InstallStartReadiness` | yes | no | no | yes | Install wizard **InstallStartPage** (Environment only; skips archives/FOMOD gate/conflicts/order/DryRun) | | `LegacyDryRunOnly` | no | yes | no | yes | Available preset; legacy Getting Started Validate currently uses `WizardFull` | | `CliFullWithDryRun` | yes | yes | no | yes | CLI `--full --dry-run --use-file-selection` | | `CliDryRunOnly` | no | no | yes | yes | CLI `--dry-run-only` | @@ -61,6 +62,7 @@ Other flags: | `SkipEnvironmentValidation` | Tests, headless fixtures without HoloPatcher | | `SkipComponentArchiveValidation` | Graph-only tests | | `SkipFomodConfigurationGate` | Tests without FOMOD fixtures | +| `SkipConflictAndOrderValidation` | Install-start readiness; graph-only skip | | `ErrorsOnly` | `--errors-only` | | `UseFileSelection` | `--use-file-selection` (default true in options type; CLI defaults differ — see below) | diff --git a/docs/plans/2026-07-29-001-feat-trust-first-install-experience-plan.md b/docs/plans/2026-07-29-001-feat-trust-first-install-experience-plan.md new file mode 100644 index 00000000..a16b074e --- /dev/null +++ b/docs/plans/2026-07-29-001-feat-trust-first-install-experience-plan.md @@ -0,0 +1,208 @@ +--- +date: 2026-07-29 +type: feat +status: active +origin: docs/brainstorms/2026-07-25-trust-first-install-experience-requirements.md +--- + +# Trust-first install experience + +## Summary + +Make long guided installs feel safe to start and safe to continue: a pre-install readiness summary with plain-language blockers, always-visible install progress, and resume/retry that survives closing and reopening the wizard — without ever implying a one-click full-folder rollback. + +The implementation already exists, uncommitted, on the current branch and builds clean with its own tests passing (9/9 across 4 new test files). This plan is a verify-and-finish pass, not a build-from-scratch pass: confirm the existing work against R1-R9, close two identified test-coverage gaps, and get it committed and shipped. + +--- + +## Problem Frame + +KOTOR full builds are long, order-sensitive, and opaque. ModSync already validates, checkpoints, and can resume components under the hood, but players experience failure as "start, fail, guess, restart" because the wizard never surfaced readiness, health, or recovery clearly. (see origin: docs/brainstorms/2026-07-25-trust-first-install-experience-requirements.md) + +Prior to this branch's uncommitted work, the installing page could not be re-entered after marking a run complete, and the deleted `InstallationErrorDialog` offered a rollback choice implying full pristine-folder restoration the product doesn't deliver. + +--- + +## Requirements + +**Install-start readiness** + +- R1. Readiness summary before install start, reusing existing validation detection. +- R2. Plain-language remediation; player cannot advance while blockers remain. +- R3. Compact pass/fail surface, pointing back to full validation detail rather than duplicating it. + +**Progress** + +- R4. Active component, overall progress, and health state always visible during install. +- R5. Copy communicates interruption is recoverable; never implies "start over from zero." + +**Recovery** + +- R6. Resume/retry from the last completed component, offered in the same session. +- R7. Resume/retry stays available after closing and reopening the wizard against the same instruction file and directories. +- R8. Completed-vs-remaining shown on failure; when resume is unavailable, UI says so and never implies one-click pristine-folder restoration. + +**Non-regression** + +- R9. Headless CLI resume-by-session behavior must not regress. + +--- + +## Current Implementation State (verified this session) + +The uncommitted code on this branch was audited directly (build + targeted test runs, not just diff review). All nine requirements have corresponding implementation: + +| Requirement | Status | Evidence | +|---|---|---| +| R1 | Done | `ValidationPipelineOptions.InstallStartReadiness` preset (environment-only); `InstallStartPage.RunReadinessAsync` runs it via the existing `InstallationValidationPipeline` | +| R2 | Done | `InstallStartPage.ValidateAsync` blocks `Next` on `_readinessHasCriticalErrors`, surfacing `_readinessErrorMessage` | +| R3 | Done | `_readinessStatusText`/`_readinessDetailText` show only the Environment stage summary, capped to 3 messages | +| R4 | Done | `InstallingPage.ProgressCallback` updates active-mod text, progress bar, count, and `_runStateText` on every callback | +| R5 | Done | `ApplyTerminalOutcomeAsync` copy: "Resume continues remaining mods; it does not restore a pristine game folder." | +| R6 | Done | `InstallingPage.ResumeRetryButton_Click` re-invokes install; `InstallationService` already skips Completed/Skipped/Blocked components | +| R7 | Done | `CheckpointManager.ProbeResumableSessionAsync` reads `install_session.json` from disk on every `InstallStartPage.OnNavigatedToAsync`, independent of in-memory session state | +| R8 | Done | `ApplyTerminalOutcomeAsync` reports completed/remaining counts; states "resume may be unavailable" when applicable; rollback-implying `InstallationErrorDialog` was deleted | +| R9 | Done | CLI resume-by-session logic in `InstallAllSelectedComponentsCoreAsync` is untouched; `--no-checkpoint` now wired to skip only Git checkpoint commits, confirmed by `NoCheckpointInstallTests` | + +Build: `dotnet build ModSync.sln` succeeds, 0 errors. New tests: `InstallStartReadinessOptionsTests` (2/2), `NoCheckpointInstallTests` (2/2), `WizardResumeReentryTests` (3/3), `InstallStartPageHeadlessTests` (2/2) — all pass individually (a combined OR filter under-matches due to a vstest filter-parsing quirk, not a real failure). + +The deleted `InstallationErrorDialog`'s `InstallationErrorEventArgs`/`ErrorAction` types are **not** dead code — they remain alive via `InstallationCoordinatorService`, consumed by the separate, pre-existing `CheckpointManagementDialog` (the Git checkpoint browser the brainstorm explicitly keeps distinct and out of scope). No removal needed. + +--- + +## Key Technical Decisions + +- **Verify-and-finish framing.** Treat the existing uncommitted code as the implementation baseline. Do not rewrite working, tested logic; this plan's units close specific verified gaps and get the result shipped. +- **Resume identity stays destination-path + GUID-overlap.** R7/AE3 say "same instruction file and directories," but the implemented check only matches on destination path and overlapping component GUIDs, not an instruction-file fingerprint. Confirmed with the user: keep the current proxy. A different build sharing components at the same destination triggering a false resume offer is judged a rare edge case not worth the added persistence complexity right now. (see origin: Outstanding Questions) +- **Checkpoint browser stays untouched.** `CheckpointManagementDialog`/`InstallationCoordinatorService` are pre-existing, distinct recovery tooling per the origin document's "three recovery concepts stay distinct" decision. This plan does not touch them. + +--- + +## Implementation Units + +### U1. Commit the existing readiness + resume implementation + +**Goal:** Get the already-implemented, already-passing trust-first install code (currently entirely uncommitted) into logical, reviewable commits. + +**Requirements:** R1-R9 + +**Dependencies:** None + +**Files:** +- `src/ModSync.Core/Installation/InstallCoordinator.cs` +- `src/ModSync.Core/Installation/ResumableSessionInfo.cs` (new) +- `src/ModSync.Core/Services/Checkpoints/CheckpointPaths.cs` +- `src/ModSync.Core/Services/InstallationService.cs` +- `src/ModSync.Core/Services/Validation/InstallationValidationPipeline.cs` +- `src/ModSync.Core/Services/Validation/ValidationPipelineOptions.cs` +- `src/ModSync.Core/CLI/ModBuildConverter.cs` +- `src/ModSync.GUI/Dialogs/WizardPages/InstallStartPage.axaml`, `.axaml.cs` +- `src/ModSync.GUI/Dialogs/WizardPages/InstallingPage.axaml`, `.axaml.cs` +- `src/ModSync.GUI/Dialogs/WizardPages/FinishedPage.axaml` +- `src/ModSync.GUI/Dialogs/WizardPages/BaseInstallCompletePage.axaml` +- `src/ModSync.GUI/Dialogs/InstallationErrorDialog.axaml`, `.axaml.cs` (deletion) +- `src/ModSync.Tests/InstallStartReadinessOptionsTests.cs` (new) +- `src/ModSync.Tests/NoCheckpointInstallTests.cs` (new) +- `src/ModSync.Tests/WizardResumeReentryTests.cs` (new) +- `src/ModSync.Tests/HeadlessUITests/InstallStartPageHeadlessTests.cs` (new) +- `src/ModSync.Tests/HeadlessUITests/FomodGateHeadlessTests.cs`, `InstallingPageHeadlessTests.cs` + +**Approach:** No new code. Re-verify build and the four new test files pass (already confirmed this session), then stage and commit in logical groups — e.g., Core services (readiness preset, checkpoint probe, `--no-checkpoint` wiring), GUI wizard pages (readiness UI, resume/retry UI, dialog deletion), and their tests together per group. + +**Test scenarios:** Test expectation: none — this unit verifies and commits existing, already-tested code; no new behavior is introduced. + +**Verification:** `dotnet build ModSync.sln` succeeds; the four new test files each pass in isolation; `git status` shows a clean tree for these files afterward. + +--- + +### U2. Close InstallStartPage resume/start-over headless coverage gap + +**Goal:** Add headless UI coverage for the resume-offer interaction paths that exist in production code but aren't exercised by any test today. + +**Requirements:** R7 (AE3) + +**Dependencies:** U1 + +**Files:** +- `src/ModSync.Tests/HeadlessUITests/InstallStartPageHeadlessTests.cs` + +**Approach:** Extend the existing headless test fixture (same `[AvaloniaFact]` pattern already in the file) to drive `ResumePreviousButton_Click`, `StartOverButton_Click`, and `ApplyStartOverAsync`, plus the "resume offered, no choice made yet" blocking state. + +**Patterns to follow:** `InstallStartPage_Readiness_BlocksWhenDirectoriesUnset` and `InstallStartPage_Readiness_AllowsWhenClean` already in the file — same harness setup, same assertion style. + +**Test scenarios:** +- Happy path: a resumable session exists for the destination; the resume offer renders and `Next` is blocked until `ResumeChoice` is set. +- Edge case: clicking **Resume previous** sets the resume choice, unblocks `Next`, and does not clear the on-disk session. +- Edge case: clicking **Start over** calls `ApplyStartOverAsync`, clears resumability, and unblocks `Next`. +- Error path: FOMOD-gate failure during readiness blocks `Next` with the gate's own remediation message (not the generic readiness message). + +**Verification:** New tests pass; existing `InstallStartPageHeadlessTests` tests continue to pass unchanged. + +--- + +### U3. Verify InstallingPage resume/retry headless coverage + +**Goal:** Confirm the already-modified `InstallingPageHeadlessTests.cs` actually exercises the `ResumeRetryButton_Click` → re-run → success path and the `RunStateText`/`FailureSummaryText` content contracts; add what's missing. + +**Requirements:** R5, R6, R8 (AE2) + +**Dependencies:** U1 + +**Files:** +- `src/ModSync.Tests/HeadlessUITests/InstallingPageHeadlessTests.cs` + +**Approach:** Read the current file in full first — this unit may turn out to be verification-only if coverage is already adequate (per Phase 2 execution loop: if the work already matches intent, confirm and move on rather than reimplementing). If gaps exist, add scenarios following the existing test patterns in the file. + +**Test scenarios:** +- Happy path: a failed run with remaining components; clicking Resume/Retry re-invokes install and completed components are skipped. +- Edge case: `RunStateDisplayText` reflects healthy/failed/cancelled states accurately across a full run. +- Integration: `ApplyTerminalOutcomeAsync`'s completed/remaining counts match the actual component states after a partial failure. + +**Verification:** All scenarios above are covered and pass; no regressions in the file's existing tests. + +--- + +### U4. Confirm remaining knowledgebase docs are in sync + +**Goal:** `docs/knowledgebase/install-lifecycle.md` was confirmed accurate this session; confirm the other three touched docs are equally in sync with the shipped behavior. + +**Requirements:** Documentation accuracy supporting R1-R9 + +**Dependencies:** U1 + +**Files:** +- `docs/knowledgebase/core-cli-reference.md` +- `docs/knowledgebase/gui-validation-surfaces.md` +- `docs/knowledgebase/validation-pipeline.md` +- `AGENTS.md` + +**Approach:** Read each doc's current (uncommitted) diff against the actual implemented behavior from U1-U3; fix any drift found. + +**Test scenarios:** Test expectation: none — documentation-only unit. + +**Verification:** Each doc's description of readiness/resume/`--no-checkpoint` behavior matches the actual code. + +--- + +## Scope Boundaries + +**In scope** +- Verifying, closing test gaps in, and shipping the already-implemented readiness/progress/resume wizard UX and its CLI non-regression. + +**Deferred for later** +- One-action pristine game-folder rollback / CAS pre-image restore (see origin: Scope Boundaries) +- Unsafe operator overrides for readiness blockers +- Redesigning the checkpoint management dialog as the primary player recovery surface +- Instruction-file-fingerprint tightening of resume identity (see Key Technical Decisions) + +**Outside this product's identity** +- Becoming a general multi-game mod manager with user-driven conflict resolution +- Embedding an MCP/headless API inside the desktop app + +--- + +## Verification + +- `dotnet build ModSync.sln` succeeds with no new errors. +- All existing and newly-added tests in the files listed above pass. +- Manual smoke check: readiness summary blocks on a bad path, install progress updates live, and closing/reopening the wizard against a partially-completed destination offers Resume. diff --git a/src/ModSync.Core/CLI/ModBuildConverter.cs b/src/ModSync.Core/CLI/ModBuildConverter.cs index d898b25e..075364fd 100644 --- a/src/ModSync.Core/CLI/ModBuildConverter.cs +++ b/src/ModSync.Core/CLI/ModBuildConverter.cs @@ -3307,7 +3307,8 @@ await Logger.LogAsync( }, cancellationToken: default, profileOverride: profileOverride, - managedDeploymentOverride: managedOverride + managedDeploymentOverride: managedOverride, + enableGitCheckpoints: !opts.NoCheckpoint ).ConfigureAwait(false); if (InstallationService.LastManagedInstallResult != null) diff --git a/src/ModSync.Core/Installation/InstallCoordinator.cs b/src/ModSync.Core/Installation/InstallCoordinator.cs index 8656f6f2..cd4a91f1 100644 --- a/src/ModSync.Core/Installation/InstallCoordinator.cs +++ b/src/ModSync.Core/Installation/InstallCoordinator.cs @@ -31,13 +31,30 @@ public InstallCoordinator() public CheckpointManager CheckpointManager { get; } public Services.GitCheckpointService CheckpointService { get; private set; } - public async Task InitializeAsync([NotNull] IList components, [NotNull] DirectoryInfo destinationPath, CancellationToken cancellationToken) + public Task InitializeAsync( + [NotNull] IList components, + [NotNull] DirectoryInfo destinationPath, + CancellationToken cancellationToken) => + InitializeAsync(components, destinationPath, cancellationToken, enableGitCheckpoints: true); + + public async Task InitializeAsync( + [NotNull] IList components, + [NotNull] DirectoryInfo destinationPath, + CancellationToken cancellationToken, + bool enableGitCheckpoints) { await CheckpointManager.InitializeAsync(components, destinationPath).ConfigureAwait(false); await CheckpointManager.EnsureSnapshotAsync(destinationPath, cancellationToken).ConfigureAwait(false); ReleaseCheckpointService(); + if (!enableGitCheckpoints) + { + await CheckpointManager.SaveAsync().ConfigureAwait(false); + List orderedWithoutGit = GetOrderedInstallList(components); + return new ResumeResult(CheckpointManager.State.SessionId, orderedWithoutGit); + } + // Initialize Git-based checkpoint system CheckpointService = new Services.GitCheckpointService(destinationPath.FullName); _checkpointServiceDirectory = NormalizeDirectoryKey(destinationPath.FullName); diff --git a/src/ModSync.Core/Installation/ResumableSessionInfo.cs b/src/ModSync.Core/Installation/ResumableSessionInfo.cs new file mode 100644 index 00000000..1bccf826 --- /dev/null +++ b/src/ModSync.Core/Installation/ResumableSessionInfo.cs @@ -0,0 +1,38 @@ +// Copyright 2021-2025 ModSync +// Licensed under the Business Source License 1.1 (BSL 1.1). +// See LICENSE.txt file in the project root for full license information. + +namespace ModSync.Core.Installation +{ + /// + /// Result of probing install_session.json for an incomplete install that can be resumed. + /// + public readonly struct ResumableSessionInfo + { + public ResumableSessionInfo( + bool isResumable, + int completedSelectedCount, + int remainingSelectedCount, + int overlappingComponentCount) + { + IsResumable = isResumable; + CompletedSelectedCount = completedSelectedCount; + RemainingSelectedCount = remainingSelectedCount; + OverlappingComponentCount = overlappingComponentCount; + } + + public bool IsResumable { get; } + + public int CompletedSelectedCount { get; } + + public int RemainingSelectedCount { get; } + + public int OverlappingComponentCount { get; } + + public static ResumableSessionInfo None { get; } = new ResumableSessionInfo( + isResumable: false, + completedSelectedCount: 0, + remainingSelectedCount: 0, + overlappingComponentCount: 0); + } +} diff --git a/src/ModSync.Core/ModComponent.cs b/src/ModSync.Core/ModComponent.cs index f1502272..88f4f1d0 100644 --- a/src/ModSync.Core/ModComponent.cs +++ b/src/ModSync.Core/ModComponent.cs @@ -807,8 +807,10 @@ public string SerializeComponent() return Services.ModComponentSerializationService.SerializeSingleComponentAsTomlString(this); } + /// The raw TOML content to deserialize. + /// See . [CanBeNull] - public static ModComponent DeserializeTomlComponent([NotNull] string tomlString) + public static ModComponent DeserializeTomlComponent([NotNull] string tomlString, bool requireName = true) { if (tomlString is null) { @@ -816,7 +818,7 @@ public static ModComponent DeserializeTomlComponent([NotNull] string tomlString) } // Use the unified deserialization service - IReadOnlyList components = Services.ModComponentSerializationService.DeserializeModComponentFromTomlString(tomlString); + IReadOnlyList components = Services.ModComponentSerializationService.DeserializeModComponentFromTomlString(tomlString, requireName); return components?.FirstOrDefault(); } public async Task InstallAsync( diff --git a/src/ModSync.Core/Parsing/MarkdownParser.cs b/src/ModSync.Core/Parsing/MarkdownParser.cs index 5c6c4da9..a125b247 100644 --- a/src/ModSync.Core/Parsing/MarkdownParser.cs +++ b/src/ModSync.Core/Parsing/MarkdownParser.cs @@ -822,7 +822,7 @@ private void ParseModSyncMetadata([NotNull] ModComponent component, [NotNull] st _logVerbose($" Detected YAML format, attempting to deserialize..."); try { - ModComponent yamlComponent = Services.ModComponentSerializationService.DeserializeYamlComponent(metadataText); + ModComponent yamlComponent = Services.ModComponentSerializationService.DeserializeYamlComponent(metadataText, requireName: false); if (yamlComponent != null) { @@ -848,7 +848,7 @@ private void ParseModSyncMetadata([NotNull] ModComponent component, [NotNull] st tomlString = "[[thisMod]]\n" + metadataText; } - var tomlComponent = ModComponent.DeserializeTomlComponent(tomlString); + var tomlComponent = ModComponent.DeserializeTomlComponent(tomlString, requireName: false); if (tomlComponent != null) { diff --git a/src/ModSync.Core/Services/Checkpoints/CheckpointPaths.cs b/src/ModSync.Core/Services/Checkpoints/CheckpointPaths.cs index 5e976c9e..cc6bba51 100644 --- a/src/ModSync.Core/Services/Checkpoints/CheckpointPaths.cs +++ b/src/ModSync.Core/Services/Checkpoints/CheckpointPaths.cs @@ -92,9 +92,25 @@ public async Task InitializeAsync([NotNull] IList components, [Not if (File.Exists(_sessionPath)) { - string json = await NetFrameworkCompatibility.ReadAllTextAsync(_sessionPath, Encoding.UTF8).ConfigureAwait(false); - InstallSessionState existingState = JsonConvert.DeserializeObject(json, s_serializerSettings); - if (existingState != null && ValidateLoadedState(existingState)) + InstallSessionState existingState = null; + try + { + string json = await NetFrameworkCompatibility.ReadAllTextAsync(_sessionPath, Encoding.UTF8).ConfigureAwait(false); + existingState = JsonConvert.DeserializeObject(json, s_serializerSettings); + } + catch (Exception ex) when (ex is JsonException || ex is IOException) + { + Logger.LogWarning($"Failed to read existing session at '{_sessionPath}': {ex.Message}"); + } + + // Only adopt a session written for this same destination. A session copied in + // from a different install (e.g. the whole game folder was copied elsewhere) + // must not silently donate its Completed/Skipped markers to an unrelated install. + bool destinationMatches = existingState is null + || string.IsNullOrWhiteSpace(existingState.DestinationPath) + || PathsEqual(existingState.DestinationPath, destinationPath.FullName); + + if (existingState != null && destinationMatches && ValidateLoadedState(existingState)) { _state = existingState; SyncComponentsWithState(components); @@ -151,6 +167,120 @@ public async Task DeleteSessionAsync() } } + /// + /// Deletes install_session.json for a destination without initializing a manager instance. + /// Does not remove Git checkpoint history under .modsync/checkpoints. + /// + public static Task DeleteSessionFileAsync([NotNull] DirectoryInfo destinationPath) + { + if (destinationPath is null) + { + throw new ArgumentNullException(nameof(destinationPath)); + } + + string sessionPath = GetSessionFilePath(destinationPath); + if (File.Exists(sessionPath)) + { + File.Delete(sessionPath); + } + + return Task.CompletedTask; + } + + /// + /// Probes an existing install session for incomplete selected work that can be resumed. + /// Identity is destination path plus overlapping component GUIDs (no build fingerprint required). + /// + [NotNull] + public static async Task ProbeResumableSessionAsync( + [NotNull] DirectoryInfo destinationPath, + [NotNull] IEnumerable selectedComponents) + { + if (destinationPath is null) + { + throw new ArgumentNullException(nameof(destinationPath)); + } + + if (selectedComponents is null) + { + throw new ArgumentNullException(nameof(selectedComponents)); + } + + string sessionPath = GetSessionFilePath(destinationPath); + if (!File.Exists(sessionPath)) + { + return ResumableSessionInfo.None; + } + + InstallSessionState state; + try + { + string json = await NetFrameworkCompatibility.ReadAllTextAsync(sessionPath, Encoding.UTF8).ConfigureAwait(false); + state = JsonConvert.DeserializeObject(json, s_serializerSettings); + } + catch (Exception ex) when (ex is JsonException || ex is IOException) + { + // A corrupted or unreadable session file (e.g. truncated by a crash mid-write) + // is treated the same as "no session" -- fail closed rather than surfacing an + // unhandled exception on the wizard's navigation path. + Logger.LogWarning($"Failed to read resumable session at '{sessionPath}': {ex.Message}"); + return ResumableSessionInfo.None; + } + + if (state is null || !ValidateLoadedState(state)) + { + return ResumableSessionInfo.None; + } + + if (!string.IsNullOrWhiteSpace(state.DestinationPath) && + !PathsEqual(state.DestinationPath, destinationPath.FullName)) + { + return ResumableSessionInfo.None; + } + + var selected = selectedComponents + .Where(c => c != null && c.IsSelected && !c.WidescreenOnly) + .ToList(); + if (selected.Count == 0) + { + return ResumableSessionInfo.None; + } + + int overlapping = 0; + int completed = 0; + int remaining = 0; + + foreach (ModComponent component in selected) + { + if (!state.Components.TryGetValue(component.Guid, out ComponentSessionEntry entry)) + { + continue; + } + + overlapping++; + if (entry.State == ModComponent.ComponentInstallState.Completed || + entry.State == ModComponent.ComponentInstallState.Skipped) + { + completed++; + } + else + { + remaining++; + } + } + + // Need overlap with the prior session and at least one incomplete selected component. + bool isResumable = overlapping > 0 && remaining > 0; + return new ResumableSessionInfo(isResumable, completed, remaining, overlapping); + } + + private static bool PathsEqual(string left, string right) + { + string a = Path.GetFullPath(left).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + string b = Path.GetFullPath(right).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + return string.Equals(a, b, StringComparison.OrdinalIgnoreCase); + } + [NotNull] public ComponentSessionEntry GetComponentEntry(Guid componentId) { diff --git a/src/ModSync.Core/Services/InstallationService.cs b/src/ModSync.Core/Services/InstallationService.cs index 2de54ed1..298d55dd 100644 --- a/src/ModSync.Core/Services/InstallationService.cs +++ b/src/ModSync.Core/Services/InstallationService.cs @@ -1048,7 +1048,8 @@ await Logger.LogErrorAsync( [CanBeNull] Action progressCallback = null, CancellationToken cancellationToken = default, [CanBeNull] string profileOverride = null, - bool? managedDeploymentOverride = null) + bool? managedDeploymentOverride = null, + bool enableGitCheckpoints = true) { if (allComponents is null) { @@ -1056,7 +1057,11 @@ await Logger.LogErrorAsync( } return await RunWithManagedInstallSessionAsync( - () => InstallAllSelectedComponentsCoreAsync(allComponents, progressCallback, cancellationToken), + () => InstallAllSelectedComponentsCoreAsync( + allComponents, + progressCallback, + cancellationToken, + enableGitCheckpoints), profileOverride, managedDeploymentOverride).ConfigureAwait(false); } @@ -1064,7 +1069,8 @@ await Logger.LogErrorAsync( private static async Task InstallAllSelectedComponentsCoreAsync( [NotNull][ItemNotNull] List allComponents, [CanBeNull] Action progressCallback, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + bool enableGitCheckpoints = true) { if (allComponents is null) { @@ -1105,7 +1111,11 @@ await Logger.LogErrorAsync( { DirectoryInfo destination = MainConfig.DestinationPath ?? throw new InvalidOperationException("DestinationPath must be set before installing."); - ResumeResult resume = await coordinator.InitializeAsync(allComponents, destination, cancellationToken).ConfigureAwait(false); + ResumeResult resume = await coordinator.InitializeAsync( + allComponents, + destination, + cancellationToken, + enableGitCheckpoints).ConfigureAwait(false); var orderedComponents = resume.OrderedComponents.Where(component => component.IsSelected).ToList(); int total = orderedComponents.Count; ModComponent.InstallExitCode exitCode = ModComponent.InstallExitCode.Success; @@ -1143,22 +1153,25 @@ await Logger.LogErrorAsync( { await Logger.LogAsync($"Install of '{component.Name}' succeeded.").ConfigureAwait(false); - // Create checkpoint after successful installation - try - { - CheckpointInfo checkpoint = await coordinator.CheckpointService.CreateCheckpointAsync( - component, - index + 1, - total, - cancellationToken - ).ConfigureAwait(false); - - coordinator.CheckpointManager.State.ComponentCheckpoints[component.Guid] = checkpoint.CommitId; - await Logger.LogAsync($"✓ Checkpoint created: {checkpoint.ShortCommitId}").ConfigureAwait(false); - } - catch (Exception ex) + // Create checkpoint after successful installation (when Git checkpoints are enabled) + if (coordinator.CheckpointService != null) { - await Logger.LogWarningAsync($"Failed to create checkpoint for '{component.Name}': {ex.Message}").ConfigureAwait(false); + try + { + CheckpointInfo checkpoint = await coordinator.CheckpointService.CreateCheckpointAsync( + component, + index + 1, + total, + cancellationToken + ).ConfigureAwait(false); + + coordinator.CheckpointManager.State.ComponentCheckpoints[component.Guid] = checkpoint.CommitId; + await Logger.LogAsync($"✓ Checkpoint created: {checkpoint.ShortCommitId}").ConfigureAwait(false); + } + catch (Exception ex) + { + await Logger.LogWarningAsync($"Failed to create checkpoint for '{component.Name}': {ex.Message}").ConfigureAwait(false); + } } await coordinator.CheckpointManager.PromoteSnapshotAsync(destination, cancellationToken).ConfigureAwait(false); @@ -1236,7 +1249,8 @@ await Logger.LogWarningAsync( [CanBeNull] Action progressCallback = null, CancellationToken cancellationToken = default, [CanBeNull] string profileOverride = null, - bool? managedDeploymentOverride = null) + bool? managedDeploymentOverride = null, + bool enableGitCheckpoints = true) { if (allComponents is null) { @@ -1248,7 +1262,8 @@ await Logger.LogWarningAsync( progressCallback, cancellationToken, profileOverride, - managedDeploymentOverride); + managedDeploymentOverride, + enableGitCheckpoints); } } diff --git a/src/ModSync.Core/Services/ModComponentSerializationService.cs b/src/ModSync.Core/Services/ModComponentSerializationService.cs index 1c805572..d4f03b6e 100644 --- a/src/ModSync.Core/Services/ModComponentSerializationService.cs +++ b/src/ModSync.Core/Services/ModComponentSerializationService.cs @@ -229,9 +229,11 @@ private static void UpdateCacheFromResourceRegistry([NotNull][ItemNotNull] IRead } #region Loading Functions + /// The raw TOML content to deserialize. + /// See . [NotNull] [ItemNotNull] - public static IReadOnlyList DeserializeModComponentFromTomlString([NotNull] string tomlContent) + public static IReadOnlyList DeserializeModComponentFromTomlString([NotNull] string tomlContent, bool requireName = true) { Logger.LogVerbose("Loading from TOML string"); if (tomlContent is null) @@ -420,7 +422,7 @@ public static IReadOnlyList DeserializeModComponentFromTomlString( Logger.LogVerbose($"TOML component does NOT have Instructions field. Available keys: {string.Join(", ", componentDict.Keys)}"); } - ModComponent thisComponent = DeserializeComponent(componentDict); + ModComponent thisComponent = DeserializeComponent(componentDict, requireName); // Assign collected instructions to this component // Only if they weren't already deserialized from componentDict @@ -932,7 +934,7 @@ public static string DetectFormatFromContent([NotNull] string content) (string Format, Func> Parse)[] cascade = { - ("toml", DeserializeModComponentFromTomlString), + ("toml", tomlText => DeserializeModComponentFromTomlString(tomlText)), ("markdown", DeserializeModComponentFromMarkdownString), ("yaml", DeserializeModComponentFromYamlString), }; @@ -1129,34 +1131,16 @@ private static void ProcessInstructionsAndOptions(Dictionary pro Logger.LogVerbose("ProcessInstructionsAndOptions: No KeyValuePair instruction items, processing individually"); var processedInstructions = new List(); - var currentInstruction = new Dictionary(StringComparer.Ordinal); - foreach (object item in instructionsList) { Logger.LogVerbose($"ProcessInstructionsAndOptions: Processing instruction item of type {item.GetType().Name}"); - if (item is KeyValuePair kvp) - { - Logger.LogVerbose($"ProcessInstructionsAndOptions: KeyValuePair - {kvp.Key} = {kvp.Value}"); - currentInstruction[kvp.Key] = kvp.Value; - - // Check if this completes an instruction (has Action field) - if (kvp.Key.Equals("Action", StringComparison.OrdinalIgnoreCase) - && !string.IsNullOrEmpty(kvp.Value?.ToString())) - { - processedInstructions.Add(new Dictionary(currentInstruction, StringComparer.Ordinal)); - currentInstruction.Clear(); - } - } - else if (item is Dictionary dict) + // This branch only runs when hasKeyValuePairs is false, so no item here can + // be a KeyValuePair (its GetType().Name always starts with + // "KeyValuePair", which would have made hasKeyValuePairs true above). + if (item is Dictionary dict) { Logger.LogVerbose($"ProcessInstructionsAndOptions: Dictionary with {dict.Count} keys: {string.Join(", ", dict.Keys)}"); - if (currentInstruction.Count > 0) - - { - processedInstructions.Add(new Dictionary(currentInstruction, StringComparer.Ordinal)); - currentInstruction.Clear(); - } processedInstructions.Add(dict); } else @@ -1165,12 +1149,6 @@ private static void ProcessInstructionsAndOptions(Dictionary pro } } - if (currentInstruction.Count > 0) - - { - processedInstructions.Add(new Dictionary(currentInstruction, StringComparer.Ordinal)); - } - Logger.LogVerbose($"ProcessInstructionsAndOptions: Processed {processedInstructions.Count} instructions"); processedDict["Instructions"] = processedInstructions; } @@ -1197,34 +1175,16 @@ private static void ProcessInstructionsAndOptions(Dictionary pro Logger.LogVerbose("ProcessInstructionsAndOptions: No KeyValuePair option items, processing individually"); var processedOptions = new List(); - var currentOption = new Dictionary(StringComparer.Ordinal); - foreach (object item in optionsList) { Logger.LogVerbose($"ProcessInstructionsAndOptions: Processing option item of type {item.GetType().Name}"); - if (item is KeyValuePair kvp) - { - Logger.LogVerbose($"ProcessInstructionsAndOptions: KeyValuePair - {kvp.Key} = {kvp.Value}"); - currentOption[kvp.Key] = kvp.Value; - - // Check if this completes an option (has Name field) - if (kvp.Key.Equals("Name", StringComparison.OrdinalIgnoreCase) - && !string.IsNullOrEmpty(kvp.Value?.ToString())) - { - processedOptions.Add(new Dictionary(currentOption, StringComparer.Ordinal)); - currentOption.Clear(); - } - } - else if (item is Dictionary dict) + // This branch only runs when hasKeyValuePairs is false, so no item here can + // be a KeyValuePair (its GetType().Name always starts with + // "KeyValuePair", which would have made hasKeyValuePairs true above). + if (item is Dictionary dict) { Logger.LogVerbose($"ProcessInstructionsAndOptions: Dictionary with {dict.Count} keys: {string.Join(", ", dict.Keys)}"); - if (currentOption.Count > 0) - - { - processedOptions.Add(new Dictionary(currentOption, StringComparer.Ordinal)); - currentOption.Clear(); - } processedOptions.Add(dict); } else @@ -1233,12 +1193,6 @@ private static void ProcessInstructionsAndOptions(Dictionary pro } } - if (currentOption.Count > 0) - - { - processedOptions.Add(new Dictionary(currentOption, StringComparer.Ordinal)); - } - Logger.LogVerbose($"ProcessInstructionsAndOptions: Processed {processedOptions.Count} options"); processedDict["Options"] = processedOptions; } @@ -1355,15 +1309,13 @@ private static List GroupKeyValuePairsIntoInstructions(List kvpL Logger.LogVerbose($"GroupKeyValuePairsIntoInstructions: Processing {key} = {value}"); - // Check if this is a new instruction (Action field marks the start of a new instruction) - if (!(key is null) && key.Equals("Action", StringComparison.OrdinalIgnoreCase) && value != null) + // A repeated key means the flat KeyValuePair stream has wrapped around to the + // next instruction (field order varies - e.g. Guid may precede or follow Action), + // so use key repetition rather than a hardcoded field name as the boundary signal. + if (!(key is null) && currentInstruction.ContainsKey(key)) { - // If we have a current instruction, save it before starting a new one - if (currentInstruction.Count > 0) - { - instructions.Add(new Dictionary(currentInstruction, StringComparer.Ordinal)); - currentInstruction.Clear(); - } + instructions.Add(new Dictionary(currentInstruction, StringComparer.Ordinal)); + currentInstruction.Clear(); } currentInstruction[key] = value; } @@ -1420,21 +1372,18 @@ private static List GroupKeyValuePairsIntoOptions(List kvpList) } Logger.LogVerbose($"GroupKeyValuePairsIntoOptions: Processing {key} = {value}"); - currentOption[key] = value; - // Check if this completes an option (has Name field and we've seen a Guid) - if ( - key?.Equals("Name", StringComparison.OrdinalIgnoreCase) == true - && !string.IsNullOrEmpty(value?.ToString()) - && currentOption.ContainsKey("Guid") - && currentOption.Count > 0 - ) + // A repeated key means the flat KeyValuePair stream has wrapped around to the + // next option (field order varies - e.g. Guid may precede or follow Name), + // so use key repetition rather than a hardcoded field name as the boundary signal. + if (!(key is null) && currentOption.ContainsKey(key)) { Logger.LogVerbose($"GroupKeyValuePairsIntoOptions: Completed option with {currentOption.Count} fields"); options.Add(new Dictionary(currentOption, StringComparer.Ordinal)); currentOption.Clear(); } + currentOption[key] = value; } } @@ -1453,13 +1402,22 @@ private static List GroupKeyValuePairsIntoOptions(List kvpList) /// Deserializes a component from a dictionary with all conditional logic unified. /// This is the migrated version from ModComponent.DeserializeComponent. /// + /// The raw key/value pairs to deserialize. + /// + /// Whether the "Name" field must be present. Standalone instruction files (TOML/JSON/XML) + /// always carry Name and should pass true. Embedded per-component metadata blocks + /// (the `<!--<<ModSync>>` markdown comment) intentionally omit Name — it lives in the + /// surrounding markdown header — so callers merging that partial data should pass false. + /// [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "MA0051:Method is too long", Justification = "")] - public static ModComponent DeserializeComponent([NotNull] IDictionary componentDict) + public static ModComponent DeserializeComponent([NotNull] IDictionary componentDict, bool requireName = true) { var component = new ModComponent(); component.Guid = GetRequiredValue(componentDict, key: "Guid"); - component.Name = GetRequiredValue(componentDict, key: "Name"); + component.Name = requireName + ? GetRequiredValue(componentDict, key: "Name") + : GetValueOrDefault(componentDict, key: "Name") ?? string.Empty; _ = Logger.LogVerboseAsync($" == Deserialize next component '{component.Name}' =="); component.Author = GetValueOrDefault(componentDict, key: "Author") ?? string.Empty; component.Heading = GetValueOrDefault(componentDict, key: "Heading") ?? string.Empty; @@ -3644,8 +3602,10 @@ private static Dictionary ConvertToStringObjectDictionary(object return result; } + /// The raw YAML content to deserialize. + /// See . [CanBeNull] - public static ModComponent DeserializeYamlComponent([NotNull] string yamlString) + public static ModComponent DeserializeYamlComponent([NotNull] string yamlString, bool requireName = true) { if (yamlString is null) { @@ -3682,7 +3642,7 @@ public static ModComponent DeserializeYamlComponent([NotNull] string yamlString) // Pre-process the component dictionary to handle duplicate fields yamlDict = PreprocessComponentDictionary(yamlDict); - ModComponent component = DeserializeComponent(yamlDict); + ModComponent component = DeserializeComponent(yamlDict, requireName); return component; } catch (Exception ex) diff --git a/src/ModSync.Core/Services/Validation/InstallationValidationPipeline.cs b/src/ModSync.Core/Services/Validation/InstallationValidationPipeline.cs index ad706cf1..b222421f 100644 --- a/src/ModSync.Core/Services/Validation/InstallationValidationPipeline.cs +++ b/src/ModSync.Core/Services/Validation/InstallationValidationPipeline.cs @@ -83,42 +83,45 @@ public static async Task RunAsync( result.PassedCount++; } - cancellationToken.ThrowIfCancellationRequested(); - step++; - progress?.Invoke(ValidationPipelineStage.Conflicts, step, totalSteps, "Checking mod conflicts..."); - ValidationPipelineStageResult conflictStage = RunConflictStage(componentsToValidate, allComponents); - result.Stages.Add(conflictStage); - if (!conflictStage.Passed) - { - result.HasCriticalErrors = true; - result.ErrorCount += conflictStage.Messages.Count(m => m.StartsWith("ERROR:", StringComparison.Ordinal)); - } - else if (conflictStage.HasWarnings) + if (!options.SkipConflictAndOrderValidation) { - result.WarningCount += conflictStage.Messages.Count(m => m.StartsWith("WARNING:", StringComparison.Ordinal)); - } - else - { - result.PassedCount++; - } + cancellationToken.ThrowIfCancellationRequested(); + step++; + progress?.Invoke(ValidationPipelineStage.Conflicts, step, totalSteps, "Checking mod conflicts..."); + ValidationPipelineStageResult conflictStage = RunConflictStage(componentsToValidate, allComponents); + result.Stages.Add(conflictStage); + if (!conflictStage.Passed) + { + result.HasCriticalErrors = true; + result.ErrorCount += conflictStage.Messages.Count(m => m.StartsWith("ERROR:", StringComparison.Ordinal)); + } + else if (conflictStage.HasWarnings) + { + result.WarningCount += conflictStage.Messages.Count(m => m.StartsWith("WARNING:", StringComparison.Ordinal)); + } + else + { + result.PassedCount++; + } - cancellationToken.ThrowIfCancellationRequested(); - step++; - progress?.Invoke(ValidationPipelineStage.InstallOrder, step, totalSteps, "Validating install order..."); - ValidationPipelineStageResult orderStage = RunInstallOrderStage(componentsToValidate); - result.Stages.Add(orderStage); - if (!orderStage.Passed) - { - result.HasCriticalErrors = true; - result.ErrorCount++; - } - else if (orderStage.HasWarnings) - { - result.WarningCount++; - } - else - { - result.PassedCount++; + cancellationToken.ThrowIfCancellationRequested(); + step++; + progress?.Invoke(ValidationPipelineStage.InstallOrder, step, totalSteps, "Validating install order..."); + ValidationPipelineStageResult orderStage = RunInstallOrderStage(componentsToValidate); + result.Stages.Add(orderStage); + if (!orderStage.Passed) + { + result.HasCriticalErrors = true; + result.ErrorCount++; + } + else if (orderStage.HasWarnings) + { + result.WarningCount++; + } + else + { + result.PassedCount++; + } } } @@ -244,8 +247,11 @@ private static int CountStages(ValidationPipelineOptions options) count++; } - // Conflicts + InstallOrder always run under FullValidation. - count += 2; + if (!options.SkipConflictAndOrderValidation) + { + // Conflicts + InstallOrder under FullValidation. + count += 2; + } } if (!options.DryRunOnly && !options.SkipComponentArchiveValidation) diff --git a/src/ModSync.Core/Services/Validation/ValidationPipelineOptions.cs b/src/ModSync.Core/Services/Validation/ValidationPipelineOptions.cs index abe17f6c..5e96b1f5 100644 --- a/src/ModSync.Core/Services/Validation/ValidationPipelineOptions.cs +++ b/src/ModSync.Core/Services/Validation/ValidationPipelineOptions.cs @@ -41,6 +41,12 @@ public sealed class ValidationPipelineOptions /// Skip FOMOD configured-only gate (tests without FOMOD fixtures). public bool SkipFomodConfigurationGate { get; set; } + /// + /// Skip Conflicts and InstallOrder stages under . + /// Used by install-start readiness so Environment can re-run without a second full validate. + /// + public bool SkipConflictAndOrderValidation { get; set; } + [CanBeNull] public MainConfig MainConfig { get; set; } @@ -57,6 +63,20 @@ public sealed class ValidationPipelineOptions UseFileSelection = true, }; + /// + /// Install-start readiness: Environment only (no DryRun / archives / conflicts / order). + /// FOMOD remains gated on InstallStartPage via FomodConfigurationGate. + /// + public static ValidationPipelineOptions InstallStartReadiness => new ValidationPipelineOptions + { + FullValidation = true, + DryRun = false, + UseFileSelection = true, + SkipComponentArchiveValidation = true, + SkipFomodConfigurationGate = true, + SkipConflictAndOrderValidation = true, + }; + /// Legacy Getting Started validate: dry-run on selected mods only. public static ValidationPipelineOptions LegacyDryRunOnly => new ValidationPipelineOptions { diff --git a/src/ModSync.GUI/Dialogs/InstallationErrorDialog.axaml b/src/ModSync.GUI/Dialogs/InstallationErrorDialog.axaml deleted file mode 100644 index fa931c78..00000000 --- a/src/ModSync.GUI/Dialogs/InstallationErrorDialog.axaml +++ /dev/null @@ -1,162 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -