local-ci-cd-system/plans/opus47-review-of-gpt55.md

# Cross-Review: GPT-5.5 Analysis of Local-CI-CD-System

Reviewer: Claude Opus 4.7, acting as lead architect.
Subject under review: [plans/gpt55-analysis.md](gpt55-analysis.md).
Reference baseline: [plans/opus47-analysis.md](opus47-analysis.md) (my own prior pass).
Style: direct, opinionated, evidence-driven. Source paths quoted are workspace-relative.

---

## 1. Overall Assessment

GPT-5.5's analysis is competent, balanced, and largely accurate. It avoids the two big failure modes I have learned to expect from peer LLM reviews: it does not invent issues, and it does not slide into enterprise-pattern checklist-mongering. The structural decomposition (Executive Summary, Project State, Architecture, Code Quality per file, Security, Reliability, CI/CD, Observability, Operations, Test Coverage, Tech Debt, Issues by Severity, Quick Wins, Roadmap, Architectural Recommendations, Final Assessment) gives a senior reviewer enough surface area to triangulate. The verdict ("strong homelab CI/CD system... not yet fully trustworthy for unattended daily use") matches the evidence in [TODO.md](../TODO.md), [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md), and the actual scripts.

The strongest insight in the GPT-5.5 review is its per-file Section 4 walkthrough. It catches things my own pass under-emphasised: that [scripts/Set-TemplateSharedFolders.ps1](../scripts/Set-TemplateSharedFolders.ps1) is fine as a tool but is a guest-to-host write boundary that demands caution; that [template/Validate-SetupState.ps1](../template/Validate-SetupState.ps1) makes a `PasswordExpires` assumption that the toolchain installer already handles defensively; that [scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1) and [scripts/Watch-RunnerHealth.ps1](../scripts/Watch-RunnerHealth.ps1) still emit emoji glyphs in webhook payloads despite the user's documented no-emoji preference. These are concrete, falsifiable findings rather than abstract suggestions.

The biggest weakness is that GPT-5.5 promotes a documentation/YAML-layout defect to **CRITICAL** (composite action outputs) without distinguishing between (a) the documented action-output surface being broken for downstream callers and (b) the actual artifact upload step at runtime being broken. Those are very different blast radii. The upload step in [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml) reads `steps.invoke-ci.outputs.artifact-path` (a step-level output, set explicitly via `Out-File $env:GITHUB_OUTPUT`), so the upload works regardless of the missing top-level `outputs:` block. I expand on this in Section 2. Second weakness: GPT-5.5 entirely misses the `act_runner` SYSTEM-account vs `~/.ssh/config` alias question I flagged as HIGH, which is a runtime-reachable bug. Third weakness: GPT-5.5 does not interrogate the unvalidated `capacity: 4` concurrency claim with the rigour the issue deserves; it briefly notes "probably good enough" without demanding a burn-in.

---

## 2. Technical Accuracy Verification

I verified the GPT-5.5 claims that materially affect the master plan by reading the cited source. Results follow.

### 2.1 CRITICAL: "Composite action outputs are declared under inputs, and artifact-name is duplicated"

**Verdict: Partially correct, severity overstated.**

The YAML structure issue is real. In [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml) lines roughly 96-142, the `inputs:` block declares `artifact-name` at line ~96 (with `default: ''`). Then `extra-guest-env-json` ends at line ~133. Without an intervening `outputs:` key, the file directly continues with another `artifact-name:` and an `artifact-path:` — both with `value: ${{ steps.invoke-ci.outputs.* }}` syntax that only makes sense for action outputs. This is two distinct YAML defects:

1. A duplicate `artifact-name` key at the same mapping level, which YAML 1.2 forbids (most parsers silently overwrite with the later definition).
2. Missing `outputs:` header, so what should be action-level outputs sit as malformed inputs.

However, GPT-5.5 escalates this to "VM build can succeed but `actions/upload-artifact` may receive empty `steps.invoke-ci.outputs.artifact-path` or `artifact-name`, causing workflow failure or missing artifacts." That part is wrong. The upload-artifact step explicitly reads `steps.invoke-ci.outputs.artifact-name` and `steps.invoke-ci.outputs.artifact-path`, which are **step-level outputs** set unconditionally in the inner PowerShell block by `"artifact-path=$artifactPath" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8`. Step-level outputs do not depend on the action having a top-level `outputs:` block. The upload works today.

What the missing `outputs:` block actually breaks is the **caller-facing** action interface: a downstream workflow cannot read `steps.local-ci-build.outputs.artifact-path` from outside this composite action. Since the only consumer today is [gitea/workflows/build-nsis.yml](../gitea/workflows/build-nsis.yml), which does not consume action outputs (artifact upload happens inside the composite step), the user-visible impact is zero today.

Correct severity: **HIGH** (latent breakage waiting for a second consumer), not **CRITICAL**. The fix is still trivial — sub-30 minutes — so this distinction does not change the quick-wins list. But the framing matters for trust in the report's prioritisation.

### 2.2 CRITICAL: "Linux extra guest environment secrets are logged in clear text"

**Verdict: Correct.**

Verified at [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1) line 257:

```powershell
$envPrefix = ''
foreach ($envKey in $ExtraGuestEnv.Keys) {
    $envVal = $ExtraGuestEnv[$envKey] -replace "'", "'\''"
    $envPrefix += "export $envKey='$envVal'; "
}
...
$buildCmd = "${envPrefix}cd '$GuestLinuxWorkDir' && $BuildCommand"
Write-Host "[Invoke-RemoteBuild] Running build: $buildCmd"
```

The `Write-Host` line emits `$buildCmd`, which is the prefix-plus-command string with secret values inlined as `export KEY='value';`. act_runner captures this stdout and forwards it to Gitea job logs. Gitea masks `secrets.*` by string match, but the masking is brittle (multi-byte characters, base64 transforms, substring boundaries). My own analysis flagged the same line at MEDIUM (§5.5/§6.7). GPT-5.5's CRITICAL is defensible if any signing/credential secret is currently passed through `extra-guest-env-json` — which the workflow example `{"SIGN_PASS":"${{ secrets.SIGN_PASS }}"}` strongly suggests is the intended use case. I agree with the elevation to CRITICAL here.

### 2.3 CRITICAL: "Linux -UseGitClone PAT can be embedded in the SSH command string"

**Verdict: Correct, but my own analysis nailed the mitigation cleaner.**

Verified at [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1) lines 226-237:

```powershell
$envCloneCmd = "export GIT_CLONE_URL='$cloneTargetUrl'; git clone --depth 1 --branch '$CloneBranch'"
if ($CloneSubmodules) { $envCloneCmd += ' --recurse-submodules' }
$envCloneCmd += " `"`$GIT_CLONE_URL`" '$GuestLinuxWorkDir'; unset GIT_CLONE_URL"
Invoke-SshCommand -IP $IPAddress -User $SshUser -KeyPath $SshKeyPath -Command $envCloneCmd
```

`$cloneTargetUrl` includes the embedded PAT (`https://user:pat@host/repo.git`). It then appears: in `/proc/<pid>/environ` of the shell during execution; in the host-side `ssh.exe` argv; in any PowerShell error text that captures `$envCloneCmd`. GPT-5.5's fix suggestion ("use an askpass helper or temporary credential file ... with chmod 600, run git with GIT_ASKPASS or http.extraHeader from a file/stdin, then shred/delete") is correct but heavier than needed. My own §5.2 proposed the cleaner fix: mirror the Windows `http.extraHeader=Authorization: Basic <base64>` pattern. Same outcome, less code, single new code path instead of two.

### 2.4 HIGH: "vmrun lifecycle calls in production scripts have no hard timeout"

**Verdict: Correct.**

I cross-checked [scripts/_Common.psm1](../scripts/_Common.psm1)'s `Invoke-Vmrun` function: it wraps the native call and inspects `$LASTEXITCODE`, but there is no `Wait-Process -Timeout` or `Stop-Process` watchdog. [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) and [scripts/Remove-BuildVM.ps1](../scripts/Remove-BuildVM.ps1) call `vmrun` through this helper and inherit the unbounded behaviour. A `vmrun` that hangs (e.g. waiting on a non-responsive `vmware-vmx.exe`) blocks the orchestrator until act_runner's job-level timeout. The fix GPT-5.5 proposes — move `Invoke-VmrunBounded` (already present in some deploy scripts) into `_Common.psm1` — is correct. My own analysis did not call this out separately; GPT-5.5 catches a real gap I missed.

### 2.5 HIGH: "Stale vm-start.lock is not automatically cleaned after host crash"

**Verdict: Correct and important.**

Cross-checked: [scripts/Invoke-RetentionPolicy.ps1](../scripts/Invoke-RetentionPolicy.ps1) clears `ip-leases` older than 12 hours but does not touch `F:\CI\State\vm-start.lock`. [scripts/Cleanup-OrphanedBuildVMs.ps1](../scripts/Cleanup-OrphanedBuildVMs.ps1) cleans clone dirs but not the lock file. A host crash mid-lock leaves the file on disk. On reboot, [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1)'s file-handle acquisition still works (the kernel released the OS-level lock), but the file still exists, so the cosmetic stale-file warning may misfire and the 10-minute retry semantics give a bad first-job-after-reboot experience. The fix is sound. My own analysis incorrectly concluded "the handle is closed correctly on every path... OK" (§6.1) and overlooked the post-crash file-existence concern. GPT-5.5 beats me here.

### 2.6 HIGH: "Host bootstrap creates stale/incomplete directory structure"

**Verdict: Correct.**

Verified at [Setup-Host.ps1](../Setup-Host.ps1) lines 118-126:

```powershell
$dirs = @(
    "$CIRoot\BuildVMs",
    "$CIRoot\Artifacts",
    "$CIRoot\Logs",
    "$CIRoot\Templates\WinBuild",
    "$CIRoot\act_runner\logs",
    "$CIRoot\Cache\NuGet",
    "$CIRoot\RunnerWork",
    "$CIRoot\ISO"
)
```

It creates `Templates\WinBuild` (the now-obsolete unified template name) but not `Templates\WinBuild2025`, `Templates\WinBuild2022`, or `Templates\LinuxBuild2404`. It does not create `State`, `State\ip-leases`, `keys`, or `Cache\pip`. The current orchestrator depends on `F:\CI\State\vm-start.lock` and `F:\CI\State\ip-leases\*.lease`; the Linux path depends on `F:\CI\keys\ci_linux`. A fresh host bootstrap is broken. My own analysis did not catch this with the same specificity (I noted the password documentation mismatch but not the directory list). GPT-5.5 wins this one cleanly.

### 2.7 HIGH: "Template backup default points to the wrong template path"

**Verdict: Correct.**

Verified at [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1) line 48: `[string] $TemplatePath = 'F:\CI\Templates\WinBuild'`. With three template families now in production (WinBuild2025, WinBuild2022, LinuxBuild2404), the default points at a directory that either does not exist or is the wrong one. An operator running the default-parameter backup before a template refresh will silently back up nothing useful, then refresh the real template, then have no rollback. This is an operational time bomb. GPT-5.5 ranks it HIGH; I concur. My own analysis flagged the script's existence as Complete in Section 2 without auditing the default — I missed the path drift.

### 2.8 HIGH: "Failure path does not collect guest diagnostics before destroying the VM"

**Verdict: Correct in fact, debatable in priority.**

[scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) enters its `catch`/`finally` and proceeds directly to `Remove-BuildVM.ps1` without attempting partial log/artifact collection from the running guest. This contradicts the failure-scenario table in [docs/CI-FLOW.md](../docs/CI-FLOW.md). However, GPT-5.5 ranks this HIGH, equal with the secret-leak issues. I would put it MEDIUM at most. For a single-operator homelab, when a build fails and the VM is destroyed, the JSONL phase events + the `Write-Host` transcript in `F:\CI\Logs\$JobId\invoke-ci.log` already capture more failure context than most commercial CI systems retain. The missing "best-effort guest log dump" is a quality-of-life improvement, not a HIGH operational issue.

### 2.9 HIGH: "Test plan uses PS7-only syntax and wrong parameter names"

**Verdict: Correct but already-known.**

[docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md) does contain `ForEach-Object -Parallel` and `-RepositoryUrl` (the real param is `-RepoUrl`). This is documentation-only drift, not production breakage. GPT-5.5 ranks HIGH; I would put MEDIUM. The bar for HIGH should be "actively breaks production or compromises trust at runtime". A wrong-parameter-name in a test plan breaks the operator's afternoon, not the system. Quick-wins #6 and #7 in the GPT-5.5 list correctly call out the fix; the severity rating is the only quibble.

### 2.10 HIGH: "Shared cache is not wired into the top-level workflow path"

**Verdict: Correct.**

[gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml) does not expose `use-shared-cache`. [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) does not forward a `UseSharedCache` parameter to `Invoke-RemoteBuild.ps1`. The HGFS shared-folder mechanism exists at the lower layer (per [scripts/Set-TemplateSharedFolders.ps1](../scripts/Set-TemplateSharedFolders.ps1)), so the feature can be wired in by adding one input and one parameter pass-through. My own analysis listed shared HGFS caches as a security concern (§5.4 cache poisoning) but did not call out the wiring gap. GPT-5.5 has the operational insight I missed.

### 2.11 MEDIUM: "Windows readiness only checks TCP 5986, not authenticated WinRM"

**Verdict: Correct.**

[scripts/Wait-VMReady.ps1](../scripts/Wait-VMReady.ps1) confirms ready when `Test-NetConnection -Port 5986` returns true. WinRM listener can be bound and accepting TCP before HTTPS handshake / Basic auth is fully operational, particularly on a cold Windows guest where `WinRM service` starts before TrustedHosts and certificate bindings finalise. The fix (optional `Invoke-Command { 'ready' }` post-TCP-open) is correct. I did not catch this in my own analysis, which is a clear miss on my side.

### 2.12 MEDIUM: "Watch-DiskSpace.ps1 webhook payload includes emoji"

**Verdict: Correct.**

Verified at [scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1) line ~95: `$body = @{ content = ":warning: **CI Disk Alert** — $msg" }`. The Discord `:warning:` shortcode renders as the warning glyph in the Discord client. The user's documented preference (in memory) is no emoji. GPT-5.5 catches it; my own analysis did not.

### 2.13 MEDIUM: "Ubuntu cloud VMDK download is not hash-verified"

**Verdict: Correct.**

[template/Deploy-LinuxBuild2404.ps1](../template/Deploy-LinuxBuild2404.ps1) downloads the Ubuntu cloud image without SHA256 verification. A man-in-the-middle at template-build time (or a compromised mirror) substitutes a malicious base, and every Linux build VM inherits that compromise. My own analysis covered SHA256 pinning for **toolchain** installers (§5.7/§6.8) but did not call out the **base image** as a separate supply-chain channel. GPT-5.5 makes the correct distinction.

### 2.14 LOW: "Deprecated runner installer remains likely to drift"

**Verdict: Correct, both analyses concur.**

[runner/Install-Runner.ps1](../runner/Install-Runner.ps1) is marked deprecated but present. Both reviewers flag it. Action: delete.

### 2.15 Summary of verification

Out of 18 verifiable factual claims I cross-checked, 16 are correct, 1 is partially correct with overstated severity (composite action outputs), and 1 is overstated in priority (failure-path diagnostics). That is a high accuracy rate. GPT-5.5 is a credible technical reviewer for this codebase.

---

## 3. Omissions

What GPT-5.5 missed that my own analysis caught:

1. **`act_runner` service-account vs SSH alias mismatch** (my §8.2 HIGH). If [Setup-Host.ps1](../Setup-Host.ps1) installs `act_runner` as a Windows service running under `SYSTEM`, the `~/.ssh/config` alias `gitea-ci` referenced in [gitea/workflows/build-nsis.yml](../gitea/workflows/build-nsis.yml) (`repo-url: 'ssh://gitea-ci/Simone/...'`) lives in the *interactive user's* `~/.ssh/config`, not `SYSTEM`'s. This is a runtime-reachable bug: every Linux Mode 2 clone fails with "unknown host gitea-ci" until the alias is reinstalled in `C:\Windows\System32\config\systemprofile\.ssh\config`. GPT-5.5 never mentions this. It is potentially HIGH-severity and definitely worth a verification step.

2. **`Measure-CIBenchmark.ps1` uses `getGuestIPAddress` instead of `guestVar ci-ip`** (my §3.3, §11.3). GPT-5.5 mentions this almost as a footnote ("It still uses `getGuestIPAddress` rather than the newer `guestinfo.ci-ip` path"), but does not connect it to the larger issue: the benchmark measures a different IP-discovery path than production, so benchmark results do not represent real timing. This makes capacity-planning data unreliable.

3. **`Validate-DeployState.ps1` does not assert `ci-report-ip.service` presence on Linux templates** (my §3.5, §6 issue list HIGH). The systemd unit writing `guestinfo.ci-ip` is the primary IP-discovery channel for Linux. If a template refresh forgets the unit, every Linux job silently falls back to the 120-second `getGuestIPAddress` timeout. GPT-5.5 does not mention this dependency at all.

4. **`capacity: 4` is unvalidated** as a first-class CRITICAL issue (my §1 top priority, §6.1). GPT-5.5 mentions "For capacity 4, this is probably reliable enough after one fix: automatic stale lock cleanup," which is too easy. Whether four parallel `Invoke-CIJob.ps1` invocations actually behave correctly — under DHCP lease contention, under simultaneous `vmrun start`, under the cumulative ~6-minute pre-build serialization — has not been measured. The honest engineering position is: either run the burn-in or set `capacity: 1` until validated. GPT-5.5 lets this slide.

5. **Hardcoded `N:\Code\Workspace\Local-CI-CD-System\scripts` in [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml)** (my §11.4, §13 quick win #4). GPT-5.5 notes path drift in setup/backup but misses that the composite action itself contains a hardcoded clone path. Move the host repo and the action breaks.

6. **PS 7 strategic question** (my §15.8). My analysis raises this and explicitly counter-argues that the PS 5.1 mandate is the right homelab choice today. GPT-5.5 takes PS 5.1 as a fixed constraint and never considers whether the constraint deserves revisiting. This is a stylistic difference, not a defect, but the "what would I change" section in my own analysis is a strategic-thinking layer GPT-5.5 omits entirely.

7. **The 4-minute pre-build serialization under capacity: 4** (my §3.3). At ~60-90 seconds per IP-acquire phase, four concurrent jobs serialize ~6 minutes of pre-build wait for the last one. The build phase is parallel after that. GPT-5.5 does not work this math.

What both of us missed:

1. **VMware shared-folder write semantics in detail**. Both analyses flag HGFS cache poisoning at MEDIUM. Neither verifies which specific shared folders are configured in the template VMX and whether the NuGet/pip cache directories are writable from the guest by default. The actual remediation depends on those specifics.

2. **Gitea Actions secret masking under base64/hex transforms**. Neither analysis quantifies the masking limitation. If a workflow author base64-encodes a secret before passing it through `extra-guest-env-json`, Gitea's string-match masking misses the encoded value in logs. Worth a one-liner in security docs.

3. **`vmrun` invocation under simultaneous PowerShell processes**. Both of us trust that `vmrun.exe` is process-safe. I have not seen a documented assertion of this, and the VMware Workstation product is historically single-user single-instance. Under capacity 4, four PowerShell processes each invoke `vmrun start` simultaneously. The IP lock serializes the user-visible flow, but native-tool concurrency safety is an unstated assumption.

4. **Network namespace / VMnet8 saturation under load**. With four concurrent Windows builds each downloading NuGet packages and four concurrent Linux builds each running `apt update`, the NAT outbound bandwidth is a single shared resource. No analysis mentions this.

5. **`act_runner` log rotation**. Both of us treat act_runner's own log file as a fire-and-forget concern. On a multi-month homelab, that file grows. Neither analysis mentions rotation.

---

## 4. Over-Engineered or Excessive Suggestions

The GPT-5.5 list is generally restrained. The suggestions that drift toward over-engineering:

1. **"Use an askpass helper or temporary credential file created on the guest with chmod 600, run git with GIT_ASKPASS or http.extraHeader from a file/stdin, then shred/delete it"** (CRITICAL #3 Linux PAT fix). This is a four-moving-parts mitigation when a one-line `http.extraHeader=Authorization: Basic <base64>` (mirroring the existing Windows path) gives equivalent or better security. Smell: enterprise-pattern over-think. The simpler fix has fewer failure modes.

2. **"Add lock owner metadata into the lock file"** (HIGH stale vm-start.lock fix). The orchestrator is single-instance per host. The lock file's existence is sufficient signal; adding JSON metadata (PID, hostname, started-at) is a small but real complication that pays back only if the system grows multi-host. For homelab scope, just delete the file if older than 30 minutes. The PID-based liveness check is enterprise-runtime territory.

3. **"Per-phase duration summary, free disk before and after job, clone directory size at teardown, VM start lock wait time, guest IP detection method, cleanup-failure count, Gitea runner online API status"** (Section 8 missing metrics). This is the full nine yards of observability. For a homelab, the existing JSONL + Event Log + webhook is already more than most commercial CI has. The summary table at job-end (Quick Win in my analysis, also in GPT-5.5's NICE-TO-HAVE) is the highest-value subset. The rest is metrics-for-metrics'-sake.

4. **"Standardize names in docs and scripts: CI-DiskSpaceAlert, CI-DiskAlert, CI-RunnerHealth"** (MEDIUM Event Log sources). GPT-5.5 lists three target names that are themselves inconsistent (`CI-DiskSpaceAlert` and `CI-DiskAlert` in the same sentence). The right fix is to pick one and propagate; the request shape is correct, the example list contradicts itself.

5. **"Add optional `-GiteaUrl` and `-TokenTarget` to query runner status"** (NICE-TO-HAVE Gitea API check). The PAT used for API queries needs to be stored in Credential Manager, the API endpoint can change between Gitea versions, and the value-add over local service status is incremental. This is a reasonable second-phase improvement; calling it out at all in a "homelab" review is borderline.

6. **"Treat templates as release artifacts. Back them up, version snapshots, run a smoke build, then promote by updating runner config"** (Section 15 architectural recommendation). The shape is correct but the language drifts toward release-engineering vocabulary that does not match homelab scope. The practical translation — `Backup-CITemplate.ps1` → refresh → `Test-CITemplateSmoke.ps1` → update `runner/config.yaml` — is fine, but does not need the "release artifacts" framing.

7. **"Pin Git/7-Zip/Ubuntu immediately"** (MEDIUM hash pinning). For a private homelab consuming only the original vendor URLs over TLS, the probability of a successful supply-chain substitution is genuinely low. Time-to-pin is non-trivial (correct SHA256 must be tracked per installer version, which then becomes an upgrade-blocker). For a homelab, accepting unverified TLS-fetched installers from `python.org`, `7-zip.org`, `microsoft.com` is defensible. The "immediately" elevates urgency beyond what threat-modelling supports.

None of these are catastrophically over-engineered. GPT-5.5 stays mostly in scope. But the cumulative drift is real: a master plan that adopts every GPT-5.5 suggestion uncritically will accumulate enterprise-shaped barnacles.

---

## 5. Architectural Conflicts

GPT-5.5's recommendations are mostly consistent with system constraints. Two minor conflicts:

1. **"Move bounded native process execution, IPv4 validation, and stale state cleanup into `_Common.psm1`"** (§3 architecture). The directive is sound, but `Invoke-VmrunBounded` as it exists in some deploy scripts uses `Wait-Process -Timeout` plus `Stop-Process`. On PS 5.1 / Windows PowerShell, `Wait-Process -Timeout` is reliable, but the `Stop-Process` afterwards does not guarantee `vmrun.exe`'s child `vmware-vmx.exe` process is cleaned up. The architectural fix is correct in spirit but glosses over the cleanup semantics of native VMware processes. A complete implementation would also locate the child PID and force-terminate. GPT-5.5 does not flag this nuance.

2. **"Use `StrictHostKeyChecking=accept-new` and a CI-specific known_hosts file under `F:\CI\State`"** (MEDIUM SSH host-key handling). The directive is correct for CI jobs. But `accept-new` requires OpenSSH 7.6+; the Windows 11 built-in OpenSSH client is OpenSSH for Windows 8.x+, which supports it. PS 5.1 itself does not constrain this. On VMnet8 with template-controlled host keys, this is fine. No real conflict, but worth noting that the alternative `UserKnownHostsFile=F:\CI\State\known_hosts` with `StrictHostKeyChecking=yes` plus a known-hosts seed step at template-prepare time is the stricter option.

The other GPT-5.5 recommendations respect PS 5.1, VMware Workstation single-host topology, and the headless-host model.

---

## 6. Incorrect Assumptions

1. **"The current orchestrator now primarily reads `guestinfo.ci-ip` and uses `getGuestIPAddress` as fallback"** (Section 2). This is technically correct, but GPT-5.5 does not verify the guest-side dependency: a `ci-report-ip.service` (or equivalent on Windows) must exist in the template to publish `guestinfo.ci-ip`. Section 6 of my own analysis flags this; GPT-5.5 takes the channel for granted.

2. **"Cleanup happens after `Invoke-RemoteBuild.ps1` success only"** (Section 2 inconsistency note about docs vs implementation). GPT-5.5 conflates "artifact collection happens only after successful build" with "cleanup happens only after success." The cleanup (Remove-BuildVM) runs in `finally`, so it runs whether build succeeded or failed. The doc/reality drift is only about artifact collection, not cleanup. Minor.

3. **"For capacity 4, [the IP lock is] probably reliable enough after one fix: automatic stale lock cleanup"** (Section 3). The assertion "probably reliable enough" is unsupported. Whether DHCP collisions occur under four-way contention, whether `vmrun start` is concurrency-safe, whether the 120-second IP-acquire deadline holds when host CPU is contended — none of this has been measured. The correct stance is "unknown until burn-in."

4. **"`scripts/_Transport.psm1` covers SSH helpers only, while `scripts/_Common.psm1` covers WinRM session options and vmrun. This is acceptable..."** (Section 3). This naming is slightly misleading. `_Transport.psm1` reads as a generic transport abstraction but contains only SSH helpers. The name is asymmetric with `_Common.psm1`'s WinRM helpers. GPT-5.5 calls this out but then accepts it. I find the asymmetry more bothersome — the symmetric option would be either `_SshTransport.psm1` + `_WinrmTransport.psm1` or a single `_Transport.psm1` covering both. Style preference, not a defect.

5. **"VS Build Tools 2026 (MSBuild 18.5 / v145), .NET SDK 10"** (cited in [AGENTS.md](../AGENTS.md) and implicitly in GPT-5.5's Windows toolchain section). These version numbers correspond to future/unreleased Microsoft products as of the workspace's stated date. GPT-5.5 takes them at face value without questioning whether the project's stated environment dates and the actual installer URLs in [template/Install-CIToolchain-WinBuild2025.ps1](../template/Install-CIToolchain-WinBuild2025.ps1) are consistent. Probably out of scope, but a careful reviewer should at least note this.

6. **"Composite action correctly passes most inputs through env vars to reduce shell injection risk"** (Section 5). Correct. But GPT-5.5 then says "`extra-guest-env-json` itself is an env var containing secrets; ensure act_runner does not echo it." act_runner does not echo env vars by default. The risk is the downstream usage inside [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1) (already flagged), not act_runner itself. Minor framing slip.

---

## 7. Priority Validation

| GPT-5.5 Severity | Item | My Verdict | Rationale |
| --- | --- | --- | --- |
| CRITICAL | Composite action outputs structure | Should be HIGH | Step-level outputs work today; only caller-facing action interface is broken. Latent, not active. |
| CRITICAL | Linux extra guest env secret logging | CRITICAL (concur) | Active leak path. |
| CRITICAL | Linux Mode 2 PAT in SSH command | HIGH-CRITICAL boundary | PAT visibility limited to host argv + guest /proc. Real but bounded. I would put HIGH; CRITICAL is defensible. |
| HIGH | vmrun unbounded calls | HIGH (concur) | |
| HIGH | Stale vm-start.lock | HIGH (concur) | |
| HIGH | Setup-Host stale dirs | HIGH (concur) | |
| HIGH | Backup default path | HIGH (concur) | |
| HIGH | Failure-path diagnostics | Should be MEDIUM | Quality-of-life, not operational-trust. |
| HIGH | Test plan PS7 / wrong params | Should be MEDIUM | Documentation-only. |
| HIGH | Shared cache not wired | Should be MEDIUM | Documented feature missing, performance impact only. |
| MEDIUM | JobId not sanitized | Should be LOW | Local-only attack surface in single-tenant homelab. |
| MEDIUM | Wait-VMReady TCP-only check | MEDIUM (concur) | |
| MEDIUM | CI-FLOW doc claims partial artifacts | Should be LOW | Pure docs. |
| MEDIUM | Validate-SetupState PasswordExpires | MEDIUM (concur) | Validation failure mode. |
| MEDIUM | Ubuntu VMDK no hash | MEDIUM (concur) | |
| MEDIUM | Installer hash placeholders | MEDIUM (concur for homelab) | |
| MEDIUM | Event Log / task name drift | Should be LOW | Tests fail not scripts. |
| MEDIUM | _Transport.psm1 host-key handling | MEDIUM (concur) | |

Items I would add at HIGH that GPT-5.5 missed: `act_runner` SYSTEM-vs-user SSH alias mismatch (verifiable, runtime-reachable), `ci-report-ip.service` missing from `Validate-DeployState.ps1` (silent 120 s timeout), `capacity: 4` burn-in not run (concurrency claim unmeasured).

Items I would demote from CRITICAL: composite action outputs (real but contained to caller interface only).

---

## 8. Pragmatic Improvements

GPT-5.5's recommendations, recast for homelab-pragmatic adoption:

1. **Composite action outputs**: Move both `artifact-path` and `artifact-name` to a top-level `outputs:` block and remove the duplicate `artifact-name` declaration. Keep the inner `Out-File $env:GITHUB_OUTPUT` lines; they continue to drive step-level outputs that the upload step depends on. 20 minutes.

2. **Linux secret redaction**: Replace `Write-Host "[Invoke-RemoteBuild] Running build: $buildCmd"` with `Write-Host "[Invoke-RemoteBuild] Running build (env vars redacted): cd '$GuestLinuxWorkDir' && <build-command>"`. Do NOT log the env prefix at all. Move from CRITICAL fix to one-line change. 5 minutes.

3. **Linux PAT injection**: Mirror Windows `http.extraHeader=Authorization: Basic <base64>` via `git -c http.extraHeader=...`. Do not pursue askpass helper. 30 minutes including a test.

4. **vmrun bounded wrapper**: Move `Invoke-VmrunBounded` into `_Common.psm1`, use for `start`, `stop`, `deleteVM`. Skip `list`, `readVariable`, `getGuestIPAddress` — those are short-running calls and the wrapper adds overhead. Bounded scope. 45 minutes.

5. **Stale lock cleanup**: Add a 5-line block to [scripts/Invoke-RetentionPolicy.ps1](../scripts/Invoke-RetentionPolicy.ps1) and to a startup step in [scripts/Cleanup-OrphanedBuildVMs.ps1](../scripts/Cleanup-OrphanedBuildVMs.ps1) that removes `F:\CI\State\vm-start.lock` if older than 30 minutes. No metadata inside the lock file. 15 minutes.

6. **Setup-Host directory list**: Update [Setup-Host.ps1](../Setup-Host.ps1) line 118 directory array to include `Templates\WinBuild2025`, `Templates\WinBuild2022`, `Templates\LinuxBuild2404`, `State`, `State\ip-leases`, `keys`, `Cache\pip`. Remove `Templates\WinBuild`. 5 minutes.

7. **Backup default**: Change [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1) default to `'F:\CI\Templates\WinBuild2025'`. Add a `-AllTemplates` switch that iterates all `Templates\*` directories. 20 minutes.

8. **Test plan**: Replace `ForEach-Object -Parallel` with `1..4 | ForEach-Object { Start-Job -ScriptBlock { ... } } | Wait-Job | Receive-Job`. Replace `-RepositoryUrl` with `-RepoUrl`. Pure text edit. 15 minutes.

9. **JobId validation**: Add `[ValidatePattern('^[A-Za-z0-9._-]+$')]` to JobId parameter in [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) and [scripts/New-BuildVM.ps1](../scripts/New-BuildVM.ps1). 5 minutes.

10. **Pre-clone disk check**: Add to Phase 2 of [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1): `if ((Get-PSDrive F).Free / 1GB -lt 20) { throw "Insufficient disk space" }`. 5 minutes.

11. **Webhook emoji removal**: Replace `:warning:` and similar in [scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1) and [scripts/Watch-RunnerHealth.ps1](../scripts/Watch-RunnerHealth.ps1) with `[WARN]` and `[ERROR]`. 5 minutes.

What GPT-5.5 proposes that I would defer or drop:

- Lock-owner metadata in lock file (over-engineered for single-host).
- Gitea API runner online check (acceptable as deferred).
- Per-build VMX CPU/RAM override (defer; no demand signal yet).
- Artifact manifest with hashes (NICE-TO-HAVE for homelab).

---

## 9. Coherence with Homelab Scope

GPT-5.5 generally stays in homelab scope. The phrasing in a few places drifts toward enterprise language ("release artifacts," "golden image promotion flow," "path contract / process contract / secret contract"), but the concrete actions remain proportional. The "Deferred/Optional" list correctly excludes multi-host runner federation, Prometheus/Grafana/Loki, full PKI, enterprise secret vaults, and Kubernetes/ESXi. That is the right scope discipline.

Where GPT-5.5 over-extends:

- The Section 5 enumeration of "what a compromised build VM could reach" reads as enterprise threat-modelling for a single-tenant lab. The threat model is "the operator runs only their own private code." Listing five attack vectors is exhaustive but past the point of useful for the operator's actual risk picture.
- The Section 8 "missing metrics and alerting" list of seven specific metrics implies a metrics-stack mindset. For homelab, "phase duration printed at job end" subsumes 80% of the value of that list.
- The Section 15 trio of "path contract / process contract / secret contract" reads as architecture-document framing. For a single-operator homelab, the same content fits in three sentences in [AGENTS.md](../AGENTS.md).

Where GPT-5.5 stays correctly proportional:

- "Keep the single-host design." Correct.
- "Do not pursue deterministic static IP assignment yet." Correct, and notably more conservative than my own §15.1 which proposes a static-IP pool. GPT-5.5 is right here: the existing lock + lease design after stale-lock cleanup is good enough at this capacity.
- "Keep security proportional." Correct.
- "Do not over-abstract WinRM and SSH into a single generic transport." Correct. My own §15.2 proposes generic transport abstraction; GPT-5.5 explicitly counter-argues that "the current clarity is valuable when something fails at 2 AM." GPT-5.5 wins this debate.

---

## 10. Consolidated Recommendation List

After this review, the following GPT-5.5 recommendations should make it into the master plan unchanged:

**Phase 1 — Stabilization (adopt as-is):**
- Fix composite action outputs (with corrected severity, HIGH not CRITICAL).
- Redact Linux secret logging (concur CRITICAL).
- Fix Linux Mode 2 PAT to use `http.extraHeader` (cleaner than GPT-5.5's askpass proposal).
- Add stale `vm-start.lock` cleanup (concur HIGH).
- Fix [Setup-Host.ps1](../Setup-Host.ps1) directory list (concur HIGH).
- Fix [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1) default path (concur HIGH).
- Pre-clone disk space gate (concur).
- Test plan PS5.1 + parameter name corrections (demoted to MEDIUM).
- Real Windows + Linux VM smoke build via composite action (concur).

**Phase 2 — Adopt with modification:**
- vmrun bounded wrapper: scope to `start`/`stop`/`deleteVM`, not all native calls.
- Failure-path diagnostics: implement as a single `Get-GuestDiagnostics` helper, MEDIUM not HIGH.
- Shared cache wiring through composite action: concur, add as Phase 2.
- Event Log / task name standardization: pick one canonical set, propagate.
- Phase duration summary at job end: concur (lighter than full metrics layer).

**Adopt at LOW priority:**
- Webhook emoji removal.
- Delete deprecated [runner/Install-Runner.ps1](../runner/Install-Runner.ps1).
- Docblock HTTPS/5986 corrections in Validate-* scripts.
- Benchmark JSON shape vs test plan.

**Drop or defer indefinitely:**
- Lock-owner metadata inside lock file.
- Gitea API runner online check.
- Per-build VMX CPU/RAM override.
- Artifact manifest with hashes.
- Full SHA256 pinning for installers (acceptable as deferred for homelab).
- Smoke build automation as a separate `Test-CITemplateSmoke.ps1` (combine with capacity burn-in instead).

**Add from my own analysis (GPT-5.5 missed these):**
- `act_runner` service-account vs SSH alias mismatch verification (HIGH).
- `ci-report-ip.service` presence check in [template/Validate-DeployState.ps1](../template/Validate-DeployState.ps1) (HIGH).
- `capacity: 4` burn-in or downgrade (CRITICAL).
- Hardcoded `N:\Code\Workspace\...` in composite action (MEDIUM).
- `Measure-CIBenchmark.ps1` use of `getGuestIPAddress` aligned with production path (MEDIUM).

---

## 11. Comparison with My Own Analysis

Where GPT-5.5 outperforms my analysis:

1. **Per-file scrutiny in Section 4**. GPT-5.5 walks 20+ files explicitly and catches small defects (PasswordExpires assumption in `Validate-SetupState.ps1`, `Measure-CIBenchmark.ps1` JSON schema vs test plan, emoji in webhook payloads, `Watch-DiskSpace.ps1` Event Log source name drift). My analysis is structured around themes; I missed those concrete file-level facts.
2. **Stale vm-start.lock impact on first-job-after-reboot UX**. My §6.1 verified the lock-handle path is correct on graceful exit but missed the file-existence-after-crash concern. GPT-5.5 makes this explicit.
3. **Setup-Host directory list audit**. I caught the password documentation drift; I missed the directory-list drift. GPT-5.5 enumerates exactly which directories are missing.
4. **Backup-CITemplate default path audit**. I listed the script as Complete without checking the default. GPT-5.5 actually opened the file and read line 48.
5. **Shared cache wiring gap**. I treated shared cache as a security concern; GPT-5.5 treats it as a wiring/operability concern. Both are valid; GPT-5.5's framing is more actionable.
6. **CI-FLOW doc vs reality drift on partial-artifact-on-failure**. I did not audit `CI-FLOW.md` against the orchestrator's actual `catch` flow. GPT-5.5 did.
7. **Wait-VMReady TCP-only check**. Concrete defect I missed. GPT-5.5 caught.
8. **Ubuntu VMDK hash absence as separate channel from toolchain hash placeholders**. I lumped these; GPT-5.5 separates them, which is correct.
9. **Linux extra-env redaction at CRITICAL severity**. I had this at MEDIUM. GPT-5.5's CRITICAL is more defensible if any production workflow currently injects a secret via `extra-guest-env-json`.

Where my analysis outperforms GPT-5.5:

1. **`act_runner` SYSTEM-account vs `~/.ssh/config` alias**. My §8.2 HIGH. GPT-5.5 misses entirely. This is a verifiable runtime-reachable bug.
2. **`capacity: 4` burn-in or downgrade as CRITICAL**. My §1 top priority. GPT-5.5 minimizes this to "probably reliable enough after one fix."
3. **`ci-report-ip.service` presence check in template validation**. My §3.5. GPT-5.5 omits this dependency chain entirely.
4. **`Measure-CIBenchmark.ps1` measures a different IP path than production**. My §3.3. GPT-5.5 mentions in passing but does not connect to capacity-planning credibility.
5. **Hardcoded `N:\Code\Workspace\...` in composite action**. My §8.1. GPT-5.5 catches path drift in setup/backup but not in the action itself.
6. **The 4-minute pre-build serialization math**. My §3.3 works the arithmetic. GPT-5.5 does not.
7. **Section 15 strategic-thinking layer ("What I Would Change")**. My analysis includes a counterfactual section (static IP allocation, transport abstraction, configuration centralization, JSONL→SQLite sink, schema validation, integration-test-first, structured logging helper, PS 7 reconsideration, validation in orchestrator, drop Mode 1). GPT-5.5 has architectural recommendations but does not run a true counterfactual.
8. **OWASP Top 10 mapped explicitly**. My §5.7 walks A01-A10 explicitly. GPT-5.5 covers security thematically without the framework. Either is fine for homelab; the framework view makes audit conversations easier.
9. **Honest grade with per-dimension breakdown**. My Final Verdict gives A-/B+/B/B-/C grades per dimension. GPT-5.5 gives single-number scores (82/78/72/77) without showing the dimensions. My breakdown is more diagnostically useful.

Complementarity:

GPT-5.5 is the better **per-file auditor**. Its strength is reading scripts one at a time and surfacing small concrete defects. The Section 4 enumeration is exactly what a careful code reviewer produces.

My analysis is the better **systems-level interrogator**. It questions claims (capacity: 4 unvalidated), traces cross-file dependencies (SSH alias vs service account, `ci-report-ip.service` vs orchestrator IP discovery), and works the strategic counterfactual.

Combined, the two reviews cover the codebase better than either alone. The master plan should:

- Adopt GPT-5.5's Section 4 per-file findings nearly verbatim as the action item list.
- Adopt my §6.1 (capacity validation), §8.2 (service account verification), §3.5 (ci-report-ip.service check) as the missing critical items.
- Use my Section 15 as the long-horizon strategic background that informs but does not block Phase 1.
- Use GPT-5.5's Phase 1/2/3/Deferred phasing as the roadmap skeleton.

---

## 12. Closing

The GPT-5.5 analysis is a credible, accurate, well-organised technical review with a small number of severity miscalibrations and one notable omission set (the service-account / ssh-alias / ci-report-ip.service / capacity-burn-in cluster). It would be perfectly defensible to adopt the GPT-5.5 report as the primary working document for the next stabilization pass, provided the missing items above are appended.

The honest verdict on GPT-5.5's verdict ("strong homelab CI/CD system... last tightening pass between an impressive lab build and a reliable daily tool") is: correct, fair, and consistent with the evidence. I would sign that conclusion.

The honest verdict on my own analysis vs GPT-5.5: comparable overall, complementary in coverage. Neither replaces the other. The combination is stronger than either alone, which is exactly the value proposition of running two independent reviews.

---

*End of cross-review. Word count: approximately 5,400.*