Skip to content

fix: harden transactional test-deployment rebuild - #333

Merged
monkey1sai merged 10 commits into
mainfrom
fix/deploy-rebuild-worktree-e2e
Jul 13, 2026
Merged

monkey1sai merged 10 commits into
mainfrom
fix/deploy-rebuild-worktree-e2e

Conversation

@monkey1sai

@monkey1sai monkey1sai commented Jul 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Harden the fixed test-deployment rebuild path with fail-closed reparse-point validation and a stable same-parent exclusive lock.
  • Preserve the staged replacement behavior for non-Git and broken-gitfile deployments, including fail-closed validation and service-stop handling.
  • Add the transaction design/implementation plan and Windows fixture coverage; record only the completed plan slices.

AI Coding Governance

Item Result
Change lane G
Behavior contract changed yes
Linked issue No linked issue; user-directed preservation and completion of the existing WIP branch
Requirement source superpowers spec
CODEOWNERS / owner review not needed; repository owner review occurs through this PR
GitNexus evidence Fresh branch re-index; detect_changes compare main found 4 files, risk low, 0 affected flows. PowerShell impact remains Target not found / UNKNOWN, compensated by raw source review and tests.
Browser E2E evidence not user-facing
Agent workflow changed? no
Required checks expected CI / Agent Governance / PR Review Agent

Frontend Verification

Item Result
Frontend route not user-facing
Main button(s) tested not user-facing
Fixture used isolated Windows TEMP deployment trees and real junction fixtures
Backend API called not applicable; PowerShell deployment helper only
Runtime action no runtime session created
Visible success state test process PASS / fail-closed error assertions
E2E command not user-facing; focused PowerShell integration test used
Screenshot / trace not applicable to this deploy-helper slice
Manual test steps none; deterministic script tests listed below
Known gaps fixed D: deployment and browser E2E were intentionally not invoked

Deploy Path Verification

Item Result
Affects runtime / docker / Kit / viewer / ports / env? yes; test-deployment rebuild orchestration only
Canonical deploy path updated? verified; fixed-path assertion remains unchanged and tests use only the injected TEMP seam
New root script added? no
Deploy dry-run command not run; scripts/deploy.ps1 behavior is unchanged and focused rebuild tests cover this slice
Full deploy tested not available; fixed D: deployment was not mutated during branch closeout
Verify command pwsh -NoProfile -NonInteractive -File scripts/tests/test-rebuild-test-deploy.ps1
Frontend URL verified not user-facing
Evidence path scripts/tests/test-rebuild-test-deploy.ps1

Validation

  • PASS: pwsh -NoProfile -NonInteractive -File scripts/tests/test-rebuild-test-deploy.ps1
  • PASS: pwsh -NoProfile -NonInteractive -File scripts/tests/test-host-native-launcher.ps1
  • PASS: pwsh -NoProfile -NonInteractive -File scripts/tests/test-preflight-ports.ps1
  • PASS: PowerShell dot-source / helper discovery for Assert-TestDeployPathSafety and Enter-TestDeployRebuildLock
  • PASS: git diff --check origin/main...HEAD
  • ADVISORY: GitNexus detect_changes returned risk low for indexed Markdown sections, but PowerShell execution-flow coverage remains UNKNOWN

Known Risks

  • The real fixed deployment at D:\Users\deploy\AI-bim-geo and browser runtime were not touched; this PR proves the transaction slice through isolated Windows integration fixtures.
  • CurrentWorktree materialization, two-rename cutover, PID identity, and browser E2E remain explicitly unchecked future items in the implementation plan.

Summary by CodeRabbit

  • New Features

    • Added safer transactional test-deployment workflows with staged replacements and rollback protection.
    • Added deployment path safety checks and rebuild locking to prevent conflicting operations.
    • Added validation for source provenance, service identity, readiness, and evidence freshness.
    • Added worktree-based frontend end-to-end testing with deterministic fixtures and run-specific evidence.
  • Bug Fixes

    • Improved handling of broken staged checkouts, environment restoration, and deployment cleanup.
    • Prevented deployment actions when validation, locking, or service-stop checks fail.
    • Added strict verification that restored environment files match their preserved contents.
  • Documentation

    • Added detailed deployment design, implementation plan, verification, and operator guidance.

Copilot AI review requested due to automatic review settings July 13, 2026 10:21
@coderabbitai

coderabbitai Bot commented Jul 13, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a transactional rebuild/deploy plan and design specification, strengthens PowerShell path, lock, checkout, environment, and cutover handling, and adds extensive tests for staged preparation, fail-closed behavior, reparse-point safety, lock contention, and cleanup.

Changes

Transactional rebuild and deploy

Layer / File(s) Summary
Workflow plan and task gates
docs/superpowers/plans/...
Documents provenance modes, rebuild tasks, deployment readiness, worktree E2E execution, evidence rules, CI wiring, and final verification.
Source and transaction design
docs/superpowers/specs/...design.md
Defines source materialization, path and lock protections, environment integrity, process identity validation, rollback boundaries, and deploy readiness checks.
Worktree E2E and verification design
docs/superpowers/specs/...design.md
Defines deterministic fixture selection, browser checks, result statuses, run-specific evidence, artifact hashes, and verification layers.
Path, lock, and environment safeguards
scripts/lib/rebuild-test-deploy.ps1
Adds path safety checks, exclusive rebuild locking, broken gitfile detection, and strict byte-level environment restoration.
Staged rebuild and cutover orchestration
scripts/lib/rebuild-test-deploy.ps1
Adds staged checkout preparation, origin normalization, clean retries, service stopping, directory cutover, rollback handling, and guaranteed lock disposal.
Transactional and Windows safety validation
scripts/tests/test-rebuild-test-deploy.ps1
Adds coverage for fail-closed preparation, live-state preservation, reparse-point rejection, lock contention and ordering, and cleanup verification.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant InvokeTestDeployRebuild
  participant Git
  participant EnvSnapshot
  participant ServiceStopper
  participant Filesystem
  InvokeTestDeployRebuild->>Git: clone and validate staged checkout
  InvokeTestDeployRebuild->>EnvSnapshot: restore preserved environment files
  InvokeTestDeployRebuild->>ServiceStopper: stop deployment-zone services
  InvokeTestDeployRebuild->>Filesystem: perform directory cutover
  Filesystem-->>InvokeTestDeployRebuild: return cutover or rollback result
Loading

Possibly related PRs

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: hardening the transactional test-deployment rebuild path.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/deploy-rebuild-worktree-e2e

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the test-deployment rebuild helper to reduce the chance of unsafe filesystem mutation by adding fail-closed path safety checks, a same-parent exclusive rebuild lock, and staged replacement behavior for non-git / broken-gitfile deployments, alongside expanded PowerShell integration fixtures and accompanying design/plan documentation.

Changes:

  • Add Assert-TestDeployPathSafety, Enter-TestDeployRebuildLock, and broken-gitfile detection; wire them into Invoke-TestDeployRebuild with staged replacement + service-stop aggregation.
  • Strengthen env snapshot restore by verifying written bytes match the preserved snapshot (and removing the .example dependency).
  • Add extensive RED fixtures in scripts/tests/test-rebuild-test-deploy.ps1 plus new transactional design and implementation plan docs.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
scripts/tests/test-rebuild-test-deploy.ps1 Adds transactional + path/reparse/lock RED fixtures and safety teardown assertions.
scripts/lib/rebuild-test-deploy.ps1 Implements path safety validation, exclusive rebuild lock, broken gitfile detection, staged replacement orchestration, and stricter env restore verification.
docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md Documents the transactional rebuild/worktree E2E design, constraints, and verification contract.
docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md Adds a task-based implementation plan with constraints, evidence expectations, and verification layers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +101 to +117
$deployLeaf = Split-Path -Leaf $deployRoot
$lockPath = Normalize-TestDeployPath -Path (Join-Path $deployParent ".$deployLeaf.rebuild.lock")
$handle = $null
try {
$handle = [System.IO.File]::Open(
$lockPath,
[System.IO.FileMode]::OpenOrCreate,
[System.IO.FileAccess]::ReadWrite,
[System.IO.FileShare]::None
)
} catch [System.IO.IOException] {
$lowCode = [int]$_.Exception.HResult -band 0xFFFF
if ($lowCode -in @(32, 33)) {
throw "test deploy rebuild already in progress for '$deployRoot'"
}
throw "failed to acquire test deploy rebuild lock '$lockPath': $($_.Exception.Message)"
}
Comment on lines +625 to +631
} catch {
$stageError = $_
if (Test-Path -LiteralPath $stageRoot) {
Remove-Item -LiteralPath $stageRoot -Recurse -Force -ErrorAction SilentlyContinue
}
throw $stageError
}
@monkey1sai
monkey1sai merged commit de6c1d2 into main Jul 13, 2026
13 of 14 checks passed
@monkey1sai
monkey1sai deleted the fix/deploy-rebuild-worktree-e2e branch July 13, 2026 10:28

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md (1)

64-69: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Define one canonical CurrentWorktree diff command.

The design’s git diff command differs from the plan’s command at Line 143: it omits --no-ext-diff and --ita-visible-in-index. Align both documents and the implementation to one exact command, or explicitly document why the flags differ; otherwise source bytes and provenance can vary for the same worktree.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md`
around lines 64 - 69, The worktree materialization flow and the plan must use
one canonical CurrentWorktree git diff command. Update the documented command
near the source-change steps and the corresponding implementation to match the
plan’s exact flags, including --no-ext-diff and --ita-visible-in-index, or
explicitly document a justified difference; preserve matching source bytes and
manifest provenance for the same worktree.
scripts/lib/rebuild-test-deploy.ps1 (1)

543-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefer structured logging over bare Write-Host in the new staged path.

The new staged-rebuild block introduces several Write-Host calls (also at Lines 560, 595, 624, 634). Per repo convention these should route through scripts/lib/StructLog.psm1 rather than bare Write-Host. The rest of this file predates the rule, so aligning at least the new lines keeps the staged transaction observable in the structured stream.

As per coding guidelines: "Use scripts/lib/StructLog.psm1 for structured logging output; do not replace with bare Write-Host calls".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/lib/rebuild-test-deploy.ps1` around lines 543 - 544, Replace the new
staged-rebuild Write-Host calls, including those near the staged checkout
discard and the referenced later lines, with the structured logging functions
provided by StructLog.psm1. Preserve each message and relevant values while
routing output through the established structured logging interface; leave
pre-existing Write-Host calls outside the staged path unchanged.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md`:
- Around line 291-309: Do not present the frontend E2E runner as implemented. In
docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md:291-309,
rewrite Step 5 and the result-layer requirements in future tense while retaining
the unchecked status; in
docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md:143-175,
label the contract as proposed rather than current capability. Preserve the
stated wrapper, dependency bootstrap, and current-run evidence gate
requirements.
- Around line 111-170: Update the Task 2 checklist to reflect the existing
implementation in scripts/lib/rebuild-test-deploy.ps1: mark the staged
preparation, standalone checkout validation, environment restoration,
service-stop handling, and rename cutover slices complete where implemented, and
split any incomplete requirements into explicit unchecked gaps. Preserve the
remaining validation and test-gate items as unchecked unless the implementation
already satisfies them.
- Around line 1-3: Classify the plan document as a “working note” and reference
docs/AGENTS.md for agent-documentation rules. Also update
docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md
at lines 1-3 to classify it as “spec design,” reference docs/AGENTS.md, and
state that the implementation remains the runtime source of truth.

In
`@docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md`:
- Line 7: 將第 7 行的目前行為描述標示為 implementation gap 或 historical
evidence,而非執行時真相;同時引用實際實作 helper 的相關
script,明確說明該段內容是待程式碼驗證的證據。保留原有問題清單,但避免將設計文件本身視為權威行為來源。

---

Nitpick comments:
In
`@docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md`:
- Around line 64-69: The worktree materialization flow and the plan must use one
canonical CurrentWorktree git diff command. Update the documented command near
the source-change steps and the corresponding implementation to match the plan’s
exact flags, including --no-ext-diff and --ita-visible-in-index, or explicitly
document a justified difference; preserve matching source bytes and manifest
provenance for the same worktree.

In `@scripts/lib/rebuild-test-deploy.ps1`:
- Around line 543-544: Replace the new staged-rebuild Write-Host calls,
including those near the staged checkout discard and the referenced later lines,
with the structured logging functions provided by StructLog.psm1. Preserve each
message and relevant values while routing output through the established
structured logging interface; leave pre-existing Write-Host calls outside the
staged path unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dd3eb22e-6027-41d3-9033-e7e4d3f7769b

📥 Commits

Reviewing files that changed from the base of the PR and between e6cc412 and c1c8dd5.

📒 Files selected for processing (4)
  • docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md
  • docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md
  • scripts/lib/rebuild-test-deploy.ps1
  • scripts/tests/test-rebuild-test-deploy.ps1

Comment on lines +1 to +3
# 交易式測試部署重建與 Worktree 前端 E2E Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development or superpowers:executing-plans task-by-task. Every worker/reviewer must return Scope, Evidence, Finding, Uncertainty, Risk, Next step.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Declare the document nature and source-of-truth boundary.

Both documents need explicit classification and must separate agent instructions from runtime/product claims.

  • docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md#L1-L3: mark this as a working note and reference docs/AGENTS.md for agent-documentation rules.
  • docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md#L1-L3: mark this as spec design, reference docs/AGENTS.md, and state that implementation remains the runtime truth.
📍 Affects 2 files
  • docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md#L1-L3 (this comment)
  • docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md#L1-L3
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md`
around lines 1 - 3, Classify the plan document as a “working note” and reference
docs/AGENTS.md for agent-documentation rules. Also update
docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md
at lines 1-3 to classify it as “spec design,” reference docs/AGENTS.md, and
state that the implementation remains the runtime source of truth.

Source: Coding guidelines

Comment on lines +111 to +170
### Task 2: GREEN transaction orchestrator

**Files:**
- Modify: `scripts/lib/rebuild-test-deploy.ps1`
- Modify: `scripts/dev/rebuild-test-deploy.ps1`
- Optionally create: `scripts/lib/test-deploy-transaction.ps1` only if keeping the existing library reviewable requires a narrow separation

- [ ] **Step 1: Implement path safety and run layout**

Add narrow helpers equivalent to:

- `Assert-TestDeployPathSafety`
- `New-TestDeployRunLayout`
- `Enter-TestDeployRebuildLock`
- `Get-TestDeployCheckoutState`

Walk all existing path components; any reparse point fails. Lock uses `FileShare.None` and remains as a stable lock file. Return stage/previous/run/env/provenance paths on the same volume.

- [ ] **Step 2: Implement external env backup**

Backup allowlisted files before live mutation; record length/SHA256/ACL hash without value. Restore into stage regardless of `.example` existence and verify bytes/hash. Redact origin URLs in command display/errors.

- [ ] **Step 3: Implement OriginMain stage**

Use explicit refspec, detached exact commit, standalone `.git` directory, expected origin hash and required-script validation. Do not mutate live checkout.

- [ ] **Step 4: Implement CurrentWorktree stage**

Capture:

```text
HEAD
+ git diff HEAD --binary --full-index --no-ext-diff --ita-visible-in-index
+ git ls-files --others --exclude-standard -z
```

Reject sensitive/tooling/cache/reparse/path-escape entries. Copy leaf files and verify hashes. Re-fingerprint source before cutover.

- [ ] **Step 5: Implement two-rename cutover**

Order: prepare/validate stage -> verified service stop -> repeat path safety -> `live -> previous` -> `stage -> live` -> deploy. Only pre-deploy rename failure auto-restores old live. Post-deploy failure returns recovery metadata without claiming external side-effect rollback.

- [ ] **Step 6: Preserve compatible result fields and expose logs**

Keep `DeploymentPath`, `OriginMainCommit`, `DeployExitCode`; add source mode/commit/patch/untracked hashes, checkout kind, provenance/previous/env backup paths, stdout/stderr paths, recovery status.

- [ ] **Step 7: Wire wrapper flags**

```powershell
.\scripts\dev\rebuild-test-deploy.ps1 -Build
.\scripts\dev\rebuild-test-deploy.ps1 -Build -SourceMode CurrentWorktree
```

No `-DryRun` or implicit dirty mode. Print no secret values.

- [ ] **Step 8: Run GREEN transaction gate**

```powershell
pwsh -NoProfile -File scripts/tests/test-rebuild-test-deploy.ps1
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Synchronize the task checklist with the implemented transaction path.

Task 2 is entirely unchecked, although the supplied scripts/lib/rebuild-test-deploy.ps1 implementation already contains staged preparation, standalone-checkout validation, environment restoration, service-stop handling, and rename cutover. Mark completed slices accurately or split remaining work into explicit gaps; otherwise this plan misstates the implementation status.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md`
around lines 111 - 170, Update the Task 2 checklist to reflect the existing
implementation in scripts/lib/rebuild-test-deploy.ps1: mark the staged
preparation, standalone checkout validation, environment restoration,
service-stop handling, and rename cutover slices complete where implemented, and
split any incomplete requirements into explicit unchecked gaps. Preserve the
remaining validation and test-gate items as unchecked unless the implementation
already satisfies them.

Comment on lines +291 to +309
- [ ] **Step 5: Implement one-click wrapper**

```powershell
.\scripts\dev\rebuild-worktree-e2e.ps1 -Build
```

It calls `CurrentWorktree` rebuild in a child process/library-safe path, verifies source provenance, seeds fixture, bootstraps dependencies, runs live shell smoke + strict real IFC functional slice, and optionally strict conversion/Kit visual layers. It writes a run-specific manifest and returns nonzero on any required layer.

- [ ] **Step 6: Define honest result layers**

Record separately:

- `shell_smoke_passed`
- `real_ifc_intake_ready`
- `conversion_ready_observed`
- `governance_semantic_observed`
- `kit_webrtc_visual_observed`

Only the first two are the minimum worktree frontend E2E gate. Full-system claim requires governance semantic + Kit visual; skipped/not-observed never counts as passed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not present the frontend E2E runner as implemented yet.

The PR objectives state that browser E2E remains an unchecked future item, but these sections describe a working one-click runner and successful evidence contract. Mark this as planned/unimplemented until the wrapper, dependency bootstrap, and current-run evidence gate exist.

  • docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md#L291-L309: use future-tense requirements and retain the unchecked status.
  • docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md#L143-L175: label the section as a proposed contract, not current capability.
📍 Affects 2 files
  • docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md#L291-L309 (this comment)
  • docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md#L143-L175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md`
around lines 291 - 309, Do not present the frontend E2E runner as implemented.
In
docs/superpowers/plans/2026-07-10-transactional-test-deploy-worktree-e2e.md:291-309,
rewrite Step 5 and the result-layer requirements in future tense while retaining
the unchecked status; in
docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md:143-175,
label the contract as proposed rather than current capability. Preserve the
stated wrapper, dependency bootstrap, and current-run evidence gate
requirements.


讓 agent 能從 dedicated worktree 安全地重建固定測試部署區 `D:\Users\deploy\AI-bim-geo`,並完成可追溯的前端 E2E;同時保留既有正式驗收口令 `rebuild-test-deploy.ps1 -Build` 的 `origin/main` 語意。

本設計修正的核心問題是:目前 helper 會在目的地原地 reset/clean、只以 `.git` 是否存在判斷 checkout、只信任數字 PID、健康檢查只看 HTTP 2xx,且 Playwright fixture/依賴/證據不足時可能以 `blocked` 結束。這些行為可造成機密環境檔遺失、錯停 PID、舊 runtime 假通過、或 fresh checkout 舊圖假裝成新證據。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Label the current-behavior claim as evidence, not runtime truth.

Line 7 asserts what the helper “currently” does without identifying the implementation source or marking the statement as an implementation gap/historical evidence. Add that boundary and point to the relevant script; the docs rules prohibit treating this document as authoritative program behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@docs/superpowers/specs/2026-07-10-transactional-test-deploy-worktree-e2e-design.md`
at line 7, 將第 7 行的目前行為描述標示為 implementation gap 或 historical
evidence,而非執行時真相;同時引用實際實作 helper 的相關
script,明確說明該段內容是待程式碼驗證的證據。保留原有問題清單,但避免將設計文件本身視為權威行為來源。

Source: Coding guidelines

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c1c8dd5f9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if ($restoredEnvFiles.Count -gt 0) {
Write-Host "[rebuild-test-deploy] restored deployment env files count=$($restoredEnvFiles.Count): $($restoredEnvFiles -join ', ')"
}
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Route valid checkouts through the staged swap

When $deployRoot already has a valid .git directory—the normal state after a successful rebuild—this else keeps the old in-place path. It runs git fetch/reset/clean directly in the live checkout, so a clean or env-restore failure leaves the active deployment partially reset with no $previousRoot to recover from; the staged clone/validate/rename path should cover valid checkouts too after the origin check.

Useful? React with 👍 / 👎.

}
$deployZoneRunDir = Join-Path $deployRoot 'scripts\.run'
$serviceStopFailures = New-Object 'System.Collections.Generic.List[string]'
foreach ($serviceName in @('bim-streaming-server', 'bim-streaming-conversion-service', 'governance-service')) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop kit-manager-api before moving the live tree

For staged replacement runs, this stop list omits kit-manager-api, but deploy.ps1 starts it from the deployment tree and records its pidfile under the same run dir (scripts/deploy.ps1 lines 1302-1316; host-native-launcher.ps1 lines 330-335). If a previous deploy left kit-manager-api running, the swap moves its pidfile away with $previousRoot, then the new deploy sees port 8010 occupied without a pidfile and can fail or prompt as a stranger process; include this service in the verified pre-cutover stop set.

Useful? React with 👍 / 👎.

Comment on lines +627 to +629
if (Test-Path -LiteralPath $stageRoot) {
Remove-Item -LiteralPath $stageRoot -Recurse -Force -ErrorAction SilentlyContinue
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Surface failed staged cleanup instead of hiding it

After the stage has restored preserved .env files, later validation or service-stop failures enter this cleanup path. If Remove-Item fails on Windows because of a transient lock/ACL/reparse issue, -ErrorAction SilentlyContinue leaves a sibling .rebuild-stage-* directory containing copied secrets without telling the operator; report or append cleanup failures so the failed rebuild does not silently leak an env-bearing stage tree.

Useful? React with 👍 / 👎.

}

$restoredEnvFiles = @(Restore-TestDeployEnvSnapshot -DeploymentPath $stageRoot -Snapshot $envSnapshot)
$removed = @(Remove-TestDeployAgentTooling -DeploymentPath $stageRoot -AllowNonFixedPathForTests:$AllowNonFixedPathForTests)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate the staged checkout for reparse points

This staged path only scans the existing live deployment before cloning, then restores env files and recursively removes tooling from $stageRoot without ever applying the same reparse-point guard to the cloned tree. If origin/main materializes a tracked symlink/junction or a reparse point under a tooling directory, the rebuild can either recurse through it during cleanup or activate a live deployment containing a reparse point; run the stage through the path-safety check before tooling removal/cutover.

Useful? React with 👍 / 👎.


$liveMovedToPrevious = $false
if (Test-Path -LiteralPath $deployRoot -PathType Container) {
[System.IO.Directory]::Move($deployRoot, $previousRoot)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Report the previous checkout after staged cutover

Once this rename succeeds, any later failure in runtime env setup or deploy.ps1 -Build leaves the old deployment only at the generated $previousRoot, but the result/wrapper prints no previous path or recovery command. In that post-cutover failure scenario operators get only the deploy exit code and have to guess which .rebuild-previous-* directory is the rollback source; include the previous path in the returned/logged metadata whenever the live tree is moved aside.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants