diff --git a/docs/plans/441-profile-path.md b/docs/plans/441-profile-path.md index 9ac9783..59e889f 100644 --- a/docs/plans/441-profile-path.md +++ b/docs/plans/441-profile-path.md @@ -8,44 +8,6 @@ updated: 2026-06-13 # Plan: #441 -- Profile path fix on OneDrive/KFM systems -## v5 Changes (revision 5) - -| # | Hole | Reviewer | Sev | Patch | -|---|------|---------|-----|-------| -| H1 | `Set-Content` in foreach body missing `-Encoding ASCII` | F-1 | HIGH | Added `-Encoding ASCII` to orphan-strip `Set-Content` (matches production line 28) | -| H2 | GG-7 exit-1 leaves stale `$LASTEXITCODE`; contaminates success-path tests | F-3 | MEDIUM | Section 5 header: each test resets `$global:LASTEXITCODE = 0` before mock redefinition | -| H3 | `TestDrive` in GG-4 contradicts Section 3 D2 (Pester rejected as scope creep) | F-2 + C-2 | MEDIUM | Replaced `TestDrive` with `Join-Path $env:TEMP "gg-test-441-$(New-Guid)"` temp-path language in GG-4; temp-dir cleanup sentence added to Section 5 header | -| H4 | GG-7 exe unspecified; false green on PS5.1-only runner | C-1 | MEDIUM | GG-7 row: `$HostExe = 'powershell'` (guaranteed on Windows); note that `'pwsh'` would mask the not-installed early-exit | -| H5 | `$ps51Fallback`/`$ps7Fallback` undefined inside `Write-PowerShellProfile` under `Set-StrictMode -Version Latest` | A-1 | MEDIUM | Two `$local:` definitions added at top of `Write-PowerShellProfile` in Section 4 (mirror production lines 17-19) | - -## v5.2 Changes (revision 5.2) - -| # | Hole | Reviewer | Sev | Patch | -|---|------|---------|-----|-------| -| JN-1 | `$local:ps51Fallback`/`$local:ps7Fallback` inside `Write-PowerShellProfile` shadow calling scope; test-scope assignment inoperable; GG-1/GG-4/GG-5 would write to real `$HOME` paths | JN-1 | MEDIUM | Parameterized `Write-PowerShellProfile` with `-Ps51Fallback`/`-Ps7Fallback` (defaults = production lines 17-18); tests pass temp paths as named parameters; Section 3 v5.2-D1 added; Section 5 GG-1/GG-4/GG-5 updated | -| JN-2 | C-2/C-3 PS7+ skip uses `Write-Host`; increments pass counter instead of skip counter on PS7+ CI | JN-2 / NF-3v4 | LOW | `Write-Host 'SKIP C-2: ...'` -> `Write-Warning '[SKIPPED] C-2: ...'` in Section 3 v3-D4; warning stream is visually distinct in PS output; no Pester dependency (D2 preserved) | - -## v5.1 Changes (revision 5.1) - -- F-4: Orphan-strip regex matches production line 27 (`\r?\n` prefix added; `.+?` -> `.*?`) -- F-5: `$local:beginMarker`/`$local:endMarker` defined in `Write-PowerShellProfile` (mirrors production lines 12-13) - ---- - -## v4 Changes (revision 4) - -| # | Griller | Sev | Patch | -|---|---------|-----|-------| -| P1 | Pluto | BLOCKING | Section 4 foreach loop body filled -- explicit strip regex + log; no stub comment | -| P2 | Pluto | BLOCKING | Section 4 algorithm wrapped explicitly in `Write-PowerShellProfile`; comment explains dot-source safety | -| P3 | Chip | HIGH | GG-7 mock calls `& $env:ComSpec /c "exit 1"` -- propagates `$LASTEXITCODE = 1` via native-command global semantics | -| P4 | Chip | SS-2 | Section 3 v3-D4 guard: `skip` replaced with `if/Write-Host/return`; guard + `$PROFILE = $path` moved INSIDE Test-Scenario body | -| P5 | Chip | MEDIUM | Section 5 header: mock isolation pattern documented -- mock redefined before each GG test; Test-Scenario child-scope model stated | -| P6 | Chip | MEDIUM | GG-4 row: both mock calls return same `$oneDrivePath`; dedup -> 1 entry; both legacy paths confirmed orphaned | -| P7 | Doc | cosmetic | Section 3 v3-D4: `$PROFILE` is conceptually (not technically) read-only, per MS Learn | - ---- - ## 1. Problem `profile.ps1` hardcodes `$HOME\Documents\...` as the profile path. On OneDrive KFM systems, PowerShell's actual `$PROFILE` resolves to `OneDrive\Documents\...`. The dev-setup block is written to a file that is never sourced; aliases silently fail to appear in new terminals. @@ -98,7 +60,7 @@ Decision: Check `$LASTEXITCODE -ne 0` after `Invoke-HostQuery`. `& $Exe` exits n Decision: GG-4 seeds BOTH legacy paths simultaneously and asserts BOTH stripped. One-path test cannot catch a loop-break bug after the first match. -**v3-D4. C-2/C-3 guarded in PS7+** *(updated v4 -- Doc/Chip)* +**v3-D4. C-2/C-3 guarded in PS7+** Decision: Guard C-2 and C-3 with proper skip. The `$PROFILE` automatic variable is conceptually read-only in PowerShell (per Microsoft Learn); assigning to it is unsupported and may cause issues in test contexts. Full refactor deferred; behavior these tests cover is superseded by this PR. Concrete mechanism: move the `$PROFILE = $path` assignment INSIDE the Test-Scenario body (it was erroneously outside as setup code), then open with `if ($PSVersionTable.PSVersion.Major -ge 7) { Write-Warning '[SKIPPED] C-2: PS7+ -- $PROFILE conceptually read-only; covered by GG tests'; return }`. This ensures the assignment never fires on PS7+ and the skip reason is logged. diff --git a/docs/plans/451-pwsh-parity-gaps.md b/docs/plans/451-pwsh-parity-gaps.md index 1530a7a..eef6fa1 100644 --- a/docs/plans/451-pwsh-parity-gaps.md +++ b/docs/plans/451-pwsh-parity-gaps.md @@ -35,7 +35,7 @@ Test count increases from 6 (T1-T6) to 9 (T1-T7, T_C, T_D), achieving parity wit --- -## CI Integration (v2 addition -- resolves Mickey BLOCKING-1/BLOCKING-2, Goofy Finding-1) +## CI Integration The `validate-ps51` job (`.github/workflows/validate.yml` lines 287-369) does NOT currently run `test_sprint_end_labels_pwsh.ps1`. The `validate-powershell` job (line 282-283) runs only the @@ -79,7 +79,7 @@ is a prerequisite to close the CI gap and is explicitly in-scope for this issue. - T_C / T_D: No setup, direct Invoke-ScriptRun on bare script - T7: Minimal test state via New-TestEnv (empty issues/prs, copy launcher logic) -**T7 detail (v2 -- replaces vague "assert shebang valid", resolves Mickey MAJOR-2):** +**T7 detail:** - Read launcher file via `[System.IO.File]::ReadAllBytes()` (binary, no encoding transform) - Assert no 0x0D (CR) bytes present (primary regression invariant) - Assert file starts with bytes 0x23 0x21 (the ASCII codes for `#!`) -- this is the shebang @@ -117,7 +117,7 @@ if ($bytes[0] -ne 0x23 -or $bytes[1] -ne 0x21) { **T_C / T_D (LOW):** Pure argument parsing, no side effects, no fixtures. - Risk: Error message wording is coupled to implementation strings in `scripts/sprint-end-labels.ps1` - and `scripts/sprint-end-labels.sh`. (v2 decision -- resolves Mickey MAJOR-1 / Goofy T_C/T_D note) + and `scripts/sprint-end-labels.sh`. - **Decision:** Use substring/regex match assertions (not full-string equality). T_C asserts exit code != 0 only (mirrors bash Test C which checks exit code = 2, no message assertion). T_D asserts exit code != 0 AND output contains `"release:shipped-"` (mirrors bash Test D). @@ -131,7 +131,7 @@ if ($bytes[0] -ne 0x23 -or $bytes[1] -ne 0x21) { **T7 (MEDIUM):** Byte-level assertion requires precise encoding handling. - Risk: `[System.IO.File]::ReadAllBytes()` MUST be used; `Get-Content` applies encoding and may hide CR bytes -- Mitigation: Use established ASCII safety pattern from chip/history.md +- Mitigation: Use the established ASCII safety pattern - Dependencies: None new (reuses New-TestEnv) --- @@ -149,7 +149,7 @@ preventing silent re-introduction of the bug. 1. **PS 5.1 coverage:** RESOLVED. validate-ps51 does NOT currently run this test file. Plan includes a 2-step YAML addition to close the gap (see CI Integration section above). -2. **Launcher byte determinism:** RESOLVED (by Goofy). Launcher content is session-deterministic +2. **Launcher byte determinism:** RESOLVED. Launcher content is session-deterministic (same $PowerShellPath within a run) but not identical across independent CI runs. T7 is a regression test, not an idempotency test -- this is acceptable. See T7 detail above. @@ -175,7 +175,7 @@ preventing silent re-introduction of the bug. --- -## Out-of-Scope Tracked Item (v3 addition -- resolves Goofy Round 2 caveat) +## Out-of-Scope Tracked Item **`$IsWindows` PS 5.1 hazard in `New-TestEnv` (line 320 of test file):** @@ -201,28 +201,3 @@ Implementer does NOT need to fix this during #451 -- it is tracked and will be a in #461. --- - -## Revision History - -- **v1 (2026-05-28T02:56:01-04:00):** Initial plan committed to `.squad/decisions/451-vertical-slice.md`. - Three parity gaps identified (T_C, T_D, T7). Open questions deferred to grill. -- **v2 (2026-05-27T23:12:03-04:00):** Addressed grill panel findings (Mickey/Goofy/Jiminy). - - BLOCKING (Mickey/Goofy): Added CI Integration section; test wired into validate-ps51 job - (YAML step at line 369+); TODO comment at line 285 marked for removal. - - MAJOR (Mickey #4): Replaced vague "assert shebang valid" with precise two-assertion spec: - no 0x0D bytes (CR regression) + file starts with 0x23 0x21 (#! shebang bytes); both - assertions required and rationale documented. - - MAJOR (Mickey #3): T_C/T_D assertion strategy defined as substring/regex match; T_C is - exit-code-only; T_D uses "release:shipped-" substring; error-message contract documented - in Risk Assessment and Done Criteria. - - MEDIUM (Jiminy): Plan moved from `.squad/decisions/451-vertical-slice.md` to - `docs/plans/451-pwsh-parity-gaps.md` per PR #441 precedent. -- **v3 (2026-05-27T23:47:00-04:00):** Addressed Round 2 grill panel (Mickey/Goofy/Jiminy). - - Mickey APPROVE: All four R1 findings confirmed resolved. Done Criteria at lines 174/172 - already cover both Mickey implementation-phase notes (error-message contract in PR - description; TODO removal). No plan text changes needed. - - Goofy APPROVE-WITH-MINOR-CAVEATS: `$IsWindows` PS 5.1 hazard at New-TestEnv line 320 - noted. Decision: out of scope for #451 (pre-existing code, not introduced by this slice). - Filed follow-up issue #461. See "Out-of-Scope Tracked Item" section above. - - Jiminy CLEAN: Commit trailer formatting noted. v3 commit uses blank-line-separated - Co-authored-by trailer per git interpret-trailers convention. diff --git a/docs/plans/468-customizable-install.md b/docs/plans/468-customizable-install.md index f41e5c0..8f8f8f8 100644 --- a/docs/plans/468-customizable-install.md +++ b/docs/plans/468-customizable-install.md @@ -8,177 +8,6 @@ updated: 2026-06-13 # Plan: #468 -- Customizable install (pick-and-choose tools) -## v14 Changelog (revision 14, 2026-05-30) - -> **Source:** Doc-2 v13 fact-check findings. Two pre-existing quoting-style nits from v5 -> survived all previous passes: one bare token and one double-quoted value in Windows prose -> and Windows e2e spec contexts. Both were flagged by Doc-2 as inconsistent with the -> single-quote convention used throughout the rest of the plan. - -1. **Line 815 Windows prose corrected (Verified):** `-Only git-hook` -> `-Only 'git-hook'`. - Bare token quoted to match single-quote convention. Surgical 1-line fix; no scope change. - -2. **Line 1108 Windows e2e spec corrected (Verified):** `-Only "gh"` -> `-Only 'gh'`. - Double-quote switched to single-quote to match single-quote convention. Surgical 1-line - fix; no scope change. - -3. **[Amend] 3 bare `-Skip winget-check` occurrences normalized (Verified):** Lines ~104, - ~106 (v7 changelog), and ~860 (foot-gun docs) each had bare `-Skip winget-check`; all - corrected to `-Skip 'winget-check'`. Same class as items 1-2 above, surfaced during v14 - sweep. Earl approved scope expansion. (Note: task brief said 4 nits; 3 actual occurrences - found and fixed -- no 4th occurrence exists in the file.) - ---- - -## v13 Changelog (Mickey -- Windows flag syntax fix, 2026-05-28) - -> **Source:** Duck-2 v12 regrill finding. Line 167 in DD-1 used Linux flag syntax -> (`--only=uv`) inside an explicitly Windows example ("fresh Windows"). Bug missed by -> Doc (v12 author), Jiminy, and Goofy on their sweeps; caught by Duck rubber-duck pass. - -1. **Line 167 Windows example corrected (Verified):** `--only=uv` on fresh Windows -> - `-Only 'uv'` on fresh Windows. Single-quote style matches all existing `-Only '...'` - usages throughout the plan. Surgical 1-line fix; no scope change. - ---- - -## v12 Changelog (Doc -- Windows example syntax fix, 2026-05-28) - -> **Source:** Duck v11 verdict (no `.squad/decisions/inbox/duck-468-v11-regrill.md` on -> disk; finding communicated inline). The two Windows npm-absent example bullets at -> lines 778-780 used Linux flag syntax (`--only=`) and the Linux tool name (`copilot-cli`). -> Windows uses PowerShell syntax (`-Only 'X'`) and the registry key is `copilot` -> (not `copilot-cli`), per grammar table lines 720-724 and registry lines 263-274 / 320-331. - -1. **Windows npm-absent example bullets corrected (Verified):** Lines 778 and 780 - updated. `--only=copilot-cli` Windows -> `-Only 'copilot'`; `--only=squad-cli` - Windows -> `-Only 'squad-cli'`. Single-quote style matches all existing `-Only '...'` - usages throughout the plan. Registry key `copilot` confirmed at `$DefaultTools` line - 273 and `$ToolRegistry` line 330. - ---- - -## v11 Changelog (Mickey -- 2x2 npm-absent matrix, 2026-05-28) - -> **Source:** Duck raised REQUEST CHANGES on v10 grill (PR #470). The Graceful Degradation -> section grouped `copilot-cli` and `squad-cli` together, but real Windows behavior differs -> between the two tools. Earl authorized Mickey as v11 author (narrow patch, deadlock break -> per Reviewer Protocol step 7). - -1. **2x2 npm-absent matrix added (Verified):** Replaced the flat per-platform bullets with a - full tool x platform matrix distinguishing all four cells. Evidence verified against source: - `scripts/linux/tools/copilot-cli.sh:46-49`, `scripts/linux/tools/squad-cli.sh:41-43`, - `scripts/windows/tools/copilot.ps1:47-50`, `scripts/windows/tools/squad-cli.ps1:37-43`. - -2. **Parity clarifier added to Non-Negotiables #2 (Duck Suggestion #2):** One sentence - distinguishing selection-semantics parity from per-tool degradation behavior. - ---- - -## v10 Changelog (Doc -- factual corrections from v9 review, 2026-05-28) - -> **Source:** Doc raised REQUEST CHANGES on v9 (PR #470). Two factual errors all nine -> prior rounds missed. Earl authorized Doc as v10 author (narrow patch only). Next grill -> panel: Duck + Jiminy + Goofy; Doc is recused as author. - -1. **Windows git-hook location corrected (Verified):** v9 documented the `Install-GitHook` - function as living in `scripts/windows/tools/git-hook.ps1`. That file does **not exist**. - Verified by directory listing of `scripts/windows/tools/` (files present: auth.ps1, - copilot.ps1, dotfiles.ps1, gh.ps1, git.ps1, nvm.ps1, profile.ps1, psmux.ps1, - squad-cli.ps1, uv.ps1, vim.ps1 -- no git-hook.ps1). `Install-GitHook` is defined at - lines 36-46 of `scripts/windows/setup.ps1`. All plan references corrected to reflect - actual location. Confidence: **Verified**. - -2. **Windows squad-cli npm-absent behavior corrected (Verified):** v9 stated - "`copilot-cli`/`squad-cli` silently skip if npm absent", implying identical behavior on - both platforms. Actual behavior is the **opposite**: Linux (`scripts/linux/tools/squad-cli.sh` - lines 41-43) emits `log_warn` and `exit 0` (silent skip); Windows - (`scripts/windows/tools/squad-cli.ps1` lines 37-43) emits `Write-Err` diagnostics and - `exit 1` (hard stop). Graceful Degradation section updated with per-platform breakdown. - Confidence: **Verified**. - ---- - -## v9 Changelog (Mickey -- semantic fix, closing Jiminy blocker) - -1. **`winget-check` registry entry is not behavior-preserving (Jiminy blocker):** v8 mapped - `'winget-check' = { Test-WingetAvailable }` -- the dispatcher invokes the scriptblock and - ignores the returned `$false`, so the missing-winget gate no longer exits. Fixed by - introducing `Invoke-WingetGate` wrapper function that preserves the original `Write-Err` + - `exit 1` semantics from `scripts/windows/setup.ps1` lines 50-55. Registry entry updated - to `'winget-check' = { Invoke-WingetGate }`. - -2. **Document `-Skip 'winget-check'` foot-gun (Jiminy non-blocking):** Added parallel - documentation to the existing `--skip=prereqs` foot-gun note, calling out that - `-Skip 'winget-check'` bypasses the App Installer availability gate on Windows. - ---- - -## v8 Changelog (Pluto -- coherence reconciliation) - -1. **v8 (Pluto):** Fixture Provenance includes Windows `winget-check` (lines 535-549), but - `$DefaultTools` and `$ToolRegistry` omitted it -- making `T_baseline_real_defaults` - impossible to pass coherently. Reconciled via path (A): added `'winget-check'` as first - entry in Windows `$DefaultTools` and `$ToolRegistry`, paralleling Linux `prereqs` as - the prerequisite-check phase. Preserves 2-concept model; AlwaysRun stays dropped. - ---- - -## v7 Changelog (Jiminy -- fixture provenance polish) - -1. **v7 (Jiminy):** Fixture Provenance Linux/Windows order corrected to match real `setup.{sh,ps1}` execution including prereqs/dotfiles/git-hook phases absorbed into DefaultTools after AlwaysRun drop. - ---- - -## v6 Changelog (Pluto -- surgical polish on Donald's v5) - -1. **`--check` mode write-then-diff bug (Duck blocker):** Restructured - `regenerate-baseline-fixtures.sh` spec so extraction populates in-memory arrays - (`${linux_defaults[@]}` / `${win_defaults[@]}`, with PowerShell `$win_defaults` - explicitly defined) ONLY; `--check` diffs in-memory against committed fixture - files and never writes. Normal mode is the sole write path. - -2. **CI workflow name misalignment (Jiminy blocker):** Slice 4 now correctly targets - `e2e-install.yml` for e2e jobs and `validate.yml` for validate-{linux,powershell,ps51}. - Added explicit macOS selective-install e2e coverage (`E2E_only_macos`). - -3. **Flag-combo coverage gaps (Jiminy blocker):** Added named Test-Scenario cases for - bash `--list --skip`, PowerShell `-List -Only` / `-List -Skip`, and both direct-child - plus root-forwarding paths for `--only` / `--skip`. - -4. **Fixture provenance anchor (Jiminy blocker):** Added "Fixture Provenance" subsection - under Baseline Fixture Mechanism. Explicit current Linux + Windows tool orders committed - as fixtures BEFORE refactor lands. `regenerate-baseline-fixtures.sh` must NOT be run - during implementation. - -5. **Re-run/flag idempotency (Jiminy blocker):** Added `T_no_selection_persistence` test - in Slice 3 -- verifies no state persists between flag invocations (bash + pwsh). - -6. **Skip-path coverage for git-hook safety (Duck non-blocking):** Added - `T_git_hook_skip_path_safe` test in Slice 3 -- asserts `--skip=prereqs` (Linux) and - `-Skip 'git'` (Windows) still allows git-hook to self-guard cleanly. - ---- - -## v5 Changelog (Donald -- polish pass on Pluto's v4) - -1. **Baseline harness format mismatch (Donald review):** Stubs now log bare tool names - (e.g. `echo "prereqs" >> "$RUN_LOG"`) instead of `RAN:prereqs`. This eliminates the - impossible diff between `RUN_LOG` (prefixed) and `defaults.txt` (bare). Option (a) - chosen -- simplest, no transform step in tests. - -2. **Git-hook safety via flag paths (Duck DK4-bis):** `git-hook.sh` and `Install-GitHook` - are now self-guarding: early-return exit 0 with skip message when `git` is not on PATH. - New test `T_git_hook_no_git_safe` in Slice 2 confirms both platforms. - -3. **Real-defaults drift test (Duck-2):** Added `T_baseline_real_defaults` (bash + pwsh) - that extracts real `DEFAULT_TOOLS`/`$DefaultTools` arrays from source via - `scripts/dev/regenerate-baseline-fixtures.sh` and diffs against committed - `tests/fixtures/baseline-tools-{linux,windows}.txt`. Runs alongside (not replacing) - the mock-dispatcher baseline test. - ---- - ## Summary Add flag-based tool selection to `scripts/linux/setup.sh` and `scripts/windows/setup.ps1` @@ -294,6 +123,8 @@ Manifest and prompt are explicitly out of scope -- they can layer on top later. ### Default Order (Linux) +> **NOTE:** squad-cli entries below are stale pending #468 implementation. + ```bash DEFAULT_TOOLS=( "prereqs" @@ -614,7 +445,7 @@ diff "$RUN_LOG" tests/fixtures/stub-tools/linux/defaults.txt The stub `defaults.txt` is the expected run-log output in order. The test proves that no-arg dispatch matches the declared default order exactly (order + set). -### Real-Defaults Drift Test (v5 fix #3 -- Duck-2) +### Real-Defaults Drift Test The mock-dispatcher baseline test above proves the dispatch mechanism works, but does NOT protect against accidental edits to the real `DEFAULT_TOOLS` / `$DefaultTools` arrays @@ -1230,6 +1061,6 @@ entries (3-line pattern). NOT added to DefaultTools (opt-in only). | DK4 (git-hook unsafe when git absent) | DD-1: git-hook is a normal tool -- `--only=uv` never triggers it | | DK4-bis (git-hook via flag paths) | v5: git-hook self-guards (`command -v git` / `Get-Command git`), `T_git_hook_no_git_safe` | | DK5 (hidden flags unprotected) | Slice 1: `T_help_no_toolsdir` negative assertions on both platforms | -| Donald-1 (baseline format mismatch) | v5: stubs log bare tool names -- `RUN_LOG` directly diffs against `defaults.txt` | -| Duck-2 (real defaults drift unprotected) | v5: `T_baseline_real_defaults` + `--check` mode in regeneration script | +| baseline format mismatch | v5: stubs log bare tool names -- `RUN_LOG` directly diffs against `defaults.txt` | +| real defaults drift unprotected | v5: `T_baseline_real_defaults` + `--check` mode in regeneration script |