Skip to content

fix: recover terminal launch assets throughout runtime lifetime - #415

Merged
DeadWaveWave merged 4 commits into
mainfrom
fix/terminal-agent-assets-lifecycle
Sep 21, 2026
Merged

DeadWaveWave merged 4 commits into
mainfrom
fix/terminal-agent-assets-lifecycle

Conversation

@DeadWaveWave

@DeadWaveWave DeadWaveWave commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner

💡 Change Scope

  • Small Change: Fast feedback, localized UI/logic, low-risk.
  • Large Change: New feature, cross-boundary logic, runtime-risk (persistence, IPC, lifecycle, recovery).

📝 What Does This PR Do?

Windows compatibility fix verified: a982704d fixes the Unicode-path script decoding failures. Native Windows CI is green: 49 E2E passed / 4 skipped / 0 failed, including all six formerly failing scenarios and both new Unicode-executable cases. The full local gate also passed. Draft status is retained while the remaining platform checks finish; the author explicitly excluded macOS animation remediation. Historical failures remain documented, with no assertions weakened or retries added.

Fix new terminals immediately exiting after a long-running runtime loses its generated shell launch assets. The old Store returned cached paths without checking that the launcher still existed. Missing files were observed; the process that removed them was not established.

  • Keep generated assets in a unique private instance beneath the configured profile, not OS temp.
  • Validate the generated manifest and restore missing/corrupt files at stable paths with staged rename, preserving existing shells' PATH shims.
  • Coalesce concurrent repair, drain creation/repair before disposal, and forbid late publication or resurrection.
  • Fail open to the original shell if optional instrumentation cannot be prepared; record bounded, redacted diagnostics.
  • Preserve history, active plans and other instances during repair; refuse unsafe links, ownership and directory replacements.
  • Emit Windows PowerShell scripts as UTF-8 with BOM; keep CMD source ASCII and resolve the colocated .ps1 through %~dpn0.ps1, without changing the user’s code page.

No node-pty upgrade/patch changes, native FD-leak fix, orphan-directory sweep or POSIX shell behavior changes. The Windows follow-up changes script representation/dispatch only, preserving invocation/cleanup ownership. The installed application was not modified or restarted.


🏗️ Large Change Spec (Required if "Large Change" is checked)

1. Context & Business Logic

Generated launch files must live at least as long as the runtime that publishes their paths. Apple filesystem guidance and Electron's application-specific userData directory support using an application-owned location instead of assuming temporary files remain available. Preflight repair protects subsequent launches without replaying a possibly executed user command. Windows replacement can fail under a file lock; fallback retains the original command rather than unlinking the destination.

The Spec and implementation Plan were explicitly approved after an isolated macOS feasibility check. The Windows-only follow-up scope/plan was also approved. Microsoft documents UTF-8 BOM for non-ASCII Windows PowerShell source and batch path parameter expansion; encoding policy lives in the existing script generator, not a second filesystem writer.

2. State Ownership & Invariants

TerminalAgentTelemetryAssetStore remains the sole lifecycle owner, delegating filesystem effects/identity checks to its private helper. Composition injects profile location and the existing bounded diagnostic sink; gateway credential ownership and PTY admission remain unchanged.

  1. Successful preparation validates the manifest at the same immutable instance paths; healthy files, history/plans and siblings are not rewritten/swept.
  2. Disposal seals admission synchronously, drains in-flight work and rejects late results before removing only the verified owned instance.
  3. Instrumentation and diagnostics cannot acquire shell-launch authority: preparation failure returns unchanged command/arguments/environment and releases any reservation.

A replacement runtime allocates a new generation; it never adopts old credentials/roots. Crash leftovers can contain private history/plans and are intentionally not age/PID-deleted. Preflight does not eliminate check/open races under continued external mutation.

3. Verification Plan & Regression Layer

  • Filesystem/lifecycle regression first: partial/full deletion, content and permissions, immutable paths, 20 concurrent callers, disposal during creation/paused repair, failed rename preserving the destination and cleaning staging, retry, symlink/hardlink/directory rejection, profile alias and sibling isolation.
  • Real bash/zsh integration: a previously running shell reuses its original shim after partial and full root deletion/repair; fresh shells execute successfully. Tests use a fake provider, not real Agent accounts.
  • macOS Electron/Playwright: actual terminal input before deletion, complete instance deletion, UI creation of a new terminal, exact-path recovery, actual output (not echoed command text), and no page errors.
  • Windows interactive Electron/PowerShell test now covers full-root repair and reusing the original shell PATH. Windows/Linux runtime results must come from their CI runners; macOS skips are not counted as platform passes.
  • Full local OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit, with required E2E retries=0 and crash fallback disabled; architecture check separately. First full run: 330 passed / 89 skipped / 1 failed. The failure is terminal-output-stability.spec.ts fit proposal equality (83x21 vs Worker/renderer 83x19). The original asset-repair E2E passed. Standalone baseline/current and preceding-test pair checks passed; causality remains unknown. After opening this PR at the author’s request, the same geometry case passed 10/10 fixed repetitions and a new complete local gate passed: 331 E2E passed / 89 skipped / 0 failed, exit 0. This suggests intermittent behavior, not a diagnosed or fixed root cause. No assertion, timeout, skip, or retry policy in the failing test was changed.

Windows follow-up — source a982704d, documentation/head 091748c7:

  • Red: 7 encoding/byte regressions failed on the original source. Green: 5 targeted files / 30 tests passed, including BOM repair and healthy-file stability.
  • Full nonempty staged gate (6 files): OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit exit 0; related tests 82 passed / 3 skipped, Electron-native recovery 8 passed, full local E2E 331 passed / 91 skipped / 0 failed. Required retries/fallback stayed disabled. Existing Node-vs-Electron SQLite ABI warnings remain in the raw unit log; the dedicated native recovery gate passed.
  • Added two native Windows E2Es using copied Node executables under Unicode/space/apostrophe/emoji paths, plus exact Unicode args and environment/plan cleanup. Strengthened the existing interactive Electron same-path-repair fixture with a Unicode root. Mac skips do not establish Windows execution.
  • Native Windows CI passed: 49 E2E passed / 4 skipped / 0 failed. This executed merge 3daf72c (head 091748c7 into main 134b4989), not a macOS mock. All original six failures, both new CMD/PowerShell Unicode-runtime cases, GUI Electron same-path repair, Ctrl-C and cleanup passed. No macOS animation code or tests changed, per the author’s scope decision.
  • Separate changelog commit 091748c7 records Windows compatibility and relocates only this PR’s entries to avoid an additive CHANGELOG conflict with main. Merge preview is clean; no unrelated product changes or history rewrite.

Historical evidence for 959696bf (retained):

  • Full OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit passed in a disposable worktree with all 19 PR files staged; staged/source tree 5cd132ac3c297ed624244c8979e56bc46ec85834 matched the committed PR tree before and after verification. This avoids claiming an empty post-commit staged gate passed.
  • Required E2E had retries=0 and crash fallback disabled; original failure and rerun results are both retained.
  • GitHub Ubuntu unit/static, all three Ubuntu E2E shards, Web continuity, and macOS shards 2/3 passed.
  • Windows CI: 41 passed / 4 skipped / 6 failed. The new same-path repair test passed, but existing Unicode-path shim cases show 路径 being interpreted as 路径, leading to MODULE_NOT_FOUND or a missing .ps1 path. This was a separate compatibility blocker at that revision, not the geometry flake; it is resolved by the verified Windows follow-up above.
  • macOS shard 1: 99 passed / 41 skipped / 1 failed. app-header.primary-sidebar-hover-animation.spec.ts:241 observed 2 distinct width samples, requiring >3. No animation code or assertions were changed in this PR; cause remains unverified.

✅ Delivery & Compliance Checklist

  • My code passes the ultimate gatekeeper: pnpm pre-commit is completely green. Latest local run and native Windows CI passed; other platform jobs may still be running, and historical failures remain recorded.
  • I have signed the CLA if required (see CLA.md). GitHub license/cla passed.
  • I have included new tests to lock down the behavior (or explicitly stated why it's untestable).
  • I have strictly adhered to the DEVELOPMENT.md architectural boundaries.
  • I have attached a screenshot or screen recording (if this touches the UI). No layout/style changes; the runtime E2E attaches terminal-after-asset-repair to its Playwright report.
  • I have updated the documentation accordingly (if adding a feature or changing a contract).

📸 Screenshots / Visual Evidence

No visual design changes. workspace-canvas.terminal-agent-assets.mac.spec.ts captures the usable terminal after repair in its Playwright report; review-only media is not committed. Acceptance uses actual shell output plus filesystem/diagnostic checks, not screenshot appearance alone.

@DeadWaveWave

Copy link
Copy Markdown
Owner Author

Reproduction update (requested PR-first workflow)

  • Code committed as cb1cc7f7; changelog-only follow-up 959696bf references this PR. Main working tree is clean.
  • Original complete gate remains recorded: 330 passed / 89 skipped / 1 failed, fit proposal 83x21 versus Worker/renderer 83x19 in terminal-output-stability.spec.ts.
  • On unchanged code and assertions, a fresh build followed by 10 fixed repetitions of that test passed 10/10, with OPENCOVE_E2E_RETRIES=0 and crash fallback disabled. This is bounded reproduction sampling, not “retry until green”; it does not establish the cause or erase the initial failure.
  • A new complete OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit run is in progress. To avoid empty staged gates after committing, it runs in an isolated worktree with the full PR diff staged (19 files). Its staged tree and PR tree both equal 5cd132ac3c297ed624244c8979e56bc46ec85834.
  • No production geometry changes, assertion relaxation, skipped test additions, or retry policy changes were made. PR remains draft while validation runs.

@DeadWaveWave

Copy link
Copy Markdown
Owner Author

Separate Windows CI blocker (not the macOS geometry flake)

Windows job: 41 passed / 4 skipped / 6 failed.

  • The newly extended interactive Electron shim test (delete the instance root, repair, reuse the existing PowerShell PATH) passed.
  • Six existing terminal-agent-shim.windows.spec.ts scenarios failed after placing assets under the fixture's Unicode profile path.
  • The diagnostic attachment successfully reads the generated scripts at their intended paths, but PowerShell invokes a launcher path containing 路径 instead of 路径, producing MODULE_NOT_FOUND. CMD-based cases report that the .ps1 path does not exist.
  • This is concrete evidence of a Windows script/path encoding compatibility problem exposed by the profile-based location, not evidence that the files disappeared again, nor justification to call all CI failures flaky.

No Windows assertion/fixture weakening or encoding patch has been applied during this requested rerun. Keep the PR draft until this compatibility blocker is corrected and verified on Windows. The unchanged-source macOS full gate rerun is still in progress; the original geometry case separately passed 10/10 fixed repetitions.

@DeadWaveWave

Copy link
Copy Markdown
Owner Author

Requested rerun completed — do not conflate the failures

Same product code (959696bf), no assertion changes:

Check Result
Original complete local E2E 330 passed, 89 skipped, 1 geometry failure
Fixed 10-repetition geometry reproduction batch 10 passed, 0 failed
New complete OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit exit 0; 331 E2E passed, 89 skipped, 0 failed

The original geometry test passed both the bounded repetition batch and the new full suite. This supports an intermittent/flaky symptom, but does not establish its root cause or turn the original failed run into a pass. Required retry/fallback policy remained unchanged.

The complete gate ran with 19 actually staged files in a disposable worktree. Its tree hash matched the PR commit before and after: 5cd132ac3c297ed624244c8979e56bc46ec85834.

Draft remains appropriate: Windows has 6 Unicode-path/encoding failures (concrete garbled launcher path evidence); macOS shard 1 separately failed sidebar animation sampling (2 distinct widths versus >3 required). Ubuntu checks/E2E/Web and macOS shards 2/3 passed. Neither CI blocker was patched, skipped, or retried into green in this rerun task.

@DeadWaveWave

Copy link
Copy Markdown
Owner Author

Windows-only CI repair completed

Per the author’s narrowed scope, no macOS animation code or tests were changed.

  • a982704d: emit UTF-8 BOM for Windows PowerShell scripts; keep CMD source ASCII and use quoted %~dpn0.ps1 runtime path expansion. No global code-page change, PTY retry, argument/exit rewrite or lifecycle-owner change.
  • 091748c7: separate changelog update. Moving only this PR’s entries resolved the additive CHANGELOG conflict; the PR is mergeable without importing unrelated source commits.
  • Regression: 7 expected failures before the fix; afterward 5 targeted files / 30 tests passed, including missing-BOM repair and no rewrite of healthy files.
  • Complete local nonempty staged OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit: exit 0; 331 E2E passed / 91 skipped / 0 failed. Native Windows cases were not counted as passed locally.
  • Actual Windows CI: 49 E2E passed / 4 skipped / 0 failed. All six original failures now pass, as do two new real Unicode runtime/profile/argument-path cases and the strengthened interactive Electron deletion/repair case. Ctrl-C, shell reuse, exact arguments, provider exit codes and plan cleanup remain covered.

The Windows job tested merge 3daf72c (PR head 091748c7 into main 134b4989). No automatic retries or assertion weakening. Working tree clean, commits pushed, installed app untouched. Draft retained; remaining platform jobs are not silently claimed green, and earlier failure evidence is preserved in this PR.

@DeadWaveWave
DeadWaveWave marked this pull request as ready for review September 21, 2026 11:58
@DeadWaveWave
DeadWaveWave merged commit adf27bf into main Sep 21, 2026
14 checks passed
@DeadWaveWave
DeadWaveWave deleted the fix/terminal-agent-assets-lifecycle branch September 21, 2026 11:58
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.

1 participant