Repository navigation
fix(core): retain shell output after undo - #56691
GoldArowana wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
The fix works. I ran the #56667 reproduction from source with the simulated model. On the base, a ! shell command after /undo renders nothing and the "1 message reverted" banner stays. With this change, the command and its output render and the staged revert is committed. The core shell/revert tests (17), the new TUI test and the core typecheck all pass.
The three-line change in Session.shell looks right. It commits after a successful start, the same way prompt commits after preparation. Re-reading the session keeps a redo made while startup was waiting. If the shell fails to start, the revert stays staged and the error message still goes to the inbox.
Public API: unchanged. The shell path now also emits the existing session.revert.committed event.
The main thing to fix is test size. The fix is three lines, but the tests add about 500 lines: most of session-shell.test.ts is re-indented, and there is a 138-line TUI test with a fake HTTP server. Please cut them down to a small core test (details inline).
| }), | ||
| ) | ||
| } | ||
| for (const staged of [false, true]) { |
There was a problem hiding this comment.
Wrapping the existing killed/timeout/spawn-failure/interrupt tests in a staged loop re-indents about 100 unchanged lines and doubles those cases. Most of the new assertions don't depend on the undo state. Please restore the original tests and keep one focused test (shell after a staged undo commits it and its output is visible), plus the failed-start case if you want to lock in that the revert stays staged. The delayed-start and concurrent-shell tests that use the plugin hook gate can probably be dropped as well, to keep this minimal.
| ]), | ||
| ) | ||
|
|
||
| it.live("keyboard shell submission after undo renders real core completion without a chat prompt", () => |
There was a problem hiding this comment.
This test hand-builds an HTTP router in fetch, relays core events into the TUI fixture and waits on frames. That's a lot of setup for a fix that lives entirely in packages/core, and the description says it timed out once with no confirmed cause. The core test already covers the behaviour, so I'd drop this file.
Issue for this PR
Closes #56667
Type of change
What does this PR do?
After undo, shell commands run but their output stays hidden behind the staged revert. Commit the current revert after successful shell startup, before recording the new shell history. Reloading session state at that point also preserves redo performed while startup waits. Failed startup keeps undo/redo available.
Three production lines, with focused core and keyboard regressions. Implemented with AI assistance and independently reviewed by an automated source/log reviewer.
How did you verify your code works?
Linux, Bun 1.4.2, frozen dependencies:
bun run test ./test/session-shell.test.ts ./test/session-revert.test.ts --rerun-each 10: 17 cases × 10 runs, all passing. Unchanged production fails 6 of those 17 cases.bun run ../core/script/test.ts ./test/session-shell-undo.test.tsx --rerun-each 10: 1 keyboard-to-core case × 10 runs, passing; unchanged production fails the output-visibility assertion.bun run check: 34/34 tasks pass with a task-local Astro config path forwarded through Turbo's strict environment filter. The default Astro config-path failure also reproduces on clean base; tracked config and lockfile are unchanged.An earlier keyboard repeat timed out. After tightening input-readiness synchronization, two 10-run repeats passed without extending timeouts; the original timeout cause remains unconfirmed.
The keyboard test uses real TUI/client and Session services through a local fixture HTTP adapter. No Windows, live-provider, production HTTP-handler, or full-workspace runtime test coverage is claimed.
Screenshots / recordings
Core behavior change; the keyboard regression verifies post-undo output becomes visible without a chat turn.
Checklist