From 780d1388fff103ff0db0d8c14de65af6225b4860 Mon Sep 17 00:00:00 2001 From: "grokkybara[bot]" <304785771+grokkybara[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 08:17:57 +0000 Subject: [PATCH] Synced from monorepo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Synced from monorepo Changes: - grok-shell: send an expired external-provider credential to the sign-in flow, not a 401 loop - pager: clickable ▲ jumps to the top of the response being read - grok-shell: keep a large task log from making the completion message too long - Plan viewer scrollbar: widen grab zone to the border column; fix striped thumb in Terminal.app - pager: poll the tmux probe teardown grace instead of sleeping it - security: vendor-compat MCP kill switch is now actually enforced when reported as on - grok-shell: restore session eviction when a leader client disconnects - Bump rust-toolchain to 1.93.0 - workspace: lexical-normalize permission path patterns before glob matching - pager: reject garbage Enter in the /resume picker - pager: show Mermaid affordances in plan mode preview - pager: drop manage-account link from /session-info - workspace: auto-approve read-only git queries; defer write floor to auto classifier - Add free-form pattern editor to the "Always allow" command prompt - grok-shell: fix /btw caching - pager: Tab walks answers in the ask_user_question card - External-provider auth refresh: single 7s attempt instead of 3×5s - pager: don't resurrect finished background tasks as Running when completion arrives first - pager: report tmux truecolor clamping in Doctor - Fix plan viewer scrollbar click+drag hijacked by comment gutter - pager/shell: stop double Recap after the same last turn - sampler: preserve x-should-retry through stream collection - pager: clear plan-mode indicator immediately when the user approves a plan - pager: tmux does not re-read its config on reattach Source-Revision: 64c4de99cc822b25ce9c54ab5a4f372093d0885d --- Cargo.lock | 130 +- Cargo.toml | 3 + SOURCE_REV | 2 +- crates/codegen/ptyctl/src/styled.rs | 4 +- crates/codegen/ptyctl/src/term.rs | 4 +- crates/codegen/xai-grok-pager-bin/Cargo.toml | 2 +- .../xai-grok-pager-minimal/src/overlay.rs | 1 + .../src/appearance/config.rs | 20 +- .../src/render/scrollbar.rs | 130 +- .../src/terminal/tmux_probe.rs | 114 +- crates/codegen/xai-grok-pager/Cargo.toml | 2 +- .../docs/user-guide/02-authentication.md | 9 + .../docs/user-guide/03-keyboard-shortcuts.md | 22 + .../docs/user-guide/05-configuration.md | 2 +- .../docs/user-guide/06-theming.md | 2 +- .../docs/user-guide/17-sessions.md | 2 +- .../docs/user-guide/21-terminal-support.md | 13 +- .../user-guide/22-permissions-and-safety.md | 8 +- .../docs/user-guide/24-monitoring-usage.md | 25 + .../src/app/acp_handler/background.rs | 108 +- .../xai-grok-pager/src/app/acp_handler/mod.rs | 5 +- .../src/app/acp_handler/permissions.rs | 31 + .../app/acp_handler/session_notification.rs | 6 + .../app/acp_handler/tests/background_tasks.rs | 215 ++ .../src/app/acp_handler/tests/mod.rs | 1 + .../src/app/acp_handler/tests/permissions.rs | 34 + .../codegen/xai-grok-pager/src/app/agent.rs | 124 + .../src/app/agent_view/input.rs | 28 +- .../src/app/agent_view/interactions.rs | 399 ++- .../xai-grok-pager/src/app/agent_view/mod.rs | 7 + .../src/app/agent_view/panes.rs | 2 +- .../xai-grok-pager/src/app/agent_view/plan.rs | 139 +- .../src/app/agent_view/render.rs | 228 +- .../src/app/agent_view/session.rs | 2 + .../src/app/agent_view/viewer.rs | 87 +- .../src/app/agent_view/viewer_tests.rs | 460 +++ .../xai-grok-pager/src/app/app_view.rs | 7 +- .../src/app/dispatch/dashboard.rs | 39 +- .../src/app/dispatch/permissions.rs | 145 +- .../src/app/dispatch/tests/permissions.rs | 211 ++ .../xai-grok-pager/src/app/effects/mod.rs | 8 +- .../xai-grok-pager/src/app/effects/tests.rs | 25 +- .../codegen/xai-grok-pager/src/app/modals.rs | 48 +- .../codegen/xai-grok-pager/src/app/mouse.rs | 10 + .../src/diagnostics/doctor_format_tests.rs | 11 +- .../xai-grok-pager/src/diagnostics/fix.rs | 133 +- .../src/diagnostics/fix_tests.rs | 71 +- .../xai-grok-pager/src/diagnostics/mod.rs | 151 +- .../xai-grok-pager/src/diagnostics/model.rs | 18 + .../src/diagnostics/probes/mod.rs | 87 +- .../src/diagnostics/probes/tmux.rs | 8 + .../xai-grok-pager/src/diagnostics/view.rs | 46 +- .../src/diagnostics/view_tests.rs | 61 +- .../xai-grok-pager/src/doctor_cmd/tests.rs | 4 + .../xai-grok-pager/src/memory_trace.rs | 134 +- .../src/scrollback/scrollback_pane.rs | 8 +- .../src/scrollback/selection.rs | 13 +- .../src/scrollback/state/nav.rs | 107 + .../xai-grok-pager/src/scrollback/sticky.rs | 18 +- .../src/views/file_search/line_viewer.rs | 307 +- .../src/views/list_pane/state/methods.rs | 26 +- .../src/views/list_pane/state/mod.rs | 38 + .../src/views/permission_view.rs | 327 +- .../xai-grok-pager/src/views/question_view.rs | 120 +- .../src/views/session_picker.rs | 23 + .../xai-grok-pager/tests/pty_e2e/common.rs | 28 + .../pty_e2e/plan_scrollbar_grab_zone_pty.rs | 164 + .../pty_e2e/question_tab_cycles_answers.rs | 236 ++ .../pty_e2e/response_top_indicator_pty.rs | 133 + .../tests/pty_e2e_scroll_selection.rs | 4 + .../xai-grok-pager/tests/pty_e2e_smoke.rs | 2 + .../src/actor/request_task.rs | 12 +- crates/codegen/xai-grok-sampler/src/client.rs | 66 +- crates/codegen/xai-grok-sampler/src/events.rs | 26 + .../xai-grok-sampler/src/stream/collect.rs | 1 + .../src/conversation.rs | 45 + .../xai-grok-sampling-types/src/types.rs | 12 + crates/codegen/xai-grok-shell/CHANGELOG.md | 25 + crates/codegen/xai-grok-shell/Cargo.toml | 3 +- crates/codegen/xai-grok-shell/README.md | 11 +- .../xai-grok-shell/changelogs/0.2.118.json | 62 + .../xai-grok-shell/changelogs/0.2.118.md | 20 + .../xai-grok-shell/src/active_sessions.rs | 5 - .../codegen/xai-grok-shell/src/agent/app.rs | 152 +- .../xai-grok-shell/src/agent/auth_method.rs | 30 +- .../xai-grok-shell/src/agent/chat_modes.rs | 6 +- .../xai-grok-shell/src/agent/config.rs | 299 +- .../src/agent/feedback_client.rs | 18 +- .../xai-grok-shell/src/agent/folder_trust.rs | 18 +- .../src/agent/handlers/session.rs | 2 +- .../codegen/xai-grok-shell/src/agent/init.rs | 5 +- .../codegen/xai-grok-shell/src/agent/mod.rs | 2 +- .../xai-grok-shell/src/agent/models.rs | 43 +- .../src/agent/models/resolution.rs | 2 +- .../src/agent/mvp_agent/acp_agent.rs | 113 +- .../src/agent/mvp_agent/agent_ops.rs | 73 +- .../src/agent/mvp_agent/code_nav.rs | 15 +- .../src/agent/mvp_agent/heap_profile.rs | 7 + .../xai-grok-shell/src/agent/mvp_agent/mod.rs | 164 +- .../src/agent/mvp_agent/replay.rs | 186 + .../src/agent/mvp_agent/replay_tests.rs | 230 ++ .../src/agent/mvp_agent/resource_telemetry.rs | 152 + .../src/agent/mvp_agent/session_lifecycle.rs | 1 + .../src/agent/mvp_agent/tests.rs | 18 +- .../xai-grok-shell/src/agent/otel_gate.rs | 184 +- .../codegen/xai-grok-shell/src/agent/proxy.rs | 4 +- .../codegen/xai-grok-shell/src/agent/relay.rs | 4 +- .../xai-grok-shell/src/agent/server.rs | 5 +- .../src/agent/session_config.rs | 4 +- .../src/agent/session_registry_client.rs | 2 +- .../xai-grok-shell/src/agent/subagent/mod.rs | 16 +- .../xai-grok-shell/src/auth/attribution.rs | 6 +- .../codegen/xai-grok-shell/src/auth/config.rs | 14 +- .../src/auth/credential_provider.rs | 14 +- .../src/auth/devbox_login_stub.rs | 4 +- .../xai-grok-shell/src/auth/device_code.rs | 12 +- .../codegen/xai-grok-shell/src/auth/error.rs | 43 +- .../xai-grok-shell/src/auth/external_auth.rs | 3 +- .../codegen/xai-grok-shell/src/auth/flow.rs | 8 +- .../xai-grok-shell/src/auth/manager.rs | 50 +- .../xai-grok-shell/src/auth/manager/remedy.rs | 322 ++ .../xai-grok-shell/src/auth/manager_tests.rs | 116 + crates/codegen/xai-grok-shell/src/auth/mod.rs | 3 +- .../codegen/xai-grok-shell/src/auth/model.rs | 6 +- .../xai-grok-shell/src/auth/oidc/protocol.rs | 2 +- .../xai-grok-shell/src/auth/recovery.rs | 8 +- .../src/auth/refresh/external_refresher.rs | 68 +- .../xai-grok-shell/src/bin/trace_classify.rs | 224 -- crates/codegen/xai-grok-shell/src/bundle.rs | 23 +- .../xai-grok-shell/src/claude_import.rs | 8 +- .../xai-grok-shell/src/claude_import_state.rs | 65 +- .../codegen/xai-grok-shell/src/config/mod.rs | 52 +- .../xai-grok-shell/src/config/reloader.rs | 2 +- .../xai-grok-shell/src/config/watcher.rs | 6 +- .../xai-grok-shell/src/extensions/bundle.rs | 2 +- .../xai-grok-shell/src/extensions/code_nav.rs | 10 +- .../xai-grok-shell/src/extensions/fs.rs | 10 +- .../xai-grok-shell/src/extensions/git.rs | 28 +- .../xai-grok-shell/src/extensions/hooks.rs | 4 +- .../src/extensions/hunk_tracker.rs | 20 +- .../xai-grok-shell/src/extensions/jj.rs | 2 +- .../src/extensions/marketplace.rs | 4 +- .../xai-grok-shell/src/extensions/mcp.rs | 19 +- .../xai-grok-shell/src/extensions/mod.rs | 14 +- .../src/extensions/notification.rs | 10 +- .../xai-grok-shell/src/extensions/plugins.rs | 2 +- .../xai-grok-shell/src/extensions/pr.rs | 6 +- .../xai-grok-shell/src/extensions/repair.rs | 2 +- .../xai-grok-shell/src/extensions/routing.rs | 5 +- .../xai-grok-shell/src/extensions/search.rs | 10 +- .../src/extensions/session_admin.rs | 53 +- .../src/extensions/session_search.rs | 4 +- .../src/extensions/session_state.rs | 4 +- .../xai-grok-shell/src/extensions/skills.rs | 2 +- .../xai-grok-shell/src/extensions/task.rs | 4 +- .../xai-grok-shell/src/extensions/terminal.rs | 16 +- .../xai-grok-shell/src/extensions/worktree.rs | 8 +- .../src/heap_profile/monitor.rs | 18 +- .../codegen/xai-grok-shell/src/inspect/mod.rs | 26 +- .../xai-grok-shell/src/leader/client.rs | 2 +- .../codegen/xai-grok-shell/src/leader/lock.rs | 21 +- .../codegen/xai-grok-shell/src/leader/mod.rs | 2 +- .../xai-grok-shell/src/leader/protocol.rs | 90 +- .../xai-grok-shell/src/leader/server.rs | 23 +- crates/codegen/xai-grok-shell/src/lib.rs | 1 - crates/codegen/xai-grok-shell/src/plugin.rs | 10 +- .../codegen/xai-grok-shell/src/relay/sync.rs | 8 +- .../xai-grok-shell/src/remote/agent.rs | 112 +- .../src/remote/chat_models_client.rs | 5 +- .../xai-grok-shell/src/remote/client.rs | 44 +- .../src/remote/conversations_client.rs | 5 +- .../codegen/xai-grok-shell/src/remote/mod.rs | 4 +- .../src/remote/skills_client.rs | 4 +- .../codegen/xai-grok-shell/src/remote/sync.rs | 2 +- .../src/remote/workspaces_client.rs | 2 +- .../xai-grok-shell/src/sampling/error.rs | 11 +- .../src/session/acp_conversion.rs | 12 +- .../xai-grok-shell/src/session/acp_mcp.rs | 4 +- .../xai-grok-shell/src/session/acp_session.rs | 1 + .../session/acp_session_impl/hook_dispatch.rs | 1 + .../acp_session_impl/laziness_classifier.rs | 20 +- .../src/session/acp_session_impl/recap.rs | 242 +- .../session/acp_session_impl/sampler_turn.rs | 114 +- .../src/session/acp_session_impl/spawn.rs | 5 +- .../src/session/acp_session_impl/stop_gate.rs | 1 + .../session/acp_session_impl/tool_calls.rs | 18 +- .../src/session/acp_session_impl/turn.rs | 4 +- .../auth_error_no_retry_tests.rs | 3 + .../auto_wake_suppression_tests.rs | 2 + .../inline_auto_compact_flow_tests.rs | 2 + .../recap_display_only_tests.rs | 398 +++ .../replay_buffer_send_update_tests.rs | 2 + .../src/session/acp_session_tests/support.rs | 14 - .../turn/auth_retry_budget_tests.rs | 62 +- .../xai-grok-shell/src/session/acp_types.rs | 34 +- .../src/session/agent_rebuild.rs | 4 +- .../src/session/announcement_state.rs | 4 +- .../src/session/chat_persistence.rs | 2 +- .../xai-grok-shell/src/session/commands.rs | 2 +- .../xai-grok-shell/src/session/compaction.rs | 2 + .../src/session/compaction_config.rs | 22 +- .../xai-grok-shell/src/session/export.rs | 8 +- .../xai-grok-shell/src/session/feedback.rs | 70 +- .../src/session/feedback_manager.rs | 18 +- .../xai-grok-shell/src/session/file_system.rs | 6 +- .../src/session/goal_classifier.rs | 8 - .../src/session/goal_tracker.rs | 43 +- .../xai-grok-shell/src/session/handle.rs | 52 +- .../src/session/helpers/chat.rs | 18 - .../src/session/helpers/memory_flush.rs | 33 - .../src/session/helpers/session_recap.rs | 72 +- .../src/session/image_describe.rs | 40 +- .../src/session/image_normalize.rs | 18 +- .../xai-grok-shell/src/session/managed_mcp.rs | 258 +- .../src/session/mcp_dispatcher.rs | 34 +- .../xai-grok-shell/src/session/mcp_restart.rs | 12 +- .../xai-grok-shell/src/session/mcp_servers.rs | 8 +- .../src/session/memory_state.rs | 18 +- .../codegen/xai-grok-shell/src/session/mod.rs | 18 +- .../src/session/normalize_cache.rs | 12 +- .../src/session/notifications.rs | 2 +- .../src/session/pending_interaction.rs | 4 +- .../xai-grok-shell/src/session/persistence.rs | 26 +- .../xai-grok-shell/src/session/plan_mode.rs | 52 +- .../src/session/prompt_history.rs | 22 +- .../src/session/prompt_parser.rs | 9 +- .../src/session/restore_stub.rs | 14 +- .../xai-grok-shell/src/session/result.rs | 2 +- .../xai-grok-shell/src/session/signals.rs | 51 +- .../src/session/slash_commands.rs | 2 +- .../xai-grok-shell/src/session/storage/mod.rs | 18 +- .../src/session/storage/search_remote_sync.rs | 552 +-- .../xai-grok-shell/src/session/telemetry.rs | 2 +- .../xai-grok-shell/src/session/tool_index.rs | 12 +- .../src/session/unified_list/facets.rs | 10 +- .../src/session/unified_list/mod.rs | 18 +- .../src/session/unified_list/row.rs | 4 +- .../src/session/user_message.rs | 10 +- .../xai-grok-shell/src/session/wire_tags.rs | 1 + .../src/session/workflow/tracker.rs | 26 +- .../xai-grok-shell/src/session/worktree.rs | 6 +- .../xai-grok-shell/src/terminal/adapter.rs | 1 + .../src/terminal/background_task.rs | 9 - .../xai-grok-shell/src/terminal/mod.rs | 10 +- .../src/terminal/pty_session.rs | 24 +- .../src/terminal/streaming_local_terminal.rs | 2 +- .../xai-grok-shell/src/tools/config.rs | 11 +- .../codegen/xai-grok-shell/src/tools/mod.rs | 1 + .../src/tools/notification_bridge.rs | 73 +- .../src/tools/task_completed_frame.rs | 272 ++ .../src/tools/task_completed_frame_tests.rs | 228 ++ .../codegen/xai-grok-shell/src/tools/todo.rs | 2 +- .../xai-grok-shell/src/tools/tool_context.rs | 21 +- .../src/trace_classifier/mod.rs | 3063 ----------------- .../src/util/config/announcements.rs | 27 +- .../src/util/config/campaigns.rs | 8 +- .../xai-grok-shell/src/util/config/hints.rs | 22 - .../xai-grok-shell/src/util/config/load.rs | 83 +- .../xai-grok-shell/src/util/config/mcp.rs | 176 +- .../src/util/config/permissions.rs | 27 +- .../xai-grok-shell/src/util/config/persist.rs | 4 - .../src/util/config/resolve/auto_mode.rs | 8 +- .../src/util/config/resolve/compaction.rs | 8 +- .../src/util/config/resolve/crash_handler.rs | 2 +- .../util/config/resolve/display_refresh.rs | 6 +- .../src/util/config/resolve/mcp.rs | 18 +- .../src/util/config/resolve/system_prompt.rs | 4 +- .../src/util/config/resolve/tool_approvals.rs | 4 +- .../src/util/config/resolve/toolset.rs | 6 +- .../src/util/config/resolve/version.rs | 2 +- .../xai-grok-shell/src/util/config/tips.rs | 33 +- .../src/util/config/worktree.rs | 28 +- .../codegen/xai-grok-shell/src/util/hooks.rs | 13 +- .../codegen/xai-grok-shell/src/util/limits.rs | 83 +- crates/codegen/xai-grok-shell/src/util/mod.rs | 2 +- .../tests/external_auth_expired_credential.rs | 448 +++ .../tests/test_leader_stdio_integration.rs | 4 +- .../tests/test_mcp_permission_persistence.rs | 2 +- crates/codegen/xai-grok-telemetry/Cargo.toml | 20 +- .../codegen/xai-grok-telemetry/src/events.rs | 66 + .../xai-grok-telemetry/src/external/config.rs | 61 + .../xai-grok-telemetry/src/external/mod.rs | 74 +- .../src/external/providers.rs | 329 +- .../xai-grok-telemetry/src/external/tests.rs | 34 + .../xai-grok-telemetry/src/otel_layer/mod.rs | 9 +- .../xai-grok-telemetry/src/otlp_http.rs | 72 +- .../tests/external_otlp_grpc_tls.rs | 107 + .../tests/otlp_collector/mod.rs | 81 + .../xai-grok-test-support/src/resources.rs | 93 +- crates/codegen/xai-grok-tools/src/bridge.rs | 1 + .../src/computer/local/lifecycle.rs | 193 ++ .../xai-grok-tools/src/computer/local/mod.rs | 4 + .../src/computer/local/terminal.rs | 282 +- .../computer/local/terminal_snapshot_tests.rs | 135 + .../xai-grok-tools/src/computer/mod.rs | 1 + .../xai-grok-tools/src/computer/task_log.rs | 54 + .../src/computer/task_log_tests.rs | 73 + .../xai-grok-tools/src/computer/types.rs | 12 + .../grok_build/task_output/mod.rs | 1 + .../src/implementations/task_output/tool.rs | 22 +- .../src/reminders/task_completion.rs | 39 +- .../xai-grok-tools/src/util/truncate.rs | 79 +- .../tests/test_subagent_soak.rs | 60 +- crates/codegen/xai-grok-version/Cargo.toml | 2 +- .../xai-grok-workspace/src/folder_trust.rs | 5 +- crates/codegen/xai-grok-workspace/src/hub.rs | 1 + .../src/permission/auto_mode.rs | 56 +- .../src/permission/bash_command_splitting.rs | 8 + .../src/permission/exec_risk.rs | 198 +- .../src/permission/gate_preflight.rs | 4 +- .../src/permission/hub_permission.rs | 2 + .../src/permission/manager.rs | 368 +- .../xai-grok-workspace/src/permission/mod.rs | 2 +- .../src/permission/policy.rs | 449 ++- .../src/permission/prompter.rs | 18 +- .../src/permission/shell_access.rs | 33 +- .../src/permission/state.rs | 6 + .../src/permission/types.rs | 8 +- crates/codegen/xai-tty-utils/src/lib.rs | 50 + .../xai-tty-utils/src/process_resources.rs | 195 ++ .../xai-tool-runtime/src/notification.rs | 3 + .../tests/notification_serde.rs | 2 + rust-toolchain.toml | 2 +- 323 files changed, 12259 insertions(+), 7227 deletions(-) create mode 100644 crates/codegen/xai-grok-pager/src/app/agent_view/viewer_tests.rs create mode 100644 crates/codegen/xai-grok-pager/tests/pty_e2e/plan_scrollbar_grab_zone_pty.rs create mode 100644 crates/codegen/xai-grok-pager/tests/pty_e2e/question_tab_cycles_answers.rs create mode 100644 crates/codegen/xai-grok-pager/tests/pty_e2e/response_top_indicator_pty.rs create mode 100644 crates/codegen/xai-grok-shell/changelogs/0.2.118.json create mode 100644 crates/codegen/xai-grok-shell/changelogs/0.2.118.md create mode 100644 crates/codegen/xai-grok-shell/src/agent/mvp_agent/replay.rs create mode 100644 crates/codegen/xai-grok-shell/src/agent/mvp_agent/replay_tests.rs create mode 100644 crates/codegen/xai-grok-shell/src/agent/mvp_agent/resource_telemetry.rs create mode 100644 crates/codegen/xai-grok-shell/src/auth/manager/remedy.rs delete mode 100644 crates/codegen/xai-grok-shell/src/bin/trace_classify.rs create mode 100644 crates/codegen/xai-grok-shell/src/tools/task_completed_frame.rs create mode 100644 crates/codegen/xai-grok-shell/src/tools/task_completed_frame_tests.rs delete mode 100644 crates/codegen/xai-grok-shell/src/trace_classifier/mod.rs create mode 100644 crates/codegen/xai-grok-shell/tests/external_auth_expired_credential.rs create mode 100644 crates/codegen/xai-grok-telemetry/tests/external_otlp_grpc_tls.rs create mode 100644 crates/codegen/xai-grok-tools/src/computer/local/lifecycle.rs create mode 100644 crates/codegen/xai-grok-tools/src/computer/local/terminal_snapshot_tests.rs create mode 100644 crates/codegen/xai-grok-tools/src/computer/task_log.rs create mode 100644 crates/codegen/xai-grok-tools/src/computer/task_log_tests.rs create mode 100644 crates/codegen/xai-tty-utils/src/process_resources.rs diff --git a/Cargo.lock b/Cargo.lock index 6e27a66..57912c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -100,7 +100,7 @@ version = "0.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bda177466b9524d59f1b12f0dd30b68696788e9992a7e959021c4a0ed96fcf59" dependencies = [ - "base64", + "base64 0.22.1", "bitflags 2.13.0", "home", "libc", @@ -530,7 +530,7 @@ source = "git+https://github.com/our-forks/async-openai.git?rev=95b52ebdedf42143 dependencies = [ "async-openai-macros", "backoff", - "base64", + "base64 0.22.1", "bytes", "derive_builder", "eventsource-stream", @@ -1121,7 +1121,7 @@ checksum = "8a18ed336352031311f4e0b4dd2ff392d4fbb370777c9d18d7fc9d7359f73871" dependencies = [ "axum-core", "axum-macros", - "base64", + "base64 0.22.1", "bytes", "form_urlencoded", "futures-util", @@ -1227,6 +1227,12 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4c7f02d4ea65f2c1853089ffd8d2787bdbc63de2f0d29dedbcf8ccdfa0ccd4cf" +[[package]] +name = "base64" +version = "0.21.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" + [[package]] name = "base64" version = "0.22.1" @@ -3717,7 +3723,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b43924e3df02cb3b846ca66a7ee58e8c13eb2556d0308c71f6154083f6980365" dependencies = [ "async-trait", - "base64", + "base64 0.22.1", "gcloud-metadata", "hex", "hmac", @@ -3756,7 +3762,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e17f9662a6966402de91daf0edb5accaae05c87f1a85479e57b95d2af7284b9f" dependencies = [ "anyhow", - "base64", + "base64 0.22.1", "bytes", "futures-util", "gcloud-auth", @@ -5179,7 +5185,7 @@ version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "futures-channel", "futures-util", @@ -5208,7 +5214,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.62.2", + "windows-core 0.61.2", ] [[package]] @@ -5693,7 +5699,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f1b46a0365a611fbf1d2143104dcf910aada96fafd295bab16c60b802bf6fa1d" dependencies = [ "ahash", - "base64", + "base64 0.22.1", "bytecount", "email_address", "fancy-regex 0.14.0", @@ -5720,7 +5726,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0529410abe238729a60b108898784df8984c87f6054c9c4fcacc47e4803c1ce1" dependencies = [ "aws-lc-rs", - "base64", + "base64 0.22.1", "ed25519-dalek", "getrandom 0.2.16", "hmac", @@ -6812,7 +6818,7 @@ version = "5.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "51e219e79014df21a225b1860a479e2dcd7cbd9130f4defd4bd0e191ea31d67d" dependencies = [ - "base64", + "base64 0.21.7", "chrono", "getrandom 0.2.16", "http 1.4.0", @@ -7164,7 +7170,7 @@ version = "0.32.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56d658ba1faf63f7b9c492cfbe6e0ec365440a16132d3270c1065f7b33f1b638" dependencies = [ - "base64", + "base64 0.22.1", "const-hex", "opentelemetry", "opentelemetry_sdk", @@ -7364,7 +7370,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ba152347f68db3aa3fbecc2b0104724d74aad532b186d3d9f4d5541681ddd91" dependencies = [ "aes 0.9.0", - "base64", + "base64 0.22.1", "bitflags 2.13.0", "brotli 8.0.2", "byteorder", @@ -7413,7 +7419,7 @@ version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d30c53c26bc5b31a98cd02d20f25a7c8567146caf63ed593a9d87b2775291be" dependencies = [ - "base64", + "base64 0.22.1", "serde_core", ] @@ -7737,7 +7743,7 @@ version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "740ebea15c5d1428f910cd1a5f52cebf8d25006245ed8ade92702f4943d91e07" dependencies = [ - "base64", + "base64 0.22.1", "indexmap", "quick-xml 0.38.3", "serde", @@ -8245,9 +8251,9 @@ dependencies = [ [[package]] name = "quinn-proto" -version = "0.11.14" +version = "0.11.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "434b42fec591c96ef50e21e886936e66d3cc3f737104fdb9b737c40ffb94c098" +checksum = "4fcb935c5bec503c2f0e306bdd3e58bb9029dcb14fa8d9ac76e3a5256ac0763e" dependencies = [ "aws-lc-rs", "bytes", @@ -8449,6 +8455,19 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "rcgen" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75e669e5202259b5314d1ea5397316ad400819437857b90861765f24c4cf80a2" +dependencies = [ + "pem", + "ring", + "rustls-pki-types", + "time", + "yasna", +] + [[package]] name = "read-fonts" version = "0.35.0" @@ -8588,7 +8607,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d0946410b9f7b082a427e4ef5c8ff541a88b357bc6c637c40db3a68ac70a36f" dependencies = [ "async-compression", - "base64", + "base64 0.22.1", "bytes", "futures-channel", "futures-core", @@ -8633,7 +8652,7 @@ version = "0.13.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "219c5811de6525e5416c7d5d53bb656d3afdbc6c5af816e0802bcfa42dbdc1c3" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "encoding_rs", "futures-channel", @@ -8801,7 +8820,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f00a32c3b81b7b254076a65abd5ab2551209146713ba38f73818657e865e9433" dependencies = [ "async-trait", - "base64", + "base64 0.22.1", "chrono", "futures", "http 1.4.0", @@ -9725,7 +9744,7 @@ version = "0.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0e7dc53c8a941858d01479622dbb7650aaa2ee0ca1fb58fb6cdddbfc3064abbb" dependencies = [ - "base64", + "base64 0.22.1", "hex", "serde", "serde_json", @@ -9744,7 +9763,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da90c3d9af638898a5fdc347479b18f950bb0dde11ecf2244f94cd28135da53b" dependencies = [ "aws-lc-rs", - "base64", + "base64 0.22.1", "const-oid 0.9.6", "der", "digest 0.10.7", @@ -9765,7 +9784,7 @@ version = "0.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c0d53558825c77716855c13794306fff766854c1bf92948e77d7890da3f2455" dependencies = [ - "base64", + "base64 0.22.1", "hex", "sigstore-crypto", "sigstore-types", @@ -9778,7 +9797,7 @@ version = "0.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "641fdaaa40d0cd6e249cf1671c2240beff1a3ece578062cf43e750779e8db2b3" dependencies = [ - "base64", + "base64 0.22.1", "hex", "reqwest 0.13.4", "serde", @@ -9796,7 +9815,7 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ee8762a4813252faffdf6f7e2040213078eb2482fbc351381134f3fc60c1be9c" dependencies = [ - "base64", + "base64 0.22.1", "chrono", "hex", "rustls-pki-types", @@ -9815,7 +9834,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0bc86c8abc1ece6832236e7e34e9baebb5d4d40490c0332b814a0d8624ca7255" dependencies = [ "aws-lc-rs", - "base64", + "base64 0.22.1", "chrono", "cmpv2", "cms", @@ -9840,7 +9859,7 @@ version = "0.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8d870c9bcfdf83396ac2bd2d6a23c5d09ec02cc9fda50fa1cbb3dd71a9bee53" dependencies = [ - "base64", + "base64 0.22.1", "chrono", "hex", "pem", @@ -9855,7 +9874,7 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6d751d608afd334fb9d8c037ad9efef69f1954494eff99cd79a0a6fc8af34ccb" dependencies = [ - "base64", + "base64 0.22.1", "chrono", "cms", "const-oid 0.9.6", @@ -10454,7 +10473,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4676b37242ccbd1aabf56edb093a4827dc49086c0ffd764a5705899e0f35f8f7" dependencies = [ "anyhow", - "base64", + "base64 0.22.1", "bitflags 2.13.0", "fancy-regex 0.11.0", "filedescriptor", @@ -10932,7 +10951,7 @@ checksum = "a286e33f82f8a1ee2df63f4fa35c0becf4a85a0cb03091a15fd7bf0b402dc94a" dependencies = [ "async-trait", "axum", - "base64", + "base64 0.22.1", "bytes", "flate2", "h2", @@ -11588,7 +11607,7 @@ version = "3.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d39cb1dbab692d82a977c0392ffac19e188bd9186a9f32806f0aaa859d75585a" dependencies = [ - "base64", + "base64 0.22.1", "log", "percent-encoding", "rustls", @@ -11604,7 +11623,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "60b4531c118335662134346048ddb0e54cc86bd7e81866757873055f0e38f5d2" dependencies = [ - "base64", + "base64 0.22.1", "http 1.4.0", "httparse", "log", @@ -11635,7 +11654,7 @@ version = "0.47.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d46cf96c5f498d36b7a9693bc6a7075c0bb9303189d61b2249b0dc3d309c07de" dependencies = [ - "base64", + "base64 0.22.1", "data-url", "flate2", "fontdb", @@ -12803,7 +12822,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08db1edfb05d9b3c1542e521aea074442088292f00b5f28e435c714a98f85031" dependencies = [ "assert-json-diff", - "base64", + "base64 0.22.1", "deadpool", "futures", "http 1.4.0", @@ -13118,7 +13137,7 @@ dependencies = [ "arc-swap", "async-trait", "axum", - "base64", + "base64 0.22.1", "chrono", "dashmap", "fastrace", @@ -13204,7 +13223,7 @@ dependencies = [ "aws-sdk-s3", "aws-smithy-http-client", "axum", - "base64", + "base64 0.22.1", "chrono", "dirs 5.0.1", "dunce", @@ -13342,7 +13361,7 @@ dependencies = [ name = "xai-grok-config" version = "0.1.0" dependencies = [ - "base64", + "base64 0.22.1", "blake3", "core-foundation", "dunce", @@ -13563,13 +13582,13 @@ dependencies = [ [[package]] name = "xai-grok-pager" -version = "0.2.117" +version = "0.2.118" dependencies = [ "agent-client-protocol", "ansi-to-tui", "anyhow", "backon", - "base64", + "base64 0.22.1", "blake3", "chrono", "clap", @@ -13653,7 +13672,7 @@ dependencies = [ [[package]] name = "xai-grok-pager-bin" -version = "0.2.117" +version = "0.2.118" dependencies = [ "anyhow", "clap", @@ -13709,7 +13728,7 @@ version = "0.1.0" dependencies = [ "alacritty_terminal", "anyhow", - "base64", + "base64 0.22.1", "clap", "dunce", "image", @@ -13735,7 +13754,7 @@ dependencies = [ "agent-client-protocol", "anstyle", "anyhow", - "base64", + "base64 0.22.1", "crossterm", "dark-light", "dirs 5.0.1", @@ -13893,7 +13912,7 @@ dependencies = [ "agent-client-protocol", "anyhow", "arboard", - "base64", + "base64 0.22.1", "dirs 6.0.0", "dunce", "image", @@ -13918,7 +13937,7 @@ dependencies = [ [[package]] name = "xai-grok-shell" -version = "0.2.117" +version = "0.2.118" dependencies = [ "agent-client-protocol", "anyhow", @@ -13927,7 +13946,7 @@ dependencies = [ "async-trait", "axum", "backon", - "base64", + "base64 0.22.1", "blake3", "bm25", "bytes", @@ -14133,6 +14152,7 @@ dependencies = [ "anyhow", "async-trait", "axum", + "base64 0.22.1", "bytes", "chrono", "ctor", @@ -14150,7 +14170,9 @@ dependencies = [ "opentelemetry_sdk", "parking_lot", "prost", + "rcgen", "reqwest 0.12.24", + "rustls", "sentry", "serde", "serde_json", @@ -14166,6 +14188,7 @@ dependencies = [ "tracing-subscriber", "url", "uuid", + "webpki-roots 0.26.11", "whoami", "xai-file-utils", "xai-grok-auth", @@ -14214,7 +14237,7 @@ dependencies = [ "async-openai", "async-stream", "async-trait", - "base64", + "base64 0.22.1", "chrono", "command-fds", "crc32fast", @@ -14331,7 +14354,7 @@ dependencies = [ [[package]] name = "xai-grok-version" -version = "0.2.117" +version = "0.2.118" dependencies = [ "semver", ] @@ -14366,7 +14389,7 @@ dependencies = [ "async-stream", "async-trait", "axum", - "base64", + "base64 0.22.1", "bstr", "chrono", "clap", @@ -14472,7 +14495,7 @@ dependencies = [ name = "xai-grok-workspace-types" version = "0.1.0" dependencies = [ - "base64", + "base64 0.22.1", "chrono", "serde", "serde_json", @@ -14519,7 +14542,7 @@ dependencies = [ name = "xai-mixpanel" version = "0.1.0" dependencies = [ - "base64", + "base64 0.22.1", "reqwest 0.12.24", "serde_json", "thiserror 2.0.18", @@ -14769,6 +14792,15 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" +[[package]] +name = "yasna" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e17bb3549cc1321ae1296b9cdc2698e2b6cb1992adfa19a8c72e5b7a738f44cd" +dependencies = [ + "time", +] + [[package]] name = "yoke" version = "0.8.0" diff --git a/Cargo.toml b/Cargo.toml index cb451e9..9e1a775 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -209,6 +209,7 @@ rand = "0.9" ratatui = { version = "0.29" } ratatui-core = "0.1" rayon = "1" +rcgen = { version = "0.13", features = ["pem"] } reflink-copy = "0.1" regex = "1" reqwest = { version = "0.12", features = ["rustls-tls", "stream", "json", "multipart", "http2", "blocking", "socks"], default-features = false } @@ -219,6 +220,7 @@ ring = "0.17" rsa = "0.9" runfiles = "0.1" rustc-hash = "2" +rustls = { version = "0.23", features = ["aws-lc-rs"] } schemars = "1" scraper = "0.23" semver = "1" @@ -277,6 +279,7 @@ vte = "0.15.0" wait-timeout = "0.2" walkdir = "2" webbrowser = { version = "1.0.4" } +webpki-roots = "0.26" which = "8" whoami = "1.4" wildmatch = "2" diff --git a/SOURCE_REV b/SOURCE_REV index 05c8181..a4180ba 100644 --- a/SOURCE_REV +++ b/SOURCE_REV @@ -1 +1 @@ -8d69c91f02bcacf01e98d5aebbf2f92547c45738 +64c4de99cc822b25ce9c54ab5a4f372093d0885d diff --git a/crates/codegen/ptyctl/src/styled.rs b/crates/codegen/ptyctl/src/styled.rs index 94d37d3..6086b4e 100644 --- a/crates/codegen/ptyctl/src/styled.rs +++ b/crates/codegen/ptyctl/src/styled.rs @@ -115,8 +115,8 @@ pub fn extract_styled_line( // Determine the character to emit. let is_cursor = cursor.row == line_number && cursor.col == col_idx + 1; - let ch = if is_cursor && opts.cursor_char.is_some() { - opts.cursor_char.unwrap() + let ch = if is_cursor && let Some(cursor_char) = opts.cursor_char { + cursor_char } else { cell.c }; diff --git a/crates/codegen/ptyctl/src/term.rs b/crates/codegen/ptyctl/src/term.rs index a2ad038..cb03638 100644 --- a/crates/codegen/ptyctl/src/term.rs +++ b/crates/codegen/ptyctl/src/term.rs @@ -274,8 +274,8 @@ impl Terminal { // Replace cursor position if requested. let is_cursor = cursor.row == line_idx + 1 && cursor.col == col_idx + 1; - if is_cursor && opts.cursor_char.is_some() { - text.push(opts.cursor_char.unwrap()); + if is_cursor && let Some(cursor_char) = opts.cursor_char { + text.push(cursor_char); } else { text.push(cell.c); if let Some(zw) = cell.zerowidth() { diff --git a/crates/codegen/xai-grok-pager-bin/Cargo.toml b/crates/codegen/xai-grok-pager-bin/Cargo.toml index cbc0d20..9cf3cc5 100644 --- a/crates/codegen/xai-grok-pager-bin/Cargo.toml +++ b/crates/codegen/xai-grok-pager-bin/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "xai-grok-pager-bin" -version = "0.2.117" +version = "0.2.118" edition.workspace = true license = "Apache-2.0" authors = ["xAI"] diff --git a/crates/codegen/xai-grok-pager-minimal/src/overlay.rs b/crates/codegen/xai-grok-pager-minimal/src/overlay.rs index f271897..df7c481 100644 --- a/crates/codegen/xai-grok-pager-minimal/src/overlay.rs +++ b/crates/codegen/xai-grok-pager-minimal/src/overlay.rs @@ -628,6 +628,7 @@ fn render_permission( area, perm, &followup, + agent.permission_pattern_edit.as_ref(), minimal_api::hovered_permission_item(agent), theme, true, diff --git a/crates/codegen/xai-grok-pager-render/src/appearance/config.rs b/crates/codegen/xai-grok-pager-render/src/appearance/config.rs index 76da40f..07069e7 100644 --- a/crates/codegen/xai-grok-pager-render/src/appearance/config.rs +++ b/crates/codegen/xai-grok-pager-render/src/appearance/config.rs @@ -354,13 +354,17 @@ impl Default for ScrollConfig { } } -/// Follow indicator display mode. +/// Scroll indicator display mode: the ▼ jump-to-bottom arrow below +/// scrollback and its ▲ jump-to-response-top mirror under the sticky +/// prompt header. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum FollowIndicator { - /// No follow indicator. + /// No scroll indicators. None, /// Show ▼ centered in the gap row below scrollback when not following - /// and there's content below the viewport. + /// and there's content below the viewport, and ▲ centered under the + /// sticky prompt header while the answer being read starts above the + /// viewport top. #[default] Center, } @@ -991,8 +995,8 @@ pub struct RawScrollConfig { /// If a scroll would be less than this percentage, scroll by this amount instead. /// 0 = minimal scroll (default), 25 = quarter page, 100 = full page. pub min_page_fraction: u8, - /// Follow indicator in the gap row below scrollback. - /// "none" = hidden, "center" = ▼ centered when content is below viewport. + /// Scroll indicators: the ▼ below scrollback and the ▲ under the sticky + /// prompt header. "none" = hidden, "center" = centered arrows. pub follow_indicator: RawFollowIndicator, /// When follow mode scrolls to new content, auto-select the latest entry. pub follow_auto_select: bool, @@ -1019,13 +1023,13 @@ impl Default for RawScrollConfig { } } -/// Follow indicator display mode (TOML format). +/// Scroll indicator display mode (TOML format). #[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "lowercase")] pub enum RawFollowIndicator { - /// No follow indicator. + /// No scroll indicators. None, - /// Show ▼ centered in the gap row below scrollback. + /// Show ▼ centered below scrollback and ▲ under the sticky prompt header. #[default] Center, } diff --git a/crates/codegen/xai-grok-pager-render/src/render/scrollbar.rs b/crates/codegen/xai-grok-pager-render/src/render/scrollbar.rs index c5dd8ee..37b583c 100644 --- a/crates/codegen/xai-grok-pager-render/src/render/scrollbar.rs +++ b/crates/codegen/xai-grok-pager-render/src/render/scrollbar.rs @@ -125,6 +125,23 @@ pub fn needs_scrollbar(total_lines: u16, viewport_lines: u16) -> bool { total_lines > viewport_lines } +/// The scrollbar's mouse grab zone: the track plus one column of slop on +/// each side. +/// +/// Users read a thumb drawn flush against a modal border as one +/// two-column widget and press the border half (reported on macOS +/// Terminal.app and ghostty over SSH), so near-miss presses must still +/// grab the thumb. +pub fn scrollbar_grab_zone(track: Rect) -> Rect { + let x = track.x.saturating_sub(SCROLLBAR_GAP_COLS); + Rect { + x, + y: track.y, + width: (track.x - x).saturating_add(track.width).saturating_add(1), + height: track.height, + } +} + /// Whether the view is at the bottom (following mode position). #[allow(dead_code)] // Useful helper, kept for future use pub fn is_at_bottom(total_lines: u16, viewport_lines: u16, offset: u16) -> bool { @@ -217,53 +234,26 @@ pub fn render_scrollbar( offset: u16, is_following: bool, ) { - if SCROLLBARS_HIDDEN.load(Ordering::Relaxed) { - return; - } - - let Some(scrollbar_area) = scrollbar_area else { - return; - }; - - if scrollbar_area.width == 0 || scrollbar_area.height == 0 { - return; - } - - if !needs_scrollbar(total_lines, viewport_lines) { - return; - } - - let lengths = ScrollLengths { - content_len: total_lines as usize, - viewport_len: viewport_lines as usize, - }; - - let scrollbar = ScrollBar::vertical(lengths).offset(offset as usize); - - // Render into ratatui-core scratch buffer - let core_area = CoreRect { - x: scrollbar_area.x, - y: scrollbar_area.y, - width: scrollbar_area.width, - height: scrollbar_area.height, - }; - let mut scratch = CoreBuffer::empty(core_area); - (&scrollbar).render(core_area, &mut scratch); - - // Copy to ratatui buffer with follow-aware styling let (track_style, thumb_style) = scrollbar_styles(is_following); - for row in 0..scrollbar_area.height { - let x = scrollbar_area.x; - let y = scrollbar_area.y + row; - let src = &scratch[(x, y)]; - let dst = &mut buf[(x, y)]; - if src.symbol() == " " { - dst.set_symbol(" "); - dst.set_style(track_style); - } else { - dst.set_symbol("\u{2588}"); - dst.set_style(thumb_style); - } + render_scrollbar_styled( + buf, + scrollbar_area, + total_lines, + viewport_lines, + offset, + track_style, + thumb_style, + ); +} + +/// Some emulators (notably macOS Terminal.app) do not stretch the `█` +/// glyph over the cell's line-gap pixels, so a foreground-only thumb +/// renders striped with dark bars; the background fill covers the whole +/// cell box. +fn thumb_fill_style(thumb_style: Style) -> Style { + match thumb_style.fg { + Some(fg) => thumb_style.bg(fg), + None => thumb_style, } } @@ -332,6 +322,7 @@ pub fn render_scrollbar_styled( (&scrollbar).render(core_area, &mut scratch); // Copy to ratatui buffer with custom styling + let thumb_fill = thumb_fill_style(thumb_style); for row in 0..scrollbar_area.height { let x = scrollbar_area.x; let y = scrollbar_area.y + row; @@ -342,7 +333,7 @@ pub fn render_scrollbar_styled( dst.set_style(track_style); } else { dst.set_symbol("\u{2588}"); - dst.set_style(thumb_style); + dst.set_style(thumb_fill); } } } @@ -398,6 +389,21 @@ mod tests { assert!(scrollbar.is_none()); } + #[test] + fn test_scrollbar_grab_zone() { + let zone = scrollbar_grab_zone(Rect::new(39, 2, 1, 10)); + assert_eq!(zone, Rect::new(38, 2, 3, 10)); + assert!(zone.contains((38, 2).into()), "gap column grabs"); + assert!(zone.contains((39, 11).into()), "track grabs"); + assert!(zone.contains((40, 5).into()), "border column grabs"); + assert!(!zone.contains((37, 5).into()), "two columns left is out"); + assert!(!zone.contains((41, 5).into()), "two columns right is out"); + assert!(!zone.contains((39, 12).into()), "rows are bounded"); + + let zone = scrollbar_grab_zone(Rect::new(0, 0, 1, 4)); + assert_eq!(zone, Rect::new(0, 0, 2, 4)); + } + #[test] fn test_needs_scrollbar() { assert!(needs_scrollbar(100, 10)); // Content > viewport @@ -473,6 +479,36 @@ mod tests { } } + /// macOS Terminal.app leaves line-gap pixels unpainted under a + /// foreground-only `█`, striping the thumb with dark bars. + #[test] + fn test_thumb_cells_fill_background() { + let area = Rect::new(0, 0, 10, 10); + let (_, scrollbar_area) = split_area_for_scrollbar(area); + let sb = scrollbar_area.unwrap(); + + let track = Style::new().bg(Color::Black); + let thumb = Style::new().fg(Color::White).bg(Color::Black); + let mut buf = Buffer::empty(area); + render_scrollbar_styled(&mut buf, scrollbar_area, 100, 10, 50, track, thumb); + + let mut thumb_cells = 0; + for y in 0..sb.height { + let cell = &buf[(sb.x, sb.y + y)]; + if cell.symbol() == "\u{2588}" { + thumb_cells += 1; + assert_eq!( + cell.style().bg, + cell.style().fg, + "thumb cell background must match the glyph color" + ); + } else { + assert_eq!(cell.style().bg, Some(Color::Black), "track keeps its bg"); + } + } + assert!(thumb_cells > 0, "a thumb must be rendered"); + } + #[test] fn test_scrollbar_thumb_position() { let area = Rect::new(0, 0, 10, 10); diff --git a/crates/codegen/xai-grok-pager-render/src/terminal/tmux_probe.rs b/crates/codegen/xai-grok-pager-render/src/terminal/tmux_probe.rs index 39a120d..37e70a7 100644 --- a/crates/codegen/xai-grok-pager-render/src/terminal/tmux_probe.rs +++ b/crates/codegen/xai-grok-pager-render/src/terminal/tmux_probe.rs @@ -9,6 +9,9 @@ const TMUX_QUERY_TIMEOUT: Duration = Duration::from_secs(2); /// into a drain timeout. The main process wait still uses only /// [`TMUX_QUERY_TIMEOUT`]. const POST_EXIT_CLEANUP_GRACE: Duration = Duration::from_millis(300); +/// How long a signalled process group may take to empty before it is killed. +const GROUP_EXIT_GRACE: Duration = Duration::from_millis(100); +const GROUP_EXIT_POLL: Duration = Duration::from_millis(1); #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum TmuxCommand<'a> { @@ -16,6 +19,7 @@ enum TmuxCommand<'a> { OptionValue(&'a str), OptionSupport(&'a str), ControlMode, + ClientFeatures, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -131,10 +135,24 @@ fn terminate_tmux_tree(group: &xai_tty_utils::ProcessGroup, child: &mut std::pro let _ = child.wait(); } +/// SIGTERM the group, then escalate to SIGKILL only if it outlives the grace. +/// +/// Callers reach this with the leader already reaped, so the group is usually +/// empty on the first check. fn terminate_owned_group(group: &xai_tty_utils::ProcessGroup) { let _ = group.terminate(); - std::thread::sleep(Duration::from_millis(100)); - // KILL is unconditional because leader state says nothing about descendants. + let deadline = std::time::Instant::now() + GROUP_EXIT_GRACE; + loop { + if group.has_live_members() == Some(false) { + // `return`, not `break`: the reaped leader's pid may already + // belong to an unrelated group, so an empty group gets no SIGKILL. + return; + } + if std::time::Instant::now() >= deadline { + break; + } + std::thread::sleep(GROUP_EXIT_POLL); + } let _ = group.kill(); } @@ -186,6 +204,25 @@ fn query_option_support_with(runner: &dyn TmuxCommandRunner, option: &str) -> Tm } } +/// The attached client's resolved terminal features, as a comma-separated list +/// (`RGB`, `clipboard`, `focus`, …). +/// +/// tmux resolves this once per client at attach time from the outer terminal's +/// terminfo plus `terminal-features` / `terminal-overrides`, and it decides +/// whether 24-bit SGR survives the multiplexer. `COLORTERM` inside the pane +/// describes only what the pane's program emits, so it cannot answer that. +/// +/// Empty output means the answer is unknown rather than negative: tmux before +/// 3.2 has no `terminal-features` and renders the unknown format as an empty +/// string, and a server with no attached client has nothing to report. +pub fn query_client_features() -> TmuxQueryResult { + query_client_features_with(&LiveTmuxCommandRunner) +} + +fn query_client_features_with(runner: &dyn TmuxCommandRunner) -> TmuxQueryResult { + parse_value(runner.run(TmuxCommand::ClientFeatures)) +} + pub fn query_control_mode() -> TmuxQueryResult { query_control_mode_with(&LiveTmuxCommandRunner) } @@ -221,6 +258,11 @@ fn build_tmux_command(command: TmuxCommand<'_>) -> Command { .stdout(Stdio::piped()) .stderr(Stdio::piped()); } + TmuxCommand::ClientFeatures => { + cmd.args(["display-message", "-p", "#{client_termfeatures}"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + } } cmd.stdin(Stdio::null()).envs(xai_tty_utils::pager_env()); xai_tty_utils::detach_std_command(&mut cmd); @@ -283,6 +325,7 @@ mod tests { TmuxCommand::OptionValue(option) => format!("value:{option}"), TmuxCommand::OptionSupport(option) => format!("support:{option}"), TmuxCommand::ControlMode => "control-mode".to_owned(), + TmuxCommand::ClientFeatures => "client-features".to_owned(), }); self.output.clone() } @@ -304,6 +347,10 @@ mod tests { TmuxCommand::ControlMode, vec!["display-message", "-p", "#{client_flags}"], ), + ( + TmuxCommand::ClientFeatures, + vec!["display-message", "-p", "#{client_termfeatures}"], + ), ]; for (request, args) in cases { let cmd = build_tmux_command(request); @@ -436,4 +483,67 @@ mod tests { assert!(output.status_success, "expected successful status"); assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "tmux 3.4"); } + + #[cfg(unix)] + #[test] + fn empty_group_teardown_does_not_wait_out_the_grace() { + let group = xai_tty_utils::ProcessGroup::new().expect("group"); + let started = std::time::Instant::now(); + terminate_owned_group(&group); + let elapsed = started.elapsed(); + assert!( + elapsed < GROUP_EXIT_GRACE / 2, + "an empty group must not wait out the grace, took {elapsed:?}" + ); + } + + #[cfg(unix)] + #[test] + fn group_that_ignores_sigterm_waits_the_grace_and_is_killed() { + let mut group = xai_tty_utils::ProcessGroup::new().expect("group"); + let mut cmd = Command::new("sh"); + cmd.arg("-c") + .arg("trap '' TERM; sleep 1000") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + xai_tty_utils::detach_std_command(&mut cmd); + #[allow(clippy::disallowed_methods)] // test fixture; the test kills it + let mut child = cmd.spawn().expect("spawn sigterm-ignoring child"); + group.attach_std(&child).expect("attach"); + // The shell installs its trap ~0.3ms after exec. Signal before that + // and it dies to the default SIGTERM, leaving a zombie that still + // reports live — the test then passes without exercising SIGKILL. + std::thread::sleep(Duration::from_millis(250)); + + let started = std::time::Instant::now(); + terminate_owned_group(&group); + let elapsed = started.elapsed(); + + assert!( + elapsed >= GROUP_EXIT_GRACE, + "an occupied group must still get the full grace, took {elapsed:?}" + ); + + // Bounded: without SIGKILL this fails in seconds rather than blocking + // the run on `sleep 1000`. + let reap_deadline = std::time::Instant::now() + Duration::from_secs(5); + let status = loop { + match child.try_wait().expect("poll child") { + Some(status) => break status, + None if std::time::Instant::now() < reap_deadline => { + std::thread::sleep(Duration::from_millis(10)); + } + None => { + let _ = child.kill(); + let _ = child.wait(); + panic!("the child survived teardown, so SIGKILL never escalated"); + } + } + }; + assert!( + !status.success(), + "the child must have been killed, got {status:?}" + ); + } } diff --git a/crates/codegen/xai-grok-pager/Cargo.toml b/crates/codegen/xai-grok-pager/Cargo.toml index 6435fce..a8323d2 100644 --- a/crates/codegen/xai-grok-pager/Cargo.toml +++ b/crates/codegen/xai-grok-pager/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "xai-grok-pager" -version = "0.2.117" +version = "0.2.118" edition.workspace = true license = "Apache-2.0" authors = ["xAI"] diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/02-authentication.md b/crates/codegen/xai-grok-pager/docs/user-guide/02-authentication.md index 3e23ca4..8a995b6 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/02-authentication.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/02-authentication.md @@ -199,6 +199,15 @@ fi echo "{\"access_token\": \"$TOKEN\", \"expires_in\": 3600}" ``` +The refresh run is headless: stdin is closed and the binary is given a few +seconds, so it cannot prompt you. If it can't produce a token there — because +your SSO session lapsed and it needs you to sign in again — Grok stops treating +the stored credential as usable and starts the interactive flow instead, the +same one you get on a machine that has never signed in. That run has a long +timeout and shows your binary's stderr, so a device-code URL or a browser prompt +reaches you. Mid-session, the turn fails with a re-auth prompt and `/login` +re-runs the binary interactively. + ### Environment Variables | Variable | Description | diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md b/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md index 468ea38..5dc7e1e 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/03-keyboard-shortcuts.md @@ -93,10 +93,32 @@ Switch between the prompt input and scrollback pane. |-----|---------|---------|--------| | `Tab` | `Space` (and `i` in vim mode) | Scrollback focused | Focus the prompt input | | `Tab` | | Prompt focused | Focus the scrollback (both simple and vim scrollback modes) | +| `Tab` | `Shift+Tab` (backwards) | Question card focused | Walk the card's answers, wrapping round at the ends. Focus stays in the card | | `Enter` | | Prompt focused | Send the current prompt | **Esc is not a focus key.** It follows the cancel / clear / rewind semantics below. The mid-turn cancel is the only branch gated on `[ui].vim_mode` (scrollback nav); nothing depends on `[ui].simple_mode` (prompt editor). Overlays, modals, slash/file dropdowns, voice, search, and selection still steal Esc first. +## Question card (`ask_user_question`) + +While the agent is waiting on an answer, the card owns the keyboard. + +| Key | Action | +|-----|--------| +| `↑` / `↓`, `j` / `k` | Move between answers (clamped at the ends) | +| `Tab` / `Shift+Tab` | Walk the answers in a loop: every answer of this question, then the next question's, and off the last answer back to the first | +| `←` / `→`, `h` / `l`, `[` / `]` | Previous / next question | +| `1`–`9`, `a`–`f` | Pick that answer directly | +| `z` | Jump to the free-text row and start typing | +| `Space` | Toggle the focused answer (multi-select), or start typing on the free-text row | +| `Enter` | Select and advance, submit on the last question, or edit the free-text row | +| `Esc` | Unselect this question's answer. It does not move focus | +| `y` | Copy the focused answer | +| `Shift+X` | Dismiss the question (the agent continues without an answer) | +| `Ctrl+F` | Fullscreen the card | + +While typing a free-text answer, `Enter` submits and `Esc` returns to the +answer rows; every other key goes to the text field. + ## Escape | State | Gesture | Effect | diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/05-configuration.md b/crates/codegen/xai-grok-pager/docs/user-guide/05-configuration.md index a746dc6..077997e 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/05-configuration.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/05-configuration.md @@ -630,7 +630,7 @@ gap_right = 0 # gap between scrollbar and screen edge [scrollback.scroll] margin = 0 # minimum context lines above/below selection min_page_fraction = 0 # minimum scroll as % of viewport (0-100) -follow_indicator = "center" # follow indicator: "center" or "none" +follow_indicator = "center" # ▼/▲ scroll indicators: "center" or "none" follow_auto_select = true # auto-select latest entry in follow mode follow_by_overscroll = true # scrolling past bottom engages follow mode anchor_on_fold = true # keep block position when folding diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/06-theming.md b/crates/codegen/xai-grok-pager/docs/user-guide/06-theming.md index 2a36cea..0cf88ec 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/06-theming.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/06-theming.md @@ -189,7 +189,7 @@ gap_right = 0 # Gap between scrollbar and screen edge (0 = at edge) [scrollback.scroll] margin = 0 # Context lines above/below selected entry (0 = edge) min_page_fraction = 0 # Minimum scroll as % of viewport (0-100) -follow_indicator = "center" # "center" = show down-arrow, "none" = hidden +follow_indicator = "center" # "center" = show the ▼/▲ scroll arrows, "none" = hidden follow_auto_select = true # Auto-select latest entry when following follow_by_overscroll = true # Scrolling past bottom engages follow mode anchor_on_fold = true # Keep block header at same screen position when folding diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md b/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md index f3ddb25..4abe1d1 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/17-sessions.md @@ -181,7 +181,7 @@ This shows: - Session title (when set) - Shell version -- Auth method (OAuth vs API key) and where to manage account and credits (https://grok.com/?_s=billing for OAuth, console.x.ai for API key; API-key sessions also suggest `grok login` for SuperGrok) +- Auth method (OAuth vs API key; API-key sessions also suggest `grok login` for SuperGrok) - Session ID - Working directory - Model (with a model hash for coding models) diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/21-terminal-support.md b/crates/codegen/xai-grok-pager/docs/user-guide/21-terminal-support.md index 338a7b7..0a86e3b 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/21-terminal-support.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/21-terminal-support.md @@ -26,11 +26,12 @@ that appear only as silence during capture. When Doctor finds an explicit unhealthy tmux setting, `/doctor fix` lists the available automatic fixes. Apply one named fix at a time, for example `/doctor fix tmux-clipboard` or `grok doctor fix dcs-passthrough --yes`. -Doctor can persist these three tmux options: +Doctor can persist these four tmux options: - `terminal.tmux-clipboard` — `set -g set-clipboard on` - `terminal.dcs-passthrough` — `set -wg allow-passthrough on` - `terminal.tmux-extended-keys` — `set -g extended-keys on` +- `terminal.tmux-truecolor` — `set -as terminal-features ",*:RGB"` A tmux fix edits only the persistent config on the computer hosting the affected tmux server, including remote sessions. Plain tmux uses the real @@ -82,6 +83,16 @@ Detection has these limitations: Run `/doctor`. A fully supported setup shows `color truecolor` and `themes all`. If it does not, Doctor shows the detected limitation and the relevant fix. +Inside tmux there are two separate questions: what color Grok emits, and what +color survives the multiplexer. The `color` line answers the first. For the +second, when the attached client is not marked `RGB`, tmux rewrites every +24-bit color to the nearest color the outer terminal's terminfo advertises, +which can be as few as eight. Themes then look washed out even though `color` +reads `truecolor`. Doctor reports this as `terminal.tmux-truecolor`. Reload +your tmux config and then detach and reattach: the server reads the new option +only on reload, and a client fixes its color depth only at attach, so neither +step alone changes anything. + ### Clipboard problems Grok writes through up to three routes, shown in `/doctor` under **Clipboard**: diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/22-permissions-and-safety.md b/crates/codegen/xai-grok-pager/docs/user-guide/22-permissions-and-safety.md index 33b1783..45fa334 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/22-permissions-and-safety.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/22-permissions-and-safety.md @@ -161,6 +161,8 @@ After splitting chained commands (on `&&`, `||`, `;`, and pipes), the following **Git (read-only):** - `git status`, `git branch`, `git log`, `git diff`, `git ls-files`, `git show`, `git rev-parse` +- `git blame`, `git describe`, `git merge-base`, `git shortlog` +- `git check-ignore`, `git check-attr`, `git cat-file`, `git ls-tree`, `git show-ref`, `git for-each-ref`, `git rev-list`, `git name-rev`, `git count-objects` **Search and inspection:** - `grep`, `rg` (not `rg --pre` / `rg --pre=…`, which spawn a preprocessor per file) @@ -337,15 +339,15 @@ A built-in list (`rm`, `chmod`, `chown`, `chgrp`, `chattr`, `pkill`, `kill`, `ki ### Read, Edit, and Grep Rules -Path patterns are globs matched against the path string the tool was called with: +Path patterns are globs matched against the tool path after lexical normalization (`.`/`..` collapsed; relative paths joined with the session working directory). A `~`-prefixed tool path is matched literally — never joined with the working directory — because tools expand `~` to the home directory only after the permission check: - `*` and `?` do not cross `/`; `**` does. `Read(src/*)` matches `src/main.rs` but not `src/nested/mod.rs`; use `Read(src/**)` for the whole tree. - A bare filename matches only that exact string. Use `**/.env` to match `.env` at any depth. - There are no anchor prefixes: a leading `//` or `~/` in a pattern is treated as literal glob text. Write absolute-path patterns or `**/` patterns instead. -- Paths are matched as given, without canonicalization. Whether a path is absolute or relative depends on how the tool was invoked, so patterns intended as boundaries should cover both forms (for example both `/repo/secrets/**` and `secrets/**`). +- Because `.`/`..` are collapsed before matching, rooted patterns cannot be escaped by traversal: `Read(./**)` scopes to the working directory (bare relatives like `src/main.rs` match; `./../../etc/passwd` does not), and `Read(src/**)` stays under `src/`. Unrooted patterns (`*`, or a leading `**` as in `**/*.rs`) intentionally match at any depth, anywhere. - `Read` rules also govern `grep` searches; `Grep(...)` rules match only grep. -`Read` and `Edit` deny rules additionally apply to file paths that shell commands touch (for example `cat` or `sed` on a denied path), including literal inline scripts passed to `bash`, `sh`, `dash`, `zsh`, or `ksh` with `-c`; that shell-level check also resolves symlinks. The direct `read_file`/`search_replace` tool checks do not resolve symlinks. For OS-level enforcement that covers every process, combine deny rules with the sandbox ([18-sandbox.md](18-sandbox.md)). +`Read` and `Edit` deny rules additionally apply to file paths that shell commands touch (for example `cat` or `sed` on a denied path), including literal inline scripts passed to `bash`, `sh`, `dash`, `zsh`, or `ksh` with `-c`; that shell-level check uses the same working-directory-aware normalization (an absolute operand under the working directory also matches rooted rules like `Read(src/**)`) and also resolves symlinks. The direct `read_file`/`search_replace` tool checks do not resolve symlinks. For OS-level enforcement that covers every process, combine deny rules with the sandbox ([18-sandbox.md](18-sandbox.md)). ### MCP Rules diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/24-monitoring-usage.md b/crates/codegen/xai-grok-pager/docs/user-guide/24-monitoring-usage.md index 7172fed..21e45ae 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/24-monitoring-usage.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/24-monitoring-usage.md @@ -66,6 +66,7 @@ without the master switch. | `OTEL_EXPORTER_OTLP_ENDPOINT` | `http://localhost:4318` for HTTP, `http://localhost:4317` for gRPC | Base endpoint. For `http/protobuf`, `/v1/logs` and `/v1/metrics` are appended per the OTLP spec; for `grpc`, the collector endpoint is used as-is. | | `OTEL_EXPORTER_OTLP_LOGS_ENDPOINT` / `..._METRICS_ENDPOINT` | — | Signal-specific overrides, used verbatim. For gRPC these should normally be collector endpoints without `/v1/...` paths. | | `OTEL_EXPORTER_OTLP_HEADERS` (+ signal-specific variants) | — | Collector auth (`k=v,k2=v2`). The **only** headers the external exporters send, and the only supported collector-auth mechanism (no config-file headers key — tokens never live on disk). | +| `OTEL_EXPORTER_OTLP_CERTIFICATE` (+ signal-specific variants) | — | Path to a PEM bundle with additional trusted CA certificate(s) for verifying the collector — for collectors behind a private/corporate CA. Additive to the default trust roots (system store and embedded Mozilla roots). | | `OTEL_EXPORTER_OTLP_TIMEOUT` | `10000` (ms) | Export timeout. | | `OTEL_METRIC_EXPORT_INTERVAL` | `60000` (ms) | Metric export interval. | | `OTEL_BLRP_SCHEDULE_DELAY` (or alias `OTEL_LOGS_EXPORT_INTERVAL`) | `5000` (ms) | Log batch interval. | @@ -115,6 +116,30 @@ the `[telemetry]` `otel_*` keys through `grok setup` managed config / requirements pins, or force-disable it fleet-wide with the same local config layers (`external_otel_disabled`, content-gate locks). +## Startup suppression (why nothing arrives for the first few seconds) + +Because xAI can force-disable this stream fleet-wide, the CLI holds emission +closed at startup until it knows whether that switch is set — it fetches the +fleet policy from `/v1/settings` and only then starts exporting. In a healthy +setup that is well under a second and invisible. + +**The wait is bounded**, so a deployment that cannot reach xAI still exports: + +- If no fleet policy can apply at all — `[features] remote_fetch = false`, or + `[endpoints] cli_chat_proxy_base_url` points somewhere other than xAI — the + stream starts immediately, governed by your local configuration. +- If the policy fetch fails or never completes (firewalled host, offline + laptop), emission starts anyway once the attempt is exhausted, and in all + cases no later than 30 seconds after startup. + +A fleet policy that arrives afterwards still applies; it can only ever +*tighten* (disable the stream or force the content gates off), never enable +something your local configuration did not. + +If your collector receives nothing at all, check the debug log +(`grok --debug`) for `external otel:` lines — they record whether the stream +resolved its configuration, and whether it is exporting or suppressed. + ## Resource attributes | Attribute | Value | diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/background.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/background.rs index ba0bc84..8d35d3a 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/background.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/background.rs @@ -132,7 +132,9 @@ pub(super) fn handle_task_backgrounded(notif: &acp::ExtNotification, app: &mut A // and backends predating that field still bake a "[monitor] " prefix // into the command — detect it and strip the prefix so those render as a // "Monitor" row instead of a bash-highlighted "[monitor] …" under Tasks. - let monitor_prefix = command.strip_prefix("[monitor] ").map(str::to_string); + let monitor_prefix = command + .strip_prefix(crate::app::agent::MONITOR_PREFIX) + .map(str::to_string); let is_monitor = monitor_description.is_some() || monitor_prefix.is_some(); // Always drain the deferred-tool suppression key now that routing is being // set up — even when we end up preferring the wire `description`. This entry @@ -153,6 +155,15 @@ pub(super) fn handle_task_backgrounded(notif: &acp::ExtNotification, app: &mut A .or_else(|| non_blank(notif_description)) .or_else(|| non_blank(deferred_description)); + // Completed-before-Backgrounded race: short bg shells can exit (and the + // terminal poll emit `TaskCompleted`) before this notification is sent. + // Never overwrite the recorded terminal state back to Running — the + // completion already came and went, so it would stick forever. + let completed_early = session + .bg_tasks + .get(&task_id) + .is_some_and(|t| t.status != BgTaskStatus::Running); + // Create central bg task state (description may still be filled from the // Execute block on demotion before we insert into the map). let mut bg_task = BgTaskState { @@ -199,7 +210,12 @@ pub(super) fn handle_task_backgrounded(notif: &acp::ExtNotification, app: &mut A scrollback.mark_height_dirty(eid); scrollback.finish_running(eid); session.tracker.remove_pending_tool(&tool_call_id); - eid + Some(eid) + } else if completed_early { + // Entry gone and the task already completed: the completion + // block is already in scrollback — nothing left to render. + session.tracker.remove_pending_tool(&tool_call_id); + None } else { // Entry was removed between the tracker lookup and now (compaction, // clear, etc.). Create a fresh BgTask so the task has UI presence. @@ -208,27 +224,34 @@ pub(super) fn handle_task_backgrounded(notif: &acp::ExtNotification, app: &mut A .with_description(description.clone()); let fallback = scrollback.push_block(RenderBlock::BgTask(block)); scrollback.set_last_running(true); - fallback + Some(fallback) } + } else if completed_early { + // The completion block is already in scrollback; a fresh "Task + // started" block would render out of order and animate forever. + None } else { let block = crate::scrollback::blocks::BgTaskBlock::started(&command, &task_id) .with_description(description.clone()); let eid = scrollback.push_block(RenderBlock::BgTask(block)); scrollback.set_last_running(true); - eid + Some(eid) }; bg_task.description = description; - session.bg_tasks.insert(task_id.clone(), bg_task); + if completed_early { + if let Some(existing) = session.bg_tasks.get_mut(&task_id) { + existing.absorb_late_backgrounded(bg_task, entry_id); + } + } else { + bg_task.scrollback_entry_id = entry_id; + session.bg_tasks.insert(task_id.clone(), bg_task); + } session .bg_tool_call_to_task .insert(tool_call_id.clone(), task_id.clone()); - if let Some(bg) = session.bg_tasks.get_mut(&task_id) { - bg.scrollback_entry_id = Some(entry_id); - } - is_active } @@ -564,6 +587,9 @@ pub(super) fn handle_task_completed(notif: &acp::ExtNotification, app: &mut AppV _ => return false, }; + // Stamps `restored_from_replay` on tombstones inserted below. + let meta = NotificationMeta::from_json(session_notif.meta.as_ref().and_then(|v| v.as_object())); + let (matched, is_active, agent) = match resolve_notif_agent(app, &session_notif.session_id) { Some(t) => t, None => return false, @@ -618,7 +644,8 @@ pub(super) fn handle_task_completed(notif: &acp::ExtNotification, app: &mut AppV bg_task.scrollback_entry_id, ) } else { - // Task we didn't know about — use snapshot data. Prefer + // Task we didn't know about — its `TaskBackgrounded` hasn't + // arrived yet. Label from the model-supplied description, else // display_command when it differs from the raw command (monitors / // isolation-wrapped shells); treat equal values as non-labels. let command = task_snapshot.command.clone(); @@ -626,21 +653,41 @@ pub(super) fn handle_task_completed(notif: &acp::ExtNotification, app: &mut AppV .end_time .and_then(|end| end.duration_since(task_snapshot.start_time).ok()) .unwrap_or_default(); - let description = task_snapshot.display_command.clone().and_then(|d| { - // Strip the baked "[monitor] " prefix so the completed label - // matches the "Task started" path (which uses the bare - // monitor description), not "[monitor] …". - let d = d - .strip_prefix("[monitor] ") - .map(str::to_string) - .unwrap_or(d); - let t = d.trim(); - if t.is_empty() || t == command.trim() { - None - } else { - Some(d) - } - }); + let description = task_snapshot + .description + .clone() + .filter(|d| !d.trim().is_empty()) + .or_else(|| { + task_snapshot.display_command.clone().and_then(|d| { + // Bare label, matching the "Task started" path. + let d = d + .strip_prefix(crate::app::agent::MONITOR_PREFIX) + .map(str::to_string) + .unwrap_or(d); + let t = d.trim(); + if t.is_empty() || t == command.trim() { + None + } else { + Some(d) + } + }) + }); + + // Record the terminal state so the late `TaskBackgrounded` merges + // into it instead of inserting a fresh Running entry. + let status = if success { + BgTaskStatus::Done + } else { + BgTaskStatus::Failed + }; + let tombstone = BgTaskState::tombstone_from_snapshot( + &task_snapshot, + status, + description.clone(), + meta.is_replay, + ); + session.bg_tasks.insert(task_id.clone(), tombstone); + (command, elapsed, description, None) }; @@ -692,7 +739,16 @@ pub(super) fn handle_task_completed(notif: &acp::ExtNotification, app: &mut AppV RenderBlock::bg_task_failed(&command, task_id, elapsed, exit_code, signal) .with_bg_task_description(description) }; - scrollback.push_block(block); + let completion_eid = scrollback.push_block(block); + + // Anchor tasks that have no "Task started" block (tombstones) to the + // completion block, so block-viewer actions don't fall back to a bogus + // EntryId(0) and immediately close. + if let Some(bg_task) = session.bg_tasks.get_mut(task_id) + && bg_task.scrollback_entry_id.is_none() + { + bg_task.scrollback_entry_id = Some(completion_eid); + } is_active } diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/mod.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/mod.rs index 22cbc94..0174c94 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/mod.rs @@ -52,7 +52,10 @@ mod workflow_ingest; #[cfg(test)] use permissions::{MCP_ARGS_MAX_LINE_CHARS, MCP_ARGS_MAX_LINES, mcp_args_lines}; -use permissions::{apply_recap_block, handle_permission_request, should_drop_late_auto_recap}; +use permissions::{ + apply_recap_block, handle_permission_request, should_drop_duplicate_auto_recap, + should_drop_late_auto_recap, +}; // Hub + child modules (via `use super::*`) need sibling symbols in this scope. use routing::{ diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/permissions.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/permissions.rs index 14bea83..ced68c6 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/permissions.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/permissions.rs @@ -436,6 +436,37 @@ pub(super) fn should_drop_late_auto_recap(auto: bool, is_replay: bool, agent_idl auto && !is_replay && !agent_idle } +/// Live auto recap when scrollback already has a recap after the last user +/// prompt. Replay still rebuilds history as stored. +pub(super) fn should_drop_duplicate_auto_recap( + auto: bool, + is_replay: bool, + scrollback: &crate::scrollback::state::ScrollbackState, +) -> bool { + auto && !is_replay && scrollback_has_recap_since_last_user(scrollback) +} + +fn scrollback_has_recap_since_last_user( + scrollback: &crate::scrollback::state::ScrollbackState, +) -> bool { + use crate::scrollback::block::RenderBlock; + use crate::scrollback::blocks::SessionEvent; + + let mut recap_since_user = false; + for (_, entry) in scrollback.iter_entries() { + if entry.block.is_user_prompt() { + recap_since_user = false; + continue; + } + if let RenderBlock::SessionEvent(b) = &entry.block + && matches!(b.event, SessionEvent::Recap { .. }) + { + recap_since_user = true; + } + } + recap_since_user +} + /// Land a `SessionRecap` block: fill a manual `/recap`'s in-flight loading /// spinner in place (and stop its animation) when one is showing, otherwise /// append a fresh block. An automatic recap never consumes the manual loading diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/session_notification.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/session_notification.rs index 2708eaa..b872bc9 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/session_notification.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/session_notification.rs @@ -797,6 +797,12 @@ pub(super) fn handle_session_notification(notif: &acp::ExtNotification, app: &mu "dropping late auto SessionRecap; agent busy (turn or command in flight)" ); false + } else if should_drop_duplicate_auto_recap(auto, meta.is_replay, &agent.scrollback) { + tracing::debug!( + "dropping duplicate live auto SessionRecap; recap already shown since last user turn" + ); + app.notification_service.focus_tracker.mark_recap_shown(); + false } else { app.notification_service.focus_tracker.mark_recap_shown(); let recap_block = RenderBlock::session_event(SessionEvent::Recap { summary, auto }); diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/background_tasks.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/background_tasks.rs index 6d51e9c..ad14146 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/background_tasks.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/background_tasks.rs @@ -635,3 +635,218 @@ ); } + /// Base snapshot for the Completed-before-Backgrounded race tests; tweak + /// fields per test (the shared helpers hardcode output/description). + fn race_snapshot( + task_id: &str, + command: &str, + exit_code: Option, + ) -> xai_grok_tools::types::TaskSnapshot { + xai_grok_tools::types::TaskSnapshot { + task_id: task_id.into(), + command: command.into(), + display_command: None, + cwd: "/tmp".into(), + start_time: std::time::SystemTime::now(), + end_time: Some(std::time::SystemTime::now()), + output: String::new(), + output_file: "/tmp/out.log".into(), + truncated: false, + exit_code, + signal: None, + completed: true, + kind: Default::default(), + block_waited: false, + explicitly_killed: false, + owner_session_id: None, + description: None, + is_backgrounded: true, + output_total_bytes: 0, + } + } + + fn completed_notif_from_snapshot( + session_id: &str, + task_snapshot: xai_grok_tools::types::TaskSnapshot, + replayed: bool, + ) -> acp::ExtNotification { + let notif = SessionNotification { + session_id: acp::SessionId::new(session_id), + update: XaiSessionUpdate::TaskCompleted { + task_snapshot, + will_wake: false, + }, + meta: replayed.then(crate::acp::meta::ReplayMetaStamp::replayed), + }; + let raw = serde_json::value::to_raw_value(¬if).unwrap(); + acp::ExtNotification::new("x.ai/task_completed", std::sync::Arc::from(raw)) + } + + /// Short bg shells can exit — and `TaskCompleted` arrive — before their + /// `TaskBackgrounded`. The late `TaskBackgrounded` must not resurrect the + /// finished task as Running or push a stray "Task started" block. + #[test] + fn completed_before_backgrounded_does_not_resurrect_running() { + let mut app = make_app_with_agent("sess-1"); + + // TaskCompleted first, for a task the pager has never seen. + let mut snapshot = race_snapshot("task-race", "echo done", Some(0)); + snapshot.output = "task output line".into(); + let done = completed_notif_from_snapshot("sess-1", snapshot, false); + assert!(handle_task_completed(&done, &mut app)); + { + let agent = app.agents.get(&AgentId(0)).unwrap(); + let task = agent + .session + .bg_tasks + .get("task-race") + .expect("unknown TaskCompleted must record terminal state"); + assert_eq!(task.status, BgTaskStatus::Done); + assert_eq!(task.stdout, "task output line"); + assert_eq!(agent.scrollback.len(), 1, "completed block rendered"); + assert!( + task.scrollback_entry_id.is_some(), + "tombstone anchored to the completion block (viewer actions need an entry)" + ); + } + + // The late TaskBackgrounded (with a wire description) arrives. + let notif = SessionNotification { + session_id: acp::SessionId::new("sess-1"), + update: XaiSessionUpdate::TaskBackgrounded { + tool_call_id: "tc-race".into(), + task_id: "task-race".into(), + command: "echo done".into(), + cwd: "/tmp".into(), + output_file: "/tmp/output.log".into(), + monitor_description: None, + description: Some("wait for build".into()), + }, + meta: None, + }; + let raw = serde_json::value::to_raw_value(¬if).unwrap(); + let late = acp::ExtNotification::new("x.ai/task_backgrounded", std::sync::Arc::from(raw)); + assert!(handle_task_backgrounded(&late, &mut app)); + + let agent = app.agents.get(&AgentId(0)).unwrap(); + let task = &agent.session.bg_tasks["task-race"]; + assert_eq!( + task.status, + BgTaskStatus::Done, + "late TaskBackgrounded must not overwrite a terminal status with Running" + ); + assert_eq!(task.tool_call_id, "tc-race", "tool_call_id backfilled"); + assert_eq!( + task.description.as_deref(), + Some("wait for build"), + "description backfilled from the late notification" + ); + assert_eq!(task.stdout, "task output line", "snapshot stdout kept"); + assert_eq!( + agent.session.bg_tool_call_to_task.get("tc-race"), + Some(&"task-race".to_string()) + ); + assert_eq!( + agent.scrollback.len(), + 1, + "no stray 'Task started' block after the completion" + ); + assert!( + !agent.scrollback.needs_animation(), + "nothing may animate as running for a finished task" + ); + } + + /// Same race on the demotion path (foreground Execute auto-backgrounded): + /// the pending Execute block is still demoted to a finished BgTask block + /// and the terminal status survives. + #[test] + fn completed_before_backgrounded_demotion_finishes_execute_block() { + let mut app = make_app_with_agent("sess-1"); + let tc_id = "call-race-demote"; + + setup_pending_execute_tool(&mut app, tc_id); + + let done = make_task_completed_notif("sess-1", "task-demote", "sleep 9999", Some(1)); + assert!(handle_task_completed(&done, &mut app)); + { + let agent = app.agents.get(&AgentId(0)).unwrap(); + assert_eq!( + agent.session.bg_tasks["task-demote"].status, + BgTaskStatus::Failed + ); + assert_eq!(agent.scrollback.len(), 2, "Execute block + failed block"); + } + + let late = make_task_backgrounded_notif("sess-1", tc_id, "task-demote", "sleep 9999"); + assert!(handle_task_backgrounded(&late, &mut app)); + + let agent = app.agents.get(&AgentId(0)).unwrap(); + let task = &agent.session.bg_tasks["task-demote"]; + assert_eq!( + task.status, + BgTaskStatus::Failed, + "terminal status survives the late demotion" + ); + assert_eq!(agent.scrollback.len(), 2, "no extra block from the demotion"); + let entry = agent.scrollback.get(0).unwrap(); + assert!( + matches!(entry.block, RenderBlock::BgTask(_)), + "Execute block demoted to BgTask" + ); + assert!( + !agent.scrollback.needs_animation(), + "the demoted entry must be finished, not animating" + ); + assert!( + agent.session.tracker.pending_tool_entry_id(tc_id).is_none(), + "pending tool drained" + ); + } + + /// A completion tombstone prefers the snapshot's model-supplied + /// description, so the race renders the same label as the normal order. + #[test] + fn unknown_completed_prefers_snapshot_description() { + let mut app = make_app_with_agent("sess-1"); + + let mut snapshot = race_snapshot("task-desc", "cargo build", Some(0)); + snapshot.description = Some("build the app".into()); + let done = completed_notif_from_snapshot("sess-1", snapshot, false); + assert!(handle_task_completed(&done, &mut app)); + + let agent = app.agents.get(&AgentId(0)).unwrap(); + assert_eq!( + agent.session.bg_tasks["task-desc"].description.as_deref(), + Some("build the app") + ); + } + + /// A monitor's completion tombstone keeps the Monitor rendering. + #[test] + fn unknown_completed_monitor_kind_marks_is_monitor() { + let mut app = make_app_with_agent("sess-1"); + + let mut snapshot = race_snapshot("task-mon", "tail -f x.log", Some(0)); + snapshot.kind = xai_grok_tools::computer::types::TaskKind::Monitor; + let done = completed_notif_from_snapshot("sess-1", snapshot, false); + assert!(handle_task_completed(&done, &mut app)); + + let agent = app.agents.get(&AgentId(0)).unwrap(); + assert!(agent.session.bg_tasks["task-mon"].is_monitor); + } + + /// A tombstone from a replayed completion is historical context: it must + /// not read as new activity (mirrors restored `TaskBackgrounded`s). + #[test] + fn replayed_unknown_completed_marks_tombstone_restored() { + let mut app = make_app_with_agent("sess-1"); + + let snapshot = race_snapshot("task-replay", "echo hi", Some(0)); + let done = completed_notif_from_snapshot("sess-1", snapshot, true); + assert!(handle_task_completed(&done, &mut app)); + + let agent = app.agents.get(&AgentId(0)).unwrap(); + assert!(agent.session.bg_tasks["task-replay"].restored_from_replay); + } + diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs index b397215..5d69a92 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/mod.rs @@ -1845,6 +1845,7 @@ pub(super) fn task_completed_notif( owner_session_id: None, description: None, is_backgrounded: false, + output_total_bytes: 0, }, will_wake, }, diff --git a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/permissions.rs b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/permissions.rs index 64b1c9c..048d0b9 100644 --- a/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/permissions.rs +++ b/crates/codegen/xai-grok-pager/src/app/acp_handler/tests/permissions.rs @@ -186,6 +186,40 @@ ); } + #[test] + fn duplicate_live_auto_recap_dropped_after_existing_recap() { + let mut agent = make_agent(Some("s1")); + agent.scrollback.push_block(recap_block("first")); + assert!(should_drop_duplicate_auto_recap( + true, + false, + &agent.scrollback + )); + assert!( + !should_drop_duplicate_auto_recap(true, true, &agent.scrollback), + "replay must still paint stored recaps" + ); + assert!( + !should_drop_duplicate_auto_recap(false, false, &agent.scrollback), + "manual /recap still allowed" + ); + } + + #[test] + fn duplicate_auto_recap_allowed_after_new_user_prompt() { + let mut agent = make_agent(Some("s1")); + agent.scrollback.push_block(recap_block("old")); + agent + .scrollback + .push_block(crate::scrollback::block::RenderBlock::user_prompt( + "next question", + )); + assert!( + !should_drop_duplicate_auto_recap(true, false, &agent.scrollback), + "new user turn re-arms auto recap" + ); + } + #[test] fn enqueue_while_scrollback_steals_focus_to_prompt() { use crate::app::agent_view::AgentPane; diff --git a/crates/codegen/xai-grok-pager/src/app/agent.rs b/crates/codegen/xai-grok-pager/src/app/agent.rs index 813cc4d..f9fd392 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent.rs @@ -166,6 +166,10 @@ pub const BG_TASK_MAX_STDOUT: usize = 10 * 1024 * 1024; /// How long to wait for a kill response before auto-clearing `pending_kill` /// so the user can retry. Applied to both bg tasks and subagents. pub const PENDING_KILL_TIMEOUT_SECS: u64 = 10; +/// Prefix baked into monitor commands by backends predating the structured +/// `monitor_description` field (and by reparented monitors). Shared +/// convention with the shell's task notifications. +pub const MONITOR_PREFIX: &str = "[monitor] "; /// Status of a background task. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum BgTaskStatus { @@ -277,6 +281,82 @@ impl BgTaskState { } self.stdout_line_count = self.stdout.lines().count(); } + /// Terminal state recorded when a `TaskCompleted` arrives for a task with + /// no `bg_tasks` entry (its `TaskBackgrounded` hasn't arrived yet — short + /// bg shells can exit on the terminal's first poll). Keeps the late + /// `TaskBackgrounded` from inserting a fresh Running entry that nothing + /// would ever complete; see [`Self::absorb_late_backgrounded`]. + pub fn tombstone_from_snapshot( + snapshot: &xai_grok_tools::types::TaskSnapshot, + status: BgTaskStatus, + description: Option, + restored_from_replay: bool, + ) -> Self { + let is_monitor = matches!( + snapshot.kind, + xai_grok_tools::computer::types::TaskKind::Monitor + ) || snapshot + .display_command + .as_deref() + .is_some_and(|d| d.starts_with(MONITOR_PREFIX)); + let mut tombstone = Self { + task_id: snapshot.task_id.clone(), + tool_call_id: String::new(), + command: snapshot.command.clone(), + description, + cwd: snapshot.cwd.clone(), + output_file: snapshot.output_file.to_string_lossy().into_owned(), + status, + start_time: snapshot.start_time, + end_time: Some(snapshot.end_time.unwrap_or_else(SystemTime::now)), + exit_code: snapshot.exit_code, + signal: snapshot.signal.clone(), + stdout: String::new(), + stdout_line_count: 0, + truncated: snapshot.truncated, + pending_kill: false, + kill_requested_at: None, + scrollback_entry_id: None, + is_monitor, + restored_from_replay, + }; + if !snapshot.output.is_empty() { + let end = crate::render::line_utils::floor_char_boundary( + &snapshot.output, + BG_TASK_MAX_STDOUT, + ); + tombstone.set_stdout(snapshot.output[..end].to_string()); + if end < snapshot.output.len() { + tombstone.truncated = true; + } + } + tombstone + } + /// Fold a late `TaskBackgrounded` into an already-terminal entry: keep the + /// terminal status/exit/timing, backfill only what the completion snapshot + /// couldn't know (blank fields, demoted-Execute stdout, scrollback entry). + pub fn absorb_late_backgrounded(&mut self, fresh: BgTaskState, entry_id: Option) { + self.tool_call_id = fresh.tool_call_id; + if self.command.trim().is_empty() { + self.command = fresh.command; + } + if self + .description + .as_ref() + .is_none_or(|d| d.trim().is_empty()) + { + self.description = fresh.description; + } + self.is_monitor |= fresh.is_monitor; + if self.stdout.is_empty() && !fresh.stdout.is_empty() { + self.stdout = fresh.stdout; + self.stdout_line_count = fresh.stdout_line_count; + self.truncated |= fresh.truncated; + } + if self.scrollback_entry_id.is_none() { + self.scrollback_entry_id = entry_id; + } + } } /// State for a scheduled (loop) task, displayed in the tasks pane. #[derive(Debug, Clone)] @@ -1779,4 +1859,48 @@ mod tests { vec!["hi /commit".to_string(), "go /push now".to_string()] ); } + /// Folding a late `TaskBackgrounded` into a terminal tombstone keeps the + /// terminal state and backfills only what the snapshot couldn't know — + /// including a blank command (gateway-bridge completions synthesize one). + #[test] + fn absorb_late_backgrounded_backfills_without_resurrecting() { + let snapshot = xai_grok_tools::types::TaskSnapshot { + task_id: "t1".into(), + command: String::new(), + display_command: None, + cwd: "/tmp".into(), + start_time: SystemTime::now(), + end_time: None, + output: String::new(), + output_file: "/tmp/out.log".into(), + truncated: false, + exit_code: Some(0), + signal: None, + completed: true, + kind: Default::default(), + block_waited: false, + explicitly_killed: false, + owner_session_id: None, + description: None, + is_backgrounded: true, + output_total_bytes: 0, + }; + let mut tombstone = + BgTaskState::tombstone_from_snapshot(&snapshot, BgTaskStatus::Done, None, false); + assert_eq!(tombstone.status, BgTaskStatus::Done); + assert!(tombstone.end_time.is_some(), "end_time falls back to now"); + let mut fresh = + BgTaskState::tombstone_from_snapshot(&snapshot, BgTaskStatus::Running, None, false); + fresh.tool_call_id = "tc-1".into(); + fresh.command = "echo hi".into(); + fresh.description = Some("say hi".into()); + fresh.set_stdout("demoted output".into()); + tombstone.absorb_late_backgrounded(fresh, None); + assert_eq!(tombstone.status, BgTaskStatus::Done, "terminal status kept"); + assert_eq!(tombstone.tool_call_id, "tc-1"); + assert_eq!(tombstone.command, "echo hi", "blank command backfilled"); + assert_eq!(tombstone.description.as_deref(), Some("say hi")); + assert_eq!(tombstone.stdout, "demoted output"); + assert_eq!(tombstone.stdout_line_count, 1); + } } diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/input.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/input.rs index 5d90892..8301241 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/input.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/input.rs @@ -844,6 +844,7 @@ impl AgentView { perm.active_idx = idx; perm.focus = crate::views::permission_view::PermissionFocus::Options; + self.permission_pattern_edit = None; if is_double_click { self.last_permission_click = None; if let Some(opt) = perm.options.get(idx) { @@ -871,13 +872,20 @@ impl AgentView { } } Event::Paste(text) => { - let in_followup = self.permission_queue.front().is_some_and(|p| { - p.focus == crate::views::permission_view::PermissionFocus::FollowupInput - }); - if in_followup { - self.route_popup_paste(text) - } else { - InputOutcome::Changed + let front_focus = self.permission_queue.front().map(|p| p.focus); + match front_focus { + Some(crate::views::permission_view::PermissionFocus::FollowupInput) => { + self.route_popup_paste(text) + } + Some(crate::views::permission_view::PermissionFocus::PatternEdit) => { + if let Some(edit) = self.permission_pattern_edit.as_mut() { + for ch in text.chars().filter(|c| *c != '\n' && *c != '\r') { + edit.insert_char(ch); + } + } + InputOutcome::Changed + } + _ => InputOutcome::Changed, } } _ => InputOutcome::Changed, @@ -1014,7 +1022,7 @@ impl AgentView { _ => InputOutcome::Unchanged, }; } - if self.question_view.is_some() && self.active_pane != AgentPane::Scrollback { + if self.is_question_focused() { return match ev { Event::Key(key) if key.kind != KeyEventKind::Release => { if key!('q', CONTROL).matches(key) { @@ -1385,6 +1393,10 @@ impl AgentView { other => resolve_action(Some(other)).unwrap_or(InputOutcome::Unchanged), } } + /// Whether an open `ask_user_question` card owns the keyboard. + pub(crate) fn is_question_focused(&self) -> bool { + self.question_view.is_some() && self.active_pane != AgentPane::Scrollback + } /// Returns `true` if the switch happened immediately, `false` if blocked. pub(crate) fn set_active_pane(&mut self, target: AgentPane, force: bool) -> bool { if target != AgentPane::Scrollback { diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/interactions.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/interactions.rs index 8ee077c..3670172 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/interactions.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/interactions.rs @@ -20,6 +20,14 @@ use crate::views::question_view::QUESTION_VIEW_HPAD; use crossterm::event::Event; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; use std::time::Instant; +/// Which neighbouring question a key asked for, and where its cursor lands. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum QuestionSwitch { + Next, + Prev, + TabForward, + TabBackward, +} impl AgentView { /// Handle key input when the permission view is active. /// @@ -130,6 +138,25 @@ impl AgentView { } return InputOutcome::Changed; } + let on_reject_once = perm.options.get(perm.active_idx).is_some_and(|o| { + o.kind == agent_client_protocol::PermissionOptionKind::RejectOnce + }); + if key.code == KeyCode::Char('e') + && key.modifiers.is_empty() + && perm.has_editable_bash_pattern() + && !on_reject_once + && let Some(idx) = perm.options.iter().position(|o| { + o.kind == agent_client_protocol::PermissionOptionKind::AllowAlways + }) + { + perm.active_idx = idx; + let initial = crate::views::permission_view::preview_command_text(perm); + self.permission_pattern_edit = Some( + crate::views::permission_view::PatternEditState::new(initial), + ); + perm.focus = PermissionFocus::PatternEdit; + return InputOutcome::Changed; + } if let Some(opt) = perm.options.get(perm.active_idx) && opt.kind == agent_client_protocol::PermissionOptionKind::RejectOnce && crate::input::key::is_text_input_key(key) @@ -141,6 +168,48 @@ impl AgentView { } InputOutcome::Changed } + PermissionFocus::PatternEdit => { + if key.code == KeyCode::Esc { + self.permission_pattern_edit = None; + perm.focus = PermissionFocus::Options; + return InputOutcome::Changed; + } + if key!('c', CONTROL).matches(key) { + return InputOutcome::Action(Action::PermissionCancel); + } + let Some(edit) = self.permission_pattern_edit.as_mut() else { + perm.focus = PermissionFocus::Options; + return InputOutcome::Changed; + }; + if key.code == KeyCode::Enter { + if edit.trimmed().is_some() + && let Some(opt) = perm.options.iter().find(|o| { + o.kind == agent_client_protocol::PermissionOptionKind::AllowAlways + }) + { + return InputOutcome::Action(Action::PermissionSelect( + opt.option_id.clone(), + )); + } + return InputOutcome::Changed; + } + let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); + let alt = key.modifiers.contains(KeyModifiers::ALT); + match key.code { + KeyCode::Backspace => edit.backspace(), + KeyCode::Delete => edit.delete(), + KeyCode::Left => edit.move_left(), + KeyCode::Right => edit.move_right(), + KeyCode::Home => edit.move_home(), + KeyCode::End => edit.move_end(), + KeyCode::Char('a') if ctrl => edit.move_home(), + KeyCode::Char('e') if ctrl => edit.move_end(), + KeyCode::Char('u') if ctrl => edit.clear(), + KeyCode::Char(c) if !ctrl && !alt => edit.insert_char(c), + _ => {} + } + InputOutcome::Changed + } } } pub(super) fn handle_cancel_turn_key(&mut self, key: &KeyEvent) -> InputOutcome { @@ -224,12 +293,13 @@ impl AgentView { /// Handle key input when the question view is active. /// /// Two modes: - /// - **Navigation**: j/k move cursor, Space toggles, Enter advances or - /// edits freeform, h/l/[/] cycle questions, 1-9/a-f jump+toggle, - /// n next, s skip, Shift-X kill (only explicit way to dismiss). + /// - **Navigation**: j/k move the cursor between answers and Tab/Shift+Tab + /// walk the same rows in a loop, Space toggles, Enter advances or edits + /// freeform, h/l/[/] cycle questions, 1-9/a-f jump+toggle, Esc unselects, + /// Shift-X kills the question tool. /// - **InputMode**: all keys go to the prompt widget; Esc exits input mode. pub(super) fn handle_question_key(&mut self, key: &KeyEvent) -> InputOutcome { - use crate::views::question_view::{QuestionFocus, QuestionSelection}; + use crate::views::question_view::{CursorMotion, QuestionFocus}; let Some(ref mut qv) = self.question_view else { return InputOutcome::Unchanged; }; @@ -346,7 +416,7 @@ impl AgentView { return InputOutcome::Changed; } let mut needs_scroll_update = false; - let mut needs_switch_question: Option = None; + let mut needs_switch_question: Option = None; if qv.is_on_freeform_row() && (key.modifiers.is_empty() || key.modifiers == KeyModifiers::SHIFT) && matches!(key.code, KeyCode::Char(c) if c != ' ') @@ -360,51 +430,37 @@ impl AgentView { KeyCode::Char('j') | KeyCode::Down if key.modifiers.is_empty() || key.modifiers == KeyModifiers::CONTROL => { - let max = qv.total_items(qv.active_tab).saturating_sub(1); - let cur = qv.cursor(); - if cur < max { - qv.set_cursor(cur + 1); - needs_scroll_update = true; - } + qv.move_cursor(CursorMotion::Next); + needs_scroll_update = true; } KeyCode::Char('k') | KeyCode::Up if key.modifiers.is_empty() || key.modifiers == KeyModifiers::CONTROL => { - let cur = qv.cursor(); - if cur > 0 { - qv.set_cursor(cur - 1); - needs_scroll_update = true; - } - } - KeyCode::Char('d') if key.modifiers == KeyModifiers::CONTROL => { - let max = qv.total_items(qv.active_tab).saturating_sub(1); - let half = (max / 2).max(1); - qv.set_cursor((qv.cursor() + half).min(max)); + qv.move_cursor(CursorMotion::Prev); needs_scroll_update = true; } - KeyCode::PageDown => { - let max = qv.total_items(qv.active_tab).saturating_sub(1); - let page = max.max(1); - qv.set_cursor((qv.cursor() + page).min(max)); + KeyCode::Char('d') if key.modifiers == KeyModifiers::CONTROL => { + qv.move_cursor(CursorMotion::HalfPageDown); needs_scroll_update = true; } KeyCode::Char('u') if key.modifiers == KeyModifiers::CONTROL => { - let half = (qv.total_items(qv.active_tab) / 2).max(1); - qv.set_cursor(qv.cursor().saturating_sub(half)); + qv.move_cursor(CursorMotion::HalfPageUp); + needs_scroll_update = true; + } + KeyCode::PageDown => { + qv.move_cursor(CursorMotion::PageDown); needs_scroll_update = true; } KeyCode::PageUp => { - let page = qv.total_items(qv.active_tab).saturating_sub(1).max(1); - qv.set_cursor(qv.cursor().saturating_sub(page)); + qv.move_cursor(CursorMotion::PageUp); needs_scroll_update = true; } KeyCode::Char('g') if key.modifiers.is_empty() => { - qv.set_cursor(0); + qv.move_cursor(CursorMotion::First); needs_scroll_update = true; } KeyCode::Char('G') if key.modifiers == KeyModifiers::SHIFT => { - let max = qv.total_items(qv.active_tab).saturating_sub(1); - qv.set_cursor(max); + qv.move_cursor(CursorMotion::Last); needs_scroll_update = true; } KeyCode::Char(' ') => { @@ -440,7 +496,7 @@ impl AgentView { } let last = qv.questions.len().saturating_sub(1); if qv.active_tab < last { - needs_switch_question = Some(true); + needs_switch_question = Some(QuestionSwitch::Next); } else { return self.submit_question_answers(false); } @@ -458,14 +514,14 @@ impl AgentView { if key.modifiers.is_empty() || key.modifiers == KeyModifiers::CONTROL => { if qv.questions.len() > 1 { - needs_switch_question = Some(true); + needs_switch_question = Some(QuestionSwitch::Next); } } KeyCode::Char('h') | KeyCode::Char('[') | KeyCode::Left if key.modifiers.is_empty() || key.modifiers == KeyModifiers::CONTROL => { if qv.questions.len() > 1 { - needs_switch_question = Some(false); + needs_switch_question = Some(QuestionSwitch::Prev); } } KeyCode::Char(c) @@ -487,7 +543,7 @@ impl AgentView { } let last = qv.questions.len().saturating_sub(1); if qv.active_tab < last { - needs_switch_question = Some(true); + needs_switch_question = Some(QuestionSwitch::Next); } else { return self.submit_question_answers(false); } @@ -520,38 +576,45 @@ impl AgentView { return self.submit_question_answers(true); } let active = qv.active_tab; - if let Some(sel) = qv.selections.get_mut(active) { - match sel { - QuestionSelection::Multi(set) => { - set.clear(); - } - QuestionSelection::Single(opt) => { - *opt = None; - } - } - } - if let Some(sel) = qv.per_question_freeform_selected.get_mut(active) { - *sel = false; - } + qv.clear_selection(active); } - KeyCode::Tab => { - self.swap_question_freeform(); - self.active_pane = AgentPane::Scrollback; - return InputOutcome::Changed; + KeyCode::Tab | KeyCode::BackTab => { + let backward = crate::input::key::is_shift_tab(key); + let crosses_questions = qv.questions.len() > 1; + needs_scroll_update = true; + match (backward, qv.is_on_first_row(), qv.is_on_last_row()) { + (false, _, false) => qv.move_cursor(CursorMotion::Next), + (true, false, _) => qv.move_cursor(CursorMotion::Prev), + (false, _, true) if crosses_questions => { + needs_switch_question = Some(QuestionSwitch::TabForward); + } + (true, true, _) if crosses_questions => { + needs_switch_question = Some(QuestionSwitch::TabBackward); + } + (false, _, true) => qv.move_cursor(CursorMotion::First), + (true, true, _) => qv.move_cursor(CursorMotion::Last), + } } KeyCode::Char('X') if key.modifiers == KeyModifiers::SHIFT => { return self.submit_question_answers(true); } _ => {} } - if let Some(forward) = needs_switch_question { + if let Some(switch) = needs_switch_question { self.last_question_click = None; self.swap_question_freeform(); if let Some(ref mut qv) = self.question_view { - if forward { - qv.next_question(); - } else { - qv.prev_question(); + match switch { + QuestionSwitch::Next => qv.next_question(), + QuestionSwitch::Prev => qv.prev_question(), + QuestionSwitch::TabForward => { + qv.wrapping_next_question(); + qv.move_cursor(CursorMotion::First); + } + QuestionSwitch::TabBackward => { + qv.wrapping_prev_question(); + qv.move_cursor(CursorMotion::Last); + } } } self.load_question_freeform(); @@ -1984,3 +2047,223 @@ mod question_freeform_chip_tests { assert_eq!(paste_chip_count(&agent), 1, "chip must stay folded"); } } +#[cfg(test)] +mod question_answer_focus_tests { + //! The question card's answer walk. Tab used to hand focus to the + //! scrollback while the card stayed drawn; these pin the walk that + //! replaced it. + use super::super::test_fixtures::make_agent; + use super::super::{AgentPane, AgentView}; + use super::question_no_freeform_tests::open_question; + use crate::actions::ActionRegistry; + use crate::app::app_view::InputOutcome; + use crate::views::prompt_widget::StashedPrompt; + use crate::views::question_view::{ + LocalQuestionKind, QuestionFocus, QuestionSelection, QuestionViewState, + }; + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + use xai_grok_tools::implementations::grok_build::ask_user_question::{ + Question, QuestionOption, + }; + fn question(prompt: &str, labels: &[&str]) -> Question { + Question { + question: prompt.into(), + options: labels + .iter() + .map(|label| QuestionOption { + label: (*label).into(), + description: "why".into(), + preview: None, + id: None, + }) + .collect(), + multi_select: Some(false), + id: None, + } + } + fn open_two_questions(agent: &mut AgentView) { + agent.question_view = Some(QuestionViewState::new( + "tc-tab".into(), + vec![ + question("First?", &["Alpha", "Beta"]), + question("Second?", &["Gamma", "Delta"]), + ], + StashedPrompt::default(), + )); + } + fn press(agent: &mut AgentView, code: KeyCode, modifiers: KeyModifiers) { + let _ = agent.handle_question_key_for_test(&KeyEvent::new(code, modifiers)); + } + fn tab(agent: &mut AgentView) { + press(agent, KeyCode::Tab, KeyModifiers::NONE); + } + fn qv(agent: &AgentView) -> &QuestionViewState { + agent.question_view.as_ref().expect("question view open") + } + /// (question index, cursor row). + fn stop(agent: &AgentView) -> (usize, usize) { + (qv(agent).active_tab, qv(agent).cursor()) + } + fn hint_labels(agent: &AgentView) -> Vec { + agent + .current_shortcut_hints(&ActionRegistry::defaults(), false) + .iter() + .map(|hint| hint.label.to_string()) + .collect() + } + #[test] + fn tab_walks_every_answer_and_wraps() { + let mut agent = make_agent(); + open_two_questions(&mut agent); + let mut visited = vec![stop(&agent)]; + for _ in 0..6 { + tab(&mut agent); + visited.push(stop(&agent)); + } + assert_eq!( + visited, + vec![(0, 0), (0, 1), (0, 2), (1, 0), (1, 1), (1, 2), (0, 0)], + "Tab visits every answer row of question 1, then question 2, then wraps" + ); + assert_eq!( + agent.active_pane, + AgentPane::Prompt, + "the card keeps the keyboard the whole way round" + ); + } + #[test] + fn tab_wraps_within_a_single_question() { + let mut agent = make_agent(); + open_question(&mut agent, false); + for expected in [1, 2, 0] { + tab(&mut agent); + assert_eq!(stop(&agent), (0, expected)); + } + assert_eq!(agent.active_pane, AgentPane::Prompt); + } + /// Both Shift+Tab encodings terminals emit walk the answers backwards. + #[test] + fn shift_tab_walks_the_answers_backwards() { + for (code, modifiers) in [ + (KeyCode::BackTab, KeyModifiers::NONE), + (KeyCode::Tab, KeyModifiers::SHIFT), + ] { + let mut agent = make_agent(); + open_two_questions(&mut agent); + for _ in 0..3 { + tab(&mut agent); + } + assert_eq!(stop(&agent), (1, 0), "parked on the second question"); + press(&mut agent, code, modifiers); + assert_eq!( + stop(&agent), + (0, 2), + "Shift+Tab off the first row enters the previous question at its last row ({code:?})" + ); + for _ in 0..2 { + press(&mut agent, code, modifiers); + } + assert_eq!(stop(&agent), (0, 0)); + press(&mut agent, code, modifiers); + assert_eq!( + stop(&agent), + (1, 2), + "before the first answer, Shift+Tab wraps to the last one ({code:?})" + ); + assert_eq!(agent.active_pane, AgentPane::Prompt); + } + } + #[test] + fn tab_from_the_scrollback_focuses_the_card() { + let mut agent = make_agent(); + open_two_questions(&mut agent); + agent.active_pane = AgentPane::Scrollback; + let registry = ActionRegistry::defaults(); + let outcome = agent + .handle_scrollback_key(&KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE), ®istry); + assert!( + matches!(outcome, InputOutcome::Changed), + "Tab in the scrollback focuses the card, got {outcome:?}" + ); + assert_eq!(agent.active_pane, AgentPane::Prompt); + } + #[test] + fn tab_skips_the_free_text_row_when_the_card_has_none() { + let mut agent = make_agent(); + open_question(&mut agent, true); + tab(&mut agent); + assert_eq!(stop(&agent), (0, 1), "two options, so one step"); + tab(&mut agent); + assert_eq!( + stop(&agent), + (0, 0), + "the last option wraps to the first when there is no free-text row" + ); + } + #[test] + fn esc_unselects_and_leaves_focus_alone() { + let mut agent = make_agent(); + open_two_questions(&mut agent); + press(&mut agent, KeyCode::Char(' '), KeyModifiers::NONE); + assert!( + matches!(qv(&agent).selections[0], QuestionSelection::Single(Some(0))), + "Space marks the focused answer" + ); + press(&mut agent, KeyCode::Esc, KeyModifiers::NONE); + assert!( + matches!(qv(&agent).selections[0], QuestionSelection::Single(None)), + "Esc clears the answer" + ); + assert_eq!(agent.active_pane, AgentPane::Prompt); + press(&mut agent, KeyCode::Esc, KeyModifiers::NONE); + assert_eq!(agent.active_pane, AgentPane::Prompt); + assert!(agent.question_view.is_some()); + } + #[test] + fn esc_still_skips_the_project_picker() { + let mut agent = make_agent(); + open_two_questions(&mut agent); + if let Some(ref mut qv) = agent.question_view { + qv.local_kind = Some(LocalQuestionKind::ProjectSelect { + resolved_paths: Vec::new(), + original_cwd: std::path::PathBuf::from("/tmp"), + stashed_prompt: String::new(), + dont_ask_index: 0, + }); + } + press(&mut agent, KeyCode::Esc, KeyModifiers::NONE); + assert!( + agent.question_view.is_none(), + "Esc submits the picker as skipped instead of unselecting" + ); + } + #[test] + fn tab_in_input_mode_stays_with_the_text_field() { + let mut agent = make_agent(); + open_two_questions(&mut agent); + press(&mut agent, KeyCode::Char('z'), KeyModifiers::NONE); + assert_eq!(qv(&agent).focus, QuestionFocus::InputMode); + tab(&mut agent); + assert_eq!( + qv(&agent).focus, + QuestionFocus::InputMode, + "Tab must not walk the answers out from under a half-typed answer" + ); + assert_eq!(agent.active_pane, AgentPane::Prompt); + } + /// The reported symptom was the bar promising one thing while Tab did + /// another, so the bar must name the walk at every stop. + #[test] + fn shortcut_hints_name_the_answer_walk() { + let mut agent = make_agent(); + open_two_questions(&mut agent); + for step in 0..7 { + let hints = hint_labels(&agent); + assert!( + hints.contains(&"next answer".to_string()), + "step {step}: the bar advertises the answer walk, got {hints:?}" + ); + tab(&mut agent); + } + } +} diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs index 6798ec2..f24bb5c 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/mod.rs @@ -1093,6 +1093,9 @@ pub struct AgentView { pub hit_plan_button: HitArea, pub hit_plan_approval_status: HitArea, pub hit_follow_indicator: HitArea, + /// ▲ jump-to-response-top indicator in the sticky header's gap row + /// (click snaps the answer's first line to the top, same as `K`). + pub hit_response_top_indicator: HitArea, /// CWD / worktree path in the status bar (click to copy). pub hit_cwd: HitArea, /// Cancel button in turn status line (`[stop]`). @@ -1326,6 +1329,10 @@ pub struct AgentView { pub permission_stashed_prompt: Option, /// Scrollback focus stolen for a permission prompt; restored when the queue empties. pub permission_stashed_pane: Option, + /// Free-form "Always allow" pattern editor buffer for the front request. + /// `Some` only in `PermissionFocus::PatternEdit`; cleared when the request + /// resolves or the edit is cancelled. + pub permission_pattern_edit: Option, /// Active plan approval view (from `exit_plan_mode` ext_method). When `Some`, /// the prompt area shows the plan approval overlay and input is modal. pub(crate) plan_approval_view: Option, diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/panes.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/panes.rs index 11d9e26..764566d 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/panes.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/panes.rs @@ -555,7 +555,7 @@ impl AgentView { } if let Some(ref mut viewer) = self.line_viewer { if let Some(area) = viewer.last_popup_area - && area.contains((col, row).into()) + && (area.contains((col, row).into()) || viewer.list_state.scrollbar_hit(col, row)) { viewer .list_state diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/plan.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/plan.rs index f8af8d4..2200cf1 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/plan.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/plan.rs @@ -9,11 +9,22 @@ use crate::app::actions::Action; use crate::app::app_view::InputOutcome; use crate::views::file_search::line_viewer::LineViewerState; use crate::views::list_pane::ListItem; -use crate::views::plan_approval_view::{PlanApprovalFocus, PlanComment, PlanReviewSource}; +use crate::views::plan_approval_view::{ + PlanApprovalFocus, PlanApprovalViewState, PlanComment, PlanReviewSource, +}; use crate::views::prompt_widget::{EnterOutcome, PromptEvent}; #[cfg(test)] use crossterm::event::KeyModifiers; use crossterm::event::{KeyCode, KeyEvent}; +/// Telemetry for every way a plan review resolves ("build", "abandon", +/// "revise"). +fn log_plan_submit(action: &str) { + use xai_grok_telemetry::events::PlanSubmit; + use xai_grok_telemetry::session_ctx::log_event; + log_event(PlanSubmit { + action: action.to_string(), + }); +} impl AgentView { /// Resolve the absolute path to the plan file for this session. fn plan_file_path(&self) -> Option { @@ -193,19 +204,7 @@ impl AgentView { None }; pav.send_approved(); - self.latest_inline_plan_content = None; - self.plan_next_comment_id = pav.next_comment_id; - self.prompt.restore(pav.stashed_prompt); - self.line_viewer = None; - self.casual_commenting_range = None; - self.casual_editing_comment_id = None; - { - use xai_grok_telemetry::events::PlanSubmit; - use xai_grok_telemetry::session_ctx::log_event; - log_event(PlanSubmit { - action: "build".to_string(), - }); - } + self.close_plan_review(pav, "build"); if let Some(text) = review_comments { return InputOutcome::Action(Action::Interject { text, @@ -219,6 +218,20 @@ impl AgentView { return InputOutcome::Changed; }; pav.send_abandoned(); + self.close_plan_review(pav, "abandon"); + InputOutcome::Changed + } + /// Shared teardown for the two plan-review decisions that end the + /// review (approve and abandon). The shell leaves plan mode as a + /// result, but its confirming `CurrentModeUpdate("default")` is + /// fire-and-forget and only arrives after the exit tool runs — so + /// flip the mode indicator optimistically here (a lost update would + /// otherwise leave the badge stuck on "plan"), restore the + /// pre-review UI, and log the decision. + /// + /// Not for the revision path (`send_plan_feedback`): the shell + /// stays in plan mode there, so the indicator must stay on. + fn close_plan_review(&mut self, pav: PlanApprovalViewState, action: &'static str) { self.plan_mode_pending = Some(false); self.latest_inline_plan_content = None; self.plan_next_comment_id = pav.next_comment_id; @@ -226,14 +239,7 @@ impl AgentView { self.line_viewer = None; self.casual_commenting_range = None; self.casual_editing_comment_id = None; - { - use xai_grok_telemetry::events::PlanSubmit; - use xai_grok_telemetry::session_ctx::log_event; - log_event(PlanSubmit { - action: "abandon".to_string(), - }); - } - InputOutcome::Changed + log_plan_submit(action); } fn send_plan_feedback(&mut self, feedback: Option) -> InputOutcome { let Some(mut pav) = self.plan_approval_view.take() else { @@ -260,13 +266,7 @@ impl AgentView { self.line_viewer = None; self.prompt.textarea.cancel_undo_group(); self.show_toast("Plan revision sent."); - { - use xai_grok_telemetry::events::PlanSubmit; - use xai_grok_telemetry::session_ctx::log_event; - log_event(PlanSubmit { - action: "revise".to_string(), - }); - } + log_plan_submit("revise"); InputOutcome::Changed } pub(crate) fn reopen_plan_approval(&mut self) { @@ -993,3 +993,84 @@ mod plan_approval_enter_tests { assert_eq!(agent.prompt.text(), "a"); } } +/// The mode indicator renders +/// `plan_mode_pending.unwrap_or(plan_mode_active)`, and the shell's +/// confirming `CurrentModeUpdate("default")` only arrives after the exit +/// tool runs (and can be lost entirely). Resolving the review with a +/// decision must therefore optimistically clear the effective plan mode +/// on BOTH decision paths — approve and abandon. +#[cfg(test)] +mod plan_approval_optimistic_mode_tests { + use super::test_fixtures::make_agent; + use super::*; + use agent_client_protocol as acp; + fn agent_in_plan_mode_with_approval() -> ( + AgentView, + tokio::sync::oneshot::Receiver>, + ) { + let mut agent = make_agent(); + agent.plan_mode_active = true; + let (tx, rx) = tokio::sync::oneshot::channel(); + let request = crate::views::plan_approval_view::ExitPlanModeExtRequest { + session_id: "test-session".into(), + tool_call_id: "call-1".into(), + plan_content: Some("# Plan\n\n## Step 1\nDo something".into()), + }; + let pav = crate::views::plan_approval_view::PlanApprovalViewState::new( + request, + agent.prompt.stash(), + tx, + ); + agent.plan_approval_view = Some(pav); + (agent, rx) + } + fn effective_plan_mode(agent: &AgentView) -> bool { + agent.plan_mode_pending.unwrap_or(agent.plan_mode_active) + } + #[test] + fn approve_plan_optimistically_clears_plan_mode() { + let (mut agent, mut rx) = agent_in_plan_mode_with_approval(); + assert!(effective_plan_mode(&agent)); + agent.approve_plan(); + assert_eq!(agent.plan_mode_pending, Some(false)); + assert!( + !effective_plan_mode(&agent), + "indicator must leave plan mode immediately on approve, \ + not wait for the shell's CurrentModeUpdate" + ); + let raw = rx + .try_recv() + .expect("approval response must be sent") + .expect("Ok"); + let parsed: serde_json::Value = serde_json::from_str(raw.0.get()).unwrap(); + assert_eq!(parsed["outcome"], "approved"); + } + /// Approve with review comments takes the early `Action::Interject` + /// return — the optimistic clear must happen before that branch. + #[test] + fn approve_plan_with_comments_still_clears_plan_mode() { + let (mut agent, _rx) = agent_in_plan_mode_with_approval(); + if let Some(ref mut pav) = agent.plan_approval_view { + pav.comments + .push(crate::views::plan_approval_view::PlanComment { + id: 1, + line_range: 1..2, + text: "use the existing helper".into(), + }); + } + let outcome = agent.approve_plan(); + assert!(matches!( + outcome, + InputOutcome::Action(Action::Interject { .. }) + )); + assert_eq!(agent.plan_mode_pending, Some(false)); + assert!(!effective_plan_mode(&agent)); + } + #[test] + fn abandon_plan_optimistically_clears_plan_mode() { + let (mut agent, _rx) = agent_in_plan_mode_with_approval(); + agent.abandon_plan(); + assert_eq!(agent.plan_mode_pending, Some(false)); + assert!(!effective_plan_mode(&agent)); + } +} diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/render.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/render.rs index 786c543..cf7ca7f 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/render.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/render.rs @@ -131,6 +131,36 @@ impl AgentView { PlanApprovalFocus::Preview => vec![HintItem::new(key!('y'), "copy plan")], } } + /// Shortcut hints for an open `ask_user_question` card. + fn question_shortcut_hints( + &self, + qv: &crate::views::question_view::QuestionViewState, + ) -> Vec { + use crate::views::question_view::QuestionFocus; + match qv.focus { + QuestionFocus::InputMode if self.prompt.file_search_visible() => { + vec![ + HintItem::paired(key!(Up), key!(Down), "nav"), + HintItem::new(key!(Tab), "accept"), + HintItem::new(key!(Right), "drill"), + HintItem::new(key!(Esc), "dismiss"), + ] + } + QuestionFocus::InputMode => { + vec![ + HintItem::new(key!(Enter), "submit"), + HintItem::new(key!(Esc), "back"), + ] + } + QuestionFocus::Navigation => { + vec![ + HintItem::new(key!(Tab), "next answer"), + HintItem::new(key!(Esc), "unselect"), + HintItem::new(key!('X'), "dismiss"), + ] + } + } + } /// Returns the *exact* hints the bottom shortcuts bar would render right now. /// /// Single source of truth for context-sensitive shortcuts (pane, overlays, @@ -165,6 +195,12 @@ impl AgentView { HintItem::new(key!(Esc), "back"), ] } + PermissionFocus::PatternEdit => { + vec![ + HintItem::new(key!(Enter), "save"), + HintItem::new(key!(Esc), "cancel"), + ] + } PermissionFocus::Options => { use crate::input::key::KeyShortcut; use crossterm::event::{KeyCode, KeyModifiers}; @@ -175,6 +211,9 @@ impl AgentView { if perm.has_adjustable_scope() { hints.push(HintItem::paired(key!(Left), key!(Right), "scope")); } + if perm.has_editable_bash_pattern() { + hints.push(HintItem::new(key!('e'), "edit pattern")); + } if !perm.description.is_empty() { let label = if perm.args_expanded { "collapse" @@ -218,31 +257,7 @@ impl AgentView { h } } else if let Some(ref qv) = self.question_view { - use crate::views::question_view::QuestionFocus; - match qv.focus { - QuestionFocus::InputMode => { - if self.prompt.file_search_visible() { - vec![ - HintItem::paired(key!(Up), key!(Down), "nav"), - HintItem::new(key!(Tab), "accept"), - HintItem::new(key!(Right), "drill"), - HintItem::new(key!(Esc), "dismiss"), - ] - } else { - vec![ - HintItem::new(key!(Enter), "submit"), - HintItem::new(key!(Esc), "back"), - ] - } - } - QuestionFocus::Navigation => { - vec![ - HintItem::new(key!(Esc), "unselect"), - HintItem::new(key!(Tab), "scrollback"), - HintItem::new(key!('X'), "dismiss"), - ] - } - } + self.question_shortcut_hints(qv) } else if self.cancel_turn_view.is_some() { vec![ HintItem::paired(key!('1'), key!('4'), "select"), @@ -1518,6 +1533,7 @@ impl AgentView { self.hit_upgrade_cta .set_unless_dropdown(upgrade_cta_rect, dropdown_open); let mut inline_edit_cursor: Option<(u16, u16)> = None; + let sticky_gap_row: Option; { self.sync_pending_user_input_marks(); self.scrollback.set_cwd(Some(self.session.cwd.clone())); @@ -1551,6 +1567,7 @@ impl AgentView { scratch, ); let sb_output = sb_rendered.output; + sticky_gap_row = sb_output.sticky_gap_row; self.update_scrollback_selection_state( sb_output.selection_model.clone(), sb_rendered.selection_boundaries, @@ -1745,6 +1762,8 @@ impl AgentView { } } } + let mut follow_indicator_y: Option = None; + let mut response_top_indicator_y: Option = None; if self.block_viewer.is_none() && !search_active { use crate::appearance::FollowIndicator; let gap_y = layout.scrollback.y + layout.scrollback.height; @@ -1771,33 +1790,35 @@ impl AgentView { } } } - let show_indicator = appearance.scrollback.scroll.follow_indicator - != FollowIndicator::None - && !self.scrollback.is_follow_mode() - && self.scrollback.has_content_below() - && content_line_y.is_none(); - if show_indicator { - let center_x = gap_x + gap_w / 2; - let indicator_style = - ratatui::style::Style::default().fg(if self.hit_follow_indicator.hovered { - theme.gray_bright - } else { - theme.gray - }); - if let Some(cell) = buf.cell_mut((center_x, gap_y)) { - cell.set_symbol("▼"); - cell.set_style(indicator_style); + if appearance.scrollback.scroll.follow_indicator != FollowIndicator::None { + if !self.scrollback.is_follow_mode() + && self.scrollback.has_content_below() + && content_line_y.is_none() + { + follow_indicator_y = Some(gap_y); + } + if self.scrollback.has_response_top_above() { + response_top_indicator_y = sticky_gap_row.map(|row| layout.scrollback.y + row); } - self.hit_follow_indicator.set(Some(Rect::new( - center_x.saturating_sub(1), - gap_y, - 3, - 1, - ))); - } else { - self.hit_follow_indicator.clear(); } } + let indicator_center_x = layout.scrollback.x + layout.scrollback.width / 2; + draw_scroll_arrow( + buf, + &theme, + indicator_center_x, + follow_indicator_y, + "▼", + &mut self.hit_follow_indicator, + ); + draw_scroll_arrow( + buf, + &theme, + indicator_center_x, + response_top_indicator_y, + "▲", + &mut self.hit_response_top_indicator, + ); if let Some(msg) = self.active_toast_message() { let sb = layout.scrollback; if let Some(toast_text) = fit_toast_text(msg, sb.width) { @@ -2345,6 +2366,7 @@ impl AgentView { perm_area, perm, followup_text, + self.permission_pattern_edit.as_ref(), self.hovered_permission_item, &theme, prompt_focused, @@ -3171,6 +3193,12 @@ impl AgentView { HintItem::new(key!(Esc), "back"), ] } + PermissionFocus::PatternEdit => { + vec![ + HintItem::new(key!(Enter), "save"), + HintItem::new(key!(Esc), "cancel"), + ] + } PermissionFocus::Options => { use crate::input::key::KeyShortcut; use crossterm::event::{KeyCode, KeyModifiers}; @@ -3181,6 +3209,9 @@ impl AgentView { if perm.has_adjustable_scope() { hints.push(HintItem::paired(key!(Left), key!(Right), "scope")); } + if perm.has_editable_bash_pattern() { + hints.push(HintItem::new(key!('e'), "edit pattern")); + } if !perm.description.is_empty() { let label = if perm.args_expanded { "collapse" @@ -3236,32 +3267,7 @@ impl AgentView { .render(layout.shortcuts, buf); } } else if let Some(ref qv) = self.question_view { - use crate::views::question_view::QuestionFocus; - use crate::views::shortcuts_bar::HintItem; - let hints = match qv.focus { - QuestionFocus::InputMode => { - if self.prompt.file_search_visible() { - vec![ - HintItem::paired(key!(Up), key!(Down), "nav"), - HintItem::new(key!(Tab), "accept"), - HintItem::new(key!(Right), "drill"), - HintItem::new(key!(Esc), "dismiss"), - ] - } else { - vec![ - HintItem::new(key!(Enter), "submit"), - HintItem::new(key!(Esc), "back"), - ] - } - } - QuestionFocus::Navigation => { - vec![ - HintItem::new(key!(Esc), "unselect"), - HintItem::new(key!(Tab), "scrollback"), - HintItem::new(key!('X'), "dismiss"), - ] - } - }; + let hints = self.question_shortcut_hints(qv); ShortcutsBar::new(&hints).render(layout.shortcuts, buf); } else if self.cancel_turn_view.is_some() { use crate::views::shortcuts_bar::HintItem; @@ -3319,7 +3325,7 @@ impl AgentView { let is_plan_viewer = self.is_plan_viewer(); let has_plan_comments = !self.plan_comments.is_empty(); let casual_commenting = self.is_casual_commenting(); - if let Some(ref mut viewer) = self.line_viewer { + if self.line_viewer.is_some() { use crate::views::file_search::line_viewer::render_line_viewer; use crate::views::shortcuts_bar::HintItem; let plan_prompt_focused = self @@ -3349,19 +3355,35 @@ impl AgentView { } else { self.plan_comments.len() }; - if let Some(ref pav) = self.plan_approval_view { - viewer.plan_mut().active_commenting_range = pav.commenting_range.clone(); - } else { - viewer.plan_mut().active_commenting_range = self.casual_commenting_range.clone(); - } - render_line_viewer( - buf, - overlay_area, - viewer, - &self.session.cwd, - &theme, - effective_comment_count, - ); + let mermaid_placements = self + .line_viewer + .as_mut() + .map(|viewer| { + if let Some(ref pav) = self.plan_approval_view { + viewer.plan_mut().active_commenting_range = pav.commenting_range.clone(); + } else { + viewer.plan_mut().active_commenting_range = + self.casual_commenting_range.clone(); + } + render_line_viewer( + buf, + overlay_area, + viewer, + &self.session.cwd, + &theme, + effective_comment_count, + ); + viewer + .last_popup_area + .map(|area| viewer.diagram_affordance_placements(area)) + .unwrap_or_default() + }) + .unwrap_or_default(); + self.inline_media_hits = super::InlineMediaHitAreas::default(); + self.paint_diagram_affordances(buf, mermaid_placements, &theme); + let Some(viewer) = self.line_viewer.as_mut() else { + return (prompt_cursor_pos, prompt_post_flush); + }; let toast_area = viewer .last_popup_area .or(viewer.last_modal_area) @@ -4361,6 +4383,34 @@ impl AgentView { (cursor, prompt_post_flush) } } +/// Draw one ▼/▲ scroll-indicator arrow centered on row `y`, or clear its +/// hit area when hidden (`y: None`). The unconditional set-or-clear is the +/// point: a hit rect must never outlive the frame that painted its arrow, +/// or an invisible click target keeps firing under whatever covers it +/// (e.g. an open block viewer). +fn draw_scroll_arrow( + buf: &mut Buffer, + theme: &Theme, + center_x: u16, + y: Option, + symbol: &str, + hit: &mut super::HitArea, +) { + let Some(y) = y else { + hit.clear(); + return; + }; + let style = Style::default().fg(if hit.hovered { + theme.gray_bright + } else { + theme.gray + }); + if let Some(cell) = buf.cell_mut((center_x, y)) { + cell.set_symbol(symbol); + cell.set_style(style); + } + hit.set(Some(Rect::new(center_x.saturating_sub(1), y, 3, 1))); +} /// Pad `msg` for the toast slot, truncating with a trailing ellipsis when it /// cannot fit in `avail_width` columns (long clipboard toasts embed backup /// file paths — dropping the whole toast would hide the copy feedback diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs index 8cd245d..b94d541 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/session.rs @@ -200,6 +200,7 @@ impl AgentView { hit_plan_button: Default::default(), hit_plan_approval_status: Default::default(), hit_follow_indicator: Default::default(), + hit_response_top_indicator: Default::default(), hit_cwd: Default::default(), hit_cancel_button: Default::default(), hit_watching_cue: Default::default(), @@ -276,6 +277,7 @@ impl AgentView { next_perm_req_id: 0, permission_stashed_prompt: None, permission_stashed_pane: None, + permission_pattern_edit: None, plan_approval_view: None, latest_inline_plan_content: None, plan_comments: Vec::new(), diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/viewer.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/viewer.rs index 859d905..b746660 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/viewer.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/viewer.rs @@ -8,7 +8,7 @@ use crate::scrollback::selection::SelectionBox; use crate::scrollback::types::DisplayMode; use crate::theme::Theme; use crate::views::btw_overlay::BTW_OVERLAY_ENTRY_IDX; -use crate::views::file_search::line_viewer::LineViewerState; +use crate::views::file_search::line_viewer::{LineViewerState, PlanViewerItem}; use crate::views::list_pane::ListItem; use crate::views::plan_approval_view::PlanApprovalFocus; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; @@ -415,6 +415,47 @@ impl AgentView { let is_plan_preview = viewer.kind == crate::views::file_search::line_viewer::LineViewerKind::PlanPreview; + let scrollbar_owns_gesture = match mouse.kind { + MouseEventKind::Down(MouseButton::Left) => { + viewer.list_state.scrollbar_hit(mouse.column, mouse.row) + } + MouseEventKind::Drag(MouseButton::Left) | MouseEventKind::Up(MouseButton::Left) => { + viewer.list_state.is_scrollbar_dragging() + } + _ => false, + }; + if scrollbar_owns_gesture { + viewer.list_state.handle_mouse_event( + mouse.kind, + mouse.column, + mouse.row, + popup_area.unwrap_or_default(), + &viewer.lines, + ); + if is_plan_preview { + viewer.plan_mut().gutter_drag_start = None; + viewer.plan_mut().gutter_drag_end = None; + } + if matches!(mouse.kind, MouseEventKind::Down(MouseButton::Left)) { + let was_commenting = self + .plan_approval_view + .as_ref() + .is_some_and(|pav| pav.focus == PlanApprovalFocus::Commenting); + if let Some(ref mut pav) = self.plan_approval_view { + pav.focus = PlanApprovalFocus::Preview; + if was_commenting { + pav.commenting_range = None; + pav.editing_comment_id = None; + pav.stashed_feedback_prompt = None; + } + } + if was_commenting { + self.prompt.set_text(""); + } + } + return InputOutcome::Changed; + } + match mouse.kind { MouseEventKind::Down(MouseButton::Left) => { // Click on close button -> cancel. @@ -470,6 +511,19 @@ impl AgentView { } return self.send_casual_plan_comments(); } + // Mermaid buttons before click-to-comment (early return ends + // the `viewer` borrow so `handle_inline_media_click` can take + // `&mut self`). + let mermaid_hit = self + .inline_media_hits + .mermaid_buttons + .iter() + .any(|(rect, _, _)| rect.contains((mouse.column, mouse.row).into())); + if mermaid_hit { + return self + .handle_inline_media_click(mouse.column, mouse.row) + .unwrap_or(InputOutcome::Changed); + } if modal_area.is_none_or(|a| !a.contains((mouse.column, mouse.row).into())) { if self.plan_approval_view.is_some() && self @@ -509,6 +563,20 @@ impl AgentView { } MouseEventKind::Moved => { let mut changed = false; + // Redraw only when mermaid button hover would change. + if self.last_mouse_pos != (mouse.column, mouse.row) { + let old = self.last_mouse_pos; + self.last_mouse_pos = (mouse.column, mouse.row); + let hits = |col: u16, row: u16| { + self.inline_media_hits + .mermaid_buttons + .iter() + .any(|(rect, _, _)| rect.contains((col, row).into())) + }; + if hits(old.0, old.1) || hits(mouse.column, mouse.row) { + changed = true; + } + } let close_hover = close_area.is_some_and(|a| a.contains((mouse.column, mouse.row).into())); if close_hover != viewer.close_hovered { @@ -684,10 +752,21 @@ impl AgentView { // edit-comment) for that row. Same shortcut as // selecting + pressing `c` / Enter. Works for both // plan-approval and casual plan-preview modes. + // Skip Mermaid affordance rows (button hits handled above). let on_list_row = mouse.row >= area.y && { let ry = (mouse.row - area.y) as usize; let vy = viewer.list_state.scroll_offset() + ry; - viewer.list_state.layout().item_at_y(vy).is_some() + viewer + .list_state + .layout() + .item_at_y(vy) + .map(|vi| { + let pi = viewer.list_state.to_physical(vi); + viewer.lines.get(pi).is_some_and(|item| { + !matches!(item, PlanViewerItem::MermaidAffordance(_)) + }) + }) + .unwrap_or(false) }; // Skip the click-to-comment trigger if the user is // already composing a comment. Without this guard, any @@ -1009,3 +1088,7 @@ impl AgentView { }) } } + +#[cfg(test)] +#[path = "viewer_tests.rs"] +mod tests; diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/viewer_tests.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/viewer_tests.rs new file mode 100644 index 0000000..4e3c992 --- /dev/null +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/viewer_tests.rs @@ -0,0 +1,460 @@ +//! Mouse-routing tests for the line viewer's plan preview: the scrollbar +//! must own a click+drag gesture end-to-end. A press on the track was +//! previously also treated as a comment-gutter anchor (row-only hit test), +//! so dragging the thumb selected plan lines for a comment instead of +//! scrolling (GB-4579: "can't click and drag scrollbar to view plan"). + +use crossterm::event::{Event, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; +use ratatui::layout::Rect; + +use crate::actions::ActionRegistry; +use crate::app::agent_view::AgentView; +use crate::app::agent_view::test_fixtures::make_agent; +use crate::views::plan_approval_view::PlanApprovalFocus; + +const POPUP: Rect = Rect { + x: 0, + y: 0, + width: 80, + height: 10, +}; +/// Scrollbar track column as split off by the list pane render +/// (`maybe_split_for_scrollbar`): last column of the popup area. +const TRACK_X: u16 = 79; + +fn mouse(kind: MouseEventKind, col: u16, row: u16) -> Event { + Event::Mouse(MouseEvent { + kind, + column: col, + row, + modifiers: KeyModifiers::empty(), + }) +} + +/// Agent showing a plan-approval preview whose plan overflows the +/// viewport, with the render-time areas planted so mouse dispatch works. +fn agent_with_scrollable_plan() -> AgentView { + let mut agent = make_agent(); + let (tx, _rx) = tokio::sync::oneshot::channel(); + let plan: String = (1..=60).fold(String::new(), |mut acc, i| { + acc.push_str(&format!("step {i}\n")); + acc + }); + let request = crate::views::plan_approval_view::ExitPlanModeExtRequest { + session_id: "test-session".into(), + tool_call_id: "call-1".into(), + plan_content: Some(plan), + }; + agent.plan_approval_view = Some( + crate::views::plan_approval_view::PlanApprovalViewState::new( + request, + crate::views::prompt_widget::StashedPrompt { + text: String::new(), + cursor: 0, + images: Vec::new(), + chip_elements: Vec::new(), + image_counter: 0, + image_undo_stash: Vec::new(), + }, + tx, + ), + ); + agent.show_plan_preview(); + + let viewer = agent + .line_viewer + .as_mut() + .expect("plan preview opens the line viewer"); + viewer.prepare_layout(POPUP.width, POPUP.height); + viewer.last_popup_area = Some(POPUP); + viewer.last_modal_area = Some(Rect::new(0, 0, 80, 12)); + viewer + .list_state + .set_scrollbar_area(Some(Rect::new(TRACK_X, POPUP.y, 1, POPUP.height))); + assert!( + viewer.list_state.total_height() > POPUP.height as usize, + "plan must overflow the viewport so the scrollbar is live" + ); + agent +} + +/// Presses on the modal border column next to the track (users read the +/// thumb + border as one two-column scrollbar) used to fall into the +/// click-outside-modal path instead of grabbing the thumb. +#[test] +fn border_column_press_grabs_scrollbar() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X + 1, 5), + ®istry, + ); + + let viewer = agent.line_viewer.as_ref().expect("viewer stays open"); + assert!( + viewer.list_state.is_scrollbar_dragging(), + "press one column right of the track (modal border) must grab the thumb" + ); + assert!( + viewer.list_state.scroll_offset() > 0, + "the press must scroll toward the clicked track position" + ); + assert!( + viewer + .plan_ref() + .and_then(|p| p.gutter_drag_start) + .is_none(), + "a border-column press must not anchor a comment-gutter drag" + ); + let pav = agent.plan_approval_view.as_ref().unwrap(); + assert_eq!(pav.focus, PlanApprovalFocus::Preview); + + let offset_after_press = agent + .line_viewer + .as_ref() + .unwrap() + .list_state + .scroll_offset(); + let _ = agent.handle_input( + &mouse(MouseEventKind::Drag(MouseButton::Left), TRACK_X + 1, 9), + ®istry, + ); + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!( + viewer.list_state.scroll_offset() > offset_after_press, + "dragging on the border column must keep scrolling (offset {} -> {})", + offset_after_press, + viewer.list_state.scroll_offset() + ); +} + +#[test] +fn gap_column_press_grabs_scrollbar() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X - 1, 5), + ®istry, + ); + + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!( + viewer.list_state.is_scrollbar_dragging(), + "press on the gap column must grab the thumb" + ); + assert!( + viewer + .plan_ref() + .and_then(|p| p.gutter_drag_start) + .is_none(), + "a gap-column press must not anchor a comment-gutter drag" + ); +} + +#[test] +fn border_column_press_does_not_close_casual_preview() { + let mut agent = agent_with_scrollable_plan(); + agent.plan_approval_view = None; + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X + 1, 5), + ®istry, + ); + + let viewer = agent + .line_viewer + .as_ref() + .expect("a border-column press must not close the casual preview"); + assert!(viewer.list_state.is_scrollbar_dragging()); +} + +#[test] +fn press_beyond_grab_zone_still_closes_casual_preview() { + let mut agent = agent_with_scrollable_plan(); + agent.plan_approval_view = None; + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X + 2, 5), + ®istry, + ); + + assert!( + agent.line_viewer.is_none(), + "a click two columns right of the track is outside the modal and must close it" + ); +} + +#[test] +fn scrollbar_press_does_not_enter_commenting() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X, 5), + ®istry, + ); + + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!( + viewer.list_state.is_scrollbar_dragging(), + "press on the track must latch a scrollbar drag" + ); + assert!( + viewer + .plan_ref() + .and_then(|p| p.gutter_drag_start) + .is_none(), + "press on the track must not anchor a comment-gutter drag" + ); + let pav = agent.plan_approval_view.as_ref().unwrap(); + assert_eq!( + pav.focus, + PlanApprovalFocus::Preview, + "press on the track must not enter commenting" + ); +} + +#[test] +fn scrollbar_drag_scrolls_plan_instead_of_selecting_lines() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X, 2), + ®istry, + ); + let offset_after_press = agent + .line_viewer + .as_ref() + .unwrap() + .list_state + .scroll_offset(); + + // Drag the thumb to the bottom of the track. + let _ = agent.handle_input( + &mouse(MouseEventKind::Drag(MouseButton::Left), TRACK_X, 9), + ®istry, + ); + + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!( + viewer.list_state.scroll_offset() > offset_after_press, + "dragging the thumb down must scroll the plan (offset {} -> {})", + offset_after_press, + viewer.list_state.scroll_offset() + ); + assert!( + viewer.plan_ref().and_then(|p| p.gutter_drag_end).is_none(), + "thumb drag must not extend a comment line selection" + ); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Up(MouseButton::Left), TRACK_X, 9), + ®istry, + ); + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!( + !viewer.list_state.is_scrollbar_dragging(), + "release must end the scrollbar drag" + ); + let pav = agent.plan_approval_view.as_ref().unwrap(); + assert_eq!( + pav.commenting_range, None, + "releasing the thumb must not open a comment on the dragged lines" + ); + assert_eq!(pav.focus, PlanApprovalFocus::Preview); +} + +/// The thumb must keep following the pointer when a drag drifts off the +/// popup rect (standard scrollbar behavior in every toolkit). +#[test] +fn scrollbar_drag_outside_popup_keeps_scrolling() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X, 8), + ®istry, + ); + let offset_after_press = agent + .line_viewer + .as_ref() + .unwrap() + .list_state + .scroll_offset(); + assert!(offset_after_press > 0, "press near the bottom scrolls down"); + + // Pointer drifts left of the track and above the popup while dragging. + let _ = agent.handle_input( + &mouse(MouseEventKind::Drag(MouseButton::Left), 40, 0), + ®istry, + ); + + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!( + viewer.list_state.scroll_offset() < offset_after_press, + "drag toward the top of the track must scroll back up (offset {} -> {})", + offset_after_press, + viewer.list_state.scroll_offset() + ); + assert!( + viewer.plan_ref().and_then(|p| p.gutter_drag_end).is_none(), + "scrollbar drag must never turn into a comment line selection" + ); +} + +/// A gutter line-selection whose Up was lost must not survive a later +/// scrollbar gesture: the track press drops the stale anchor, so a stray +/// release afterwards cannot commit the leftover lines as a comment. +#[test] +fn scrollbar_gesture_drops_stale_gutter_anchor() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + // Anchor + extend a comment line selection, then lose the Up. + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), 10, 4), + ®istry, + ); + let _ = agent.handle_input( + &mouse(MouseEventKind::Drag(MouseButton::Left), 10, 6), + ®istry, + ); + { + let viewer = agent.line_viewer.as_ref().unwrap(); + let start = viewer.plan_ref().and_then(|p| p.gutter_drag_start); + let end = viewer.plan_ref().and_then(|p| p.gutter_drag_end); + assert!( + start.is_some() && end.is_some() && start != end, + "precondition: a multi-line gutter drag is live (start {start:?}, end {end:?})" + ); + } + // Scrollbar click + release: the track press must drop the stale anchor. + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X, 5), + ®istry, + ); + { + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!(viewer.list_state.is_scrollbar_dragging()); + assert!( + viewer + .plan_ref() + .and_then(|p| p.gutter_drag_start) + .is_none() + && viewer.plan_ref().and_then(|p| p.gutter_drag_end).is_none(), + "track press must drop a stale comment-gutter anchor" + ); + } + let _ = agent.handle_input( + &mouse(MouseEventKind::Up(MouseButton::Left), TRACK_X, 5), + ®istry, + ); + + // The track press also discarded the in-progress comment draft + // (same rule as clicking back into the modal). + let pav = agent.plan_approval_view.as_ref().unwrap(); + assert_eq!(pav.commenting_range, None); + assert_eq!(pav.focus, PlanApprovalFocus::Preview); + + // A stray release on content must not commit the leftover lines. + let _ = agent.handle_input( + &mouse(MouseEventKind::Up(MouseButton::Left), 10, 6), + ®istry, + ); + let pav = agent.plan_approval_view.as_ref().unwrap(); + assert_eq!( + pav.commenting_range, None, + "stale gutter lines must not be committed as a comment range" + ); + assert_eq!( + pav.focus, + PlanApprovalFocus::Preview, + "a stray release must not re-enter commenting" + ); +} + +/// A lost mouse-up after a track press must not make the next plan-line +/// click skip gutter / click-to-comment (sticky `is_scrollbar_dragging`). +#[test] +fn lost_scrollbar_up_does_not_block_next_line_click() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X, 5), + ®istry, + ); + assert!( + agent + .line_viewer + .as_ref() + .unwrap() + .list_state + .is_scrollbar_dragging(), + "precondition: track press latched a thumb drag" + ); + + // No Up — simulate a dropped release, then click a plan line. + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), 10, 4), + ®istry, + ); + + let viewer = agent.line_viewer.as_ref().unwrap(); + assert!( + !viewer.list_state.is_scrollbar_dragging(), + "content Down must clear the stale scrollbar latch" + ); + assert!( + viewer + .plan_ref() + .and_then(|p| p.gutter_drag_start) + .is_some(), + "content Down must still anchor a comment-gutter drag" + ); + let pav = agent.plan_approval_view.as_ref().unwrap(); + assert_eq!( + pav.focus, + PlanApprovalFocus::Commenting, + "content Down must still enter click-to-comment" + ); +} + +#[test] +fn wheel_on_border_column_scrolls_plan() { + let mut agent = agent_with_scrollable_plan(); + let registry = ActionRegistry::defaults(); + + let _ = agent.handle_input( + &mouse(MouseEventKind::Down(MouseButton::Left), TRACK_X + 1, 9), + ®istry, + ); + let _ = agent.handle_input( + &mouse(MouseEventKind::Up(MouseButton::Left), TRACK_X + 1, 9), + ®istry, + ); + let off = agent + .line_viewer + .as_ref() + .unwrap() + .list_state + .scroll_offset(); + assert!(off > 0, "border click near track bottom scrolls down"); + + agent.handle_scroll(-3, TRACK_X + 1, 5); + let off_after = agent + .line_viewer + .as_ref() + .unwrap() + .list_state + .scroll_offset(); + assert!( + off_after < off, + "wheel-up on the border column must scroll up ({off} -> {off_after})" + ); +} diff --git a/crates/codegen/xai-grok-pager/src/app/app_view.rs b/crates/codegen/xai-grok-pager/src/app/app_view.rs index f696eeb..43cab82 100644 --- a/crates/codegen/xai-grok-pager/src/app/app_view.rs +++ b/crates/codegen/xai-grok-pager/src/app/app_view.rs @@ -3525,9 +3525,10 @@ fn handle_welcome_input(ev: &Event, ctx: &mut WelcomeInputCtx<'_>) -> InputOutco None => return InputOutcome::Changed, }, PickerOutcome::SubmitQuery => { - let query = ctx.sp_state.query().trim().to_string(); - if !query.is_empty() { - return InputOutcome::Action(Action::LoadSession(query, None, false)); + if let Some(sid) = + crate::views::session_picker::session_id_for_direct_load(ctx.sp_state.query()) + { + return InputOutcome::Action(Action::LoadSession(sid.to_string(), None, false)); } return InputOutcome::Unchanged; } diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs index 213465b..94faedf 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/dashboard.rs @@ -2367,41 +2367,10 @@ pub(super) fn dispatch_dashboard_permission_select( return vec![]; }; - let meta = if let Some(scope) = perm - .mcp_scope - .as_ref() - .filter(|_| option_id.0.as_ref() == "allow-always-mcp") - { - let selection = match scope.selected { - crate::views::permission_view::McpScope::Tool => { - xai_grok_workspace::permission::McpScopeSelection::Tool { - tool_name: scope.tool_name.clone(), - } - } - crate::views::permission_view::McpScope::Server => match &scope.server_prefix { - Some(prefix) => xai_grok_workspace::permission::McpScopeSelection::Server { - server: prefix.clone(), - }, - None => xai_grok_workspace::permission::McpScopeSelection::Tool { - tool_name: scope.tool_name.clone(), - }, - }, - }; - serde_json::to_value(selection) - .ok() - .and_then(|v| v.as_object().cloned()) - } else if let Some(ref h) = perm.bash_highlights - && perm.bash_selection_count > 0 - { - let parts: Vec = h.highlighted_words[..perm.bash_selection_count].to_vec(); - serde_json::to_value(xai_grok_workspace::permission::BashCommandSelectedTerms { - command_parts: parts, - }) - .ok() - .and_then(|v| v.as_object().cloned()) - } else { - None - }; + // Share the main dispatch's meta logic so dashboard peek honors an edited + // pattern (and the glob routing) identically instead of dropping it. + let edited_pattern = super::permissions::take_edited_pattern(agent, &perm); + let meta = super::permissions::build_selection_meta(&perm, &option_id, edited_pattern); perm.request .response_tx diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/permissions.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/permissions.rs index f76e797..61c0c92 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/permissions.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/permissions.rs @@ -10,6 +10,104 @@ use agent_client_protocol as acp; // Permission dispatch // --------------------------------------------------------------------------- +use crate::views::permission_view::{McpScope, PermissionFocus, PermissionViewState}; +use xai_grok_workspace::permission::{BashCommandSelectedTerms, McpScopeSelection}; + +/// Free-form pattern taken from the editor on confirm. +pub(super) struct EditedPattern { + pub pattern: String, + /// True when the buffer was mutated — routes to `allowed_bash_globs`. + pub is_glob: bool, +} + +/// Take the free-form pattern-editor buffer, honoring it only when the resolved +/// request was in `PatternEdit` focus (and always clearing it, so an abandoned +/// edit can't leak into a later prompt). +pub(super) fn take_edited_pattern( + agent: &mut AgentView, + perm: &PermissionViewState, +) -> Option { + agent + .permission_pattern_edit + .take() + .filter(|_| perm.focus == PermissionFocus::PatternEdit) + .and_then(|e| { + e.trimmed().map(|s| EditedPattern { + pattern: s.to_owned(), + is_glob: e.is_dirty(), + }) + }) +} + +/// Build the ACP response meta for a permission selection — the single source +/// of truth shared by the main and dashboard dispatch paths. MCP scope wins for +/// the `allow-always-mcp` id; otherwise a free-form edited pattern (glob when +/// dirty) wins over the arrow word-scope (literal prefix). `None` when there is +/// nothing to scope. +pub(super) fn build_selection_meta( + perm: &PermissionViewState, + option_id: &acp::PermissionOptionId, + edited: Option, +) -> Option> { + let obj = |v: serde_json::Value| v.as_object().cloned(); + + if let Some(scope) = perm + .mcp_scope + .as_ref() + .filter(|_| option_id.0.as_ref() == "allow-always-mcp") + { + let selection = match scope.selected { + McpScope::Tool => McpScopeSelection::Tool { + tool_name: scope.tool_name.clone(), + }, + // Defensive: render disables Server when there is no prefix. + McpScope::Server => match &scope.server_prefix { + Some(prefix) => McpScopeSelection::Server { + server: prefix.clone(), + }, + None => McpScopeSelection::Tool { + tool_name: scope.tool_name.clone(), + }, + }, + }; + return serde_json::to_value(selection).ok().and_then(obj); + } + + if let Some(edited) = edited.filter(|_| { + // The editor authors an *allow* pattern, so apply it only to the bash + // allow-always option. A different selection (reject-always, allow-once) + // made while the editor is open must fall through to the arrow word-scope + // — otherwise the allow text would land in the deny set. + option_id.0.as_ref() == "allow-always-command" && perm.bash_highlights.is_some() + }) { + // Glob only when the editor is dirty. Unedited save is a literal grant + // of the pre-filled command, so metacharacters that came from the + // command itself (e.g. `find . -name *.rs`) stay literal. + return serde_json::to_value(BashCommandSelectedTerms { + command_parts: vec![edited.pattern], + is_glob: edited.is_glob, + }) + .ok() + .and_then(obj); + } + + if let Some(h) = perm + .bash_highlights + .as_ref() + .filter(|_| perm.bash_selection_count > 0) + { + // Arrow word-scope: a literal command prefix, never a glob. + return serde_json::to_value(BashCommandSelectedTerms { + command_parts: h.highlighted_words[..perm.bash_selection_count].to_vec(), + is_glob: false, + }) + .ok() + .and_then(obj); + } + + None +} + /// Handle permission option selection (AllowOnce, AllowAlways, RejectAlways). /// /// Pops the front request, sends the response, and handles queue transitions @@ -40,6 +138,8 @@ pub(super) fn dispatch_permission_select( return vec![]; }; + let edited_pattern = take_edited_pattern(agent, &perm); + // Detect the "enable always-approve mode" id BEFORE moving option_id // into the response. Cheap str compare on the `Arc` interior. let enable_always_approve = @@ -72,46 +172,7 @@ pub(super) fn dispatch_permission_select( ); } - // Build response meta. MCP and bash flows are mutually exclusive at - // the per-request level; check MCP first because it owns the - // `allow-always-mcp` option id and the bash branch is the existing - // fallback. - let meta = if let Some(scope) = perm - .mcp_scope - .as_ref() - .filter(|_| option_id.0.as_ref() == "allow-always-mcp") - { - let selection = match scope.selected { - crate::views::permission_view::McpScope::Tool => { - xai_grok_workspace::permission::McpScopeSelection::Tool { - tool_name: scope.tool_name.clone(), - } - } - crate::views::permission_view::McpScope::Server => match &scope.server_prefix { - Some(prefix) => xai_grok_workspace::permission::McpScopeSelection::Server { - server: prefix.clone(), - }, - // Defensive: render path should disable Server when no prefix. - None => xai_grok_workspace::permission::McpScopeSelection::Tool { - tool_name: scope.tool_name.clone(), - }, - }, - }; - serde_json::to_value(selection) - .ok() - .and_then(|v| v.as_object().cloned()) - } else if let Some(ref h) = perm.bash_highlights - && perm.bash_selection_count > 0 - { - let parts: Vec = h.highlighted_words[..perm.bash_selection_count].to_vec(); - serde_json::to_value(xai_grok_workspace::permission::BashCommandSelectedTerms { - command_parts: parts, - }) - .ok() - .and_then(|v| v.as_object().cloned()) - } else { - None - }; + let meta = build_selection_meta(&perm, &option_id, edited_pattern); perm.request .response_tx @@ -252,6 +313,9 @@ pub(super) fn drain_permission_queue(agent: &mut AgentView) { /// - Queue still has items → clear prompt text and reset next front to Options. pub(crate) fn resolve_permission_queue_transition(agent: &mut AgentView) { agent.last_permission_click = None; + // The pattern editor is front-request scoped: drop any buffer when the + // front request is resolved (covers cancel/followup/select paths). + agent.permission_pattern_edit = None; if agent.permission_queue.is_empty() { restore_permission_stashes(agent); } else { @@ -267,6 +331,7 @@ pub(crate) fn resolve_permission_queue_transition(agent: &mut AgentView) { /// Restore composer + pane stashes when the permission queue empties. pub(super) fn restore_permission_stashes(agent: &mut AgentView) { + agent.permission_pattern_edit = None; if let Some(stashed) = agent.permission_stashed_prompt.take() { agent.prompt.restore(stashed); } diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/permissions.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/permissions.rs index 4f1b577..95d248c 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/permissions.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/permissions.rs @@ -568,3 +568,214 @@ fn permission_select_reject_does_not_steer_sticky_cursor() { "reject selection must not steer the sticky cursor" ); } + +/// Push a bash "Always allow" prompt (id `allow-always-command`) whose +/// arrow-scope covers `gh api`, returning the response receiver. +fn push_bash_allow_always( + agent: &mut crate::app::agent_view::AgentView, + focus: crate::views::permission_view::PermissionFocus, +) -> tokio::sync::oneshot::Receiver> { + use crate::views::permission_view::PermissionViewState; + use std::sync::Arc; + use xai_grok_workspace::permission::bash_command_splitting::BashCommandHighlights; + + let (tx, rx) = tokio::sync::oneshot::channel(); + let request = acp::RequestPermissionRequest::new( + acp::SessionId::new(Arc::from("sess")), + acp::ToolCallUpdate::new( + acp::ToolCallId::new(Arc::from("tc")), + acp::ToolCallUpdateFields::default(), + ), + vec![ + acp::PermissionOption::new( + acp::PermissionOptionId::new(Arc::from("allow-always-command")), + "Always allow", + acp::PermissionOptionKind::AllowAlways, + ), + acp::PermissionOption::new( + acp::PermissionOptionId::new(Arc::from("reject-always-command")), + "Never allow", + acp::PermissionOptionKind::RejectAlways, + ), + ], + ); + let options = request.options.clone(); + agent.permission_queue.push_back(PermissionViewState { + request: xai_acp_lib::AcpArgs { + request, + response_tx: tx, + }, + id: 1, + focus, + options, + active_idx: 0, + bash_highlights: Some(BashCommandHighlights { + prefix: vec![], + highlighted_words: vec!["gh".into(), "api".into()], + suffix: vec![], + }), + bash_selection_count: 2, + bash_command_raw: Some("gh api repos/owner/repo/pulls".into()), + mcp_scope: None, + title: "Allow command?".into(), + description: vec![], + args_expanded: false, + desc_scroll: 0, + subagent_label: None, + options_area_height: 0, + options_scroll_offset: 0, + }); + rx +} + +fn selected_terms( + resp: acp::RequestPermissionResponse, +) -> xai_grok_workspace::permission::BashCommandSelectedTerms { + let meta = resp.meta.expect("bash selection meta"); + serde_json::from_value(serde_json::Value::Object(meta)).expect("selection terms") +} + +/// An edit abandoned before dispatch (focus back to `Options`, e.g. via a +/// mouse click) must not persist its buffer: the resolved rule uses the +/// arrow-scope words, and the buffer is cleared. +#[test] +fn abandoned_pattern_edit_is_not_persisted() { + use crate::views::permission_view::{PatternEditState, PermissionFocus}; + use std::sync::Arc; + + let mut app = test_app_with_agent(); + let agent = app.agents.get_mut(&AgentId(0)).unwrap(); + let mut rx = push_bash_allow_always(agent, PermissionFocus::Options); + agent.permission_pattern_edit = Some(PatternEditState::new("rm -rf /")); + + let _ = dispatch( + Action::PermissionSelect(acp::PermissionOptionId::new(Arc::from( + "allow-always-command", + ))), + &mut app, + ); + + assert!(app.agents[&AgentId(0)].permission_pattern_edit.is_none()); + let terms = selected_terms(rx.try_recv().expect("response").expect("ok")); + assert_eq!(terms.command_parts, vec!["gh", "api"]); + assert!(!terms.is_glob, "arrow-scope grant is literal, not a glob"); +} + +/// Seed a dirty editor buffer with the given text (insert+undo marks dirty). +fn dirty_pattern_edit(text: &str) -> crate::views::permission_view::PatternEditState { + let mut e = crate::views::permission_view::PatternEditState::new(text); + e.insert_char('x'); + e.backspace(); + debug_assert_eq!(e.buffer, text); + debug_assert!(e.is_dirty()); + e +} + +/// A pattern confirmed from `PatternEdit` focus is persisted verbatim as a glob +/// when the editor is dirty. +#[test] +fn confirmed_pattern_edit_is_persisted() { + use crate::views::permission_view::PermissionFocus; + use std::sync::Arc; + + let mut app = test_app_with_agent(); + let agent = app.agents.get_mut(&AgentId(0)).unwrap(); + let mut rx = push_bash_allow_always(agent, PermissionFocus::PatternEdit); + agent.permission_pattern_edit = Some(dirty_pattern_edit("gh api repos/owner/*")); + + let _ = dispatch( + Action::PermissionSelect(acp::PermissionOptionId::new(Arc::from( + "allow-always-command", + ))), + &mut app, + ); + + assert!(app.agents[&AgentId(0)].permission_pattern_edit.is_none()); + let terms = selected_terms(rx.try_recv().expect("response").expect("ok")); + assert_eq!(terms.command_parts, vec!["gh api repos/owner/*"]); + assert!(terms.is_glob, "dirty editor routes to the glob set"); +} + +/// A non-allow selection (e.g. reject-always) made while the editor is open +/// must not carry the edited *allow* text — it falls back to the arrow scope, so +/// the allow pattern can never land in the deny set. +#[test] +fn edited_pattern_not_applied_to_reject_option() { + use crate::views::permission_view::PermissionFocus; + use std::sync::Arc; + + let mut app = test_app_with_agent(); + let agent = app.agents.get_mut(&AgentId(0)).unwrap(); + let mut rx = push_bash_allow_always(agent, PermissionFocus::PatternEdit); + agent.permission_pattern_edit = Some(dirty_pattern_edit("gh api repos/owner/*")); + + let _ = dispatch( + Action::PermissionSelect(acp::PermissionOptionId::new(Arc::from( + "reject-always-command", + ))), + &mut app, + ); + + let terms = selected_terms(rx.try_recv().expect("response").expect("ok")); + assert_eq!( + terms.command_parts, + vec!["gh", "api"], + "reject must use arrow scope, not the edited allow pattern" + ); + assert!(!terms.is_glob); +} + +/// Opening the editor and saving without edits is an exact grant, not a glob — +/// so a metacharacter that came from the command itself is not a wildcard. +#[test] +fn unedited_pattern_edit_is_literal_not_glob() { + use crate::views::permission_view::{PatternEditState, PermissionFocus}; + use std::sync::Arc; + + let mut app = test_app_with_agent(); + let agent = app.agents.get_mut(&AgentId(0)).unwrap(); + let mut rx = push_bash_allow_always(agent, PermissionFocus::PatternEdit); + // Clean pre-fill (dirty=false); content may even contain metacharacters. + agent.permission_pattern_edit = Some(PatternEditState::new("gh api")); + + let _ = dispatch( + Action::PermissionSelect(acp::PermissionOptionId::new(Arc::from( + "allow-always-command", + ))), + &mut app, + ); + + let terms = selected_terms(rx.try_recv().expect("response").expect("ok")); + assert!( + !terms.is_glob, + "unedited (clean) save is an exact grant, not a glob" + ); +} + +/// Editing then restoring the original text still counts as glob intent — +/// routing follows dirty, not string equality with the pre-fill. +#[test] +fn retyped_same_text_is_still_glob() { + use crate::views::permission_view::PermissionFocus; + use std::sync::Arc; + + let mut app = test_app_with_agent(); + let agent = app.agents.get_mut(&AgentId(0)).unwrap(); + let mut rx = push_bash_allow_always(agent, PermissionFocus::PatternEdit); + // Pre-fill is `gh api`; dirty but buffer equals pre-fill. + agent.permission_pattern_edit = Some(dirty_pattern_edit("gh api")); + + let _ = dispatch( + Action::PermissionSelect(acp::PermissionOptionId::new(Arc::from( + "allow-always-command", + ))), + &mut app, + ); + + let terms = selected_terms(rx.try_recv().expect("response").expect("ok")); + assert_eq!(terms.command_parts, vec!["gh api"]); + assert!( + terms.is_glob, + "dirty editor is a glob even when text matches pre-fill" + ); +} diff --git a/crates/codegen/xai-grok-pager/src/app/effects/mod.rs b/crates/codegen/xai-grok-pager/src/app/effects/mod.rs index 8875841..3b53874 100644 --- a/crates/codegen/xai-grok-pager/src/app/effects/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/effects/mod.rs @@ -4569,7 +4569,7 @@ fn format_session_info( "{title_line} Shell version: {version_display}\n{auth_lines} Session ID: {session_id}{conversation_line}\n Working directory: {cwd}\n Model: {model_display}{model_hash_line}{backend_line}{sandbox_line}{turn_line}\n Context: {used} / {total} tokens ({pct}%)" ) } -/// Auth section for `/session-info` — login method + where to manage account/credits. +/// Auth section for `/session-info` — active login method. /// /// This reflects the process login / ACP auth method, not per-model sampling /// credentials (a model `api_key`/`env_key` can still own the turn). @@ -4581,12 +4581,10 @@ fn format_auth_lines(is_api_key_auth: bool, api_key_env_set: bool) -> String { " Auth method: API key\n" }; return format!( - "{method} Manage account and credits: console.x.ai\n Run `grok login` to use your SuperGrok subscription instead.\n" + "{method} Run `grok login` to use your SuperGrok subscription instead.\n" ); } - String::from( - " Auth method: OAuth\n Manage account and credits: https://grok.com/?_s=billing\n", - ) + String::from(" Auth method: OAuth\n") } /// Build the single text content block for a plain `Effect::SendPrompt`. /// diff --git a/crates/codegen/xai-grok-pager/src/app/effects/tests.rs b/crates/codegen/xai-grok-pager/src/app/effects/tests.rs index 04472a7..6f0c6f0 100644 --- a/crates/codegen/xai-grok-pager/src/app/effects/tests.rs +++ b/crates/codegen/xai-grok-pager/src/app/effects/tests.rs @@ -2338,10 +2338,7 @@ fn format_session_info_session_auth_ignores_api_key_env() { let info = make_session_info("auto", None, 1000, 10000); let text = format_session_info(&info, None, false, false, true); assert!(text.contains("Auth method: OAuth"), "{text}"); - assert!( - text.contains("Manage account and credits: https://grok.com/?_s=billing"), - "{text}" - ); + assert!(!text.contains("Manage account and credits"), "{text}"); assert!(!text.contains("Also present: XAI_API_KEY"), "{text}"); assert!(!text.contains("console.x.ai"), "{text}"); assert!(!text.contains("grok login"), "{text}"); @@ -2352,10 +2349,7 @@ fn format_session_info_api_key_without_env() { let text = format_session_info(&info, None, false, true, false); assert!(text.contains("Auth method: API key\n"), "{text}"); assert!(!text.contains("XAI_API_KEY"), "{text}"); - assert!( - text.contains("Manage account and credits: console.x.ai"), - "{text}" - ); + assert!(!text.contains("Manage account and credits"), "{text}"); assert!( text.contains("Run `grok login` to use your SuperGrok subscription instead."), "{text}" @@ -2363,30 +2357,25 @@ fn format_session_info_api_key_without_env() { assert!(!text.contains("grok.com"), "{text}"); } #[test] -fn format_session_info_api_key_auth_notes_console_billing() { +fn format_session_info_api_key_auth_suggests_grok_login() { let info = make_session_info("auto", None, 1000, 10000); let text = format_session_info(&info, None, false, true, true); assert!(text.contains("Auth method: API key (XAI_API_KEY)"), "{text}"); - assert!( - text.contains("Manage account and credits: console.x.ai"), - "{text}" - ); + assert!(!text.contains("Manage account and credits"), "{text}"); assert!( text.contains("Run `grok login` to use your SuperGrok subscription instead."), "{text}" ); assert!(!text.contains("Also present: XAI_API_KEY"), "{text}"); + assert!(!text.contains("console.x.ai"), "{text}"); assert!(!text.contains("grok.com"), "{text}"); } #[test] -fn format_session_info_session_only_manage_at_grok_com() { +fn format_session_info_session_only_shows_oauth() { let info = make_session_info("auto", None, 1000, 10000); let text = format_session_info(&info, None, false, false, false); assert!(text.contains("Auth method: OAuth"), "{text}"); - assert!( - text.contains("Manage account and credits: https://grok.com/?_s=billing"), - "{text}" - ); + assert!(!text.contains("Manage account and credits"), "{text}"); assert!(!text.contains("Also present: XAI_API_KEY"), "{text}"); assert!(!text.contains("console.x.ai"), "{text}"); assert!(!text.contains("grok login"), "{text}"); diff --git a/crates/codegen/xai-grok-pager/src/app/modals.rs b/crates/codegen/xai-grok-pager/src/app/modals.rs index f6feb5d..828edd6 100644 --- a/crates/codegen/xai-grok-pager/src/app/modals.rs +++ b/crates/codegen/xai-grok-pager/src/app/modals.rs @@ -1089,10 +1089,15 @@ impl AgentView { } } PickerOutcome::SubmitQuery => { - let query = state.query().trim().to_string(); - if !query.is_empty() { + // Free-text load only for a UUID session id. + // Own the id before clearing the modal (state is a + // reborrow of `active_modal`). + let load_id = + crate::views::session_picker::session_id_for_direct_load(state.query()) + .map(str::to_owned); + if let Some(sid) = load_id { self.active_modal = None; - InputOutcome::Action(Action::LoadSession(query, None, false)) + InputOutcome::Action(Action::LoadSession(sid, None, false)) } else { InputOutcome::Unchanged } @@ -2729,6 +2734,43 @@ mod session_picker_delete_tests { "typing a query restores the selection highlight" ); } + + /// Paste garbage + Enter with no rows must not LoadSession. + #[test] + fn enter_with_garbage_query_does_not_load_session() { + let mut agent = make_agent(); + open_picker(&mut agent, vec![]); + if let Some(ActiveModal::SessionPicker { state, .. }) = agent.active_modal.as_mut() { + state.set_query("this is pasted garbage!!!"); + } + let out = agent.handle_palette_or_arg_input(&key_code(KeyCode::Enter)); + assert!( + matches!(out, InputOutcome::Unchanged), + "garbage query must be a no-op, got {out:?}" + ); + assert!( + matches!(agent.active_modal, Some(ActiveModal::SessionPicker { .. })), + "picker must stay open" + ); + } + + #[test] + fn enter_with_uuid_query_loads_session() { + let mut agent = make_agent(); + open_picker(&mut agent, vec![]); + let sid = "019fb61a-85a5-7ba0-a4ec-24647dca1893"; + if let Some(ActiveModal::SessionPicker { state, .. }) = agent.active_modal.as_mut() { + state.set_query(sid); + } + let out = agent.handle_palette_or_arg_input(&key_code(KeyCode::Enter)); + assert!( + matches!( + out, + InputOutcome::Action(Action::LoadSession(ref id, None, false)) if id == sid + ), + "UUID query should direct-load, got {out:?}" + ); + } } #[cfg(test)] diff --git a/crates/codegen/xai-grok-pager/src/app/mouse.rs b/crates/codegen/xai-grok-pager/src/app/mouse.rs index 32d12a1..d3a6339 100644 --- a/crates/codegen/xai-grok-pager/src/app/mouse.rs +++ b/crates/codegen/xai-grok-pager/src/app/mouse.rs @@ -273,6 +273,13 @@ impl AgentView { self.scrollback.goto_bottom(); return InputOutcome::Changed; } + if self + .hit_response_top_indicator + .contains(mouse.column, mouse.row) + { + self.scrollback.prev_response(); + return InputOutcome::Changed; + } if let Some(hd_area) = self.history_dropdown_area && hd_area.contains((mouse.column, mouse.row).into()) && self.prompt.history_search.is_active() @@ -1098,6 +1105,9 @@ impl AgentView { changed |= self .hit_follow_indicator .update_hover(mouse.column, mouse.row); + changed |= self + .hit_response_top_indicator + .update_hover(mouse.column, mouse.row); changed |= self.hit_cancel_button.update_hover(mouse.column, mouse.row); changed |= self.hit_bg_button.update_hover(mouse.column, mouse.row); changed |= self.hit_watching_cue.update_hover(mouse.column, mouse.row); diff --git a/crates/codegen/xai-grok-pager/src/diagnostics/doctor_format_tests.rs b/crates/codegen/xai-grok-pager/src/diagnostics/doctor_format_tests.rs index 0871a61..193fbee 100644 --- a/crates/codegen/xai-grok-pager/src/diagnostics/doctor_format_tests.rs +++ b/crates/codegen/xai-grok-pager/src/diagnostics/doctor_format_tests.rs @@ -43,6 +43,7 @@ fn unavailable_tmux() -> TmuxProbeFacts { allow_passthrough_support: TmuxProbeResult::Unavailable, allow_passthrough: TmuxProbeResult::Unavailable, control_mode: TmuxProbeResult::Unavailable, + client_features: TmuxProbeResult::Unavailable, } } @@ -171,6 +172,7 @@ fn tmux_config_and_reload_notes_output_is_stable() { allow_passthrough_support: TmuxProbeResult::Available(()), allow_passthrough: TmuxProbeResult::Available("off".to_owned()), control_mode: TmuxProbeResult::Available(false), + client_features: TmuxProbeResult::Unavailable, }, &TMUX_ROUTE, "pbcopy", @@ -202,17 +204,17 @@ fn tmux_config_and_reload_notes_output_is_stable() { " ! terminal.tmux-clipboard `set-clipboard` is off in tmux, so OSC 52 clipboard copies are blocked\n", " Automatic setup: `grok doctor fix tmux-clipboard`\n", " Add `set -g set-clipboard on` to ~/.byobu/.tmux.conf\n", - " Note: Reload tmux with `tmux source-file ~/.byobu/.tmux.conf`, or detach and reattach.\n", + " Note: Reload tmux with `tmux source-file ~/.byobu/.tmux.conf`, or restart the tmux server.\n", "\n", " ! terminal.dcs-passthrough `allow-passthrough` is off in tmux, which can block clipboard copies in nested sessions\n", " Automatic setup: `grok doctor fix dcs-passthrough`\n", " Add `set -wg allow-passthrough on` to ~/.byobu/.tmux.conf\n", - " Note: Reload tmux with `tmux source-file ~/.byobu/.tmux.conf`, or detach and reattach.\n", + " Note: Reload tmux with `tmux source-file ~/.byobu/.tmux.conf`, or restart the tmux server.\n", "\n", " ! terminal.tmux-extended-keys `extended-keys` is off in tmux, so some shortcuts may not work\n", " Automatic setup: `grok doctor fix tmux-extended-keys`\n", " Add `set -g extended-keys on` to ~/.byobu/.tmux.conf\n", - " Note: Reload tmux with `tmux source-file ~/.byobu/.tmux.conf`, or detach and reattach.\n", + " Note: Reload tmux with `tmux source-file ~/.byobu/.tmux.conf`, or restart the tmux server.\n", ) ); } @@ -393,6 +395,7 @@ fn unavailable_and_error_probes_do_not_create_false_issues() { allow_passthrough_support: TmuxProbeResult::Unavailable, allow_passthrough: TmuxProbeResult::Error("query failed".to_owned()), control_mode: TmuxProbeResult::Unavailable, + client_features: TmuxProbeResult::Unavailable, }, &TMUX_ROUTE, "pbcopy", @@ -488,6 +491,7 @@ fn runtime_merge_does_not_duplicate_view_findings() { allow_passthrough_support: TmuxProbeResult::Available(()), allow_passthrough: TmuxProbeResult::Available("off".to_owned()), control_mode: TmuxProbeResult::Available(false), + client_features: TmuxProbeResult::Unavailable, }, &TMUX_ROUTE, "pbcopy", @@ -620,6 +624,7 @@ fn keyboard_fact_formats_from_explicit_target_evidence() { set_clipboard: crate::diagnostics::TmuxOptionFact::Unavailable, allow_passthrough_support: crate::diagnostics::TmuxSupportFact::Unavailable, allow_passthrough: crate::diagnostics::TmuxOptionFact::Unavailable, + color_passthrough: crate::diagnostics::TmuxColorPassthrough::Unknown, }, color: ColorFacts { level: RuntimeFact::Available(ColorLevel::TrueColor), diff --git a/crates/codegen/xai-grok-pager/src/diagnostics/fix.rs b/crates/codegen/xai-grok-pager/src/diagnostics/fix.rs index a9f6eda..ecb71cb 100644 --- a/crates/codegen/xai-grok-pager/src/diagnostics/fix.rs +++ b/crates/codegen/xai-grok-pager/src/diagnostics/fix.rs @@ -8,13 +8,16 @@ use xai_grok_config::managed_text::{ ManagedConfigStatus, ManagedItem, ManagedItemState, SyntaxValidator, }; -use crate::diagnostics::{DiagnosticId, DiagnosticReport, TmuxOptionFact, TmuxSupportFact}; +use crate::diagnostics::{ + DiagnosticId, DiagnosticReport, TmuxColorPassthrough, TmuxOptionFact, TmuxSupportFact, +}; use crate::terminal::{ByobuBackend, TerminalContext}; pub const SSH_WRAP_ID: DiagnosticId = DiagnosticId::new("terminal", "ssh-wrap"); pub const TMUX_CLIPBOARD_ID: DiagnosticId = DiagnosticId::new("terminal", "tmux-clipboard"); pub const DCS_PASSTHROUGH_ID: DiagnosticId = DiagnosticId::new("terminal", "dcs-passthrough"); pub const TMUX_EXTENDED_KEYS_ID: DiagnosticId = DiagnosticId::new("terminal", "tmux-extended-keys"); +pub const TMUX_TRUECOLOR_ID: DiagnosticId = DiagnosticId::new("terminal", "tmux-truecolor"); pub const SSH_WRAP_FIX_COMMAND: &str = "grok doctor fix terminal.ssh-wrap"; pub const SSH_WRAP_ONE_OFF: &str = "grok wrap ssh "; @@ -405,6 +408,20 @@ enum TmuxEvidence { Clipboard, DcsPassthrough, ExtendedKeys, + ColorPassthrough, +} + +/// How a tmux remedy reaches its healthy state, which decides whether an +/// existing line elsewhere in the config can defeat Grok's managed block. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum TmuxRemedy { + /// `set -g