Skip to content

Fix shellv2 history loading and ordering#1924

Merged
KCarretto merged 2 commits intomainfrom
fix-shellv2-history-10825082860140443234
Feb 27, 2026
Merged

Fix shellv2 history loading and ordering#1924
KCarretto merged 2 commits intomainfrom
fix-shellv2-history-10825082860140443234

Conversation

@KCarretto
Copy link
Copy Markdown
Collaborator

This PR fixes an issue where shell history was not displayed or was delayed when connecting to a ShellV2 session, particularly in interactive mode.

Changes:

  1. Immediate Polling: The pollForShellTasks function is now called once before the main event loop in writeMessagesFromShell. This ensures that any existing shell tasks (history) are fetched and sent to the client immediately upon connection, rather than waiting for the first ticker event (which could be delayed or skipped in interactive mode).
  2. Ordered History: The database query for shell tasks now explicitly orders results by created_at in ascending order. This guarantees that the command history is displayed in the correct sequence.
  3. Regression Test: A new integration test TestShellHistory_Interactive has been added to tavern/internal/http/shell/integration_test.go. This test verifies that a client connecting to a shell with existing history receives that history immediately, even if the shell is in interactive mode (active portal).

Verification:

  • Ran go test -v tavern/internal/http/shell/integration_test.go and verified that the new test case passes, along with existing tests.

PR created automatically by Jules for task 10825082860140443234 started by @KCarretto

- Modify `tavern/internal/http/shell/handler.go` to invoke `pollForShellTasks` immediately upon WebSocket connection, ensuring history is sent without waiting for the first polling tick.
- Add `.Order(shelltask.ByCreatedAt(sql.OrderAsc()))` to the shell task query to ensure history is replayed in the correct chronological order.
- Add `TestShellHistory_Interactive` integration test to verify that shell history is received immediately even when an active portal is present.

Co-authored-by: KCarretto <16250309+KCarretto@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@github-actions
Copy link
Copy Markdown
Contributor

Summary

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Other ❓ Flaky 🍂 Duration ⏱️
2691    ±0 2691    ±0 0    ±0 0    ±0 0    ±0 0    ±0 1ms    ±0

Previous Results

Build 🏗️ Result 🧪 Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Other ❓ Flaky 🍂 Duration ⏱️
#1317 2691 2691 0 0 0 0 43.9s

Insights

Average Tests per Run Total Flaky Tests Total Failed Slowest Test (p95)
2691 0 0 5.5s

Slowest Tests

Test 📝 Results 📊 Duration (avg) ⏱️ Duration (p95) ⏱️
TestDockerExecutor_Build_ContextCancellation 1 5.5s 5.5s
eldritch-libsys: std::dll_inject_impl::tests::test_dll_inject_simple 1 5.2s 5.2s
TestInteractiveShell 1 5.1s 5.1s
TestOtherStreamOutput 1 5.0s 5.0s
imix::bin/imix: install::tests::test_install_execution 3 1.6s 4.8s
imix::bin/imix: install::tests::test_install_execution 3 1.6s 4.8s
imix::bin/imix: install::tests::test_install_execution 3 1.6s 4.8s
imix::bin/imix: tests::task_tests::test_task_streaming_output 3 3.0s 3.0s
imix::bin/imix: tests::task_tests::test_task_streaming_output 3 3.0s 3.0s
imix::bin/imix: tests::task_tests::test_task_streaming_output 3 3.0s 3.0s

🎉 No failed tests in this run. | 🍂 No flaky tests in this run.

Github Test Reporter by CTRF 💚

@KCarretto KCarretto marked this pull request as ready for review February 27, 2026 00:33
@KCarretto KCarretto merged commit 0adc9df into main Feb 27, 2026
8 checks passed
@KCarretto KCarretto deleted the fix-shellv2-history-10825082860140443234 branch February 27, 2026 00:34
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