feat: restore terminal after stopped program #80

Activity

1. kbuffardi

   added

   enhancementNew feature or requestNew feature or request

   on Aug 28, 2026
2. kbuffardi commented on Aug 29, 2026

   

   kbuffardi

   on Aug 29, 2026

   OwnerAuthor

   More actions

   Implementation plan

   Planned by Codex in response to the request: “make a plan for issue #80”.

   Scope and diagnosis

   Stopping a CPU-bound WASM process deliberately terminates the compiler worker. The terminal immediately restores its prompt, while a replacement worker must reload Clang and rebind the toolbar. The recovery path spans terminal state, toolbar readiness, cached binary data, worker identity, and the replacement worker’s compiler-ready event. Existing tests cover pieces of this path, but not the complete stopped-run recovery flow.

   Tasks

   1. Lock the failure with lifecycle regression coverage (S; tests)

      • Extend terminal-stop and toolbar harnesses to model a running process, STOP, worker replacement, loading, and compiler-ready.
      • Assert direct g++ main.cpp, toolbar Compile, and Compile & Run all work after the same stopped run.
      • Assert delayed messages from the old worker cannot affect current UI state.
      • Likely files: scripts/e2e-terminal-stop.test.mjs, scripts/e2e-workspace-file-tracking.test.mjs.

   2. Make post-stop worker recovery explicit (S; 1–2 files)

      • Centralize replacement-worker setup in the app lifecycle: detach/terminate the old worker, create the replacement, bind it to the toolbar, and retain editor/workspace state plus cached compiled bytes.
      • Ensure terminal callbacks always dispatch to the current worker, STOP remains idempotent, and only the active replacement worker can restore readiness.
      • Likely files: src/ui/app.js, possibly src/ui/toolbar.js.

   3. Harden active-worker readiness state (S; 1 file)

      • Keep Compile and Compile & Run disabled during replacement loading, then enable them from that worker’s compiler-ready event when a valid C++ workspace is available.
      • Clear stale Compile & Run state from the killed worker while retaining a previously compiled artifact for a later Run where appropriate.
      • Likely file: src/ui/toolbar.js.

   4. Verify all issue acceptance paths (M; tests)

      • After STOP and replacement readiness, terminal g++ main.cpp must post a compile request to the replacement worker.
      • Compile must post to that worker and render compiler results; Compile & Run must compile then start the newly built artifact.
      • Cover repeated STOP, STOP while the replacement is loading, and stale old-worker stdout, stderr, run-result, run-start, and compile-result events.

   Checkpoint and validation

   Run targeted tests first, then:

   npm run lint
   npm run build
   npm run test:e2e

   Finally, manually load the unpacked extension, run a non-terminating program, press STOP, wait for compiler readiness, and exercise the three acceptance flows.

   Risks

   • Readiness race: the prompt appears before the compiler reload completes. Keep the prompt responsive but gate compile controls on the current worker’s explicit readiness event.
   • Stale events: detach the old handler and preserve the toolbar worker-identity guard.
   • Cancellation regression: retain worker termination for CPU-bound WASM; improve only recovery afterwards.

   Delivery

   Implement on fix/restore-after-stop; open a PR with Closes #80 for human review and merge.
3. kbuffardi commented on Aug 29, 2026

   

   kbuffardi

   on Aug 29, 2026

   OwnerAuthor

   More actions

   Plan review and required revisions

   Critical correction

   The plan must not assume worker replacement wiring is missing: app.js already terminates the old worker, creates a new one, and passes it to toolbar.setWorker(). The more concrete likely defect is a replacement-readiness race. After STOP, terminal.stopRun() immediately redraws a shell prompt. A user can issue g++ main.cpp while the replacement worker is loading; compiler.worker.js returns Compiler not ready. whenever ensureReady() observes compilerState === "loading". This directly conflicts with the issue requirement that the prompt be usable after STOP.

   Required plan amendments

   1. Decide and document the recovery UX before implementation.

      • Preferred: do not expose a usable prompt until the replacement worker has either emitted compiler-ready or emitted compiler-error; show an explicit recovery/loading line in the interim.
      • Alternative: retain the immediate prompt but queue compile/run requests until replacement readiness. This needs bounded, cancellable request ownership and is materially more complex.
      • Do not silently accept a failed g++ because the compiler is loading.

   2. Move readiness-race coverage ahead of lifecycle refactoring.

      • Add a deterministic worker-level test for a compile received during compilerState: loading; it must either wait for readiness or be prevented by the UI contract.
      • Add UI integration coverage for STOP → replacement still loading → attempted terminal command, followed by STOP → replacement ready → terminal g++, Compile, and Compile & Run.
      • Fake-worker toolbar tests alone cannot prove this condition because they do not exercise ensureReady().

   3. Narrow the implementation target.

      • Keep worker termination for CPU-bound WASM cancellation.
      • Coordinate terminal.js, app.js, and toolbar.js so the terminal recovery state and toolbar control state are both driven by the same active worker terminal event (compiler-ready or compiler-error).
      • Preserve the active-worker identity guards and reset recovery state on replacement error as well as success.

   4. Strengthen acceptance criteria.

      • While replacement loading, users receive an explicit recoverable loading state—not a prompt that accepts commands doomed to fail.
      • Once the prompt is shown, g++ main.cpp succeeds in dispatching to a ready compiler.
      • A replacement compiler error restores a clear terminal error/prompt state without leaving the UI permanently disabled.
      • STOP during replacement loading is safe and idempotent; stale old-worker messages and callbacks cannot restore controls or prompts.

   5. Add real-browser validation.

      • Automated UI/worker tests establish the state contract; a Chrome extension smoke/reproduction case must confirm actual Worker termination/recreation and Clang reload timing. Record the time-to-ready and any console errors.

   Revised sequencing

   1. Write failing readiness-race tests (worker-level plus UI lifecycle).
   2. Choose the prompt-gating or request-queue contract; prefer prompt-gating for the smallest safe change.
   3. Implement coordinated recovery state and error handling.
   4. Add post-ready regression cases for terminal g++, Compile, and Compile & Run.
   5. Run focused tests, lint, build, full E2E, and the browser smoke case.

   The original plan is otherwise sound on stale-message protection, idempotent STOP, and keeping the fix scoped. Its tasks should be revised to make this readiness contract the foundation rather than a secondary risk.
4. kbuffardi commented on Aug 29, 2026

   

   kbuffardi

   on Aug 29, 2026

   OwnerAuthor

   More actions

   Review addendum: confirmed lifecycle defect

   The plan review identified a concrete defect that changes the priority of the work: initToolbar() in src/ui/toolbar.js does not return a controller object, but src/ui/app.js assigns its return to toolbarController and later calls optional controller methods:

   • toolbarController?.setWorker(worker) after STOP
   • toolbarController?.getLastRunBinaryBytes() when running
   • toolbarController?.setRunPreparing(isPreparing) during terminal state changes

   Because the value is undefined, the post-STOP setWorker call is a no-op. The replacement worker starts loading, but no toolbar message handler is attached and its compiler-ready event never reenables Compile / Compile & Run. This exactly matches the reported behavior.

   Required first implementation task

   1. Correct the toolbar/app interface contract before any broader recovery refactor.
      • Either have initToolbar() return a narrow controller with setWorker, getLastRunBinaryBytes, and setRunPreparing, or import/use those exported functions directly from app.js.
      • Prefer the smallest API-consistent solution and document ownership of the worker lifecycle.
      • Add a regression test that proves the result of initToolbar() exposes every method used by app.js and that STOP binds Worker B, receives its compiler-ready, and reenables compile controls.

   Revised ordering

   1. Add the app/toolbar controller-contract regression test.
   2. Fix the missing controller binding and demonstrate Worker B receives the handler.
   3. Add STOP → Worker B ready → terminal g++, Compile, and Compile & Run coverage.
   4. Add the previously noted loading/error/stale-async cases as hardening.
   5. Run focused tests, lint, build, full E2E, and browser validation.

   The earlier readiness-race critique remains relevant for commands typed while Worker B loads, but it is secondary to the confirmed missing binding.
5. kbuffardi

   added this to the rc milestone on Aug 29, 2026
6. kbuffardi commented on Aug 29, 2026

   

   kbuffardi

   on Aug 29, 2026

   OwnerAuthor

   More actions

   Implementation update from Codex, responding to the user request: “implement plan and raise a pr”.

   The toolbar controller contract was fixed and covered by regression tests. Pull request #81 is open for human review: #81

   Validation completed: focused toolbar E2E, lint, full E2E, production build, and Chrome browser smoke.
7. kbuffardi

   closed this as completedin #81on Aug 29, 2026