From 8adf9013a0929e5c7f1d4e849492d2387837a28d Mon Sep 17 00:00:00 2001 From: "grokkybara[bot]" <304785771+grokkybara[bot]@users.noreply.github.com> Date: Thu, 16 Jul 2026 20:27:30 +0100 Subject: [PATCH] Synced from monorepo Changes: - grok-shell: request workspaces:read/write OAuth2 scopes - security: fix SSRF bypass via HTTP redirect in hook runner - fix(grok-build): enterprise STT WSS URL + API-key voice bearer - Harden identity-change purge and sync-marker invariants - sandbox + workspace-server: delete the legacy ready-file arm - Show billing URL when browser cannot open - fix(pager): show folder-trust UI in minimal mode - fix(pager): drain task_backgrounded before no-wait headless exit - grok-agent-sdk: stop SDK-spawned agents from staging self-updates they can never adopt - Split settings_modal into directory module - Delegate VS Code SSH file links - grok-shell: release the workspace session binding when a session is removed - keep skills reachable when their name collides with a client builtin - Preserve semantic link targets --- Cargo.lock | 8 +- README.md | 18 +- SOURCE_REV | 1 + crates/codegen/xai-fast-worktree/src/api.rs | 16 +- .../xai-fast-worktree/src/discovery.rs | 11 +- crates/codegen/xai-grok-config/src/lib.rs | 21 +- crates/codegen/xai-grok-config/src/loader.rs | 3 + .../xai-grok-config/src/managed_cache.rs | 103 +- .../src/managed_cache/tests.rs | 137 + crates/codegen/xai-grok-config/src/paths.rs | 7 +- crates/codegen/xai-grok-env/Cargo.toml | 6 +- crates/codegen/xai-grok-env/src/lib.rs | 10 +- .../codegen/xai-grok-hooks/src/runner/http.rs | 67 +- crates/codegen/xai-grok-memory/Cargo.toml | 3 + .../codegen/xai-grok-memory/src/embedding.rs | 4 +- crates/codegen/xai-grok-memory/src/storage.rs | 2 +- crates/codegen/xai-grok-pager-bin/Cargo.toml | 3 +- crates/codegen/xai-grok-pager-bin/src/main.rs | 118 +- .../codegen/xai-grok-pager-minimal/Cargo.toml | 2 +- .../xai-grok-pager-minimal/src/auth.rs | 270 +- .../xai-grok-pager-minimal/src/live.rs | 7 +- .../xai-grok-pager-minimal/src/overlay.rs | 11 +- .../codegen/xai-grok-pager-render/Cargo.toml | 5 +- .../src/clipboard/mod.rs | 18 +- .../xai-grok-pager-render/src/gboom/game.rs | 2 +- .../xai-grok-pager-render/src/gboom/mod.rs | 2 +- .../xai-grok-pager-render/src/link_opener.rs | 175 +- .../src/prompt_images.rs | 2 +- .../xai-grok-pager-render/src/render/osc8.rs | 650 +- .../src/terminal/image.rs | 14 +- .../src/terminal/keyboard.rs | 2 +- .../xai-grok-pager-render/src/terminal/mod.rs | 15 + .../src/terminal/test.rs | 46 + .../xai-grok-pager-render/src/theme/cache.rs | 10 +- .../src/theme/system_appearance.rs | 14 +- crates/codegen/xai-grok-pager/Cargo.toml | 9 +- .../docs/user-guide/14-headless-mode.md | 6 + .../app/acp_handler/session_notification.rs | 42 +- .../codegen/xai-grok-pager/src/app/actions.rs | 2 + .../codegen/xai-grok-pager/src/app/agent.rs | 4 +- .../src/app/agent_view/interactions.rs | 10 +- .../src/app/agent_view/links.rs | 192 +- .../xai-grok-pager/src/app/agent_view/mod.rs | 17 +- .../src/app/agent_view/notices.rs | 24 + .../src/app/agent_view/panes.rs | 13 +- .../src/app/agent_view/render.rs | 19 +- .../src/app/agent_view/session.rs | 8 +- .../xai-grok-pager/src/app/app_view.rs | 3 +- .../src/app/dispatch/billing.rs | 2 +- .../xai-grok-pager/src/app/dispatch/ctx.rs | 25 + .../xai-grok-pager/src/app/dispatch/router.rs | 38 +- .../src/app/dispatch/tests/billing.rs | 132 + .../xai-grok-pager/src/app/event_loop.rs | 8 +- crates/codegen/xai-grok-pager/src/app/mod.rs | 2 +- .../codegen/xai-grok-pager/src/app/mouse.rs | 19 +- crates/codegen/xai-grok-pager/src/headless.rs | 79 +- .../codegen/xai-grok-pager/src/minimal/api.rs | 20 +- .../src/scrollback/blocks/tool/edit.rs | 33 +- .../src/scrollback/blocks/tool/read.rs | 33 +- .../xai-grok-pager/src/scrollback/link_map.rs | 261 +- .../xai-grok-pager/src/scrollback/render.rs | 446 +- .../src/scrollback/state/mod.rs | 11 +- .../xai-grok-pager/src/scrollback/types.rs | 13 +- .../xai-grok-pager/src/slash/registry.rs | 136 +- crates/codegen/xai-grok-pager/src/tracing.rs | 2 +- .../xai-grok-pager/src/views/btw_overlay.rs | 40 +- .../src/views/settings_modal.rs | 12619 ---------------- .../src/views/settings_modal/input.rs | 1309 ++ .../src/views/settings_modal/mod.rs | 37 + .../src/views/settings_modal/render.rs | 2943 ++++ .../src/views/settings_modal/state.rs | 915 ++ .../src/views/settings_modal/tests.rs | 7455 +++++++++ crates/codegen/xai-grok-shell-base/Cargo.toml | 9 +- .../xai-grok-shell-base/src/cpu_profile.rs | 4 +- crates/codegen/xai-grok-shell-base/src/env.rs | 42 +- .../xai-grok-shell-base/src/util/mod.rs | 9 +- crates/codegen/xai-grok-shell/Cargo.toml | 15 +- .../xai-grok-shell/src/agent/chat_modes.rs | 11 +- .../xai-grok-shell/src/agent/config.rs | 11 +- .../xai-grok-shell/src/agent/mvp_agent/mod.rs | 11 +- .../src/agent/mvp_agent/session_lifecycle.rs | 20 +- .../src/agent/mvp_agent/tests.rs | 49 + .../src/agent/subagent/handle_request.rs | 22 +- .../codegen/xai-grok-shell/src/auth/config.rs | 8 +- .../xai-grok-shell/src/auth/manager.rs | 132 +- .../xai-grok-shell/src/auth/manager_tests.rs | 196 + .../codegen/xai-grok-shell/src/config/mod.rs | 13 +- .../extensions/chat_conversation_history.rs | 5 +- .../xai-grok-shell/src/leader/server.rs | 1 - .../xai-grok-shell/src/managed_config.rs | 320 +- .../src/managed_config/response.rs | 34 +- .../src/managed_config/tests.rs | 72 + .../src/session/acp_session_impl/spawn.rs | 19 +- .../session/acp_session_impl/tool_calls.rs | 4 +- .../src/session/slash_commands.rs | 104 +- .../src/session/unified_list/mod.rs | 14 +- .../tests/signed_managed_config.rs | 282 - .../tests/signed_managed_config/common.rs | 220 - .../tests/signed_managed_config_extended.rs | 296 - .../tests/team_managed_config.rs | 434 +- .../tests/test_built_binary_e2e.rs | 7 +- crates/codegen/xai-grok-tools/Cargo.toml | 4 +- .../xai-grok-tools/src/registry/types.rs | 7 +- crates/codegen/xai-grok-version/Cargo.toml | 2 +- .../xai-grok-voice/src/bin/voice_probe.rs | 11 +- crates/codegen/xai-grok-voice/src/config.rs | 318 +- crates/codegen/xai-grok-workspace/Cargo.toml | 3 +- .../src/bin/workspace_server.rs | 26 +- .../xai-grok-workspace/src/daemonize.rs | 7 - .../xai-grok-workspace/src/diag_server.rs | 9 +- .../src/foreign_sessions/capability/mod.rs | 19 +- .../src/foreign_sessions/codex/tests/db.rs | 10 +- .../codegen/xai-grok-workspace/src/handle.rs | 13 +- crates/codegen/xai-grok-workspace/src/hub.rs | 66 +- .../src/session/checkpoint.rs | 3 +- .../src/session/tool_config.rs | 2 +- .../xai-grok-workspace/src/workspace_ops.rs | 2 +- 117 files changed, 17017 insertions(+), 14559 deletions(-) create mode 100644 SOURCE_REV delete mode 100644 crates/codegen/xai-grok-pager/src/views/settings_modal.rs create mode 100644 crates/codegen/xai-grok-pager/src/views/settings_modal/input.rs create mode 100644 crates/codegen/xai-grok-pager/src/views/settings_modal/mod.rs create mode 100644 crates/codegen/xai-grok-pager/src/views/settings_modal/render.rs create mode 100644 crates/codegen/xai-grok-pager/src/views/settings_modal/state.rs create mode 100644 crates/codegen/xai-grok-pager/src/views/settings_modal/tests.rs delete mode 100644 crates/codegen/xai-grok-shell/tests/signed_managed_config.rs delete mode 100644 crates/codegen/xai-grok-shell/tests/signed_managed_config/common.rs delete mode 100644 crates/codegen/xai-grok-shell/tests/signed_managed_config_extended.rs diff --git a/Cargo.lock b/Cargo.lock index 695ef59..855dda1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13270,7 +13270,7 @@ dependencies = [ [[package]] name = "xai-grok-pager" -version = "0.1.220-alpha.4" +version = "0.2.101" dependencies = [ "agent-client-protocol", "ansi-to-tui", @@ -13359,7 +13359,7 @@ dependencies = [ [[package]] name = "xai-grok-pager-bin" -version = "0.1.220-alpha.4" +version = "0.2.101" dependencies = [ "anyhow", "clap", @@ -13618,7 +13618,7 @@ dependencies = [ [[package]] name = "xai-grok-shell" -version = "0.1.220-alpha.4" +version = "0.2.101" dependencies = [ "agent-client-protocol", "anyhow", @@ -14009,7 +14009,7 @@ dependencies = [ [[package]] name = "xai-grok-version" -version = "0.2.0-dev" +version = "0.2.101" dependencies = [ "semver", ] diff --git a/README.md b/README.md index 4ad87c4..4569db8 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,9 @@ Protocol (ACP). This repository contains the Rust source for the `grok` CLI/TUI and its agent runtime. It is synced periodically from the SpaceXAI monorepo. +A small `SOURCE_REV` file at the root records the full monorepo commit SHA +for the version of the code present in this tree. + --- @@ -54,9 +57,18 @@ Requirements: - **Rust** — the toolchain is pinned by [`rust-toolchain.toml`](rust-toolchain.toml); `rustup` installs it automatically on first build. -- **protoc** — proto codegen resolves [`bin/protoc`](bin/protoc) (a - [dotslash](https://dotslash-cli.com) launcher) or falls back to a `protoc` on - `PATH` / `$PROTOC`. +- **[DotSlash](https://dotslash-cli.com)** — required so hermetic tools under + [`bin/`](bin/) (notably [`bin/protoc`](bin/protoc)) can download and run. + Install it and ensure `dotslash` is on your `PATH` **before** building: + + ```sh + cargo install dotslash + # or: prebuilt packages — https://dotslash-cli.com/docs/installation/ + /usr/bin/env dotslash --help # sanity check + ``` + +- **protoc** — proto codegen resolves [`bin/protoc`](bin/protoc) via DotSlash, + or falls back to a `protoc` on `PATH` / `$PROTOC`. - macOS and Linux are supported build hosts; Windows builds are best-effort and not currently tested from this tree. diff --git a/SOURCE_REV b/SOURCE_REV new file mode 100644 index 0000000..b7282d2 --- /dev/null +++ b/SOURCE_REV @@ -0,0 +1 @@ +2ec0f0c8488842da03a71eeee3c61154957ca919 diff --git a/crates/codegen/xai-fast-worktree/src/api.rs b/crates/codegen/xai-fast-worktree/src/api.rs index 19889cb..70a593c 100644 --- a/crates/codegen/xai-fast-worktree/src/api.rs +++ b/crates/codegen/xai-fast-worktree/src/api.rs @@ -1411,11 +1411,15 @@ pub(crate) fn register_worktree( return; } }; + // Same canonical path as discovery rebuild / WorktreeDb::get so macOS + // /var vs /private/var (and other symlink roots) do not create duplicate rows. + let path = dunce::canonicalize(worktree_path).unwrap_or_else(|_| worktree_path.to_path_buf()); + let source = dunce::canonicalize(source).unwrap_or_else(|_| source.to_path_buf()); let record = db::WorktreeRecord { - id: worktree_id.unwrap_or_else(|| db::id_from_path(worktree_path)), - path: worktree_path.to_path_buf(), - source_repo: source.to_path_buf(), - repo_name: db::repo_name_from_path(source), + id: worktree_id.unwrap_or_else(|| db::id_from_path(&path)), + path, + source_repo: source.clone(), + repo_name: db::repo_name_from_path(&source), kind, creation_mode: creation_mode.to_owned(), git_ref: Some(git_ref.to_owned()), @@ -1435,7 +1439,9 @@ pub(crate) fn register_worktree( #[cfg(feature = "metadata")] fn unregister_worktree(worktree_path: &std::path::Path) { if let Ok(db) = crate::db::WorktreeDb::open_default() { - let _ = db.unregister_by_path(worktree_path); + let path = + dunce::canonicalize(worktree_path).unwrap_or_else(|_| worktree_path.to_path_buf()); + let _ = db.unregister_by_path(&path); } } diff --git a/crates/codegen/xai-fast-worktree/src/discovery.rs b/crates/codegen/xai-fast-worktree/src/discovery.rs index 3d8159e..6677994 100644 --- a/crates/codegen/xai-fast-worktree/src/discovery.rs +++ b/crates/codegen/xai-fast-worktree/src/discovery.rs @@ -125,10 +125,12 @@ impl DiscoveredWorktree { .unwrap_or_else(|| "unknown".to_string()); let source_repo = self.source_repo.unwrap_or_else(|| PathBuf::from("unknown")); let created_at = fs_creation_time(&self.path); + // Match `WorktreeDb::get`, which looks up by canonical path. + let path = dunce::canonicalize(&self.path).unwrap_or(self.path); WorktreeRecord { - id: id_from_path(&self.path), - path: self.path, + id: id_from_path(&path), + path, source_repo, repo_name, kind: self.kind, @@ -163,8 +165,9 @@ pub fn rebuild_worktree_db( }; for wt in discovery.found { - let id = id_from_path(&wt.path); - let path_str = wt.path.to_string_lossy(); + let path = dunce::canonicalize(&wt.path).unwrap_or_else(|_| wt.path.clone()); + let id = id_from_path(&path); + let path_str = path.to_string_lossy(); if db.get_by_id(&id)?.is_some() || db.get(&path_str)?.is_some() { report.already_tracked += 1; continue; diff --git a/crates/codegen/xai-grok-config/src/lib.rs b/crates/codegen/xai-grok-config/src/lib.rs index ab6de2d..7dc682f 100644 --- a/crates/codegen/xai-grok-config/src/lib.rs +++ b/crates/codegen/xai-grok-config/src/lib.rs @@ -32,22 +32,23 @@ pub use campaigns::{ }; pub use loader::{ CampaignsState, ConfigLayers, MANAGED_CONFIG_FILENAME, ManagedConfigLayer, - apply_version_overrides_with_registered, campaigns_application_disabled, campaigns_state_path, - deep_merge_toml, expand_env_vars_in_string, expand_env_vars_in_toml, load_config_file, - load_dismissed_ids_from_home, load_effective_config_disk_only, load_from_disk, - load_managed_config, load_system_managed_config, load_toml_file, managed_config_layers, - managed_config_layers_at, toml_error_detail, + REQUIREMENTS_FILENAME, apply_version_overrides_with_registered, campaigns_application_disabled, + campaigns_state_path, deep_merge_toml, expand_env_vars_in_string, expand_env_vars_in_toml, + load_config_file, load_dismissed_ids_from_home, load_effective_config_disk_only, + load_from_disk, load_managed_config, load_system_managed_config, load_toml_file, + managed_config_layers, managed_config_layers_at, toml_error_detail, }; pub use macos_managed::MDM_REQUIREMENTS_SOURCE; pub use managed_cache::{ - ServingIdentity, SyncMarker, is_managed_config_hard_stale_for, is_managed_config_stale_for, - managed_config_identity_changed, managed_deployment_id, managed_policy_compromised_for, - mark_managed_config_synced, + MANAGED_CONFIG_CACHE_FILE, ServingIdentity, SyncMarker, confirmed_team_switch, + confirmed_team_switch_at, is_managed_config_hard_stale_for, is_managed_config_stale_for, + managed_config_identity_changed_at, managed_deployment_id, managed_policy_compromised_for, + mark_managed_config_synced, mark_managed_config_synced_at, normalize_identity, }; pub use paths::{ claude_managed_settings_path, claude_managed_settings_probe_path, decode_cwd_from_dirname, - default_grok_home, encode_cwd_dirname, ensure_sessions_cwd_dir, grok_application, grok_home, - sessions_cwd_dir, system_config_dir, user_grok_home, + default_grok_home, encode_cwd_dirname, ensure_sessions_cwd_dir, grok_application, + grok_application_in, grok_home, sessions_cwd_dir, system_config_dir, user_grok_home, }; pub use validation::{ RequirementsError, RequirementsLayer, RequirementsSource, fail_closed_flag_from_str, diff --git a/crates/codegen/xai-grok-config/src/loader.rs b/crates/codegen/xai-grok-config/src/loader.rs index f9c710b..595f597 100644 --- a/crates/codegen/xai-grok-config/src/loader.rs +++ b/crates/codegen/xai-grok-config/src/loader.rs @@ -87,6 +87,9 @@ pub fn load_from_disk() -> std::io::Result { /// Managed config filename, shared by the loaders in this module. pub const MANAGED_CONFIG_FILENAME: &str = "managed_config.toml"; +/// Requirements (cloud-cache) filename — the sibling server-synced artifact. +pub const REQUIREMENTS_FILENAME: &str = "requirements.toml"; + pub fn load_managed_config() -> std::io::Result { load_user_config_layer(user_grok_home().as_deref(), MANAGED_CONFIG_FILENAME) } diff --git a/crates/codegen/xai-grok-config/src/managed_cache.rs b/crates/codegen/xai-grok-config/src/managed_cache.rs index de4e941..8ed59c4 100644 --- a/crates/codegen/xai-grok-config/src/managed_cache.rs +++ b/crates/codegen/xai-grok-config/src/managed_cache.rs @@ -10,8 +10,9 @@ use std::path::Path; use crate::paths::user_grok_home; -/// Sync marker; staleness keys on this, not mtimes. -const MANAGED_CONFIG_CACHE_FILE: &str = "managed_config_cache.json"; +/// Sync marker; staleness keys on this, not mtimes. Public so removal code can name it +/// apart from the policy artifacts (removed last). +pub const MANAGED_CONFIG_CACHE_FILE: &str = "managed_config_cache.json"; /// The on-disk marker: unsigned, detects only deletion / identity change, not /// in-place edits (see the module doc). @@ -84,10 +85,11 @@ fn managed_deployment_id_at(home: &Path, key_fingerprint: &str) -> Option) { +/// [`mark_managed_config_synced`] for an explicit `home` (apply-lock holder: same dir as lock). +pub fn mark_managed_config_synced_at(home: &Path, marker: SyncMarker<'_>) { let SyncMarker { principal, had_managed_config, @@ -101,11 +103,12 @@ fn mark_managed_config_synced_at(home: &Path, marker: SyncMarker<'_>) { .ok(); let cache = ManagedConfigCache { synced_at, - principal: principal.map(str::to_owned), - // What THIS sync served, not on-disk presence — a confirmed switch already evicted any prior files. + // Blank → None: marker must never record "unknown" as a tenant. + principal: normalize_identity(principal), + // What THIS sync served (not on-disk); switch already evicted priors. had_managed_config, had_requirements, - key_fingerprint: key_fingerprint.map(str::to_owned), + key_fingerprint: normalize_identity(key_fingerprint), fail_closed, }; match serde_json::to_string(&cache) { @@ -146,21 +149,10 @@ fn read_managed_config_cache(home: &Path) -> Option { } } -/// A confirmed identity switch vs the marker — both sides of a dimension present and differing (team id or fingerprint). -/// Callers evict prior artifacts on true; a missing marker / `None` / pre-upgrade never counts (first sync / signed-out / legacy never evict). -/// A blank/whitespace value on either side of either dimension (principal or key fingerprint) -/// is "unknown", not a distinct tenant — a malformed `auth.json` parse blip must not confirm a -/// switch and shed a real tenant's policy. -pub fn managed_config_identity_changed( - new_principal: Option<&str>, - new_key_fingerprint: Option<&str>, -) -> bool { - user_grok_home().is_some_and(|home| { - managed_config_identity_changed_at(&home, new_principal, new_key_fingerprint) - }) -} - -fn managed_config_identity_changed_at( +/// Confirmed identity switch vs the marker (both sides of a dimension known and differing). +/// Missing marker / blank / pre-upgrade never counts. Callers evict prior artifacts on true. +/// Takes the apply-lock holder's `home` (same dir as the lock). +pub fn managed_config_identity_changed_at( home: &Path, new_principal: Option<&str>, new_key_fingerprint: Option<&str>, @@ -168,41 +160,78 @@ fn managed_config_identity_changed_at( let Some(cache) = read_managed_config_cache(home) else { return false; }; - let principal_changed = matches!( - (cache.principal.as_deref(), new_principal), - (Some(old), Some(new)) - if !old.trim().is_empty() && !new.trim().is_empty() && old != new - ); - let key_changed = matches!( - (cache.key_fingerprint.as_deref(), new_key_fingerprint), - (Some(old), Some(new)) - if !old.trim().is_empty() && !new.trim().is_empty() && old != new - ); - principal_changed || key_changed + confirmed_switch(cache.principal.as_deref(), new_principal).is_some() + || confirmed_switch(cache.key_fingerprint.as_deref(), new_key_fingerprint).is_some() +} + +/// Present non-blank value, else `None` (blank/whitespace is "unknown", not a tenant). Untrimmed. +fn known(value: Option<&str>) -> Option<&str> { + value.filter(|v| !v.trim().is_empty()) +} + +/// [`known`] then trim — the one normalization for storing or deriving an identity +/// (whitespace is not identity). Shared with the shell's identity derivation. +pub fn normalize_identity(value: Option<&str>) -> Option { + known(value).map(|v| v.trim().to_owned()) +} + +/// Both sides known and differing after trim (older markers may be untrimmed). Returns recorded value. +fn confirmed_switch<'a>(recorded: Option<&'a str>, current: Option<&str>) -> Option<&'a str> { + match (known(recorded), known(current)) { + (Some(old), Some(new)) if old.trim() != new.trim() => Some(old), + _ => None, + } +} + +/// Offline tenant-purge detector: confirmed team switch vs marker → evicted principal. +/// Key-scoped markers never confirm (key owns the machine's policy, not the team). +pub fn confirmed_team_switch(new_team_id: &str) -> Option { + user_grok_home().and_then(|home| confirmed_team_switch_at(&home, new_team_id)) +} + +/// [`confirmed_team_switch`] for an explicit `home` (purge-lock holder: same dir as delete). +pub fn confirmed_team_switch_at(home: &Path, new_team_id: &str) -> Option { + let cache = read_managed_config_cache(home)?; + if known(cache.key_fingerprint.as_deref()).is_some() { + return None; + } + confirmed_switch(cache.principal.as_deref(), Some(new_team_id)).map(str::to_owned) } /// True when an artifact the marker recorded serving is now absent. Only served artifacts count, so a config-less /// principal (or legacy marker) isn't misread as stale. Detects deletion, not edits. fn cache_missing_required_artifact(cache: &ManagedConfigCache, home: &Path) -> bool { - (cache.had_requirements && !home.join("requirements.toml").exists()) - || (cache.had_managed_config && !home.join("managed_config.toml").exists()) + (cache.had_requirements && !home.join(crate::loader::REQUIREMENTS_FILENAME).exists()) + || (cache.had_managed_config && !home.join(crate::loader::MANAGED_CONFIG_FILENAME).exists()) } /// Whether the cached principal differs from the team serving now — the team dimension only. /// Deploy-key identity is verified by fingerprint ([`cache_key_fingerprint_mismatch`]); `None` never fires. +/// Trim-aware (same rule as marker write): whitespace alone is not a mismatch. fn cache_identity_mismatch(cache: &ManagedConfigCache, identity: &ServingIdentity) -> bool { match identity { - ServingIdentity::Team(team_id) => cache.principal.as_deref() != Some(team_id.as_str()), + ServingIdentity::Team(team_id) => match ( + known(cache.principal.as_deref()), + known(Some(team_id.as_str())), + ) { + // Both blank → no team to compare. + (None, None) => false, + // Both known → trim-compare. + (Some(a), Some(b)) => a.trim() != b.trim(), + // One-sided: treat as mismatch (first install / cleared principal field). + _ => true, + }, ServingIdentity::DeploymentKey { .. } | ServingIdentity::None => false, } } /// Whether the configured deployment key differs from the cache's, by one-way fingerprint (never the raw key) — /// the only identity verifiable offline. A pre-upgrade marker (no fingerprint) never fires; only a *changed* key. +/// Trim-aware; both sides must be known (unlike the team principal path). fn cache_key_fingerprint_mismatch(cache: &ManagedConfigCache, identity: &ServingIdentity) -> bool { match identity { ServingIdentity::DeploymentKey { fingerprint } => { - matches!(cache.key_fingerprint.as_deref(), Some(recorded) if recorded != fingerprint) + confirmed_switch(cache.key_fingerprint.as_deref(), Some(fingerprint.as_str())).is_some() } ServingIdentity::Team(_) | ServingIdentity::None => false, } diff --git a/crates/codegen/xai-grok-config/src/managed_cache/tests.rs b/crates/codegen/xai-grok-config/src/managed_cache/tests.rs index 4aadd04..89564ca 100644 --- a/crates/codegen/xai-grok-config/src/managed_cache/tests.rs +++ b/crates/codegen/xai-grok-config/src/managed_cache/tests.rs @@ -1010,3 +1010,140 @@ fn gate_retries_once_on_a_compromised_verdict() { assert!(refused); assert_eq!(evals, 1); } + +/// The offline purge detector: fires only on a marker-recorded TEAM switch, returning the +/// evicted principal; a key-scoped marker means the key owns the machine's policy, so a +/// team mismatch (even with live config unreadable/blipping) must never confirm. +#[test] +fn confirmed_team_switch_scopes_to_marker() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + + // No marker → no switch (first run / signed-out). + assert_eq!(confirmed_team_switch_at(home, "team-b"), None); + + // Team marker A → B confirms and reports the evicted principal; same team doesn't. + mark_managed_config_synced_at( + home, + SyncMarker { + principal: Some("team-a"), + had_managed_config: true, + had_requirements: true, + key_fingerprint: None, + fail_closed: true, + }, + ); + assert_eq!( + confirmed_team_switch_at(home, "team-b").as_deref(), + Some("team-a") + ); + assert_eq!(confirmed_team_switch_at(home, "team-a"), None); + + // Key-scoped marker (dk-synced): a differing team NEVER confirms — the regression + // shape is a dk machine with a team user signed in and config resolution blipping. + mark_managed_config_synced_at( + home, + SyncMarker { + principal: Some("dk-deployment-1"), + had_managed_config: true, + had_requirements: true, + key_fingerprint: Some("fp-1"), + fail_closed: true, + }, + ); + assert_eq!(confirmed_team_switch_at(home, "team-b"), None); +} + +/// Blank identity values normalize to `None` at the marker WRITE, so no reader can +/// treat "unknown" as a distinct tenant (the detectors' blank guards stay as +/// defense in depth). +#[test] +fn marker_write_normalizes_blank_identities() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + mark_managed_config_synced_at( + home, + SyncMarker { + principal: Some(" "), + had_managed_config: true, + had_requirements: true, + key_fingerprint: Some(""), + fail_closed: true, + }, + ); + let cache = read_managed_config_cache(home).expect("marker written"); + assert_eq!( + cache.principal, None, + "blank principal must not be recorded" + ); + assert_eq!( + cache.key_fingerprint, None, + "blank fingerprint must not be recorded" + ); + // And a blank-recorded marker can't confirm a switch. + assert_eq!(confirmed_team_switch_at(home, "team-b"), None); +} + +/// Identity values are stored TRIMMED at the marker write, so a marker can never +/// differ from a live value by surrounding whitespace alone. +#[test] +fn marker_write_trims_identity_values() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path(); + mark_managed_config_synced_at( + home, + SyncMarker { + principal: Some(" team-a "), + had_managed_config: true, + had_requirements: true, + key_fingerprint: Some(" fp-1 "), + fail_closed: false, + }, + ); + let cache = read_managed_config_cache(home).expect("marker written"); + assert_eq!(cache.principal.as_deref(), Some("team-a")); + assert_eq!(cache.key_fingerprint.as_deref(), Some("fp-1")); +} + +/// The one home of the blank + trim rules ([`known`] + `confirmed_switch`): both sides +/// known and differing on their trimmed forms, else `None`. +#[test] +fn confirmed_switch_requires_two_known_differing_sides() { + assert_eq!(confirmed_switch(Some("a"), Some("b")), Some("a")); + assert_eq!(confirmed_switch(Some("a"), Some("a")), None); + assert_eq!(confirmed_switch(Some(" "), Some("b")), None); + assert_eq!(confirmed_switch(Some("a"), Some("")), None); + assert_eq!(confirmed_switch(None, Some("b")), None); + assert_eq!(confirmed_switch(Some("a"), None), None); + assert_eq!(confirmed_switch(None, None), None); + // Whitespace is not identity: a marker written untrimmed by an older build must + // not read as a tenant switch against the same (trimmed) value... + assert_eq!(confirmed_switch(Some("team-a "), Some("team-a")), None); + assert_eq!(confirmed_switch(Some("team-a"), Some("team-a ")), None); + // ...while genuinely different trimmed values still switch (the recorded value + // is returned verbatim for logging). + assert_eq!( + confirmed_switch(Some(" team-a "), Some("team-b")), + Some(" team-a ") + ); +} + +/// Staleness identity compare is trim-aware (sibling of marker write normalize). +#[test] +fn cache_identity_mismatch_ignores_whitespace_only_diffs() { + let cache = ManagedConfigCache { + principal: Some("team-a".into()), + ..Default::default() + }; + assert!( + !cache_identity_mismatch(&cache, &team(" team-a ")), + "whitespace-only team id diff must not hard-stale" + ); + assert!( + cache_identity_mismatch(&cache, &team("team-b")), + "a real team switch must still mismatch" + ); + // One-sided known still mismatches (first install / cleared marker fields). + let empty = ManagedConfigCache::default(); + assert!(cache_identity_mismatch(&empty, &team("team-a"))); +} diff --git a/crates/codegen/xai-grok-config/src/paths.rs b/crates/codegen/xai-grok-config/src/paths.rs index d8fb8f5..c749779 100644 --- a/crates/codegen/xai-grok-config/src/paths.rs +++ b/crates/codegen/xai-grok-config/src/paths.rs @@ -59,8 +59,13 @@ pub fn user_grok_home() -> Option { /// Canonical grok application path: `$GROK_HOME/bin/grok` (Unix) or `grok.exe` (Windows). pub fn grok_application() -> PathBuf { + grok_application_in(&grok_home()) +} + +/// [`grok_application`] under an explicit home instead of `$GROK_HOME`. +pub fn grok_application_in(home: &std::path::Path) -> PathBuf { let name = if cfg!(windows) { "grok.exe" } else { "grok" }; - grok_home().join("bin").join(name) + home.join("bin").join(name) } /// System-wide config directory: `/etc/grok/` on Unix, `None` on Windows. diff --git a/crates/codegen/xai-grok-env/Cargo.toml b/crates/codegen/xai-grok-env/Cargo.toml index f0bab3d..f38377f 100644 --- a/crates/codegen/xai-grok-env/Cargo.toml +++ b/crates/codegen/xai-grok-env/Cargo.toml @@ -6,8 +6,10 @@ edition.workspace = true description = "Backend environment presets for the Grok CLI crate family: endpoint URL defaults and env-var test support." [features] -# single shared rlib per crate, so downstream Bazel test targets need the -default-bazel = [] +# Exposes `EnvVarGuard` (and its process-wide env lock) to downstream +# crates' test targets. +test-support = [] +default-bazel = ["test-support"] [dependencies] tracing = { workspace = true } diff --git a/crates/codegen/xai-grok-env/src/lib.rs b/crates/codegen/xai-grok-env/src/lib.rs index 6a39e8a..658fba3 100644 --- a/crates/codegen/xai-grok-env/src/lib.rs +++ b/crates/codegen/xai-grok-env/src/lib.rs @@ -97,21 +97,21 @@ impl std::fmt::Display for GrokBuildEnvironment { } } /// Serializes env-var mutation across tests; `std::env` is process-global. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] fn env_lock() -> std::sync::MutexGuard<'static, ()> { ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()) } /// RAII env-var override for tests: constructors snapshot the prior value /// under [`ENV_LOCK`], `Drop` restores it, panics included. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub struct EnvVarGuard { key: &'static str, prev: Option, _lock: std::sync::MutexGuard<'static, ()>, } -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] impl EnvVarGuard { pub fn set(key: &'static str, value: &str) -> Self { let lock = env_lock(); @@ -138,7 +138,7 @@ impl EnvVarGuard { unsafe { std::env::set_var(self.key, value) }; } } -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] impl Drop for EnvVarGuard { fn drop(&mut self) { match self.prev.take() { diff --git a/crates/codegen/xai-grok-hooks/src/runner/http.rs b/crates/codegen/xai-grok-hooks/src/runner/http.rs index 78a3836..f003ac0 100644 --- a/crates/codegen/xai-grok-hooks/src/runner/http.rs +++ b/crates/codegen/xai-grok-hooks/src/runner/http.rs @@ -131,6 +131,16 @@ async fn validate_hook_url(url: &str) -> Result<(), String> { Ok(()) } +/// Build the reqwest client used to send a hook request. +fn build_hook_client(timeout_ms: u64) -> reqwest::Client { + reqwest::Client::builder() + .timeout(Duration::from_millis(timeout_ms)) + // `validate_hook_url` only vets the initial URL, not redirect targets. + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap_or_default() +} + /// Run a single HTTP hook. /// /// POSTs the serialized `HookEventEnvelope` as JSON to `spec.url`. @@ -223,10 +233,7 @@ pub async fn run_http_hook( } }; - let client = reqwest::Client::builder() - .timeout(Duration::from_millis(spec.timeout_ms)) - .build() - .unwrap_or_default(); + let client = build_hook_client(spec.timeout_ms); let response = match client .post(url) @@ -924,6 +931,58 @@ mod tests { assert_eq!(info.raw_url.as_deref(), Some(raw)); } + /// The hook client must not follow HTTP redirects: `validate_hook_url` + /// only vets the initial URL, so a followed 3xx would reach an unvalidated + /// target. The local server answers every request with a 302 pointing at a + /// blocked address; with redirects disabled the client returns the 302 + /// verbatim and never issues a second request to the target. + #[tokio::test] + async fn hook_client_does_not_follow_redirects() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let requests = Arc::new(AtomicUsize::new(0)); + let server_requests = Arc::clone(&requests); + + tokio::spawn(async move { + loop { + let Ok((mut socket, _)) = listener.accept().await else { + return; + }; + server_requests.fetch_add(1, Ordering::SeqCst); + let mut buf = [0u8; 1024]; + let _ = socket.read(&mut buf).await; + let response = "HTTP/1.1 302 Found\r\n\ + Location: http://169.254.169.254/latest/meta-data/\r\n\ + Content-Length: 0\r\n\r\n"; + let _ = socket.write_all(response.as_bytes()).await; + let _ = socket.flush().await; + } + }); + + let client = build_hook_client(5000); + let resp = client + .post(format!("http://{addr}/hook")) + .body("{}") + .send() + .await + .expect("request should succeed without following the redirect"); + + assert_eq!( + resp.status().as_u16(), + 302, + "redirect must be surfaced, not followed" + ); + assert_eq!( + requests.load(Ordering::SeqCst), + 1, + "client must not issue a second request to the redirect target" + ); + } + /// Unresolved `${VAR}` refs are preserved verbatim by the helper, /// which means `validate_hook_url` will reject the URL with an /// "invalid URL" error. This is the desired behaviour: a hook diff --git a/crates/codegen/xai-grok-memory/Cargo.toml b/crates/codegen/xai-grok-memory/Cargo.toml index 12d8b5a..4ee8fdc 100644 --- a/crates/codegen/xai-grok-memory/Cargo.toml +++ b/crates/codegen/xai-grok-memory/Cargo.toml @@ -5,6 +5,9 @@ version = "0.1.0" edition.workspace = true [features] +# Exposes `#[cfg(test)]`-only helpers (`MemoryStorage::with_paths`, +# `MockEmbeddingProvider`) to downstream crates' test targets. +test-support = [] [dependencies] anyhow = { workspace = true } diff --git a/crates/codegen/xai-grok-memory/src/embedding.rs b/crates/codegen/xai-grok-memory/src/embedding.rs index 2183317..0778321 100644 --- a/crates/codegen/xai-grok-memory/src/embedding.rs +++ b/crates/codegen/xai-grok-memory/src/embedding.rs @@ -214,12 +214,12 @@ impl EmbeddingProvider for ApiEmbeddingProvider { /// A mock embedding provider for testing that returns deterministic vectors. /// Uses blake3 hash of text → float values for reproducible results. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub struct MockEmbeddingProvider { pub dimensions: usize, } -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] #[async_trait] impl EmbeddingProvider for MockEmbeddingProvider { async fn embed_batch( diff --git a/crates/codegen/xai-grok-memory/src/storage.rs b/crates/codegen/xai-grok-memory/src/storage.rs index c648b43..a8863f0 100644 --- a/crates/codegen/xai-grok-memory/src/storage.rs +++ b/crates/codegen/xai-grok-memory/src/storage.rs @@ -74,7 +74,7 @@ impl MemoryStorage { } /// Create a `MemoryStorage` with explicit paths (for testing). - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] pub fn with_paths(global_dir: PathBuf, workspace_dir: PathBuf) -> Self { Self { global_dir, diff --git a/crates/codegen/xai-grok-pager-bin/Cargo.toml b/crates/codegen/xai-grok-pager-bin/Cargo.toml index b875052..a102002 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.1.220-alpha.4" +version = "0.2.101" edition.workspace = true license = "Apache-2.0" authors = ["xAI"] @@ -92,6 +92,5 @@ default-bazel = [ "sandbox-enforce", ] jemalloc = ["dep:tikv-jemallocator", "dep:tikv-jemalloc-sys", "dep:tikv-jemalloc-ctl"] -# Forward the sandbox-enforcement feature to the library. sandbox-enforce = ["xai-grok-pager/sandbox-enforce"] release-dist = ["xai-grok-pager/release-dist"] diff --git a/crates/codegen/xai-grok-pager-bin/src/main.rs b/crates/codegen/xai-grok-pager-bin/src/main.rs index 4f0f5ae..de17b9a 100644 --- a/crates/codegen/xai-grok-pager-bin/src/main.rs +++ b/crates/codegen/xai-grok-pager-bin/src/main.rs @@ -200,6 +200,11 @@ async fn run_setup_command(json: bool) { "Your team doesn't have a managed configuration yet. A team admin can set one up at console.x.ai." ); } + SetupOutcome::Skipped => { + eprintln!( + "Managed configuration was not applied this run (another process held the apply lock, or the credential changed during the fetch). Run `grok setup` again." + ); + } SetupOutcome::Failed(e) => { eprintln!("Couldn't apply managed configuration. {e}"); std::process::exit(1); @@ -1081,9 +1086,16 @@ async fn run_agent_command( leader_eligible, ); tracing::info!(use_leader, ?policy_disable_reason, "leader mode resolved"); - if stdio_direct_update_eligible(is_stdio, use_leader) - && should_check_for_updates(no_auto_update) - { + let managed_install = is_managed_install( + std::env::current_exe().ok(), + &xai_grok_shell::util::grok_home::grok_home(), + ); + if stdio_auto_update_enabled( + is_stdio, + use_leader, + should_check_for_updates(no_auto_update), + managed_install, + ) { let update_config = update_config.clone(); tokio::spawn(async move { auto_update::run_update_if_available( @@ -1094,6 +1106,8 @@ async fn run_agent_command( .await .ok(); }); + } else if is_stdio && !use_leader && !managed_install { + tracing::debug!("stdio auto-update skipped: not the managed install"); } if use_leader { if !agent_args.plugin_dirs.is_empty() { @@ -2082,8 +2096,8 @@ fn build_update_config() -> UpdateConfig { } config } -/// Centralized gate for all auto-update checks. Add new suppression -/// rules here — not at each call site. +/// Central gate for auto-update checks; add new suppression rules here, +/// not at call sites. fn should_check_for_updates(no_auto_update_flag: bool) -> bool { if cfg!(debug_assertions) { return false; @@ -2091,21 +2105,35 @@ fn should_check_for_updates(no_auto_update_flag: bool) -> bool { if no_auto_update_flag { return false; } - if std::env::var_os("GROK_DISABLE_AUTOUPDATER").is_some() { + !std::env::var_os("GROK_DISABLE_AUTOUPDATER") + .is_some_and(|v| env_flag_enabled(&v.to_string_lossy())) +} +/// Gate for the stdio agent's background auto-update: only the direct stdio +/// agent, from the managed install. Other modes update in `run_agent_command`. +fn stdio_auto_update_enabled( + is_stdio: bool, + use_leader: bool, + updates_enabled: bool, + managed_install: bool, +) -> bool { + is_stdio && !use_leader && updates_enabled && managed_install +} +/// True when `exe` is the binary `/bin/grok` resolves to, the +/// install that adopts a staged update on respawn. Both sides are +/// canonicalized; any failure reports unmanaged and skips the update. The +/// npm shim hardcodes `~/.grok`, so a custom `GROK_HOME` skips here too. +fn is_managed_install(exe: Option, grok_home: &std::path::Path) -> bool { + if grok_home.as_os_str().is_empty() { return false; } - true -} -/// Mode-gate for the direct stdio agent's background auto-update. -/// -/// Only the *direct* stdio agent is newly eligible: every other agent mode -/// already self-updates at the top of `run_agent_command`, and a leader-backed -/// stdio process is a thin bridge whose updates are owned by the leader -/// (`LeaderAutoUpdateConfig`). Update suppression (`--no-auto-update`, -/// `GROK_DISABLE_AUTOUPDATER`, debug builds) is layered on separately via -/// [`should_check_for_updates`]. -fn stdio_direct_update_eligible(is_stdio: bool, use_leader: bool) -> bool { - is_stdio && !use_leader + let Some(exe) = exe else { + return false; + }; + let managed = xai_grok_config::grok_application_in(grok_home); + match (dunce::canonicalize(&exe), dunce::canonicalize(&managed)) { + (Ok(exe), Ok(managed)) => exe == managed, + _ => false, + } } /// Map the mutually-exclusive channel flags to a channel name. clap enforces /// that at most one is set, so the order is irrelevant. @@ -2380,27 +2408,55 @@ mod tests { xai_grok_shell::heap_profile::dump_to_path(dump.path()).expect("shell dump"); dump.assert_nonempty_dump(); } - /// Only the direct (non-leader) stdio agent is newly eligible for the - /// background auto-update. Leader-backed stdio defers to the leader's own - /// updater, and non-stdio modes already update at the top of - /// `run_agent_command`. + #[cfg(unix)] #[test] - fn stdio_direct_update_eligible_only_for_non_leader_stdio() { + fn is_managed_install_matches_only_the_bin_grok_target() { + let home = + std::env::temp_dir().join(format!("grok-pager-managed-install-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&home); + std::fs::create_dir_all(home.join("bin")).unwrap(); + std::fs::create_dir_all(home.join("downloads")).unwrap(); + assert!(!is_managed_install( + Some(home.join("bin").join("grok")), + &home + )); + assert!(!is_managed_install(None, &home)); + assert!(!is_managed_install( + Some(home.join("bin").join("grok")), + std::path::Path::new("") + )); + let target = home.join("downloads").join("grok-1.2.3"); + std::fs::write(&target, b"binary").unwrap(); + std::os::unix::fs::symlink(&target, home.join("bin").join("grok")).unwrap(); + assert!(is_managed_install( + Some(home.join("bin").join("grok")), + &home + )); + assert!(is_managed_install(Some(target.clone()), &home)); + let pinned = home.join("bin").join("grok-9.9.9"); + std::fs::write(&pinned, b"binary").unwrap(); + assert!(!is_managed_install(Some(pinned), &home)); + let _ = std::fs::remove_dir_all(&home); + } + /// Pins the gate composition; a dropped conjunct fails its named case. + #[test] + fn stdio_auto_update_requires_direct_stdio_enabled_and_managed() { + assert!(stdio_auto_update_enabled(true, false, true, true)); assert!( - stdio_direct_update_eligible(true, false), - "direct stdio agent should be eligible", + !stdio_auto_update_enabled(true, true, true, true), + "leader bridge" ); assert!( - !stdio_direct_update_eligible(true, true), - "leader-backed stdio defers to the leader's updater", + !stdio_auto_update_enabled(false, false, true, true), + "non-stdio" ); assert!( - !stdio_direct_update_eligible(false, false), - "non-stdio modes update at the top of run_agent_command", + !stdio_auto_update_enabled(true, false, false, true), + "updates off" ); assert!( - !stdio_direct_update_eligible(false, true), - "non-stdio leader path is not stdio-eligible", + !stdio_auto_update_enabled(true, false, true, false), + "pinned binary" ); } use clap::Parser as _; diff --git a/crates/codegen/xai-grok-pager-minimal/Cargo.toml b/crates/codegen/xai-grok-pager-minimal/Cargo.toml index 11a360e..dcda4dc 100644 --- a/crates/codegen/xai-grok-pager-minimal/Cargo.toml +++ b/crates/codegen/xai-grok-pager-minimal/Cargo.toml @@ -44,6 +44,6 @@ xai-token-estimation = { workspace = true } # The unit tests reuse the pager's test-only view-model constructors # (`test_agent_view`, yolo/auto setters), gated behind its test-only # helpers. -xai-grok-pager = { path = "../xai-grok-pager", features = [] } +xai-grok-pager = { path = "../xai-grok-pager", features = ["test-support"] } [features] diff --git a/crates/codegen/xai-grok-pager-minimal/src/auth.rs b/crates/codegen/xai-grok-pager-minimal/src/auth.rs index 4f517d0..4eef0be 100644 --- a/crates/codegen/xai-grok-pager-minimal/src/auth.rs +++ b/crates/codegen/xai-grok-pager-minimal/src/auth.rs @@ -1,24 +1,26 @@ -//! Minimal-mode sign-in rendering for the live region. +//! Minimal-mode sign-in / folder-trust rendering for the live region. //! //! Before any agent session exists (unauthenticated / folder-trust pending) the //! minimal live region shows the sign-in flow itself — device or external-command -//! flow, a sign-in error, or a brief "starting" transient once authenticated — -//! since minimal has no welcome screen. [`draw_live`](super::live::draw_live) -//! computes a [`MinimalAuthHint`] from the app's [`AuthState`] and renders it via -//! [`render_auth`]. +//! flow, a sign-in error, the folder-trust question, or a brief "starting" +//! transient once both gates are open — since minimal has no welcome screen. +//! [`draw_live`](super::live::draw_live) computes a [`MinimalAuthHint`] from the +//! app's [`AuthState`] + [`TrustState`] and renders it via [`render_auth`]. + +use std::path::PathBuf; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; -use xai_grok_pager::app::app_view::AuthState; +use xai_grok_pager::app::app_view::{AuthState, TrustState}; use xai_grok_pager::theme::Theme; /// What the minimal live region shows when there is no active agent yet: the -/// in-region sign-in flow (device or external-command), a sign-in error, or a -/// brief "starting" transient once authenticated. Computed from [`AuthState`] -/// before the draw closure so the closure can own it. +/// in-region sign-in flow (device or external-command), a sign-in error, the +/// folder-trust question, or a brief "starting" transient once authenticated +/// (and trusted). Computed before the draw closure so the closure can own it. pub(super) enum MinimalAuthHint { /// Interactive sign-in underway — show the URL (when known) and the device /// code (when the URL carries one). Covers device flow and the external @@ -30,12 +32,26 @@ pub(super) enum MinimalAuthHint { }, /// The last sign-in attempt failed; show the error. Failed(String), - /// Authenticated — the session is being created (brief transient). + /// Authenticated, but the cwd has untrusted repo-local config — ask before + /// creating a session. Input (y/Enter trust, n/Esc quit) is handled by the + /// welcome interceptor in `AppView::handle_input`; this is render-only. + TrustFolder { workspace: PathBuf }, + /// Authenticated (+ trusted) — the session is being created (brief transient). Starting, } -/// Map the app's [`AuthState`] to what the no-agent live region should show. -pub(super) fn minimal_auth_hint(auth: &AuthState) -> MinimalAuthHint { +/// Map the app's auth + trust state to what the no-agent live region should show. +/// +/// Mirrors the welcome screen's gate order: trust is only offered after auth is +/// `Done`, when the user has access and is not ZDR-blocked (those gates already +/// block sessions, and the input interceptor only answers trust under the same +/// conditions). +pub(super) fn minimal_auth_hint( + auth: &AuthState, + trust: &TrustState, + has_access: bool, + is_zdr_blocked: bool, +) -> MinimalAuthHint { match auth { AuthState::Authenticating { auth_url, .. } => MinimalAuthHint::SigningIn { url: auth_url.clone(), @@ -51,10 +67,59 @@ pub(super) fn minimal_auth_hint(auth: &AuthState) -> MinimalAuthHint { url: None, code: None, }, + AuthState::Done if has_access && !is_zdr_blocked => { + if let TrustState::Pending { workspace } = trust { + MinimalAuthHint::TrustFolder { + workspace: workspace.clone(), + } + } else { + MinimalAuthHint::Starting + } + } AuthState::Done => MinimalAuthHint::Starting, } } +/// Rows the no-agent live region needs for `hint` (before path wrap). Used by +/// the overlay host so the viewport grows enough to show the trust question +/// instead of clipping to the idle prompt height. +pub(super) fn auth_hint_rows(hint: &MinimalAuthHint, width: u16) -> u16 { + match hint { + // header + blank + "Opening browser…" + MinimalAuthHint::SigningIn { url: None, code: _ } => 3, + // header + blank + "Open this URL" + url rows + optional code block + + // blank + "Waiting…" + MinimalAuthHint::SigningIn { + url: Some(url), + code, + } => { + let url_rows = wrapped_char_rows(url, width); + let code_rows = if code.is_some() { 2 } else { 0 }; // blank + "Code: …" + 3 + url_rows + code_rows + 2 + } + // "Sign-in failed" + blank + error + MinimalAuthHint::Failed(_) => 3, + // question + path rows + blank + 2 warning + blank + 2 menu + blank + hint + MinimalAuthHint::TrustFolder { workspace } => { + let path = workspace.display().to_string(); + let path_rows = wrapped_char_rows(&path, width); + 1 + path_rows + 1 + 2 + 1 + 2 + 1 + 1 + } + MinimalAuthHint::Starting => 1, + } +} + +/// How many rows `text` needs when painted char-by-char at `width` (no +/// wrap-inserted spaces) — same layout as [`render_url`]. +fn wrapped_char_rows(text: &str, width: u16) -> u16 { + let width = width.max(1) as usize; + let chars = text.chars().filter(|c| !c.is_control()).count(); + if chars == 0 { + return 1; + } + chars.div_ceil(width) as u16 +} + /// Parse the device-flow `user_code` from a verification URL (`None` if absent /// or malformed). Mirrors `views::welcome::extract_user_code`, kept local so /// minimal does not depend on welcome-screen internals. @@ -120,8 +185,8 @@ fn render_url( y.saturating_add(1) } -/// Render the sign-in flow (or transient status) in the live region when no -/// agent exists yet. Top-aligned in `area`; clips to its height. +/// Render the sign-in / trust flow (or transient status) in the live region when +/// no agent exists yet. Top-aligned in `area`; clips to its height. pub(super) fn render_auth(buf: &mut Buffer, area: Rect, theme: &Theme, hint: &MinimalAuthHint) { if area.width == 0 || area.height == 0 { return; @@ -221,6 +286,77 @@ pub(super) fn render_auth(buf: &mut Buffer, area: Rect, theme: &Theme, hint: &Mi Line::from(Span::styled(err.clone(), gray)), ); } + MinimalAuthHint::TrustFolder { workspace } => { + // Mirrors `render_welcome_trust` copy, flush-left for minimal. + y = put_line( + buf, + area, + y, + bottom, + Line::from(Span::styled( + "Do you trust the contents of this directory?", + bold, + )), + ); + y = render_url( + buf, + area, + y, + bottom, + &workspace.display().to_string(), + Style::default().fg(theme.accent_user).bg(Color::Reset), + ); + y = put_line(buf, area, y, bottom, Line::default()); + y = put_line( + buf, + area, + y, + bottom, + Line::from(Span::styled( + "Grok Build may run or modify contents in this directory,", + gray, + )), + ); + y = put_line( + buf, + area, + y, + bottom, + Line::from(Span::styled("posing security risks.", gray)), + ); + y = put_line(buf, area, y, bottom, Line::default()); + y = put_line( + buf, + area, + y, + bottom, + Line::from(vec![ + Span::styled("y", bold), + Span::styled(" Yes, proceed", gray), + ]), + ); + y = put_line( + buf, + area, + y, + bottom, + Line::from(vec![ + Span::styled("n", bold), + Span::styled(" No, quit", gray), + ]), + ); + y = put_line(buf, area, y, bottom, Line::default()); + let _ = put_line( + buf, + area, + y, + bottom, + Line::from(Span::styled( + "Enter or y to trust \u{00b7} n or Esc to quit", + gray, + )), + ); + } MinimalAuthHint::Starting => { let _ = put_line( buf, @@ -257,6 +393,8 @@ mod tests { fn auth_hint_maps_auth_state() { use xai_grok_pager::app::app_view::AuthMode; + let trust_done = TrustState::Done; + // Device flow → SigningIn carrying the URL and the parsed code. let st = AuthState::Authenticating { request_seq: 1, @@ -264,7 +402,7 @@ mod tests { auth_url: Some("https://accounts.x.ai/device?user_code=ABCD-EFGH".into()), mode: AuthMode::Device, }; - match minimal_auth_hint(&st) { + match minimal_auth_hint(&st, &trust_done, true, false) { MinimalAuthHint::SigningIn { url, code } => { assert_eq!( url.as_deref(), @@ -282,7 +420,7 @@ mod tests { auth_url: Some("https://provider.example/login".into()), mode: AuthMode::Command, }; - match minimal_auth_hint(&st) { + match minimal_auth_hint(&st, &trust_done, true, false) { MinimalAuthHint::SigningIn { url, code } => { assert_eq!(url.as_deref(), Some("https://provider.example/login")); assert!(code.is_none()); @@ -291,17 +429,52 @@ mod tests { } assert!(matches!( - minimal_auth_hint(&AuthState::Done), + minimal_auth_hint(&AuthState::Done, &trust_done, true, false), MinimalAuthHint::Starting )); assert!(matches!( - minimal_auth_hint(&AuthState::Pending { - error: Some("nope".into()) - }), + minimal_auth_hint( + &AuthState::Pending { + error: Some("nope".into()) + }, + &trust_done, + true, + false + ), MinimalAuthHint::Failed(_) )); } + #[test] + fn auth_hint_maps_pending_trust_after_auth() { + let trust = TrustState::Pending { + workspace: PathBuf::from("/tmp/untrusted-repo"), + }; + match minimal_auth_hint(&AuthState::Done, &trust, true, false) { + MinimalAuthHint::TrustFolder { workspace } => { + assert_eq!(workspace, PathBuf::from("/tmp/untrusted-repo")); + } + _ => panic!("expected TrustFolder"), + } + + // Access / ZDR gates suppress the trust question (matches welcome + + // the input interceptor). + assert!(matches!( + minimal_auth_hint(&AuthState::Done, &trust, false, false), + MinimalAuthHint::Starting + )); + assert!(matches!( + minimal_auth_hint(&AuthState::Done, &trust, true, true), + MinimalAuthHint::Starting + )); + + // Trust is not offered while auth is still in flight. + assert!(matches!( + minimal_auth_hint(&AuthState::Pending { error: None }, &trust, true, false), + MinimalAuthHint::SigningIn { .. } + )); + } + #[test] fn render_auth_shows_url_and_code() { let theme = Theme::current(); @@ -312,14 +485,7 @@ mod tests { code: Some("ABCD-EFGH".into()), }; render_auth(&mut buf, area, &theme, &hint); - let mut text = String::new(); - for y in 0..area.height { - for x in 0..area.width { - if let Some(c) = buf.cell((x, y)) { - text.push_str(c.symbol()); - } - } - } + let text = buffer_text(&buf, area); assert!(text.contains("Sign in to Grok"), "header: {text:?}"); assert!(text.contains("accounts.x.ai/device"), "url: {text:?}"); assert!(text.contains("ABCD-EFGH"), "device code: {text:?}"); @@ -328,4 +494,52 @@ mod tests { "waiting line: {text:?}" ); } + + #[test] + fn render_auth_shows_trust_question() { + let theme = Theme::current(); + let area = Rect::new(0, 0, 80, 14); + let mut buf = Buffer::empty(area); + let hint = MinimalAuthHint::TrustFolder { + workspace: PathBuf::from("/home/agent/project"), + }; + render_auth(&mut buf, area, &theme, &hint); + let text = buffer_text(&buf, area); + assert!( + text.contains("Do you trust the contents of this directory?"), + "question: {text:?}" + ); + assert!( + text.contains("/home/agent/project"), + "workspace path: {text:?}" + ); + assert!(text.contains("Yes, proceed"), "yes option: {text:?}"); + assert!(text.contains("No, quit"), "no option: {text:?}"); + assert!(text.contains("Enter or y to trust"), "hint line: {text:?}"); + assert!(text.contains("posing security risks"), "warning: {text:?}"); + } + + #[test] + fn auth_hint_rows_covers_trust_path_wrap() { + let long = "x".repeat(200); + let hint = MinimalAuthHint::TrustFolder { + workspace: PathBuf::from(long), + }; + let rows = auth_hint_rows(&hint, 40); + // path alone needs 5 rows at width 40 (200/40); total well above base. + assert!(rows >= 12, "expected room for wrapped path, got {rows}"); + } + + fn buffer_text(buf: &Buffer, area: Rect) -> String { + let mut text = String::new(); + for y in 0..area.height { + for x in 0..area.width { + if let Some(c) = buf.cell((x, y)) { + text.push_str(c.symbol()); + } + } + text.push('\n'); + } + text + } } diff --git a/crates/codegen/xai-grok-pager-minimal/src/live.rs b/crates/codegen/xai-grok-pager-minimal/src/live.rs index f825235..9a6767c 100644 --- a/crates/codegen/xai-grok-pager-minimal/src/live.rs +++ b/crates/codegen/xai-grok-pager-minimal/src/live.rs @@ -71,7 +71,12 @@ pub(super) fn prompt_style( /// Draw the pinned live region (tail + status + prompt) into the inline viewport. pub fn draw_live(app: &mut AppView, terminal: &mut PagerTerminal) { let force_todos = minimal_api::minimal_show_todos(app); - let auth_hint = crate::auth::minimal_auth_hint(&app.auth_state); + let auth_hint = crate::auth::minimal_auth_hint( + &app.auth_state, + &app.trust_state, + app.has_access(), + app.is_zdr_blocked(), + ); let pending_hint = minimal_pending_hint(&app.pending_action); let transcript_hint = if minimal_api::minimal_ctrl_o_opens_transcript(app) { "ctrl+o transcript" diff --git a/crates/codegen/xai-grok-pager-minimal/src/overlay.rs b/crates/codegen/xai-grok-pager-minimal/src/overlay.rs index 501d134..070fdb5 100644 --- a/crates/codegen/xai-grok-pager-minimal/src/overlay.rs +++ b/crates/codegen/xai-grok-pager-minimal/src/overlay.rs @@ -230,7 +230,16 @@ fn compute_target(app: &mut AppView, term_h: u16, width: u16) -> u16 { let content_w = width as usize; let ActiveView::Agent(id) = &app.active_view else { - return base; + // No agent yet: size for the in-region sign-in / folder-trust UI so the + // trust question isn't clipped to the idle prompt height. + let hint = super::auth::minimal_auth_hint( + &app.auth_state, + &app.trust_state, + app.has_access(), + app.is_zdr_blocked(), + ); + let needed = super::auth::auth_hint_rows(&hint, width); + return needed.max(base).min(ceiling); }; let id = *id; let Some(agent) = app.agents.get_mut(&id) else { diff --git a/crates/codegen/xai-grok-pager-render/Cargo.toml b/crates/codegen/xai-grok-pager-render/Cargo.toml index dc692eb..032a9cd 100644 --- a/crates/codegen/xai-grok-pager-render/Cargo.toml +++ b/crates/codegen/xai-grok-pager-render/Cargo.toml @@ -102,9 +102,12 @@ serial_test = { workspace = true } tempfile = { workspace = true } [features] +# Exposes `#[cfg(test)]` test-only helpers (mock setters, test guards, +# deterministic pinning) to downstream crates' test builds. Enabled by +test-support = [] # CI builds a single shared rlib variant per crate, so the pager's CI # test target links the same render lib as production. Enabling the test-only # helpers in the CI default set keeps them present for the pager test while # leaving cargo's `default` (and thus cargo production builds) without them. # The helpers are inert unless their mock setters are called. -default-bazel = [] +default-bazel = ["test-support"] diff --git a/crates/codegen/xai-grok-pager-render/src/clipboard/mod.rs b/crates/codegen/xai-grok-pager-render/src/clipboard/mod.rs index 94927e0..040587e 100644 --- a/crates/codegen/xai-grok-pager-render/src/clipboard/mod.rs +++ b/crates/codegen/xai-grok-pager-render/src/clipboard/mod.rs @@ -420,7 +420,7 @@ pub struct ClipboardTextReadError; /// Read CLIPBOARD text while distinguishing emptiness from failure. pub fn system_clipboard_read_text() -> Result, ClipboardTextReadError> { - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(text) = test_support::hook_text_result() { return text; } @@ -438,7 +438,7 @@ pub fn system_clipboard_get() -> Option { /// Read X11 PRIMARY text for an unmodified Linux middle-button press. #[cfg(target_os = "linux")] pub fn system_primary_selection_get() -> Option { - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(available) = test_support::hook_x11_primary_available() { if !available { return None; @@ -705,7 +705,7 @@ pub fn system_clipboard_probe_attachments( if !attachment_probe_would_run(clipboard_text) { return Ok((None, None)); } - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(canned) = test_support::hook_attachments() { return canned; } @@ -786,7 +786,7 @@ pub use xai_grok_shared::clipboard::ImageData; /// single native pass (macOS native, sub-millisecond, no data read). `(None, /// false)` off-macOS or when AppKit cannot be loaded. pub fn clipboard_image_snapshot() -> (Option, bool) { - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(snapshot) = test_support::hook_image_snapshot() { return snapshot; } @@ -799,7 +799,7 @@ pub fn clipboard_image_snapshot() -> (Option, bool) { /// [`clipboard_image_snapshot`] classification. `None` off-macOS. pub fn clipboard_change_count() -> Option { // Seam consistency: a hooked snapshot's change_count is the changeCount. - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some((change_count, _)) = test_support::hook_image_snapshot() { return change_count; } @@ -809,7 +809,7 @@ pub fn clipboard_change_count() -> Option { /// Whether the fast image probe exists on this platform. Gates the /// focus-driven clipboard-image tip so non-macOS never probes. pub fn clipboard_image_probe_supported() -> bool { - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(supported) = test_support::hook_image_probe_supported() { return supported; } @@ -880,7 +880,7 @@ pub fn system_clipboard_get_image() -> Option { /// (the off-thread probe reads the REAL pasteboard there), so tests exercise /// deferral by asserting the enqueued effect and then driving /// `complete_clipboard_attachment_paste` directly with a canned outcome. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub mod test_support { use super::{ClipboardProbeError, ClipboardTextReadError, ImageData}; use std::cell::{Cell, RefCell}; @@ -1039,9 +1039,9 @@ pub mod test_support { } } -#[cfg(all(test, target_os = "linux"))] +#[cfg(all(any(test, feature = "test-support"), target_os = "linux"))] pub use test_support::primary_selection_read_call_count; -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub use test_support::{ ClipboardProbeHook, clear_clipboard_probe_hook, clipboard_probe_call_count, set_clipboard_probe_hook, diff --git a/crates/codegen/xai-grok-pager-render/src/gboom/game.rs b/crates/codegen/xai-grok-pager-render/src/gboom/game.rs index 3d76b14..5b55aae 100644 --- a/crates/codegen/xai-grok-pager-render/src/gboom/game.rs +++ b/crates/codegen/xai-grok-pager-render/src/gboom/game.rs @@ -404,7 +404,7 @@ impl Game { /// Whether any movement control is currently held (latched or within /// the repeat-bridging window). Used to assert hold-clearing behavior. - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] pub fn any_held(&self) -> bool { self.hold.iter().any(|&h| h > 0.0) } diff --git a/crates/codegen/xai-grok-pager-render/src/gboom/mod.rs b/crates/codegen/xai-grok-pager-render/src/gboom/mod.rs index 596d1ac..42915d2 100644 --- a/crates/codegen/xai-grok-pager-render/src/gboom/mod.rs +++ b/crates/codegen/xai-grok-pager-render/src/gboom/mod.rs @@ -276,7 +276,7 @@ impl GboomState { /// Whether the game currently holds a latched movement control. Lets the /// app layer assert that backgrounded games drop their holds. - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] pub fn any_movement_held(&self) -> bool { self.game.any_held() } diff --git a/crates/codegen/xai-grok-pager-render/src/link_opener.rs b/crates/codegen/xai-grok-pager-render/src/link_opener.rs index ca0aed6..d907daa 100644 --- a/crates/codegen/xai-grok-pager-render/src/link_opener.rs +++ b/crates/codegen/xai-grok-pager-render/src/link_opener.rs @@ -4,17 +4,66 @@ //! code path (keyboard navigation, mouse click, action dispatch) can //! open a link safely without duplicating platform-specific logic. +use std::collections::HashMap; + use crate::terminal::hyperlinks::SchemeFilter; +/// Outcome of attempting to open a URL in the system browser/handler. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OpenUrlResult { + /// Opener was launched (or the test seam recorded the URL). + Opened, + /// Scheme was rejected by the safety filter. + RejectedScheme, + /// Browser cannot run here (headless / no display) or the opener + /// failed to spawn. Callers should surface the URL for manual open. + BrowserUnavailable, +} + +/// Whether the environment looks capable of opening a GUI browser. +/// +/// Pure helper for tests. On Linux/BSD, requires a non-empty `DISPLAY` or +/// `WAYLAND_DISPLAY` (or a non-empty `BROWSER` override). macOS/Windows +/// are treated as available at the env level (spawn failure is still +/// reported by [`open_url`]). +pub fn browser_open_likely_available_from_env(env: &HashMap) -> bool { + if cfg!(any(target_os = "macos", target_os = "windows")) { + return true; + } + // Explicit BROWSER override: allow even without a display server so + // scripted/headless setups that point at a CLI browser still try. + if env.get("BROWSER").is_some_and(|v| !v.is_empty()) { + return true; + } + env.get("WAYLAND_DISPLAY").is_some_and(|v| !v.is_empty()) + || env.get("DISPLAY").is_some_and(|v| !v.is_empty()) +} + +/// Whether this process likely has a GUI browser available right now. +pub fn browser_open_likely_available() -> bool { + let env = crate::host::collect_unicode_env(); + browser_open_likely_available_from_env(&env) +} + +/// User-facing copy when the browser opener cannot run. Includes the full +/// URL on its own line so it is easy to select/copy in the TUI. +pub fn browser_unavailable_message(url: &str) -> String { + format!("Could not open a browser. Open this URL manually:\n{url}") +} + /// Open a URL in the system's default browser/handler. /// /// Spawns the platform-native opener (`open` on macOS, `xdg-open` on /// Linux, `cmd /c start` on Windows) with fully detached stdio so it /// cannot block the pager. /// +/// Returns `true` when the opener was launched (or the test seam recorded +/// the URL). Returns `false` when the environment looks headless or spawn +/// fails — callers should show [`browser_unavailable_message`]. +/// /// **Callers handling untrusted input** should call [`is_safe_to_open`] -/// first, or use [`open_url_if_safe`] which combines both steps. -pub fn open_url(url: &str) { +/// first, or use [`open_url_if_safe`] / [`try_open_url`] which combine both. +pub fn open_url(url: &str) -> bool { // Test seam: PTY e2e must observe the open without launching a real // browser. When set, append the URL to the file and skip the OS opener. if let Ok(path) = std::env::var("GROK_TEST_OPEN_URL_FILE") { @@ -28,8 +77,17 @@ pub fn open_url(url: &str) { .and_then(|mut f| writeln!(f, "{url}")) { tracing::warn!(error = %e, path, "GROK_TEST_OPEN_URL_FILE write failed"); + return false; } - return; + return true; + } + + // Skip the doomed spawn on headless Linux VMs (no DISPLAY / Wayland) + // so billing Upgrade / Buy-credits clicks can fall back to showing the + // URL instead of silently no-op'ing. + if !browser_open_likely_available() { + tracing::info!("skipping browser open: no display server / BROWSER"); + return false; } #[cfg(target_os = "macos")] @@ -48,16 +106,20 @@ pub fn open_url(url: &str) { .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()); xai_grok_tools::util::detach_std_command(&mut command); - if let Err(e) = command.spawn() { - // Redact URL to avoid leaking sensitive query params to logs. - let redacted = url::Url::parse(url) - .map(|mut u| { - u.set_query(None); - u.set_fragment(None); - u.to_string() - }) - .unwrap_or_else(|_| "".to_string()); - tracing::warn!(url = %redacted, error = %e, "failed to open URL"); + match command.spawn() { + Ok(_) => true, + Err(e) => { + // Redact URL to avoid leaking sensitive query params to logs. + let redacted = url::Url::parse(url) + .map(|mut u| { + u.set_query(None); + u.set_fragment(None); + u.to_string() + }) + .unwrap_or_else(|_| "".to_string()); + tracing::warn!(url = %redacted, error = %e, "failed to open URL"); + false + } } } @@ -199,14 +261,26 @@ pub fn is_safe_to_open(url: &str, filter: SchemeFilter) -> bool { false } -/// Validate scheme and open a URL if permitted. Returns `true` if opened. +/// Validate scheme and open a URL if permitted. +/// +/// Returns `true` only when the scheme is allowed **and** the opener was +/// launched. Distinguishes scheme rejection from browser unavailability +/// via [`try_open_url`]. pub fn open_url_if_safe(url: &str, filter: SchemeFilter) -> bool { - if is_safe_to_open(url, filter) { - open_url(url); - true - } else { + matches!(try_open_url(url, filter), OpenUrlResult::Opened) +} + +/// Validate scheme and attempt to open. Prefer this when the caller needs +/// to show a manual-URL fallback on [`OpenUrlResult::BrowserUnavailable`]. +pub fn try_open_url(url: &str, filter: SchemeFilter) -> OpenUrlResult { + if !is_safe_to_open(url, filter) { tracing::debug!(url, "URL scheme not permitted"); - false + return OpenUrlResult::RejectedScheme; + } + if open_url(url) { + OpenUrlResult::Opened + } else { + OpenUrlResult::BrowserUnavailable } } @@ -446,4 +520,67 @@ mod tests { SchemeFilter::Standard )); } + + fn env(pairs: &[(&str, &str)]) -> HashMap { + pairs + .iter() + .map(|(k, v)| ((*k).to_owned(), (*v).to_owned())) + .collect() + } + + #[test] + fn browser_available_with_x11_display() { + assert!(browser_open_likely_available_from_env(&env(&[( + "DISPLAY", ":0" + )]))); + } + + #[test] + fn browser_available_with_wayland() { + assert!(browser_open_likely_available_from_env(&env(&[( + "WAYLAND_DISPLAY", + "wayland-0" + )]))); + } + + #[test] + fn browser_available_with_browser_env_override() { + // Headless boxes can still open via BROWSER=… even without DISPLAY. + assert!(browser_open_likely_available_from_env(&env(&[( + "BROWSER", "firefox" + )]))); + } + + #[test] + fn browser_unavailable_when_display_vars_empty_or_missing() { + if cfg!(any(target_os = "macos", target_os = "windows")) { + // Desktop OSes do not gate on DISPLAY. + assert!(browser_open_likely_available_from_env(&env(&[]))); + return; + } + assert!(!browser_open_likely_available_from_env(&env(&[]))); + assert!(!browser_open_likely_available_from_env(&env(&[ + ("DISPLAY", ""), + ("WAYLAND_DISPLAY", ""), + ("BROWSER", ""), + ]))); + } + + #[test] + fn browser_unavailable_message_includes_full_url() { + let url = "https://grok.com/supergrok?referrer=grok-build"; + let msg = browser_unavailable_message(url); + assert!(msg.contains("Could not open a browser")); + assert!(msg.contains(url)); + // URL on its own line for easy select/copy in the TUI. + assert!(msg.lines().any(|l| l == url)); + } + + #[test] + fn try_open_url_rejects_unsafe_scheme_without_opening() { + assert_eq!( + try_open_url("javascript:alert(1)", SchemeFilter::Standard), + OpenUrlResult::RejectedScheme + ); + } } diff --git a/crates/codegen/xai-grok-pager-render/src/prompt_images.rs b/crates/codegen/xai-grok-pager-render/src/prompt_images.rs index e67d2df..9c40de7 100644 --- a/crates/codegen/xai-grok-pager-render/src/prompt_images.rs +++ b/crates/codegen/xai-grok-pager-render/src/prompt_images.rs @@ -703,7 +703,7 @@ impl PromptImagePreview { self.finish(PromptImagePreviewResult::Failed); } - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] pub fn ready_for_test(bytes: Vec, dimensions: (u32, u32)) -> Self { let preview = Self::default(); preview.finish(PromptImagePreviewResult::Ready { diff --git a/crates/codegen/xai-grok-pager-render/src/render/osc8.rs b/crates/codegen/xai-grok-pager-render/src/render/osc8.rs index 75748bc..3af3db5 100644 --- a/crates/codegen/xai-grok-pager-render/src/render/osc8.rs +++ b/crates/codegen/xai-grok-pager-render/src/render/osc8.rs @@ -13,13 +13,86 @@ use linkify::{LinkFinder, LinkKind}; use ratatui::text::Line; use unicode_width::UnicodeWidthStr; +/// Semantic destination of a pager link. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum LinkTarget { + Url(Arc), + File(Arc), +} + +/// Whether the painted text can independently identify its semantic target. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum LinkPresentation { + #[default] + Opaque, + SelfResolvingPath, +} + +/// Output and activation policy for a semantic link target. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedLinkTarget { + /// Terminal-owned OSC 8 destination, or `None` when plain text owns discovery. + pub osc8_url: Option>, + /// App-owned activation target, or `None` when activation is delegated. + pub open_target: Option, +} + +/// Resolve one semantic target using the current terminal context. +pub fn resolve_link_target(target: &LinkTarget) -> Option { + resolve_link_target_with_presentation(target, LinkPresentation::Opaque) +} + +pub fn resolve_link_target_with_presentation( + target: &LinkTarget, + presentation: LinkPresentation, +) -> Option { + resolve_link_target_for_context(target, presentation, crate::terminal::terminal_context()) +} + +/// Resolve one semantic target for both OSC 8 output and app-owned activation. +pub fn resolve_link_target_for_context( + target: &LinkTarget, + presentation: LinkPresentation, + terminal: &crate::terminal::TerminalContext, +) -> Option { + match target { + LinkTarget::Url(url) => { + let filter = crate::terminal::hyperlinks::SchemeFilter::Standard; + crate::link_opener::is_safe_to_open(url, filter).then(|| ResolvedLinkTarget { + osc8_url: Some(Arc::clone(url)), + open_target: Some(LinkTarget::Url(Arc::clone(url))), + }) + } + LinkTarget::File(_) + if terminal.brand == crate::terminal::TerminalName::VsCode + && terminal.is_official_vscode_remote + && presentation == LinkPresentation::SelfResolvingPath => + { + Some(ResolvedLinkTarget { + osc8_url: None, + open_target: None, + }) + } + LinkTarget::File(path) => Some(ResolvedLinkTarget { + osc8_url: file_path_to_url(path), + open_target: Some(LinkTarget::File(Arc::clone(path))), + }), + } +} + +/// Resolve the target for app-owned activation. +pub fn resolve_link_open_target(target: &LinkTarget) -> Option { + resolve_link_target(target).and_then(|resolved| resolved.open_target) +} + /// A single link region on screen. #[derive(Debug, Clone)] pub struct OverlayLink { pub screen_row: u16, pub col_start: u16, pub col_end: u16, - pub url: Arc, + pub target: LinkTarget, + pub presentation: LinkPresentation, pub id: Option, } @@ -154,10 +227,10 @@ fn quoted_file_path_regex() -> &'static regex::Regex { }) } -/// Turn a display path (`/abs/…` or `~/…`) into a `file://` URL, expanding `~/`. -/// Relative paths fail — use [`tool_path_file_url`] to join cwd first. -pub fn path_to_file_url(path: &str) -> Option> { - tool_path_file_url(path, None) +/// Turn a display path (`/abs/…` or `~/…`) into a semantic filesystem target. +/// Relative paths fail — use [`tool_path_file_target`] to join cwd first. +pub fn path_to_file_target(path: &str) -> Option { + tool_path_file_target(path, None) } fn file_path_to_url(path: &Path) -> Option> { @@ -167,24 +240,76 @@ fn file_path_to_url(path: &Path) -> Option> { } #[cfg(test)] -fn tool_path_file_url_with_home( +fn tool_path_file_target_with_home( path: &str, cwd: Option<&Path>, home: Option<&Path>, -) -> Option> { +) -> Option { let target = crate::render::tool_paths::resolve_tool_path_target_with_home(Path::new(path), cwd, home)?; - file_path_to_url(&target) + Some(LinkTarget::File(Arc::from(target))) } -/// `file://` URL for a Read/Edit target, joining ordinary relative paths to `cwd`. -pub fn tool_path_file_url(path: &str, cwd: Option<&Path>) -> Option> { - let target = crate::render::tool_paths::resolve_tool_path_target(path, cwd)?; - file_path_to_url(&target) +/// Semantic target for a Read/Edit path, joining ordinary relative paths to `cwd`. +pub fn tool_path_file_target(path: &str, cwd: Option<&Path>) -> Option { + crate::render::tool_paths::resolve_tool_path_target(path, cwd) + .map(|path| LinkTarget::File(Arc::from(path))) } -/// Resolve a markdown link destination that names a local file into a `file://` -/// URL, so paths the model emits (`[videos/1.mp4](videos/1.mp4)`) open on click. +fn file_link_presentation_for_resolved( + painted: &str, + target: &LinkTarget, + cwd: Option<&Path>, + resolved: Option<&Path>, +) -> LinkPresentation { + let LinkTarget::File(target_path) = target else { + return LinkPresentation::Opaque; + }; + let painted_path = Path::new(painted); + let is_absolute = painted_path.is_absolute() + || matches!( + painted_path.components().next(), + Some(std::path::Component::Prefix(_)) + ); + let is_home_relative = + painted == "~" || painted.starts_with("~/") || painted.starts_with(r"~\"); + if !is_absolute && !is_home_relative && (!painted.contains(['/', '\\']) || cwd.is_none()) { + return LinkPresentation::Opaque; + } + resolved + .filter(|resolved| *resolved == target_path.as_ref()) + .map_or(LinkPresentation::Opaque, |_| { + LinkPresentation::SelfResolvingPath + }) +} + +/// Classify painted file text only when it independently resolves to `target`. +pub fn file_link_presentation( + painted: &str, + target: &LinkTarget, + cwd: Option<&Path>, +) -> LinkPresentation { + let resolved = crate::render::tool_paths::resolve_tool_path_target(painted, cwd); + file_link_presentation_for_resolved(painted, target, cwd, resolved.as_deref()) +} + +#[cfg(test)] +fn file_link_presentation_with_home( + painted: &str, + target: &LinkTarget, + cwd: Option<&Path>, + home: Option<&Path>, +) -> LinkPresentation { + let resolved = crate::render::tool_paths::resolve_tool_path_target_with_home( + Path::new(painted), + cwd, + home, + ); + file_link_presentation_for_resolved(painted, target, cwd, resolved.as_deref()) +} + +/// Resolve a markdown link destination that names a local file into a semantic +/// filesystem target, so model paths (`[videos/1.mp4](videos/1.mp4)`) open on click. /// /// Web/scheme URLs, `mailto:`/`tel:`, and anchors return `None`. /// @@ -195,7 +320,7 @@ pub fn tool_path_file_url(path: &str, cwd: Option<&Path>) -> Option> { /// each short path to the exact file its message produced (correct across /// forks/resumes) and never opens an arbitrary or out-of-session file; an /// ambiguous or absent match is left unlinked. -pub fn local_link_to_file_url(dest: &str, media_paths: &[PathBuf]) -> Option> { +pub fn local_link_to_file_target(dest: &str, media_paths: &[PathBuf]) -> Option { let dest = dest.trim(); if dest.is_empty() || dest.starts_with('#') || dest.contains("://") { return None; @@ -222,9 +347,7 @@ pub fn local_link_to_file_url(dest: &str, media_paths: &[PathBuf]) -> Option, - url: &Arc, + target: &LinkTarget, + presentation: LinkPresentation, overlay: &mut LinkOverlay, ) -> bool { let mut segments: Vec<(u16, u16, u16)> = Vec::new(); @@ -349,7 +473,8 @@ fn push_link_segments( screen_row, col_start, col_end, - url: Arc::clone(url), + target: target.clone(), + presentation, id: None, }); } @@ -390,13 +515,14 @@ fn scan_logical_line( .get_or_insert_with(Vec::new) .push(link.start()..link.end()); - let url: Arc = Arc::from(url); + let target = LinkTarget::Url(Arc::from(url)); push_link_segments( text, rows, content_x, link.start()..link.end(), - &url, + &target, + LinkPresentation::Opaque, overlay, ); } @@ -419,7 +545,7 @@ fn scan_logical_line( if range_overlaps_urls(path_m.start(), path_m.end()) { continue; } - let Some(file_url) = path_to_file_url(path_m.as_str()) else { + let Some(file_target) = path_to_file_target(path_m.as_str()) else { continue; }; @@ -429,7 +555,8 @@ fn scan_logical_line( rows, content_x, path_m.start()..path_m.end(), - &file_url, + &file_target, + file_link_presentation(path_m.as_str(), &file_target, None), overlay, ) { path_byte_ranges.push(path_m.start()..path_m.end()); @@ -462,7 +589,7 @@ fn scan_logical_line( continue; } let path_end = m.start() + path.len(); - let Some(file_url) = path_to_file_url(path) else { + let Some(file_target) = path_to_file_target(path) else { continue; }; @@ -471,7 +598,8 @@ fn scan_logical_line( rows, content_x, m.start()..path_end, - &file_url, + &file_target, + file_link_presentation(path, &file_target, None), overlay, ) { path_byte_ranges.push(m.start()..path_end); @@ -503,7 +631,7 @@ fn scan_logical_line( let path = m .as_str() .trim_end_matches(['.', ',', ';', ':', '!', '?', ')']); - let Some(file_url) = local_link_to_file_url(path, media_paths) else { + let Some(file_target) = local_link_to_file_target(path, media_paths) else { continue; }; let path_end = m.start() + path.len(); @@ -513,7 +641,8 @@ fn scan_logical_line( rows, content_x, m.start()..path_end, - &file_url, + &file_target, + LinkPresentation::Opaque, overlay, ) { path_byte_ranges.push(m.start()..path_end); @@ -541,7 +670,7 @@ mod tests { scan_lines_for_url_overlays(rows.into_iter(), content_x, media_paths, overlay); } - // ── local_link_to_file_url ── + // ── local_link_to_file_target ── #[test] fn local_link_relative_resolves_to_generated_media() { @@ -551,7 +680,10 @@ mod tests { let media = vec![dir.path().join("images/1.jpg")]; // Short session-relative path matches the generated media by suffix. - let url = local_link_to_file_url("images/1.jpg", &media).unwrap(); + let target = local_link_to_file_target("images/1.jpg", &media).unwrap(); + assert_eq!(target, LinkTarget::File(Arc::from(media[0].as_path()))); + let resolved = resolve_link_target(&target).expect("resolved target"); + let url = resolved.osc8_url.expect("OSC 8 URL"); assert!( url.starts_with("file://") && url.ends_with("/images/1.jpg"), "got {url}" @@ -565,13 +697,13 @@ mod tests { std::fs::write(dir.path().join("images/1.jpg"), b"x").unwrap(); let media = vec![dir.path().join("images/1.jpg")]; - assert!(local_link_to_file_url("https://x.ai", &media).is_none()); - assert!(local_link_to_file_url("mailto:a@b.c", &media).is_none()); - assert!(local_link_to_file_url("#section", &media).is_none()); + assert!(local_link_to_file_target("https://x.ai", &media).is_none()); + assert!(local_link_to_file_target("mailto:a@b.c", &media).is_none()); + assert!(local_link_to_file_target("#section", &media).is_none()); // Relative path that isn't a known generated media file. - assert!(local_link_to_file_url("images/2.jpg", &media).is_none()); + assert!(local_link_to_file_target("images/2.jpg", &media).is_none()); // No known media at all. - assert!(local_link_to_file_url("images/1.jpg", &[]).is_none()); + assert!(local_link_to_file_target("images/1.jpg", &[]).is_none()); } #[test] @@ -587,49 +719,82 @@ mod tests { dir.path().join("a/images/1.jpg"), dir.path().join("b/images/1.jpg"), ]; - assert!(local_link_to_file_url("images/1.jpg", &media).is_none()); + assert!(local_link_to_file_target("images/1.jpg", &media).is_none()); // A `..` never matches a clean absolute media path, so it can't escape. - assert!(local_link_to_file_url("../images/1.jpg", &media).is_none()); + assert!(local_link_to_file_target("../images/1.jpg", &media).is_none()); } - // ── tool_path_file_url ── + // ── tool_path_file_target ── #[test] - fn tool_path_file_url_resolves_relative_against_cwd() { + fn tool_path_file_target_resolves_relative_against_cwd() { let cwd = Path::new("/Users/me/project"); - let url = tool_path_file_url("src/main.rs", Some(cwd)).expect("url"); - assert!(url.starts_with("file://"), "got {url}"); - assert!(url.contains("/Users/me/project/src/main.rs"), "got {url}"); + let target = tool_path_file_target("src/main.rs", Some(cwd)).expect("target"); + assert_eq!( + target, + LinkTarget::File(Arc::from(Path::new("/Users/me/project/src/main.rs"))) + ); + assert_eq!( + resolve_link_target(&target) + .unwrap() + .osc8_url + .unwrap() + .as_ref(), + "file:///Users/me/project/src/main.rs" + ); } #[test] - fn tool_path_file_url_accepts_absolute_without_existing_file() { - let url = tool_path_file_url("/tmp/does-not-exist-xyz/foo.rs", None).expect("url"); - assert!(url.starts_with("file://"), "got {url}"); - assert!(url.contains("foo.rs"), "got {url}"); + fn tool_path_file_target_accepts_absolute_without_existing_file() { + let target = tool_path_file_target("/tmp/does-not-exist-xyz/foo.rs", None).expect("target"); + assert_eq!( + target, + LinkTarget::File(Arc::from(Path::new("/tmp/does-not-exist-xyz/foo.rs"))) + ); + assert!( + resolve_link_target(&target) + .unwrap() + .osc8_url + .unwrap() + .contains("foo.rs") + ); } #[test] - fn tool_path_file_url_preserves_parent_segments_for_os_resolution() { - let url = tool_path_file_url("/repo/link/../target.rs", None).expect("url"); - assert!(url.contains("/repo/link/../target.rs"), "got {url}"); + fn tool_path_file_target_preserves_parent_segments_for_os_resolution() { + let target = tool_path_file_target("/repo/link/../target.rs", None).expect("target"); + let LinkTarget::File(path) = target else { + panic!("expected file target"); + }; + assert_eq!(&*path, Path::new("/repo/link/../target.rs")); + assert!( + file_path_to_url(&path) + .unwrap() + .contains("/repo/link/../target.rs") + ); } #[test] fn unresolved_tilde_never_manufactures_a_cwd_file_url() { assert!( - tool_path_file_url_with_home("~/target.rs", Some(Path::new("/repo")), None).is_none() + tool_path_file_target_with_home("~/target.rs", Some(Path::new("/repo")), None) + .is_none() ); } #[cfg(unix)] #[test] - fn tool_path_file_url_preserves_non_utf8_cwd_bytes() { + fn tool_path_file_target_preserves_non_utf8_cwd_bytes() { use std::ffi::OsString; - use std::os::unix::ffi::OsStringExt; + use std::os::unix::ffi::{OsStrExt, OsStringExt}; let cwd = PathBuf::from(OsString::from_vec(b"/tmp/non-utf8-\x80".to_vec())); - let url = tool_path_file_url("main.rs", Some(&cwd)).expect("url"); + let target = tool_path_file_target("main.rs", Some(&cwd)).expect("target"); + let LinkTarget::File(path) = &target else { + panic!("expected file target"); + }; + assert_eq!(path.as_os_str().as_bytes(), b"/tmp/non-utf8-\x80/main.rs"); + let url = resolve_link_target(&target).unwrap().osc8_url.unwrap(); assert!(url.contains("/tmp/non-utf8-%80/main.rs"), "got {url}"); assert!( !url.contains("%EF%BF%BD"), @@ -639,6 +804,205 @@ mod tests { // ── LinkOverlay ── + #[test] + fn resolve_target_keeps_standard_scheme_filter_and_file_open_path() { + let web = LinkTarget::Url(Arc::from("https://example.com/a")); + assert_eq!( + resolve_link_target(&web).unwrap(), + ResolvedLinkTarget { + osc8_url: Some(Arc::from("https://example.com/a")), + open_target: Some(web.clone()), + } + ); + assert_eq!(resolve_link_open_target(&web), Some(web)); + let unsafe_url = LinkTarget::Url(Arc::from("javascript:alert(1)")); + assert!(resolve_link_target(&unsafe_url).is_none()); + assert!(resolve_link_open_target(&unsafe_url).is_none()); + + let file = LinkTarget::File(Arc::from(Path::new("/tmp/a b.rs"))); + assert_eq!( + resolve_link_target(&file).unwrap(), + ResolvedLinkTarget { + osc8_url: Some(Arc::from("file:///tmp/a%20b.rs")), + open_target: Some(file.clone()), + } + ); + assert_eq!(resolve_link_open_target(&file), Some(file)); + } + + #[test] + fn official_vscode_remote_file_delegation_is_exact() { + use crate::terminal::{TerminalContext, TerminalName}; + + struct Case { + name: &'static str, + terminal: TerminalContext, + target: LinkTarget, + presentation: LinkPresentation, + expected_osc8: Option<&'static str>, + expected_open: bool, + } + + let file = LinkTarget::File(Arc::from(Path::new("/worktree/src/main.rs"))); + let web = LinkTarget::Url(Arc::from("https://example.com/docs")); + let official_remote = TerminalContext { + brand: TerminalName::VsCode, + is_ssh: true, + is_official_vscode_remote: true, + ..Default::default() + }; + let cases = [ + Case { + name: "local VS Code file", + terminal: TerminalContext { + brand: TerminalName::VsCode, + ..Default::default() + }, + target: file.clone(), + presentation: LinkPresentation::SelfResolvingPath, + expected_osc8: Some("file:///worktree/src/main.rs"), + expected_open: true, + }, + Case { + name: "official VS Code SSH self-resolving file", + terminal: official_remote.clone(), + target: file.clone(), + presentation: LinkPresentation::SelfResolvingPath, + expected_osc8: None, + expected_open: false, + }, + Case { + name: "official VS Code SSH opaque file", + terminal: official_remote.clone(), + target: file.clone(), + presentation: LinkPresentation::Opaque, + expected_osc8: Some("file:///worktree/src/main.rs"), + expected_open: true, + }, + Case { + name: "unproven VS Code SSH file", + terminal: TerminalContext { + brand: TerminalName::VsCode, + is_ssh: true, + ..Default::default() + }, + target: file.clone(), + presentation: LinkPresentation::SelfResolvingPath, + expected_osc8: Some("file:///worktree/src/main.rs"), + expected_open: true, + }, + Case { + name: "official VS Code SSH web", + terminal: official_remote, + target: web, + presentation: LinkPresentation::Opaque, + expected_osc8: Some("https://example.com/docs"), + expected_open: true, + }, + Case { + name: "Cursor SSH file", + terminal: TerminalContext { + brand: TerminalName::Cursor, + is_ssh: true, + ..Default::default() + }, + target: file.clone(), + presentation: LinkPresentation::SelfResolvingPath, + expected_osc8: Some("file:///worktree/src/main.rs"), + expected_open: true, + }, + Case { + name: "Kitty SSH file", + terminal: TerminalContext { + brand: TerminalName::Kitty, + is_ssh: true, + ..Default::default() + }, + target: file, + presentation: LinkPresentation::SelfResolvingPath, + expected_osc8: Some("file:///worktree/src/main.rs"), + expected_open: true, + }, + ]; + + for case in cases { + let resolved = + resolve_link_target_for_context(&case.target, case.presentation, &case.terminal) + .unwrap_or_else(|| panic!("{} should resolve", case.name)); + assert_eq!( + resolved.osc8_url.as_deref(), + case.expected_osc8, + "{} OSC 8 policy", + case.name + ); + assert_eq!( + resolved.open_target.is_some(), + case.expected_open, + "{} activation policy", + case.name + ); + } + } + + #[test] + fn file_presentation_requires_exact_path_shaped_resolution() { + let target = LinkTarget::File(Arc::from(Path::new("/worktree/src/main.rs"))); + let cwd = Path::new("/worktree"); + + assert_eq!( + file_link_presentation("/worktree/src/main.rs", &target, Some(cwd)), + LinkPresentation::SelfResolvingPath + ); + assert_eq!( + file_link_presentation("src/main.rs", &target, Some(cwd)), + LinkPresentation::SelfResolvingPath + ); + assert_eq!( + file_link_presentation("src/main.rs", &target, None), + LinkPresentation::Opaque + ); + let home = Path::new("/home/me"); + let home_target = LinkTarget::File(Arc::from(home.join("src/main.rs"))); + assert_eq!( + file_link_presentation_with_home("~/src/main.rs", &home_target, None, Some(home)), + LinkPresentation::SelfResolvingPath + ); + assert_eq!( + file_link_presentation_with_home("~/src/main.rs", &home_target, None, None), + LinkPresentation::Opaque + ); + assert_eq!( + file_link_presentation_with_home("~/src/other.rs", &home_target, None, Some(home)), + LinkPresentation::Opaque + ); + for painted in [ + "main.rs", + "main\u{2026}", + "src/other.rs", + "\u{2026}/src/main.rs", + "src/main.rs (1 of 2)", + ] { + assert_eq!( + file_link_presentation(painted, &target, Some(cwd)), + LinkPresentation::Opaque, + "{painted}" + ); + } + } + + #[test] + fn open_target_preserves_a_relative_file_that_cannot_be_encoded_for_osc8() { + let file = LinkTarget::File(Arc::from(Path::new("relative.rs"))); + assert_eq!( + resolve_link_target(&file), + Some(ResolvedLinkTarget { + osc8_url: None, + open_target: Some(file.clone()), + }) + ); + assert_eq!(resolve_link_open_target(&file), Some(file)); + } + #[test] fn overlay_empty_by_default() { let overlay = LinkOverlay::new(); @@ -653,7 +1017,8 @@ mod tests { screen_row: 5, col_start: 10, col_end: 20, - url: "https://example.com".into(), + target: LinkTarget::Url("https://example.com".into()), + presentation: LinkPresentation::Opaque, id: Some(1), }); assert!(!overlay.is_empty()); @@ -686,7 +1051,12 @@ mod tests { assert_eq!(overlay.links().len(), 1); let link = &overlay.links()[0]; - assert_eq!(&*link.url, "https://example.com"); + assert_eq!( + &*resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://example.com" + ); assert_eq!(link.screen_row, 5); // "See " = 4 display cols, content_x = 2 assert_eq!(link.col_start, 6); @@ -701,8 +1071,18 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 2); - assert_eq!(&*overlay.links()[0].url, "https://a.example"); - assert_eq!(&*overlay.links()[1].url, "https://b.example"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://a.example" + ); + assert_eq!( + &*resolve_link_target(&overlay.links()[1].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://b.example" + ); assert!(overlay.links()[0].col_end <= overlay.links()[1].col_start); } @@ -717,7 +1097,12 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "https://example.com"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://example.com" + ); // "Visit " = 6 display cols (in first span) // The URL is in its own span, so col_start = 6 assert_eq!(overlay.links()[0].col_start, 6); @@ -765,7 +1150,9 @@ mod tests { assert_eq!(overlay.links().len(), 1); assert_eq!( - &*overlay.links()[0].url, + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), "https://example.com", "trailing dot should be excluded by linkify" ); @@ -779,7 +1166,9 @@ mod tests { assert_eq!(overlay.links().len(), 1); assert_eq!( - &*overlay.links()[0].url, + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), "https://example.com/path?key=val#sec" ); } @@ -804,7 +1193,12 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "file:///Users/foo/src/main.rs"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "file:///Users/foo/src/main.rs" + ); } #[test] @@ -828,7 +1222,9 @@ mod tests { let mut overlay = LinkOverlay::new(); scan_unjoined(std::iter::once((0, &line)), 0, &media, &mut overlay); assert_eq!(overlay.links().len(), 1, "{line_text}"); - let url = &*overlay.links()[0].url; + let url = resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"); assert!( url.starts_with("file://") && url.ends_with(suffix), "got {url}" @@ -865,7 +1261,9 @@ mod tests { assert_eq!(overlay.links().len(), 1); assert_eq!( - &*overlay.links()[0].url, + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), // `%` is itself percent-encoded (`%25`) when building the file URL. "file:///Users/alice/.grok/sessions/%252Fabc/00000000/images/1.jpg", ); @@ -889,7 +1287,12 @@ mod tests { let expected_url = "file:///Users/alice/.grok/sessions/%252FUsers%252Fali\ ce%252Fcode%252Fxai/00000000-0000-0000-0000-000000000001/images/1.jpg"; for link in overlay.links() { - assert_eq!(&*link.url, expected_url); + assert_eq!( + &*resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + expected_url + ); } // Row 0: path starts after the prose and runs to the row's end. let prose = "Image generated and saved to "; @@ -928,7 +1331,9 @@ mod tests { assert_eq!(overlay.links().len(), 2); for link in overlay.links() { assert_eq!( - &*link.url, + &*resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), "file:///Users/me/.grok/sessions/%252Fabc/019f3a86/images/1.jpg" ); } @@ -958,9 +1363,9 @@ mod tests { assert_eq!(overlay.links().len(), 2); for link in overlay.links() { assert!( - link.url.starts_with("file://") && link.url.ends_with("/images/1.png"), + resolve_link_target(&link.target).and_then(|resolved| resolved.osc8_url).is_some_and(|url| url.starts_with("file://") && url.ends_with("/images/1.png")), "got {}", - link.url + resolve_link_target(&link.target).and_then(|resolved| resolved.osc8_url).expect("url") ); } } @@ -976,7 +1381,12 @@ mod tests { assert_eq!(overlay.links().len(), 2); for link in overlay.links() { - assert_eq!(&*link.url, "https://example.com/some/long/path?key=val"); + assert_eq!( + &*resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://example.com/some/long/path?key=val" + ); } } @@ -994,7 +1404,12 @@ mod tests { assert_eq!(overlay.links().len(), 2); for link in overlay.links() { - assert_eq!(&*link.url, "file:///tmp/release/Demo%20App.app"); + assert_eq!( + &*resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "file:///tmp/release/Demo%20App.app" + ); } // Row 1's region covers only `App.app` (the joiner space belongs // to no row). @@ -1017,7 +1432,12 @@ mod tests { scan_lines_for_url_overlays(rows.into_iter(), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "file:///Users/alice"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "file:///Users/alice" + ); assert_eq!(overlay.links()[0].screen_row, 0); } @@ -1034,7 +1454,12 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "file:///Users/foo/images/1.jpg"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "file:///Users/foo/images/1.jpg" + ); assert_eq!( overlay.links()[0].col_start, UnicodeWidthStr::width("Saved to ") as u16 @@ -1049,7 +1474,9 @@ mod tests { assert_eq!(overlay.links().len(), 1); assert_eq!( - &*overlay.links()[0].url, + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), "file:///Users/foo/bar.rs", "colon-delimited line number should be excluded" ); @@ -1077,7 +1504,9 @@ mod tests { assert_eq!(overlay.links().len(), 1); assert_eq!( - &*overlay.links()[0].url, + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), "https://example.com/foo/bar", "URL should be detected, not the path portion" ); @@ -1090,9 +1519,20 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 2); - let urls: Vec<&str> = overlay.links().iter().map(|l| &*l.url).collect(); - assert!(urls.contains(&"https://docs.rs/foo")); - assert!(urls.contains(&"file:///Users/me/src/lib.rs")); + let urls: Vec> = overlay + .links() + .iter() + .map(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + }) + .collect(); + assert!(urls.iter().any(|url| url.as_ref() == "https://docs.rs/foo")); + assert!( + urls.iter() + .any(|url| url.as_ref() == "file:///Users/me/src/lib.rs") + ); } #[test] @@ -1102,7 +1542,12 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "file:///tmp/grok-impl-summary.md"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "file:///tmp/grok-impl-summary.md" + ); } #[test] @@ -1113,7 +1558,9 @@ mod tests { assert_eq!(overlay.links().len(), 1); assert_eq!( - &*overlay.links()[0].url, + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), "file:///node_modules/@scope/package/index.js" ); } @@ -1134,7 +1581,10 @@ mod tests { ); let link = &overlay.links()[0]; assert_eq!( - &*link.url, "file:///Users/alice/src/app/release/mac-arm64/Demo%20App.app", + &*resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "file:///Users/alice/src/app/release/mac-arm64/Demo%20App.app", "space must be percent-encoded in the file URL" ); // Clickable region must cover the *entire* displayed path, including @@ -1158,7 +1608,9 @@ mod tests { assert_eq!(overlay.links().len(), 1); assert_eq!( - &*overlay.links()[0].url, + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), "file:///tmp/release/Demo%20App.app" ); assert_eq!(overlay.links()[0].col_start, 5); // "open " @@ -1177,7 +1629,12 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "file:///tmp/foo/bar"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "file:///tmp/foo/bar" + ); // "See " = 4 cols; path is 12 cols (`/tmp/foo/bar`). assert_eq!(overlay.links()[0].col_start, 4); assert_eq!(overlay.links()[0].col_end, 4 + 12); @@ -1202,9 +1659,12 @@ mod tests { assert_eq!(overlay.links().len(), 1); let link = &overlay.links()[0]; // `~` is expanded to the home directory in the file URL. - assert_eq!(&*link.url, expected.as_str()); - assert!(link.url.starts_with("file:///")); - assert!(!link.url.contains('~'), "tilde must be expanded in the URL"); + let url = resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"); + assert_eq!(&*url, expected.as_str()); + assert!(url.starts_with("file:///")); + assert!(!url.contains('~'), "tilde must be expanded in the URL"); // The clickable region covers the displayed `~/…` text, tilde included. // "Findings report " = 16 display cols. assert_eq!(link.col_start, 16); @@ -1222,7 +1682,12 @@ mod tests { scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, expected.as_str()); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + expected.as_str() + ); assert_eq!(overlay.links()[0].col_start, 0); } @@ -1272,7 +1737,8 @@ mod tests { screen_row: 5, col_start: 10, col_end: 20, - url: Arc::from("https://a.example"), + target: LinkTarget::Url(Arc::from("https://a.example")), + presentation: LinkPresentation::Opaque, id: None, }); assert!(overlay.overlaps(5, 10, 20)); @@ -1286,7 +1752,8 @@ mod tests { screen_row: 0, col_start: 10, col_end: 20, - url: Arc::from("https://a.example"), + target: LinkTarget::Url(Arc::from("https://a.example")), + presentation: LinkPresentation::Opaque, id: None, }); assert!(overlay.overlaps(0, 15, 25)); // right overlap @@ -1305,7 +1772,8 @@ mod tests { screen_row: 0, col_start: 4, col_end: 23, - url: Arc::from("https://example.com"), + target: LinkTarget::Url(Arc::from("https://example.com")), + presentation: LinkPresentation::Opaque, id: None, }); scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); @@ -1322,7 +1790,8 @@ mod tests { screen_row: 0, col_start: 50, col_end: 70, - url: Arc::from("https://first.example"), + target: LinkTarget::Url(Arc::from("https://first.example")), + presentation: LinkPresentation::Opaque, id: None, }); scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); @@ -1339,7 +1808,8 @@ mod tests { screen_row: 0, col_start: 9, col_end: 31, - url: Arc::from("file:///Users/foo/src/main.rs"), + target: LinkTarget::File(Arc::from(Path::new("/Users/foo/src/main.rs"))), + presentation: LinkPresentation::Opaque, id: None, }); scan_unjoined(std::iter::once((0, &line)), 0, &[], &mut overlay); diff --git a/crates/codegen/xai-grok-pager-render/src/terminal/image.rs b/crates/codegen/xai-grok-pager-render/src/terminal/image.rs index 9914b77..802229b 100644 --- a/crates/codegen/xai-grok-pager-render/src/terminal/image.rs +++ b/crates/codegen/xai-grok-pager-render/src/terminal/image.rs @@ -75,7 +75,7 @@ pub fn scrollback_inline_overlay_forced_off() -> bool { INLINE_OVERLAY_FORCE_OFF.load(Ordering::Relaxed) } -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] thread_local! { /// Per-test override so tests don't depend on the host terminal or the /// process-wide `GRAPHICS_PROTOCOL` cache. @@ -85,7 +85,7 @@ thread_local! { /// Detect and cache the graphics protocol for the current terminal. pub fn detect_graphics_protocol() -> GraphicsProtocol { - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(p) = TEST_PROTOCOL_OVERRIDE.with(|c| c.get()) { return p; } @@ -119,12 +119,12 @@ pub fn scrollback_inline_overlay_active() -> bool { scrollback_inline_overlay_active_for_brand(protocol, terminal_context().brand) } -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] fn test_protocol_override_active() -> bool { TEST_PROTOCOL_OVERRIDE.with(|c| c.get().is_some()) } -#[cfg(not(test))] +#[cfg(not(any(test, feature = "test-support")))] fn test_protocol_override_active() -> bool { false } @@ -145,17 +145,17 @@ fn scrollback_inline_overlay_active_for_brand( /// Set a per-thread protocol override for tests. Returns a guard that /// clears it on drop. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_protocol_for_test(p: GraphicsProtocol) -> TestProtocolGuard { TEST_PROTOCOL_OVERRIDE.with(|c| c.set(Some(p))); TestProtocolGuard } /// RAII guard that clears the test protocol override on drop. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub struct TestProtocolGuard; -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] impl Drop for TestProtocolGuard { fn drop(&mut self) { TEST_PROTOCOL_OVERRIDE.with(|c| c.set(None)); diff --git a/crates/codegen/xai-grok-pager-render/src/terminal/keyboard.rs b/crates/codegen/xai-grok-pager-render/src/terminal/keyboard.rs index 9376e17..c09eb46 100644 --- a/crates/codegen/xai-grok-pager-render/src/terminal/keyboard.rs +++ b/crates/codegen/xai-grok-pager-render/src/terminal/keyboard.rs @@ -49,7 +49,7 @@ impl ModifierDelivery { /// Construct a delivery from explicit fates. `#[non_exhaustive]` blocks /// struct-literal construction from other crates, so downstream test /// builds use this constructor. - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] pub fn new_for_test(cmd: ModifierFate, opt: ModifierFate) -> Self { Self { cmd, opt } } diff --git a/crates/codegen/xai-grok-pager-render/src/terminal/mod.rs b/crates/codegen/xai-grok-pager-render/src/terminal/mod.rs index 89d6da8..54dbff1 100644 --- a/crates/codegen/xai-grok-pager-render/src/terminal/mod.rs +++ b/crates/codegen/xai-grok-pager-render/src/terminal/mod.rs @@ -282,6 +282,8 @@ pub struct TerminalContext { pub tmux_meta: TmuxClientMeta, /// Whether the session is inside a remote SSH connection. pub is_ssh: bool, + /// Positive evidence that SSH is hosted by the official VS Code remote server. + pub is_official_vscode_remote: bool, /// The raw `TERM` environment variable (e.g. `xterm-256color`, `screen`). pub term_var: Option, /// The tmux server version (e.g. `"tmux 3.4"`), populated only when @@ -666,6 +668,16 @@ fn env_get<'a>(env: &'a HashMap, key: &str) -> Option<&'a str> { env.get(key).map(|v| v.as_str()).filter(|v| !v.is_empty()) } +fn is_official_vscode_remote_askpass(path: &str) -> bool { + std::path::Path::new(path).components().any(|component| { + matches!( + component, + std::path::Component::Normal(name) + if name == ".vscode-server" || name == ".vscode-server-insiders" + ) + }) +} + /// Detect the terminal brand from an injected environment map. /// /// This is the pure equivalent of the original `detect_terminal_info`. @@ -935,6 +947,8 @@ pub fn build_terminal_context_from_env(env: &HashMap) -> Termina let is_ssh = env_get(env, "SSH_CONNECTION").is_some() || env_get(env, "SSH_TTY").is_some() || env_get(env, "SSH_CLIENT").is_some(); + let is_official_vscode_remote = is_ssh + && env_get(env, "VSCODE_GIT_ASKPASS_MAIN").is_some_and(is_official_vscode_remote_askpass); let term_var = env_get(env, "TERM").map(|s| s.to_owned()); let vte_version = env_get(env, "VTE_VERSION").map(|s| s.to_owned()); // SSH strips TERM_PROGRAM_VERSION; iTerm2 LC_TERMINAL_VERSION survives. @@ -950,6 +964,7 @@ pub fn build_terminal_context_from_env(env: &HashMap) -> Termina embedded_editor, tmux_meta, is_ssh, + is_official_vscode_remote, term_var, tmux_version: None, vte_version, diff --git a/crates/codegen/xai-grok-pager-render/src/terminal/test.rs b/crates/codegen/xai-grok-pager-render/src/terminal/test.rs index 910ee34..cde7b2b 100644 --- a/crates/codegen/xai-grok-pager-render/src/terminal/test.rs +++ b/crates/codegen/xai-grok-pager-render/src/terminal/test.rs @@ -1054,6 +1054,52 @@ fn brand_vscode_from_askpass_without_term_program() { assert_eq!(detect_terminal_brand_from_env(&env), TerminalName::VsCode); } +#[test] +fn context_official_vscode_remote_from_askpass_and_ssh() { + for server_dir in [".vscode-server", ".vscode-server-insiders"] { + let askpass = format!("/home/user/{server_dir}/bin/abc/askpass"); + let env = env_from(&[ + ("VSCODE_GIT_ASKPASS_MAIN", &askpass), + ("SSH_CONNECTION", "192.0.2.1 50000 192.0.2.2 22"), + ]); + let ctx = build_terminal_context_from_env(&env); + assert_eq!(ctx.brand, TerminalName::VsCode); + assert!(ctx.is_ssh); + assert!(ctx.is_official_vscode_remote, "{server_dir}"); + } +} + +#[test] +fn context_unofficial_vscode_remote_markers_are_not_official() { + for askpass in [ + "/home/user/.vscode-server-oss/bin/abc/askpass", + "/home/user/.vscodium-server/bin/abc/askpass", + "/home/user/.code-oss-server/bin/abc/askpass", + "/home/user/cache/.vscode-server-oss/.vscode-serverish/askpass", + "/usr/local/bin/askpass-main.js", + ] { + let env = env_from(&[ + ("VSCODE_GIT_ASKPASS_MAIN", askpass), + ("SSH_CONNECTION", "192.0.2.1 50000 192.0.2.2 22"), + ]); + let ctx = build_terminal_context_from_env(&env); + assert_eq!(ctx.brand, TerminalName::VsCode); + assert!(ctx.is_ssh); + assert!(!ctx.is_official_vscode_remote, "{askpass}"); + } +} + +#[test] +fn official_vscode_server_marker_without_ssh_is_not_remote() { + let env = env_from(&[( + "VSCODE_GIT_ASKPASS_MAIN", + "/home/user/.vscode-server/bin/abc/askpass", + )]); + let ctx = build_terminal_context_from_env(&env); + assert!(!ctx.is_ssh); + assert!(!ctx.is_official_vscode_remote); +} + // -- Zellij detection from ZELLIJ_VERSION (no ZELLIJ or SESSION_NAME) ----- #[test] diff --git a/crates/codegen/xai-grok-pager-render/src/theme/cache.rs b/crates/codegen/xai-grok-pager-render/src/theme/cache.rs index adaf8ba..0c65472 100644 --- a/crates/codegen/xai-grok-pager-render/src/theme/cache.rs +++ b/crates/codegen/xai-grok-pager-render/src/theme/cache.rs @@ -21,7 +21,7 @@ use super::system_appearance; /// `load_from_disk()`, then kept in sync by `set()`. static CURRENT: AtomicU8 = AtomicU8::new(ThemeKind::GrokNight as u8); static LOADED: AtomicBool = AtomicBool::new(false); -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] static TEST_LOCK: Mutex<()> = Mutex::new(()); /// Whether auto-switching mode is active. Set when the config file @@ -270,7 +270,7 @@ fn load_auto_theme_config() -> AutoThemeConfig { // -- Test support ------------------------------------------------------------ -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn reset_for_test() { // Tests are serialized via TEST_LOCK so the AtomicU8/AtomicBool // pair is safe to reset without any cross-thread coordination. @@ -284,12 +284,12 @@ pub fn reset_for_test() { /// Seed `AUTO_THEME_CONFIG` with explicit defaults so `auto_theme_config()` /// never falls through to `load_auto_theme_config()` (which reads the /// user's real `config.toml`). Call from test setup after `reset_for_test()`. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn seed_auto_theme_defaults_for_test() { *AUTO_THEME_CONFIG.lock().unwrap_or_else(|e| e.into_inner()) = Some(AutoThemeConfig::default()); } -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn test_lock() -> &'static Mutex<()> { &TEST_LOCK } @@ -300,7 +300,7 @@ pub fn test_lock() -> &'static Mutex<()> { /// `set_theme` tests mutate) and `Theme::current()` reads the global color /// level; holding the shared test lock blocks a mid-test theme change. Hold the /// returned guard for the whole test. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn pin_theme() -> std::sync::MutexGuard<'static, ()> { let guard = test_lock().lock().unwrap_or_else(|e| e.into_inner()); set(ThemeKind::GrokNight); diff --git a/crates/codegen/xai-grok-pager-render/src/theme/system_appearance.rs b/crates/codegen/xai-grok-pager-render/src/theme/system_appearance.rs index 747360f..0c9c13b 100644 --- a/crates/codegen/xai-grok-pager-render/src/theme/system_appearance.rs +++ b/crates/codegen/xai-grok-pager-render/src/theme/system_appearance.rs @@ -36,7 +36,7 @@ pub enum SystemAppearance { /// directly) is also controllable from tests. #[must_use] pub fn detect() -> Option { - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(v) = mock_override() { return v; } @@ -56,7 +56,7 @@ pub fn detect() -> Option { /// live [`SystemAppearanceWatcher`] uses [`detect`] (without OSC 11). #[must_use] pub fn detect_with_osc11_fallback() -> Option { - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] if let Some(v) = mock_override() { return v; } @@ -78,7 +78,7 @@ fn detect_without_mock() -> Option { /// /// Returns `Some(value)` when a mock is active, `None` when real /// detection should proceed. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] fn mock_override() -> Option> { *MOCK_APPEARANCE.lock().unwrap_or_else(|e| e.into_inner()) } @@ -175,25 +175,25 @@ impl Drop for SystemAppearanceWatcher { // -- Test support ---------------------------------------------------------- -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] use std::sync::Mutex; /// Mock override for `detect()`. When set to `Some(value)`, `detect()` /// returns the mock value instead of calling `dark_light::detect()`. /// This ensures the `SystemAppearanceWatcher` polling loop (which calls /// `detect()` directly) is also controllable from tests. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] static MOCK_APPEARANCE: Mutex>> = Mutex::new(None); /// Override `detect()` for tests. Set to `Some(value)` to mock a specific /// appearance, or `None` to mock detection failure. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_mock(value: Option) { *MOCK_APPEARANCE.lock().unwrap_or_else(|e| e.into_inner()) = Some(value); } /// Clear the mock override, restoring real detection behavior. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn clear_mock() { *MOCK_APPEARANCE.lock().unwrap_or_else(|e| e.into_inner()) = None; } diff --git a/crates/codegen/xai-grok-pager/Cargo.toml b/crates/codegen/xai-grok-pager/Cargo.toml index e205ecf..2584c2c 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.1.220-alpha.4" +version = "0.2.101" edition.workspace = true license = "Apache-2.0" authors = ["xAI"] @@ -161,7 +161,7 @@ windows-sys = { version = "0.59", features = ["Win32_System_Console"] } [dev-dependencies] # Enable the render crate's test-only helpers for the pager's test build. -xai-grok-pager-render = { path = "../xai-grok-pager-render", features = [] } +xai-grok-pager-render = { path = "../xai-grok-pager-render", features = ["test-support"] } pretty_assertions = { workspace = true } insta = { workspace = true } criterion = { workspace = true } @@ -273,6 +273,7 @@ default = ["jemalloc", "sandbox-enforce"] default-bazel = [ "jemalloc", "sandbox-enforce", + "test-support", ] # No-op on this crate: the actual `#[global_allocator]` (and the # `tikv-jemallocator` dep) live on the composition-root binary @@ -282,3 +283,7 @@ default-bazel = [ jemalloc = [] sandbox-enforce = ["xai-grok-sandbox/enforce"] release-dist = [] +# Exposes test-only view-model constructors/setters (e.g. `test_agent_view`, +# `AgentSession::set_yolo_mode_for_test`) to sibling crates' test builds — used +# by `xai-grok-pager-minimal`'s unit tests. Never enabled in production builds. +test-support = ["xai-grok-pager-render/test-support"] diff --git a/crates/codegen/xai-grok-pager/docs/user-guide/14-headless-mode.md b/crates/codegen/xai-grok-pager/docs/user-guide/14-headless-mode.md index c58631c..1cf6ac6 100644 --- a/crates/codegen/xai-grok-pager/docs/user-guide/14-headless-mode.md +++ b/crates/codegen/xai-grok-pager/docs/user-guide/14-headless-mode.md @@ -556,6 +556,12 @@ grok -p "..." --no-auto-update | Non-TTY stderr (auto-detected) | Automatic | | `[cli] auto_update = false` | Persistent| +`GROK_DISABLE_AUTOUPDATER` set to a falsy value (`0`, `false`, `off`, `no`, or empty, any +case) counts as not set. The agent SDKs +inject `GROK_DISABLE_AUTOUPDATER=1` for the non-leader agents they spawn (a falsy value in +the SDK's isolation env keeps updates on), and the stdio agent skips its background update +unless it runs from the managed install (`$GROK_HOME/bin/grok`). + Update messages go to **stderr**. Stdout stays clean for `--output-format json`. See also [Environment Variables for Headless](#environment-variables-for-headless). --- 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 be0f38e..43c0065 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 @@ -546,30 +546,30 @@ pub(super) fn handle_session_notification(notif: &acp::ExtNotification, app: &mu )), }; agent.scrollback.push_block(block); - } else if let Some(eid) = entry_id { - if let Some(entry) = agent.scrollback.get_by_id_mut(eid) { - if let RenderBlock::Subagent(ref mut sb) = entry.block { - match status.as_str() { - "completed" => { - sb.kind = crate::scrollback::blocks::SubagentBlockKind::Completed { - elapsed: elapsed_dur, - }; - } - "cancelled" => { - sb.kind = crate::scrollback::blocks::SubagentBlockKind::Cancelled { - elapsed: elapsed_dur, - }; - } - _ => { - sb.kind = crate::scrollback::blocks::SubagentBlockKind::Failed { - elapsed: elapsed_dur, - error: error.clone(), - }; - } + } else if let Some(eid) = entry_id + && let Some(entry) = agent.scrollback.get_by_id_mut(eid) + { + if let RenderBlock::Subagent(ref mut sb) = entry.block { + match status.as_str() { + "completed" => { + sb.kind = crate::scrollback::blocks::SubagentBlockKind::Completed { + elapsed: elapsed_dur, + }; + } + "cancelled" => { + sb.kind = crate::scrollback::blocks::SubagentBlockKind::Cancelled { + elapsed: elapsed_dur, + }; + } + _ => { + sb.kind = crate::scrollback::blocks::SubagentBlockKind::Failed { + elapsed: elapsed_dur, + error: error.clone(), + }; } } - entry.invalidate_cache(); } + entry.invalidate_cache(); } let mut was_running = false; if let Some(info) = agent.subagent_sessions.get_mut(&child_session_id) { diff --git a/crates/codegen/xai-grok-pager/src/app/actions.rs b/crates/codegen/xai-grok-pager/src/app/actions.rs index 4101350..3b2cf0c 100644 --- a/crates/codegen/xai-grok-pager/src/app/actions.rs +++ b/crates/codegen/xai-grok-pager/src/app/actions.rs @@ -62,6 +62,8 @@ pub enum Action { CheckSubscription, /// Open an arbitrary URL in the system browser (with scheme validation). OpenUrl(String), + /// Open a semantic scrollback link. + OpenLink(crate::render::osc8::LinkTarget), /// Open grok.com managed connectors, appending session teamId when set. OpenManagedConnectors, /// Cycle to the next visible link (or highlight the first if none selected). diff --git a/crates/codegen/xai-grok-pager/src/app/agent.rs b/crates/codegen/xai-grok-pager/src/app/agent.rs index 37d3d2a..900cf03 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent.rs @@ -759,12 +759,12 @@ impl AgentSession { /// Test-only setter for `yolo_mode` (the field is private; production toggles /// it via the permission-mode facade). Available to sibling crates' test /// builds through the test-only helpers. - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] pub(crate) fn set_yolo_mode_for_test(&mut self, on: bool) { self.yolo_mode = on; } /// Test-only setter for `auto_mode`. See [`Self::set_yolo_mode_for_test`]. - #[cfg(test)] + #[cfg(any(test, feature = "test-support"))] pub(crate) fn set_auto_mode_for_test(&mut self, on: bool) { self.auto_mode = on; } 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 525b883..45913bf 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 @@ -643,11 +643,11 @@ impl AgentView { modifiers: mouse.modifiers, }; let _ = self.prompt.handle_mouse(&event); - } else if let Some((scroll_top, scroll_bottom)) = self.question_scroll_region { - if mouse.row >= scroll_top && mouse.row < scroll_bottom { - self.apply_question_scroll(delta); - } - } else { + } else if let Some((scroll_top, scroll_bottom)) = self.question_scroll_region + && mouse.row >= scroll_top + && mouse.row < scroll_bottom + { + self.apply_question_scroll(delta); } InputOutcome::Changed } diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/links.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/links.rs index d2b4c92..0b16cff 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/links.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/links.rs @@ -56,11 +56,17 @@ impl AgentView { } }); } - /// Return the URL of the currently highlighted link, if any. - pub fn highlighted_link_url(&self) -> Option<&str> { + /// Return the semantic target of the currently highlighted link, if any. + pub fn highlighted_link_target(&self) -> Option<&crate::render::osc8::LinkTarget> { self.highlighted_link_idx .and_then(|idx| self.visible_link_map.links().get(idx)) - .map(|link| &*link.url) + .map(|link| &link.target) + } + /// Return the current OSC 8 URL for the highlighted link preview. + pub fn highlighted_link_url(&self) -> Option> { + self.highlighted_link_target() + .and_then(crate::render::osc8::resolve_link_target) + .and_then(|resolved| resolved.osc8_url) } /// True when `(x, y)` lies inside an overlay drawn over the scrollback this /// frame (dropdown, goal detail). Such positions belong to the overlay, not @@ -275,17 +281,33 @@ mod link_click_tests { agent.active_pane = AgentPane::Scrollback; } /// Add a link to the visible_link_map covering (col_start..col_end, row). - fn add_visible_link(agent: &mut AgentView, row: u16, col_start: u16, col_end: u16, url: &str) { + fn add_visible_target( + agent: &mut AgentView, + row: u16, + col_start: u16, + col_end: u16, + target: crate::render::osc8::LinkTarget, + ) { let mut overlay = LinkOverlay::new(); overlay.push(OverlayLink { screen_row: row, col_start, col_end, - url: Arc::from(url), + target, + presentation: crate::render::osc8::LinkPresentation::Opaque, id: Some(1), }); agent.visible_link_map.rebuild(1, &overlay, vec![]); } + fn add_visible_link(agent: &mut AgentView, row: u16, col_start: u16, col_end: u16, url: &str) { + add_visible_target( + agent, + row, + col_start, + col_end, + crate::render::osc8::LinkTarget::Url(Arc::from(url)), + ); + } fn mouse_down(col: u16, row: u16) -> MouseEvent { MouseEvent { kind: MouseEventKind::Down(MouseButton::Left), @@ -1032,7 +1054,11 @@ mod link_click_tests { let mut agent = make_agent(); let area = Rect::new(0, 0, 80, 24); setup_scrollback_area(&mut agent, area); - agent.pending_link_click = Some((15, 5, "https://example.com".into())); + agent.pending_link_click = Some(( + 15, + 5, + crate::render::osc8::LinkTarget::Url("https://example.com".into()), + )); agent.left_mouse_down = true; let outcome = agent.handle_mouse(&mouse_drag(16, 5)); assert!(matches!( @@ -1042,22 +1068,29 @@ mod link_click_tests { assert!(agent.pending_link_click.is_none()); } #[test] - fn up_at_same_position_returns_open_url_action() { + fn up_at_same_position_returns_open_link_action() { let mut agent = make_agent(); let area = Rect::new(0, 0, 80, 24); setup_scrollback_area(&mut agent, area); - agent.pending_link_click = Some((15, 5, "https://example.com".into())); + agent.pending_link_click = Some(( + 15, + 5, + crate::render::osc8::LinkTarget::Url("https://example.com".into()), + )); agent.left_mouse_down = true; let outcome = agent.handle_mouse(&mouse_up(15, 5)); match outcome { - InputOutcome::Action(Action::OpenUrl(url)) => { - assert_eq!(url, "https://example.com"); + InputOutcome::Action(Action::OpenLink(target)) => { + assert_eq!( + target, + crate::render::osc8::LinkTarget::Url("https://example.com".into()) + ); } - other => panic!("expected Action::OpenUrl, got {other:?}"), + other => panic!("expected Action::OpenLink, got {other:?}"), } } - /// A modifier+click on a `file://` link dispatches `OpenUrl` (Ctrl on - /// Linux/Windows; macOS polls CoreGraphics so the Down step isn't + /// A modifier+click preserves a filesystem target through app activation + /// (Ctrl on Linux/Windows; macOS polls CoreGraphics so the Down step isn't /// reproducible in a unit test). #[test] #[cfg(not(target_os = "macos"))] @@ -1065,21 +1098,41 @@ mod link_click_tests { let mut agent = make_agent(); let area = Rect::new(0, 0, 80, 24); setup_scrollback_area(&mut agent, area); - add_visible_link(&mut agent, 5, 10, 30, "file:///tmp/session/images/1.png"); + add_visible_target( + &mut agent, + 5, + 10, + 30, + crate::render::osc8::LinkTarget::File(Arc::from(std::path::Path::new( + "/tmp/session/images/1.png", + ))), + ); let mut down = mouse_down(15, 5); down.modifiers = crossterm::event::KeyModifiers::CONTROL; assert!(matches!(agent.handle_mouse(&down), InputOutcome::Changed)); match agent.handle_mouse(&mouse_up(15, 5)) { - InputOutcome::Action(Action::OpenUrl(url)) => { - assert_eq!(url, "file:///tmp/session/images/1.png"); + InputOutcome::Action(Action::OpenLink(target)) => { + assert_eq!( + target, + crate::render::osc8::LinkTarget::File(Arc::from(std::path::Path::new( + "/tmp/session/images/1.png", + ))) + ); } - other => panic!("expected Action::OpenUrl(file://…), got {other:?}"), + other => panic!("expected Action::OpenLink(file), got {other:?}"), } } fn test_link(url: &str, painted_w: u16) -> crate::scrollback::VisibleLink { crate::scrollback::VisibleLink { rects: vec![Rect::new(0, 0, painted_w, 1)], - url: std::sync::Arc::from(url), + target: crate::render::osc8::LinkTarget::Url(std::sync::Arc::from(url)), + id: None, + } + } + fn test_file_link(path: &std::path::Path, painted_w: u16) -> crate::scrollback::VisibleLink { + crate::scrollback::VisibleLink { + rects: vec![Rect::new(0, 0, painted_w, 1)], + target: crate::render::osc8::LinkTarget::File(Arc::from(path)), id: None, } } @@ -1115,13 +1168,14 @@ mod link_click_tests { true, &test_link(bare, bare_w.saturating_add(40)) )); + let file_path = std::path::Path::new("/tmp/session/images/1.png"); assert!(app_should_open_link_on_click_with( true, - &test_link(file, file_w) + &test_file_link(file_path, file_w) )); assert!(app_should_open_link_on_click_with( true, - &test_link(file, 8) + &test_file_link(file_path, 8) )); } /// Regression: while the plan preview (line viewer) is open and the @@ -1192,10 +1246,17 @@ mod link_click_tests { let mut agent = make_agent(); let area = Rect::new(0, 0, 80, 24); setup_scrollback_area(&mut agent, area); - agent.pending_link_click = Some((15, 5, "https://example.com".into())); + agent.pending_link_click = Some(( + 15, + 5, + crate::render::osc8::LinkTarget::Url("https://example.com".into()), + )); agent.left_mouse_down = true; let outcome = agent.handle_mouse(&mouse_up(16, 5)); - assert!(!matches!(outcome, InputOutcome::Action(Action::OpenUrl(_)))); + assert!(!matches!( + outcome, + InputOutcome::Action(Action::OpenLink(_)) + )); assert!(agent.pending_link_click.is_none()); } #[test] @@ -1204,7 +1265,11 @@ mod link_click_tests { let area = Rect::new(0, 0, 80, 24); setup_scrollback_area(&mut agent, area); add_visible_link(&mut agent, 5, 10, 30, "https://example.com"); - agent.pending_link_click = Some((15, 5, "https://example.com".into())); + agent.pending_link_click = Some(( + 15, + 5, + crate::render::osc8::LinkTarget::Url("https://example.com".into()), + )); let outcome = agent.handle_mouse(&mouse_down(5, 3)); assert!(matches!(outcome, InputOutcome::Changed)); assert!(agent.pending_link_click.is_none()); @@ -1214,7 +1279,11 @@ mod link_click_tests { let mut agent = make_agent(); let area = Rect::new(0, 0, 80, 24); setup_scrollback_area(&mut agent, area); - agent.pending_link_click = Some((15, 5, "https://example.com".into())); + agent.pending_link_click = Some(( + 15, + 5, + crate::render::osc8::LinkTarget::Url("https://example.com".into()), + )); agent.left_mouse_down = true; agent.pending_text_drag = Some(PendingTextDrag { start_col: 15, @@ -1238,14 +1307,21 @@ mod link_click_tests { let mut agent = make_agent(); setup_scrollback_area(&mut agent, Rect::new(0, 0, 80, 20)); agent.active_pane = AgentPane::Prompt; - agent.pending_link_click = Some((15, 5, "https://example.com".into())); + agent.pending_link_click = Some(( + 15, + 5, + crate::render::osc8::LinkTarget::Url("https://example.com".into()), + )); agent.left_mouse_down = true; let outcome = agent.handle_mouse(&mouse_up(15, 5)); match outcome { - InputOutcome::Action(Action::OpenUrl(url)) => { - assert_eq!(url, "https://example.com"); + InputOutcome::Action(Action::OpenLink(target)) => { + assert_eq!( + target, + crate::render::osc8::LinkTarget::Url("https://example.com".into()) + ); } - other => panic!("expected Action::OpenUrl, got {other:?}"), + other => panic!("expected Action::OpenLink, got {other:?}"), } } /// `/btw` panel links share the pane-agnostic Up path: once Down records @@ -1258,14 +1334,21 @@ mod link_click_tests { agent.active_pane = AgentPane::Prompt; agent.btw_focused = true; add_visible_link(&mut agent, 20, 4, 40, "https://example.com/btw"); - agent.pending_link_click = Some((10, 20, "https://example.com/btw".into())); + agent.pending_link_click = Some(( + 10, + 20, + crate::render::osc8::LinkTarget::Url("https://example.com/btw".into()), + )); agent.left_mouse_down = true; let outcome = agent.handle_mouse(&mouse_up(10, 20)); match outcome { - InputOutcome::Action(Action::OpenUrl(url)) => { - assert_eq!(url, "https://example.com/btw"); + InputOutcome::Action(Action::OpenLink(target)) => { + assert_eq!( + target, + crate::render::osc8::LinkTarget::Url("https://example.com/btw".into()) + ); } - other => panic!("expected Action::OpenUrl for btw link, got {other:?}"), + other => panic!("expected Action::OpenLink for btw link, got {other:?}"), } } /// On mouse-fallback terminals, Down on a `/btw` link with the link @@ -1292,11 +1375,12 @@ mod link_click_tests { let outcome = agent.handle_mouse(&down); assert!(matches!(outcome, InputOutcome::Changed)); assert_eq!( - agent - .pending_link_click - .as_ref() - .map(|(c, r, u)| (*c, *r, u.as_str())), - Some((10, 20, "https://example.com/btw")) + agent.pending_link_click.as_ref(), + Some(&( + 10, + 20, + crate::render::osc8::LinkTarget::Url("https://example.com/btw".into()) + )) ); } } @@ -1311,7 +1395,10 @@ mod link_click_tests { screen_row: 20, col_start: 4, col_end: 40, - url: Arc::from("https://example.com/btw"), + target: crate::render::osc8::LinkTarget::Url(Arc::from( + "https://example.com/btw", + )), + presentation: crate::render::osc8::LinkPresentation::Opaque, id: Some(1), }); o @@ -1362,7 +1449,8 @@ mod link_click_tests { screen_row: i as u16, col_start: 0, col_end: 10, - url: Arc::from(*url), + target: crate::render::osc8::LinkTarget::Url(Arc::from(*url)), + presentation: crate::render::osc8::LinkPresentation::Opaque, id: Some(i as u32), }); } @@ -1374,7 +1462,10 @@ mod link_click_tests { add_multiple_links(&mut agent); agent.cycle_highlighted_link(true); assert_eq!(agent.highlighted_link_idx, Some(0)); - assert_eq!(agent.highlighted_link_url(), Some("https://a.com")); + assert_eq!( + agent.highlighted_link_url().as_deref(), + Some("https://a.com") + ); } #[test] fn cycle_backward_from_none_selects_last() { @@ -1382,7 +1473,10 @@ mod link_click_tests { add_multiple_links(&mut agent); agent.cycle_highlighted_link(false); assert_eq!(agent.highlighted_link_idx, Some(2)); - assert_eq!(agent.highlighted_link_url(), Some("https://c.com")); + assert_eq!( + agent.highlighted_link_url().as_deref(), + Some("https://c.com") + ); } #[test] fn cycle_forward_wraps_around() { @@ -1417,10 +1511,13 @@ mod link_click_tests { let enter = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); let outcome = agent.handle_scrollback_key(&enter, ®istry); match outcome { - InputOutcome::Action(Action::OpenUrl(url)) => { - assert_eq!(url, "https://b.com"); + InputOutcome::Action(Action::OpenLink(target)) => { + assert_eq!( + target, + crate::render::osc8::LinkTarget::Url("https://b.com".into()) + ); } - other => panic!("expected Action::OpenUrl, got {other:?}"), + other => panic!("expected Action::OpenLink, got {other:?}"), } assert_eq!(agent.highlighted_link_idx, None); } @@ -1433,7 +1530,10 @@ mod link_click_tests { let registry = ActionRegistry::defaults(); let enter = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); let outcome = agent.handle_scrollback_key(&enter, ®istry); - assert!(!matches!(outcome, InputOutcome::Action(Action::OpenUrl(_)))); + assert!(!matches!( + outcome, + InputOutcome::Action(Action::OpenLink(_)) + )); } /// Enter with a previous user prompt selected enters inline edit mode /// (edit-and-resubmit) instead of falling through to OpenBlockViewer. @@ -1603,7 +1703,7 @@ mod link_click_tests { let mut agent = make_agent(); add_multiple_links(&mut agent); agent.highlighted_link_idx = Some(99); - assert_eq!(agent.highlighted_link_url(), None); + assert!(agent.highlighted_link_url().is_none()); } fn make_search_agent() -> (AgentView, ActionRegistry) { use crate::scrollback::block::RenderBlock; 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 6890282..8929d1c 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 @@ -475,8 +475,13 @@ pub(super) fn app_should_open_link_on_click_with( if !native_plain_url_open { return true; } + let Some(url) = crate::render::osc8::resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + else { + return true; + }; if !crate::app::link_opener::is_safe_to_open( - &link.url, + &url, crate::terminal::hyperlinks::SchemeFilter::Standard, ) { return true; @@ -1000,9 +1005,9 @@ pub struct AgentView { pub last_btw_area: Rect, /// Pending plain scrollback click that should dispatch on mouse-up if no drag starts. pub pending_scrollback_click: Option<(u16, u16)>, - /// Pending link click: (col, row, url). Set on Down(Left) when a link is hit, + /// Pending link click: (col, row, target). Set on Down(Left) when a link is hit, /// consumed on Up(Left) at the same position, cleared on drag. - pub pending_link_click: Option<(u16, u16, String)>, + pub pending_link_click: Option<(u16, u16, crate::render::osc8::LinkTarget)>, /// Absolute paths of media generated in this transcript, used to resolve the /// short relative paths the model prints (`images/1.jpg`) to clickable /// links. Rebuilt from scrollback only when its generation changes. @@ -2110,7 +2115,7 @@ fn collect_citation_links( for url in &ws.citations { links.push(VisibleLink { rects: vec![block_geom.content_area], - url: Arc::from(url.as_str()), + target: crate::render::osc8::LinkTarget::Url(Arc::from(url.as_str())), id: None, }); } @@ -2119,7 +2124,7 @@ fn collect_citation_links( if !wf.url.is_empty() { links.push(VisibleLink { rects: vec![block_geom.content_area], - url: Arc::from(wf.url.as_str()), + target: crate::render::osc8::LinkTarget::Url(Arc::from(wf.url.as_str())), id: None, }); } @@ -3133,7 +3138,7 @@ pub(super) mod test_fixtures { /// the lazy Mermaid glue (which needs a session dir) can be exercised from the /// `mermaid_worker` test module without duplicating the large `AgentSession` /// literal. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub(crate) fn test_agent_view(session_id: Option<&str>, cwd: std::path::PathBuf) -> AgentView { let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); AgentView::new( diff --git a/crates/codegen/xai-grok-pager/src/app/agent_view/notices.rs b/crates/codegen/xai-grok-pager/src/app/agent_view/notices.rs index 9557036..64bb06e 100644 --- a/crates/codegen/xai-grok-pager/src/app/agent_view/notices.rs +++ b/crates/codegen/xai-grok-pager/src/app/agent_view/notices.rs @@ -327,6 +327,30 @@ impl AgentView { .as_mut() .is_some_and(|m| m.tick_result_notice()) } + + /// Open `url` in the system browser. When the opener cannot run (headless + /// Linux VM, missing `xdg-open`, etc.), push a scrollback system message + /// with the full URL so the user can copy it, and best-effort copy to the + /// clipboard (OSC 52 works over SSH even without a local display). + /// + /// Unsafe schemes are rejected silently (same as [`open_url_if_safe`]). + pub(crate) fn open_url_or_show(&mut self, url: &str) { + use crate::app::link_opener::{OpenUrlResult, browser_unavailable_message, try_open_url}; + use crate::scrollback::block::RenderBlock; + use crate::terminal::hyperlinks::SchemeFilter; + + match try_open_url(url, SchemeFilter::Standard) { + OpenUrlResult::Opened | OpenUrlResult::RejectedScheme => {} + OpenUrlResult::BrowserUnavailable => { + self.scrollback + .push_block(RenderBlock::system(browser_unavailable_message(url))); + // Best-effort clipboard so SSH/VM users can paste into a + // browser on another machine without selecting TUI text. + let _ = crate::clipboard::SystemClipboard::try_set(url); + self.show_toast("Browser unavailable - URL shown above"); + } + } + } } #[cfg(test)] 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 62cad0a..213e10e 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 @@ -43,10 +43,10 @@ impl AgentView { return InputOutcome::Action(Action::FocusPrompt); } if key!(Enter).matches(key) - && let Some(url) = self.highlighted_link_url().map(String::from) + && let Some(target) = self.highlighted_link_target().cloned() { self.highlighted_link_idx = None; - return InputOutcome::Action(Action::OpenUrl(url)); + return InputOutcome::Action(Action::OpenLink(target)); } if key!(Enter).matches(key) && !self.scrollback.is_selected_group_header() @@ -602,10 +602,11 @@ impl AgentView { modifiers: crossterm::event::KeyModifiers::NONE, }; let _ = self.prompt.handle_mouse(&event); - } else if let Some((scroll_top, scroll_bottom)) = self.question_scroll_region { - if row >= scroll_top && row < scroll_bottom { - self.apply_question_scroll(lines); - } + } else if let Some((scroll_top, scroll_bottom)) = self.question_scroll_region + && row >= scroll_top + && row < scroll_bottom + { + self.apply_question_scroll(lines); } return; } 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 32a72b6..43c7a6b 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 @@ -4151,12 +4151,19 @@ impl AgentView { && r.x < link.col_end }) }) - .map(|link| xai_ratatui_inline::LinkSpan { - row: link.screen_row, - col_start: link.col_start, - col_end: link.col_end, - url: link.url.clone(), - id: if emit_id { link.id } else { None }, + .filter_map(|link| { + crate::render::osc8::resolve_link_target_with_presentation( + &link.target, + link.presentation, + ) + .and_then(|resolved| resolved.osc8_url) + .map(|url| xai_ratatui_inline::LinkSpan { + row: link.screen_row, + col_start: link.col_start, + col_end: link.col_end, + url, + id: if emit_id { link.id } else { None }, + }) }) .collect(); self.push_promo_cta_link_span( 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 5cdcc20..227d2d4 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 @@ -428,10 +428,10 @@ impl AgentView { /// load's batch/replay bookkeeping — and defer its results. The window's /// pending re-init completion later no-ops (generation gone). pub(crate) fn abort_session_reload(&mut self) { - if let Some(reload) = self.session_reload.take() { - if self.apply_reload_outcome(reload, false) { - crate::memory_release::release_retained_memory_with("reload-abort"); - } + if let Some(reload) = self.session_reload.take() + && self.apply_reload_outcome(reload, false) + { + crate::memory_release::release_retained_memory_with("reload-abort"); } } /// Finalize the reload window opened for `generation`. 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 81412b4..4abb120 100644 --- a/crates/codegen/xai-grok-pager/src/app/app_view.rs +++ b/crates/codegen/xai-grok-pager/src/app/app_view.rs @@ -5653,7 +5653,8 @@ pub(crate) mod tests { screen_row: 2, col_start: 0, col_end: 10, - url: Arc::from("https://example.com"), + target: crate::render::osc8::LinkTarget::Url(Arc::from("https://example.com")), + presentation: crate::render::osc8::LinkPresentation::Opaque, id: Some(1), }); agent.visible_link_map.rebuild(1, &overlay, vec![]); diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/billing.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/billing.rs index 6706aee..d04601f 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/billing.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/billing.rs @@ -578,7 +578,7 @@ pub(super) fn dispatch_open_supergrok_url(app: &mut AppView) -> Vec { // being correctly configured. If the URL already specifies a // referrer it's left alone. let url = crate::app::link_opener::ensure_query_param(url, "referrer", "grok-build"); - crate::app::link_opener::open_url(&url); + super::ctx::open_url_or_show(app, &url); vec![] } diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/ctx.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/ctx.rs index 4f5e13d..157b84a 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/ctx.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/ctx.rs @@ -34,6 +34,31 @@ pub(super) fn with_active_agent(app: &mut AppView, f: impl FnOnce(&mut AgentView } } +/// Open `url` via the system browser, falling back to a visible URL when the +/// browser cannot open (headless VM / missing opener). Prefer this over raw +/// `open_url_if_safe` for user-initiated billing/upgrade CTAs. +/// +/// When no agent is active (welcome/gate screen), still attempts the open and +/// falls back to clipboard + toast. +pub(super) fn open_url_or_show(app: &mut AppView, url: &str) { + if let Some(agent) = get_active_agent_mut(app) { + agent.open_url_or_show(url); + return; + } + + use crate::app::link_opener::{OpenUrlResult, browser_unavailable_message, try_open_url}; + use crate::terminal::hyperlinks::SchemeFilter; + + match try_open_url(url, SchemeFilter::Standard) { + OpenUrlResult::Opened | OpenUrlResult::RejectedScheme => {} + OpenUrlResult::BrowserUnavailable => { + let _ = crate::clipboard::SystemClipboard::try_set(url); + // No scrollback on the welcome screen — toast carries the URL. + app.show_toast(&browser_unavailable_message(url)); + } + } +} + /// Get a shared reference to the active agent view (if any). pub(super) fn get_active_agent(app: &AppView) -> Option<&AgentView> { if let ActiveView::Agent(id) = app.active_view diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs index f6cd154..a47efd2 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/router.rs @@ -5,7 +5,7 @@ use super::auth::{ }; use super::billing::dispatch_open_supergrok_url; use super::ctx::{ - active_agent_session_id, get_active_agent_mut, navigate_clearing_selection, + active_agent_session_id, get_active_agent_mut, navigate_clearing_selection, open_url_or_show, sync_sleep_inhibitor, with_active_agent, with_scrollback, }; use super::dashboard::{ @@ -583,10 +583,7 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec { surface: xai_grok_telemetry::events::CreditLimitUpsellSurface::InlineCard, choice, }); - crate::app::link_opener::open_url_if_safe( - &url, - crate::terminal::hyperlinks::SchemeFilter::Standard, - ); + open_url_or_show(app, &url); } else { dispatch_open_block_viewer(app); } @@ -849,16 +846,17 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec { } } Action::AnnouncementsOpenCta(surface) => { - use crate::terminal::hyperlinks::SchemeFilter; if let Some((promo, url)) = crate::views::announcements::promo_cta_target( &app.active_announcements, &app.hidden_announcement_ids, ) { + let url = url.to_owned(); + let promo_id = promo.id.clone(); log_event(xai_grok_telemetry::events::AnnouncementCtaClicked { - id: promo.id.clone(), + id: promo_id, source: surface, }); - crate::app::link_opener::open_url_if_safe(url, SchemeFilter::Standard); + open_url_or_show(app, &url); } vec![] } @@ -962,7 +960,6 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec { Action::CheckSubscription => vec![Effect::CheckSubscription { verify: None }], Action::OpenSupergrokUrl => dispatch_open_supergrok_url(app), Action::OpenUrl(url) => { - use crate::terminal::hyperlinks::SchemeFilter; if url.starts_with("file://") { let opened = url::Url::parse(&url) .ok() @@ -974,14 +971,31 @@ pub(crate) fn dispatch(action: Action, app: &mut AppView) -> Vec { "Could not open file" }); } else { - crate::app::link_opener::open_url_if_safe(&url, SchemeFilter::Standard); + open_url_or_show(app, &url); + } + vec![] + } + Action::OpenLink(target) => { + use crate::render::osc8::LinkTarget; + match crate::render::osc8::resolve_link_open_target(&target) { + Some(LinkTarget::File(path)) => { + let opened = crate::app::link_opener::open_path(&path); + app.show_toast(if opened { + "Opening in default app\u{2026}" + } else { + "Could not open file" + }); + } + Some(LinkTarget::Url(url)) => { + crate::app::link_opener::open_url(&url); + } + None => {} } vec![] } Action::OpenManagedConnectors => { - use crate::terminal::hyperlinks::SchemeFilter; let url = crate::views::mcps_modal::managed_connectors_url(app.team_id.as_deref()); - crate::app::link_opener::open_url_if_safe(&url, SchemeFilter::Standard); + open_url_or_show(app, &url); vec![] } Action::OpenNextLink => { diff --git a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/billing.rs b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/billing.rs index 79bfeff..538e0f8 100644 --- a/crates/codegen/xai-grok-pager/src/app/dispatch/tests/billing.rs +++ b/crates/codegen/xai-grok-pager/src/app/dispatch/tests/billing.rs @@ -1058,3 +1058,135 @@ fn unknown_non_restricted_command_still_passes_through() { "no upsell for genuinely unknown commands" ); } + +// ── Browser-unavailable URL fallback ──────────────────────────────── + +/// When the OS browser opener cannot run (simulated via a broken +/// `GROK_TEST_OPEN_URL_FILE` seam), `Action::OpenUrl` for a billing CTA +/// must push a scrollback system message that includes the full URL — +/// the headless-VM fix for silent Upgrade / Buy-more-credits no-ops. +#[serial_test::serial(GROK_TEST_OPEN_URL_FILE)] +#[test] +fn open_url_shows_manual_url_when_browser_unavailable() { + // Point the test seam at a path whose parent dir does not exist so the + // write fails and `open_url` returns false (BrowserUnavailable). + let bad = std::env::temp_dir().join(format!( + "grok-open-url-missing-{}/out.txt", + std::process::id() + )); + // SAFETY: serialized via `serial_test` so no other test races the env var. + unsafe { std::env::set_var("GROK_TEST_OPEN_URL_FILE", &bad) }; + + let mut app = test_app_with_agent(); + let before = agent_scrollback_len(&app); + let url = UPSELL_URL_UPGRADE; + let effects = dispatch(Action::OpenUrl(url.to_string()), &mut app); + assert!(effects.is_empty()); + + assert_eq!( + agent_scrollback_len(&app), + before + 1, + "must push a system message with the URL" + ); + let text = last_system_text(&app, AgentId(0)); + assert!( + text.contains("Could not open a browser"), + "fallback copy missing: {text}" + ); + assert!( + text.contains(url), + "full billing URL must be visible for copy: {text}" + ); + let toast = app.agents[&AgentId(0)] + .toast + .as_ref() + .map(|(m, _)| m.as_str()); + assert_eq!(toast, Some("Browser unavailable - URL shown above")); + + // SAFETY: serialized via `serial_test`; restore the env for other tests. + unsafe { std::env::remove_var("GROK_TEST_OPEN_URL_FILE") }; +} + +/// Successful open (test seam write OK) must not spam a fallback system message. +#[serial_test::serial(GROK_TEST_OPEN_URL_FILE)] +#[test] +fn open_url_does_not_show_fallback_when_opener_succeeds() { + let url_file = + std::env::temp_dir().join(format!("grok-open-url-ok-{}.txt", std::process::id())); + let _ = std::fs::remove_file(&url_file); + // SAFETY: serialized via `serial_test`. + unsafe { std::env::set_var("GROK_TEST_OPEN_URL_FILE", &url_file) }; + + let mut app = test_app_with_agent(); + let before = agent_scrollback_len(&app); + let url = UPSELL_URL_PAYG; + let _ = dispatch(Action::OpenUrl(url.to_string()), &mut app); + + assert_eq!( + agent_scrollback_len(&app), + before, + "successful open must not push a fallback system message" + ); + let recorded = std::fs::read_to_string(&url_file).unwrap_or_default(); + assert!( + recorded.lines().any(|l| l == url), + "opener seam must record the URL; got {recorded:?}" + ); + + // SAFETY: serialized via `serial_test`. + unsafe { std::env::remove_var("GROK_TEST_OPEN_URL_FILE") }; + let _ = std::fs::remove_file(&url_file); +} + +/// Credit-limit upsell Q&A submit routes through OpenUrl; when the browser +/// is unavailable the full option URL must land in scrollback. +#[serial_test::serial(GROK_TEST_OPEN_URL_FILE)] +#[test] +fn credit_limit_upsell_submit_shows_url_when_browser_unavailable() { + use crate::app::agent_view::translate_local_submit_for_test; + use crate::app::app_view::InputOutcome; + use crate::views::question_view::{LocalQuestionKind, QuestionSelection}; + + let bad = std::env::temp_dir().join(format!( + "grok-open-url-upsell-missing-{}/out.txt", + std::process::id() + )); + // SAFETY: serialized via `serial_test`. + unsafe { std::env::set_var("GROK_TEST_OPEN_URL_FILE", &bad) }; + + let mut app = test_app_with_agent(); + open_upsell_qa(&mut app, CreditLimitUpsellMode::UnifiedCredits); + let mut qv = app + .agents + .get_mut(&AgentId(0)) + .unwrap() + .question_view + .take() + .expect("expected credit-limit upsell modal"); + // Select option 1 = "Buy more credits" (credits / usage URL). + qv.selections[0] = QuestionSelection::Single(Some(1)); + let kind = LocalQuestionKind::CreditLimitUpsell { + choices: vec![ + xai_grok_telemetry::events::CreditLimitChoice::UpgradeTier, + xai_grok_telemetry::events::CreditLimitChoice::PurchaseCredits, + ], + }; + let InputOutcome::Action(Action::OpenUrl(url)) = + translate_local_submit_for_test(&qv, kind, false) + else { + panic!("expected OpenUrl from upsell submit"); + }; + assert_eq!(url, UPSELL_URL_PAYG); + + let before = agent_scrollback_len(&app); + let _ = dispatch(Action::OpenUrl(url.clone()), &mut app); + let text = last_system_text(&app, AgentId(0)); + assert_eq!(agent_scrollback_len(&app), before + 1); + assert!( + text.contains(&url), + "upsell URL missing from fallback: {text}" + ); + + // SAFETY: serialized via `serial_test`. + unsafe { std::env::remove_var("GROK_TEST_OPEN_URL_FILE") }; +} diff --git a/crates/codegen/xai-grok-pager/src/app/event_loop.rs b/crates/codegen/xai-grok-pager/src/app/event_loop.rs index 965619a..e093cd6 100644 --- a/crates/codegen/xai-grok-pager/src/app/event_loop.rs +++ b/crates/codegen/xai-grok-pager/src/app/event_loop.rs @@ -781,7 +781,13 @@ pub(crate) async fn run( crate::notifications::load_notification_config(raw), ); if let Some(table) = raw.as_table() { - app.voice_config = xai_grok_voice::VoiceConfig::from_config_table(table); + // Voice inherits the same resolved endpoints base as chat + // (config > GROK_XAI_API_BASE_URL env > default). + let endpoints_base = + xai_grok_shell::agent::config::EndpointsConfig::from_config_value(raw) + .xai_api_base_url; + app.voice_config = + xai_grok_voice::VoiceConfig::from_config_table(table, Some(&endpoints_base)); } } // Stamp request-identity headers so the STT handshake attributes voice usage diff --git a/crates/codegen/xai-grok-pager/src/app/mod.rs b/crates/codegen/xai-grok-pager/src/app/mod.rs index 917fe38..9b1f500 100644 --- a/crates/codegen/xai-grok-pager/src/app/mod.rs +++ b/crates/codegen/xai-grok-pager/src/app/mod.rs @@ -112,7 +112,7 @@ static MINIMAL_SHOW_SWITCH_BACK_TO_FULLSCREEN: AtomicBool = AtomicBool::new(fals pub fn minimal_show_switch_back_to_fullscreen() -> bool { MINIMAL_SHOW_SWITCH_BACK_TO_FULLSCREEN.load(Ordering::Acquire) } -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_minimal_show_switch_back_to_fullscreen_for_test(on: bool) { MINIMAL_SHOW_SWITCH_BACK_TO_FULLSCREEN.store(on, Ordering::Release); } diff --git a/crates/codegen/xai-grok-pager/src/app/mouse.rs b/crates/codegen/xai-grok-pager/src/app/mouse.rs index 38cb9c2..a6c0d25 100644 --- a/crates/codegen/xai-grok-pager/src/app/mouse.rs +++ b/crates/codegen/xai-grok-pager/src/app/mouse.rs @@ -370,7 +370,7 @@ impl AgentView { && let Some(link) = self.visible_link_map.link_at(mouse.column, mouse.row) { self.pending_link_click = app_should_open_link_on_click(link) - .then(|| (mouse.column, mouse.row, link.url.to_string())); + .then(|| (mouse.column, mouse.row, link.target.clone())); self.pending_scrollback_click = None; return InputOutcome::Changed; } @@ -425,11 +425,9 @@ impl AgentView { } if let Some(id) = self.queue.send_now_click(mouse.column, mouse.row) && self.session.state.is_turn_running() + && let InputOutcome::Action(action) = self.force_interject_queue_row(id) { - if let InputOutcome::Action(action) = self.force_interject_queue_row(id) - { - return InputOutcome::Action(action); - } + return InputOutcome::Action(action); } self.set_active_pane(AgentPane::Queue, false); self.queue.handle_mouse( @@ -618,7 +616,7 @@ impl AgentView { self.visible_link_map.link_at(mouse.column, mouse.row) { self.pending_link_click = app_should_open_link_on_click(link) - .then(|| (mouse.column, mouse.row, link.url.to_string())); + .then(|| (mouse.column, mouse.row, link.target.clone())); self.pending_scrollback_click = None; return InputOutcome::Changed; } @@ -697,11 +695,11 @@ impl AgentView { } let had_pending_text_drag = self.pending_text_drag.take().is_some(); let _had_pending_block_drag = self.pending_block_drag.take().is_some(); - if let Some((lc, lr, url)) = self.pending_link_click.take() + if let Some((lc, lr, target)) = self.pending_link_click.take() && mouse.column == lc && mouse.row == lr { - return InputOutcome::Action(Action::OpenUrl(url)); + return InputOutcome::Action(Action::OpenLink(target)); } if self.active_pane == AgentPane::Scrollback { if let Some((click_col, click_row)) = self.pending_scrollback_click.take() { @@ -793,10 +791,7 @@ impl AgentView { surface: xai_grok_telemetry::events::CreditLimitUpsellSurface::InlineCard, choice, }); - crate::app::link_opener::open_url_if_safe( - &url, - crate::terminal::hyperlinks::SchemeFilter::Standard, - ); + self.open_url_or_show(&url); self.last_click = None; return InputOutcome::Changed; } diff --git a/crates/codegen/xai-grok-pager/src/headless.rs b/crates/codegen/xai-grok-pager/src/headless.rs index b8023e1..4af6912 100644 --- a/crates/codegen/xai-grok-pager/src/headless.rs +++ b/crates/codegen/xai-grok-pager/src/headless.rs @@ -13,7 +13,7 @@ use clap::ValueEnum; use tokio_util::sync::CancellationToken; use agent_client_protocol as acp; -use xai_acp_lib::{AcpAgentTx, AcpClientMessageBox, acp_send}; +use xai_acp_lib::{AcpAgentTx, AcpClientMessageBox, AcpClientRx, acp_send}; use xai_grok_shell::agent::auth_method::AuthMethodKind; use xai_grok_shell::agent::config::Config as AgentConfig; use xai_grok_shell::extensions::task::{CancelSubagentRequest, KillTaskRequest}; @@ -1223,19 +1223,18 @@ pub async fn run_single_turn( prompt_result = Some(res); prompt_done_at = Some(Instant::now()); if !options.wait_for_background { - // Drain notifications already queued ahead of the response. - while let Ok(msg) = acp_rx.try_recv() { - handle_headless_acp_message( - msg.boxed(), - &mut emitter, - t_prompt, - &mut ttf_logged, - options.yolo, - options.output_format, - &mut pending_bg, - &mut completed_before_bg, - ); - } + drain_acp_with_grace( + &mut acp_rx, + Duration::from_millis(750), + &mut emitter, + t_prompt, + &mut ttf_logged, + options.yolo, + options.output_format, + &mut pending_bg, + &mut completed_before_bg, + ) + .await; break; } // With wait_for_background: keep draining ACP for task_completed. @@ -1461,6 +1460,58 @@ fn track_background_lifecycle( // ── ACP client message handling (select arm + pre-exit drain) ──────────── +#[allow(clippy::too_many_arguments)] +async fn drain_acp_with_grace( + acp_rx: &mut AcpClientRx, + grace: Duration, + emitter: &mut HeadlessEmitter, + t_prompt: Instant, + ttf_logged: &mut bool, + yolo: bool, + output_format: OutputFormat, + pending_bg: &mut HashSet, + completed_before_bg: &mut HashSet, +) { + let deadline = Instant::now() + grace; + loop { + while let Ok(msg) = acp_rx.try_recv() { + handle_headless_acp_message( + msg.boxed(), + emitter, + t_prompt, + ttf_logged, + yolo, + output_format, + pending_bg, + completed_before_bg, + ); + } + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining.is_zero() { + break; + } + tokio::select! { + biased; + msg = acp_rx.recv() => { + let Some(msg) = msg else { break; }; + handle_headless_acp_message( + msg.boxed(), + emitter, + t_prompt, + ttf_logged, + yolo, + output_format, + pending_bg, + completed_before_bg, + ); + } + _ = tokio::time::sleep(remaining) => { + break; + } + } + } +} + /// Process one inbound ACP client message. Used by both `acp_rx.recv()` and /// `try_recv()` so buffered `task_backgrounded` is not dropped when /// `PromptResponse` completes first. diff --git a/crates/codegen/xai-grok-pager/src/minimal/api.rs b/crates/codegen/xai-grok-pager/src/minimal/api.rs index da9dd16..916df8e 100644 --- a/crates/codegen/xai-grok-pager/src/minimal/api.rs +++ b/crates/codegen/xai-grok-pager/src/minimal/api.rs @@ -32,7 +32,7 @@ use ratatui::style::Color; use crate::acp::tracker::TurnActivity; // Only the test-only setters below reference `AgentSession`. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] use crate::app::agent::AgentSession; use crate::app::agent_view::{AgentView, McpInitProgress}; use crate::app::app_view::{ActiveView, AppView, SessionPickerEntry}; @@ -581,49 +581,49 @@ pub fn record_committed_for_expand(sb: &mut ScrollbackState, id: EntryId) { // ── Test-only surface (minimal's unit tests, via the test-only helpers) ── /// [`crate::app::agent_view::test_agent_view`]. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn test_agent_view(session_id: Option<&str>, cwd: std::path::PathBuf) -> AgentView { crate::app::agent_view::test_agent_view(session_id, cwd) } /// Test-only setter for `AgentView::extensions_modal`. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_extensions_modal(v: &mut AgentView, val: Option) { v.extensions_modal = val; } /// Test-only setter for `AgentView::question_view`. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_question_view(v: &mut AgentView, val: Option) { v.question_view = val; } /// Test-only setter for `AgentView::plan_mode_active`. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_plan_mode_active(v: &mut AgentView, on: bool) { v.plan_mode_active = on; } /// Test-only setter for `AgentView::plan_mode_pending`. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_plan_mode_pending(v: &mut AgentView, val: Option) { v.plan_mode_pending = val; } /// Test-only mutable access to `PromptWidget::suggestions`. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn prompt_suggestions_mut(pw: &mut PromptWidget) -> &mut SuggestionController { &mut pw.suggestions } /// Test-only setter for `AgentSession`'s yolo mode. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_yolo_mode_for_test(session: &mut AgentSession, on: bool) { session.set_yolo_mode_for_test(on); } /// Test-only setter for `AgentSession`'s auto mode. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_auto_mode_for_test(session: &mut AgentSession, on: bool) { session.set_auto_mode_for_test(on); } @@ -632,7 +632,7 @@ pub fn set_auto_mode_for_test(session: &mut AgentSession, on: bool) { /// toggle. Thinking blocks render zero rows when this is off (the default), so /// minimal's commit-height tests must force it on to exercise a thinking /// block's committed height instead of getting an order-dependent 0. -#[cfg(test)] +#[cfg(any(test, feature = "test-support"))] pub fn set_show_thinking_blocks(enabled: bool) { crate::appearance::cache::set_show_thinking_blocks(enabled); } diff --git a/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/edit.rs b/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/edit.rs index 75af8df..9b51915 100644 --- a/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/edit.rs +++ b/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/edit.rs @@ -938,9 +938,8 @@ impl EditToolCallBlock { Line::from(spans) } - /// Absolute `file://` for OSC8 regardless of painted path surface. - fn path_link_url(&self, cwd: Option<&Path>) -> Option> { - crate::render::osc8::tool_path_file_url(&self.path, cwd) + fn path_link_target(&self, cwd: Option<&Path>) -> Option { + crate::render::osc8::tool_path_file_target(&self.path, cwd) } /// Render this block's hunks for its current highlight phase — the single @@ -1152,7 +1151,7 @@ impl EditToolCallBlock { edit_cfg.effective_line_summary(crate::appearance::cache::load_collapsed_edit_blocks()); let cwd = ctx.cwd.as_deref(); - let link_url = self.path_link_url(cwd); + let link_target = self.path_link_target(cwd); match ctx.mode { DisplayMode::Collapsed => { @@ -1179,7 +1178,7 @@ impl EditToolCallBlock { selection_range: Some(TOOL_HEADER_RANGE), // Copy the painted path span (basename when collapsed). content: line, - link_url, + link_target, ..Default::default() }], }) @@ -1232,7 +1231,7 @@ impl EditToolCallBlock { selection_text: line.selection_text, joiner: line.joiner, content: line.content, - link_url: if has_path { link_url.clone() } else { None }, + link_target: if has_path { link_target.clone() } else { None }, ..Default::default() }); } @@ -1656,13 +1655,23 @@ mod tests { } #[test] - fn header_link_url_is_absolute_file_url_for_all_surfaces() { + fn header_link_target_is_absolute_file_for_all_surfaces() { let abs = "/Users/me/project/src/foo.rs"; let cwd = Path::new("/Users/me/project"); let block = EditToolCallBlock::new(abs, vec![]); - let url = block.path_link_url(Some(cwd)).expect("file url"); - assert!(url.starts_with("file://"), "got {url}"); - assert!(url.contains("foo.rs"), "got {url}"); + let target = block.path_link_target(Some(cwd)).expect("file target"); + assert_eq!( + target, + crate::render::osc8::LinkTarget::File(Arc::from(Path::new(abs))) + ); + assert_eq!( + crate::render::osc8::resolve_link_target(&target) + .unwrap() + .osc8_url + .unwrap() + .as_ref(), + "file:///Users/me/project/src/foo.rs" + ); let mut ctx = test_ctx(); ctx.cwd = Some(cwd.to_path_buf()); @@ -1672,7 +1681,7 @@ mod tests { collapsed.lines[0].content.spans[1].content.as_ref(), "foo.rs" ); - assert_eq!(collapsed.lines[0].link_url.as_deref(), Some(url.as_ref())); + assert_eq!(collapsed.lines[0].link_target.as_ref(), Some(&target)); ctx.mode = DisplayMode::Expanded; let expanded = block.output(&ctx); @@ -1680,7 +1689,7 @@ mod tests { expanded.lines[0].content.spans[1].content.as_ref(), "src/foo.rs" ); - assert_eq!(expanded.lines[0].link_url.as_deref(), Some(url.as_ref())); + assert_eq!(expanded.lines[0].link_target.as_ref(), Some(&target)); } #[test] diff --git a/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/read.rs b/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/read.rs index 8e16612..d53ac09 100644 --- a/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/read.rs +++ b/crates/codegen/xai-grok-pager/src/scrollback/blocks/tool/read.rs @@ -233,19 +233,19 @@ impl ReadToolCallBlock { /// /// Spans: `["Read ", path, optional_range_suffix, optional_extra_suffix]` /// or `["Skill ", skill_name]`. Prefix/suffixes excluded (no `selection_text` - /// override). Sets absolute `file://` `link_url` for non-skill paths. + /// override). Attaches a semantic filesystem target for non-skill paths. fn header_block_line(&self, line: Line<'static>, cwd: Option<&std::path::Path>) -> BlockLine { let path_end = 2.min(line.spans.len()).max(1); - let link_url = if self.skill_name().is_some() { + let link_target = if self.skill_name().is_some() { None } else { - crate::render::osc8::tool_path_file_url(&self.path, cwd) + crate::render::osc8::tool_path_file_target(&self.path, cwd) }; BlockLine { selectable: Selectable::Spans(1..path_end), selection_range: Some(TOOL_HEADER_RANGE), content: line, - link_url, + link_target, ..Default::default() } } @@ -615,16 +615,31 @@ mod tests { } #[test] - fn header_link_url_is_absolute_for_collapsed_and_expanded() { + fn header_link_target_is_absolute_file_for_collapsed_and_expanded() { let abs = "/Users/me/project/src/main.rs"; let block = ReadToolCallBlock::new(abs); let mut ctx = make_ctx(); ctx.cwd = Some(std::path::PathBuf::from("/Users/me/project")); let collapsed = block.output(&ctx); - let url = collapsed.lines[0].link_url.as_ref().expect("link_url"); - assert!(url.starts_with("file://"), "got {url}"); - assert!(url.contains("main.rs"), "got {url}"); + let target = collapsed.lines[0] + .link_target + .as_ref() + .expect("link target"); + assert_eq!( + target, + &crate::render::osc8::LinkTarget::File( + std::sync::Arc::from(std::path::Path::new(abs),) + ) + ); + assert_eq!( + crate::render::osc8::resolve_link_target(target) + .unwrap() + .osc8_url + .unwrap() + .as_ref(), + "file:///Users/me/project/src/main.rs" + ); assert_eq!( collapsed.lines[0].content.spans[1].content.as_ref(), "main.rs" @@ -636,7 +651,7 @@ mod tests { expanded.lines[0].content.spans[1].content.as_ref(), "src/main.rs" ); - assert_eq!(expanded.lines[0].link_url.as_deref(), Some(url.as_ref())); + assert_eq!(expanded.lines[0].link_target.as_ref(), Some(target)); } #[test] diff --git a/crates/codegen/xai-grok-pager/src/scrollback/link_map.rs b/crates/codegen/xai-grok-pager/src/scrollback/link_map.rs index 82ca723..8cafdc8 100644 --- a/crates/codegen/xai-grok-pager/src/scrollback/link_map.rs +++ b/crates/codegen/xai-grok-pager/src/scrollback/link_map.rs @@ -5,9 +5,8 @@ //! tool blocks. Used by the mouse handler for click-to-open. use ratatui::layout::Rect; -use std::sync::Arc; -use crate::render::osc8::LinkOverlay; +use crate::render::osc8::{LinkOverlay, LinkTarget}; /// A clickable link region on screen. /// @@ -16,7 +15,7 @@ use crate::render::osc8::LinkOverlay; #[derive(Debug, Clone)] pub struct VisibleLink { pub rects: Vec, - pub url: Arc, + pub target: LinkTarget, pub id: Option, } @@ -32,8 +31,11 @@ impl VisibleLink { /// True when painted cell width equals the URL's display width (bare URL /// text on screen, not a short label or wide citation block). pub fn looks_like_bare_url_text(&self) -> bool { + let LinkTarget::Url(url) = &self.target else { + return false; + }; let painted: usize = self.rects.iter().map(|r| usize::from(r.width)).sum(); - painted == unicode_width::UnicodeWidthStr::width(self.url.as_ref()) + painted == unicode_width::UnicodeWidthStr::width(url.as_ref()) } } @@ -65,13 +67,37 @@ impl VisibleLinkMap { generation: u64, overlay: &LinkOverlay, citation_links: Vec, + ) { + self.rebuild_for_context( + generation, + overlay, + citation_links, + crate::terminal::terminal_context(), + ); + } + + fn rebuild_for_context( + &mut self, + generation: u64, + overlay: &LinkOverlay, + citation_links: Vec, + terminal: &crate::terminal::TerminalContext, ) { self.links.clear(); self.generation = generation; self.links .reserve(overlay.links().len() + citation_links.len()); - self.push_overlay_links(overlay, /* merge_from */ 0); - self.links.extend(citation_links); + self.push_overlay_links(overlay, /* merge_from */ 0, terminal); + self.links + .extend(citation_links.into_iter().filter_map(|mut link| { + link.target = crate::render::osc8::resolve_link_target_for_context( + &link.target, + crate::render::osc8::LinkPresentation::Opaque, + terminal, + )? + .open_target?; + Some(link) + })); } /// Append overlay links (e.g. `/btw`) without changing generation. @@ -88,14 +114,27 @@ impl VisibleLinkMap { /// each frame's links will accumulate. pub fn append_from_overlay(&mut self, overlay: &LinkOverlay) { let start_len = self.links.len(); - self.push_overlay_links(overlay, start_len); + self.push_overlay_links(overlay, start_len, crate::terminal::terminal_context()); } /// Push overlay segments, merging same-`id` only with entries at /// indices `>= merge_from` (0 for rebuild; map length for append). - fn push_overlay_links(&mut self, overlay: &LinkOverlay, merge_from: usize) { + fn push_overlay_links( + &mut self, + overlay: &LinkOverlay, + merge_from: usize, + terminal: &crate::terminal::TerminalContext, + ) { self.links.reserve(overlay.links().len()); for link in overlay.links() { + let Some(target) = crate::render::osc8::resolve_link_target_for_context( + &link.target, + link.presentation, + terminal, + ) + .and_then(|resolved| resolved.open_target) else { + continue; + }; let width = link.col_end.saturating_sub(link.col_start); if width == 0 { continue; @@ -110,7 +149,7 @@ impl VisibleLinkMap { } else { self.links.push(VisibleLink { rects: vec![rect], - url: Arc::clone(&link.url), + target, id: link.id, }); } @@ -144,7 +183,8 @@ impl VisibleLinkMap { #[cfg(test)] mod tests { use super::*; - use crate::render::osc8::{LinkOverlay, OverlayLink}; + use crate::render::osc8::{LinkOverlay, LinkPresentation, OverlayLink, resolve_link_target}; + use crate::terminal::{TerminalContext, TerminalName}; use std::sync::Arc; fn make_overlay(links: Vec<(u16, u16, u16, &str, Option)>) -> LinkOverlay { @@ -154,7 +194,8 @@ mod tests { screen_row: row, col_start, col_end, - url: Arc::from(url), + target: LinkTarget::Url(Arc::from(url)), + presentation: LinkPresentation::Opaque, id, }); } @@ -168,7 +209,7 @@ mod tests { .enumerate() .map(|(i, w)| Rect::new(0, i as u16, *w, 1)) .collect(), - url: Arc::from(url), + target: LinkTarget::Url(Arc::from(url)), id: None, } } @@ -193,6 +234,120 @@ mod tests { assert!(!link(url, &[url_w.saturating_add(40)]).looks_like_bare_url_text()); } + #[test] + fn file_target_provenance_survives_overlay_to_visible_map() { + let path = Arc::::from(std::path::Path::new( + "/tmp/non-display-target/file name.rs", + )); + let mut overlay = LinkOverlay::new(); + overlay.push(OverlayLink { + screen_row: 3, + col_start: 4, + col_end: 10, + target: LinkTarget::File(Arc::clone(&path)), + presentation: crate::render::osc8::LinkPresentation::Opaque, + id: None, + }); + + let mut map = VisibleLinkMap::default(); + map.rebuild(1, &overlay, vec![]); + + assert_eq!(map.links()[0].target, LinkTarget::File(Arc::clone(&path))); + let resolved = resolve_link_target(&map.links()[0].target).expect("resolved file target"); + assert_eq!(resolved.open_target, Some(LinkTarget::File(path))); + assert_eq!( + resolved.osc8_url.unwrap().as_ref(), + "file:///tmp/non-display-target/file%20name.rs" + ); + assert!(!map.links()[0].looks_like_bare_url_text()); + } + + #[test] + fn official_vscode_remote_file_is_excluded_from_activation_map() { + let file = LinkTarget::File(Arc::from(std::path::Path::new("/worktree/src/main.rs"))); + let web = LinkTarget::Url(Arc::from("https://example.com/docs")); + let mut overlay = LinkOverlay::new(); + for (row, target, presentation) in [ + (3, file, LinkPresentation::SelfResolvingPath), + (4, web.clone(), LinkPresentation::Opaque), + ] { + overlay.push(OverlayLink { + screen_row: row, + col_start: 4, + col_end: 20, + target, + presentation, + id: None, + }); + } + let terminal = TerminalContext { + brand: TerminalName::VsCode, + is_ssh: true, + is_official_vscode_remote: true, + ..Default::default() + }; + + let mut map = VisibleLinkMap::default(); + map.rebuild_for_context(1, &overlay, vec![], &terminal); + + assert_eq!(map.links().len(), 1); + assert_eq!(map.links()[0].target, web); + assert!(map.link_at(5, 3).is_none()); + assert!(map.link_at(5, 4).is_some()); + } + + #[test] + fn cwd_change_stales_map_before_presentation_ownership_flip() { + let target = LinkTarget::File(Arc::from(std::path::Path::new("/worktree/src/main.rs"))); + let painted = "src/main.rs"; + let mut state = crate::scrollback::ScrollbackState::new(); + let terminal = TerminalContext { + brand: TerminalName::VsCode, + is_ssh: true, + is_official_vscode_remote: true, + ..Default::default() + }; + let overlay_for = |cwd: Option<&std::path::Path>| { + let mut overlay = LinkOverlay::new(); + overlay.push(OverlayLink { + screen_row: 3, + col_start: 4, + col_end: 15, + target: target.clone(), + presentation: crate::render::osc8::file_link_presentation(painted, &target, cwd), + id: None, + }); + overlay + }; + + state.set_cwd(Some(std::path::PathBuf::from("/other"))); + let mut map = VisibleLinkMap::default(); + map.rebuild_for_context( + state.generation(), + &overlay_for(state.cwd()), + vec![], + &terminal, + ); + assert_eq!(map.len(), 1, "opaque relative paint stays Grok-owned"); + assert!(!map.is_stale(state.generation())); + + let new_cwd = std::path::PathBuf::from("/worktree"); + state.set_cwd(Some(new_cwd.clone())); + assert!(map.is_stale(state.generation())); + map.rebuild_for_context( + state.generation(), + &overlay_for(state.cwd()), + vec![], + &terminal, + ); + assert!(map.is_empty(), "self-resolving paint delegates to VS Code"); + + let generation = state.generation(); + state.set_cwd(Some(new_cwd)); + assert_eq!(state.generation(), generation); + assert!(!map.is_stale(state.generation())); + } + #[test] fn link_at_hit_and_miss() { let mut map = VisibleLinkMap::default(); @@ -203,7 +358,13 @@ mod tests { // Hit inside the link let hit = map.link_at(15, 5); assert!(hit.is_some()); - assert_eq!(&*hit.unwrap().url, "https://example.com"); + assert_eq!( + &*resolve_link_target(&hit.unwrap().target) + .unwrap() + .osc8_url + .unwrap(), + "https://example.com" + ); // Miss: wrong row assert!(map.link_at(15, 6).is_none()); // Miss: before start col @@ -240,7 +401,13 @@ mod tests { ]); map.rebuild(2, &overlay2, vec![]); assert_eq!(map.links().len(), 2); - assert_eq!(&*map.links()[0].url, "https://second.com"); + assert_eq!( + &*resolve_link_target(&map.links()[0].target) + .unwrap() + .osc8_url + .unwrap(), + "https://second.com" + ); } #[test] @@ -252,7 +419,13 @@ mod tests { ]); map.rebuild(1, &overlay, vec![]); assert_eq!(map.links().len(), 1); - assert_eq!(&*map.links()[0].url, "https://valid.com"); + assert_eq!( + &*resolve_link_target(&map.links()[0].target) + .unwrap() + .osc8_url + .unwrap(), + "https://valid.com" + ); } #[test] @@ -261,7 +434,7 @@ mod tests { let overlay = make_overlay(vec![(0, 0, 5, "https://md-link.com", Some(1))]); let citations = vec![VisibleLink { rects: vec![Rect::new(2, 10, 30, 1)], - url: Arc::from("https://citation.com"), + target: LinkTarget::Url(Arc::from("https://citation.com")), id: None, }]; map.rebuild(1, &overlay, citations); @@ -270,12 +443,24 @@ mod tests { // Markdown link let hit = map.link_at(3, 0); assert!(hit.is_some()); - assert_eq!(&*hit.unwrap().url, "https://md-link.com"); + assert_eq!( + &*resolve_link_target(&hit.unwrap().target) + .unwrap() + .osc8_url + .unwrap(), + "https://md-link.com" + ); // Citation link let hit = map.link_at(15, 10); assert!(hit.is_some()); - assert_eq!(&*hit.unwrap().url, "https://citation.com"); + assert_eq!( + &*resolve_link_target(&hit.unwrap().target) + .unwrap() + .osc8_url + .unwrap(), + "https://citation.com" + ); } #[test] @@ -291,7 +476,13 @@ mod tests { // Position 5 is in both links; first match wins (iter order) let hit = map.link_at(5, 5); assert!(hit.is_some()); - assert_eq!(&*hit.unwrap().url, "https://first.com"); + assert_eq!( + &*resolve_link_target(&hit.unwrap().target) + .unwrap() + .osc8_url + .unwrap(), + "https://first.com" + ); } #[test] @@ -316,7 +507,13 @@ mod tests { // Should be 1 logical link with 2 rects assert_eq!(map.links().len(), 1); assert_eq!(map.links()[0].rects.len(), 2); - assert_eq!(&*map.links()[0].url, "https://wrapped.com"); + assert_eq!( + &*resolve_link_target(&map.links()[0].target) + .unwrap() + .osc8_url + .unwrap(), + "https://wrapped.com" + ); // Hit on first row segment assert!(map.link_at(15, 3).is_some()); @@ -364,8 +561,20 @@ mod tests { 2, "colliding per-doc ids must not merge across append" ); - assert_eq!(&*map.link_at(5, 0).unwrap().url, "https://scrollback.com"); - assert_eq!(&*map.link_at(5, 5).unwrap().url, "https://btw.com"); + assert_eq!( + &*resolve_link_target(&map.link_at(5, 0).unwrap().target) + .unwrap() + .osc8_url + .unwrap(), + "https://scrollback.com" + ); + assert_eq!( + &*resolve_link_target(&map.link_at(5, 5).unwrap().target) + .unwrap() + .osc8_url + .unwrap(), + "https://btw.com" + ); } #[test] @@ -397,6 +606,12 @@ mod tests { map.append_from_overlay(&make_overlay(vec![(2, 0, 5, "https://new-btw.com", None)])); assert_eq!(map.len(), 2); assert!(map.link_at(1, 1).is_none()); - assert_eq!(&*map.link_at(1, 2).unwrap().url, "https://new-btw.com"); + assert_eq!( + &*resolve_link_target(&map.link_at(1, 2).unwrap().target) + .unwrap() + .osc8_url + .unwrap(), + "https://new-btw.com" + ); } } diff --git a/crates/codegen/xai-grok-pager/src/scrollback/render.rs b/crates/codegen/xai-grok-pager/src/scrollback/render.rs index 47a0b47..2e0eb3f 100644 --- a/crates/codegen/xai-grok-pager/src/scrollback/render.rs +++ b/crates/codegen/xai-grok-pager/src/scrollback/render.rs @@ -629,9 +629,8 @@ pub(crate) fn render_scrolled_entries_with_selection_boundaries( } }); - // Tool-header link_url overlays before plain-text scan (basename/relative - // paint still needs absolute file://). Hit box = selectable path span - // (respects bullet prepend + Selectable shift). + // Basename/relative tool headers need the stored absolute target. + // Hit box = selectable path span (respects bullet prepend + Selectable shift). { for (idx, bl) in cached_output.lines.iter().enumerate().skip(content_skip) { let visible_offset = (idx - content_skip) as u16; @@ -639,7 +638,7 @@ pub(crate) fn render_scrolled_entries_with_selection_boundaries( if screen_row >= max_y { break; } - let Some(url) = bl.link_url.as_ref() else { + let Some(target) = bl.link_target.as_ref() else { continue; }; let Some(cols) = selectable_cols_usize(&bl.content, &bl.selectable) else { @@ -664,11 +663,18 @@ pub(crate) fn render_scrolled_entries_with_selection_boundaries( if result.link_overlay.overlaps(screen_row, col_start, col_end) { continue; } + let painted = derive_selection_text(bl); + let fully_visible = cols.end <= visible_width; result.link_overlay.push(OverlayLink { screen_row, col_start, col_end, - url: Arc::clone(url), + target: target.clone(), + presentation: if fully_visible { + crate::render::osc8::file_link_presentation(&painted, target, cwd) + } else { + crate::render::osc8::LinkPresentation::Opaque + }, id: None, }); } @@ -683,7 +689,7 @@ pub(crate) fn render_scrolled_entries_with_selection_boundaries( .iter() .enumerate() .skip(content_skip) - .filter(|(_, bl)| bl.link_url.is_none()) + .filter(|(_, bl)| bl.link_target.is_none()) .map(|(idx, bl)| { let visible_offset = (idx - content_skip) as u16; let screen_row = first_visible_content_y + visible_offset; @@ -942,18 +948,16 @@ pub(crate) fn map_hyperlinks_to_overlay( // Map each hyperlink to screen-space OverlayLinks. Unsafe schemes // (javascript:, data:, …) are dropped since OSC 8 URLs reach the terminal - // without the link_opener scheme filter. A non-web destination may still be - // a local file the model linked (`[videos/1.mp4](videos/1.mp4)`): resolve it - // to a `file://` URL (relative paths matched against this transcript's - // generated media) so it opens like an absolute path. + // without the link_opener scheme filter. Local-file destinations such as + // `[videos/1.mp4](videos/1.mp4)` resolve against generated media. let scheme_filter = crate::terminal::hyperlinks::SchemeFilter::Standard; for h in hyperlinks { - let url: Arc = if crate::app::link_opener::is_safe_to_open(&h.url, scheme_filter) { - Arc::from(h.url.as_str()) - } else if let Some(file_url) = - crate::render::osc8::local_link_to_file_url(&h.url, media_paths) + let target = if crate::app::link_opener::is_safe_to_open(&h.url, scheme_filter) { + crate::render::osc8::LinkTarget::Url(Arc::from(h.url.as_str())) + } else if let Some(file_target) = + crate::render::osc8::local_link_to_file_target(&h.url, media_paths) { - file_url + file_target } else { continue; }; @@ -987,7 +991,8 @@ pub(crate) fn map_hyperlinks_to_overlay( screen_row, col_start: content_x + local_col_start, col_end: content_x + local_col_end, - url: Arc::clone(&url), + target: target.clone(), + presentation: crate::render::osc8::LinkPresentation::Opaque, id: Some(h.id), }); } @@ -998,6 +1003,9 @@ pub(crate) fn map_hyperlinks_to_overlay( mod tests { use super::*; use crate::appearance::AppearanceConfig; + use crate::render::osc8::{ + LinkPresentation, resolve_link_target, resolve_link_target_for_context, + }; use crate::scrollback::RenderBlock; use crate::scrollback::block::BlockContent; use crate::scrollback::types::DisplayMode; @@ -1071,13 +1079,38 @@ mod tests { scroll_offset: usize, selected_idx: Option, ) -> ScrollRenderResult { + render_with_scratch_and_buffer(entries, viewport, scroll_offset, selected_idx).0 + } + + fn render_with_scratch_and_buffer( + entries: &[ScrollbackEntry], + viewport: Rect, + scroll_offset: usize, + selected_idx: Option, + ) -> (ScrollRenderResult, Buffer) { + render_with_scratch_and_buffer_with_cwd( + entries, + viewport, + scroll_offset, + selected_idx, + None, + ) + } + + fn render_with_scratch_and_buffer_with_cwd( + entries: &[ScrollbackEntry], + viewport: Rect, + scroll_offset: usize, + selected_idx: Option, + cwd: Option<&std::path::Path>, + ) -> (ScrollRenderResult, Buffer) { let theme = Theme::current(); let appearance = AppearanceConfig::default(); let layouts = compute_layouts(entries, viewport.width, &appearance); let refs: Vec<&ScrollbackEntry> = entries.iter().collect(); let mut buf = Buffer::empty(viewport); - render_scrolled_entries_with_scratch( + let result = render_scrolled_entries_with_scratch( &mut buf, viewport, &refs, @@ -1094,8 +1127,9 @@ mod tests { 0, &[], None, - None, - ) + cwd, + ); + (result, buf) } fn render_with_selection_boundaries( @@ -2330,7 +2364,12 @@ mod tests { assert_eq!(link.screen_row, 10); assert_eq!(link.col_start, 4); assert_eq!(link.col_end, 9); - assert_eq!(&*link.url, "https://a.com"); + assert_eq!( + &*resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://a.com" + ); assert_eq!(link.id, Some(1)); } @@ -2374,11 +2413,15 @@ mod tests { None, ); - let links: Vec<&str> = result + let links: Vec> = result .link_overlay .links() .iter() - .map(|l| &*l.url) + .map(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + }) .collect(); assert!( links @@ -2409,7 +2452,14 @@ mod tests { assert_eq!(overlay.links()[1].screen_row, 1); assert_eq!(overlay.links()[1].col_start, 0); assert_eq!(overlay.links()[1].col_end, 5); - assert_eq!(&*overlay.links()[0].url, &*overlay.links()[1].url); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + &*resolve_link_target(&overlay.links()[1].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + ); } #[test] @@ -2425,7 +2475,12 @@ mod tests { map_hyperlinks_to_overlay(&links, &output, 2, 0, 10, 0, 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "https://visible.com"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://visible.com" + ); assert_eq!(overlay.links()[0].screen_row, 0); } @@ -2441,7 +2496,12 @@ mod tests { map_hyperlinks_to_overlay(&links, &output, 0, 0, 2, 0, 0, &[], &mut overlay); assert_eq!(overlay.links().len(), 1); - assert_eq!(&*overlay.links()[0].url, "https://visible.com"); + assert_eq!( + &*resolve_link_target(&overlay.links()[0].target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + "https://visible.com" + ); } #[test] @@ -2521,14 +2581,19 @@ mod tests { !result.link_overlay.is_empty(), "execute block output should have linkified URLs" ); - let urls: Vec<&str> = result + let urls: Vec> = result .link_overlay .links() .iter() - .map(|l| &*l.url) + .map(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + }) .collect(); assert!( - urls.contains(&"https://docs.example.com/api"), + urls.iter() + .any(|url| url.as_ref() == "https://docs.example.com/api"), "expected URL from stdout in overlay, got: {:?}", urls, ); @@ -2554,7 +2619,11 @@ mod tests { .link_overlay .links() .iter() - .filter(|l| &*l.url == expected_url.as_str()) + .filter(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.as_ref() == expected_url.as_str()) + }) .collect(); assert!( path_links.len() >= 2, @@ -2563,7 +2632,14 @@ mod tests { .link_overlay .links() .iter() - .map(|l| (&*l.url, l.screen_row, l.col_start, l.col_end)) + .map(|l| ( + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + l.screen_row, + l.col_start, + l.col_end + )) .collect::>() ); // Regions land on consecutive distinct rows. @@ -2589,7 +2665,11 @@ mod tests { .link_overlay .links() .iter() - .filter(|l| &*l.url == "https://example.com") + .filter(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.as_ref() == "https://example.com") + }) .count(); assert_eq!( url_count, 1, @@ -2632,11 +2712,15 @@ mod tests { let viewport = Rect::new(0, 0, 80, 10); let result = render_with_scratch(&entries, viewport, 0, None); - let urls: Vec<&str> = result + let urls: Vec> = result .link_overlay .links() .iter() - .map(|l| &*l.url) + .map(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + }) .collect(); assert!( urls.iter() @@ -2720,7 +2804,17 @@ mod tests { .link_overlay .links() .iter() - .map(|l| (l.screen_row, l.col_start, l.col_end, l.url.to_string())) + .map(|l| { + ( + l.screen_row, + l.col_start, + l.col_end, + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + .to_string(), + ) + }) .collect(); assert!( links @@ -2811,7 +2905,15 @@ mod tests { .link_overlay .links() .iter() - .map(|l| (l.screen_row, l.url.to_string())) + .map(|l| { + ( + l.screen_row, + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + .to_string(), + ) + }) .collect(); assert!( links @@ -3072,7 +3174,11 @@ mod tests { .link_overlay .links() .iter() - .filter(|l| l.url.contains("a1.rs")) + .filter(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.contains("a1.rs")) + }) .map(|l| l.screen_row) .collect::>() }; @@ -3386,18 +3492,24 @@ mod tests { let viewport = Rect::new(0, 0, 80, 30); let result = render_with_scratch(&entries, viewport, 0, None); - let urls: Vec<&str> = result + let urls: Vec> = result .link_overlay .links() .iter() - .map(|l| &*l.url) + .map(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url") + }) .collect(); assert!( - urls.contains(&"https://head.example.com/first"), + urls.iter() + .any(|url| url.as_ref() == "https://head.example.com/first"), "URL in head section should be detected, got: {urls:?}", ); assert!( - urls.contains(&"https://tail.example.com/last"), + urls.iter() + .any(|url| url.as_ref() == "https://tail.example.com/last"), "URL in tail section should be detected, got: {urls:?}", ); // The ellipsis separator line should not produce spurious links. @@ -3411,7 +3523,7 @@ mod tests { /// Collapsed Edit header: after bullet prepend the path is span 2, and the /// OSC8 overlay must cover path cols only (not the verb or bullet). #[test] - fn tool_header_link_url_overlay_covers_path_after_bullet() { + fn tool_header_link_target_overlay_covers_path_after_bullet() { use crate::appearance::ToolBullet; use crate::scrollback::types::{BlockContext, selectable_cols}; use unicode_width::UnicodeWidthStr; @@ -3443,9 +3555,11 @@ mod tests { ); let path_span = header.content.spans[2].content.as_ref(); assert_eq!(path_span, "foo.rs"); - let url = header.link_url.as_ref().expect("link_url on header"); - assert!(url.starts_with("file://"), "got {url}"); - assert!(url.contains("foo.rs"), "got {url}"); + let target = header.link_target.as_ref().expect("link target on header"); + assert_eq!( + target, + &crate::render::osc8::LinkTarget::File(Arc::from(std::path::Path::new(abs))) + ); let cols = selectable_cols(&header.content, &header.selectable) .expect("path span should be selectable"); @@ -3487,7 +3601,11 @@ mod tests { .link_overlay .links() .iter() - .filter(|l| l.url.contains("foo.rs") && l.url.starts_with("file://")) + .filter(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.contains("foo.rs") && url.starts_with("file://")) + }) .collect(); assert_eq!( file_links.len(), @@ -3508,6 +3626,202 @@ mod tests { ); } + fn official_vscode_remote_context() -> crate::terminal::TerminalContext { + crate::terminal::TerminalContext { + brand: crate::terminal::TerminalName::VsCode, + is_ssh: true, + is_official_vscode_remote: true, + ..Default::default() + } + } + + fn file_link_policy( + link: &OverlayLink, + terminal: &crate::terminal::TerminalContext, + ) -> crate::render::osc8::ResolvedLinkTarget { + resolve_link_target_for_context(&link.target, link.presentation, terminal) + .expect("file target policy") + } + + #[test] + fn official_vscode_remote_delegates_scanned_absolute_path() { + let path = "/worktree/src/main.rs"; + let entry = make_markdown_entry(path); + let viewport = Rect::new(0, 0, 80, 5); + let (result, buf) = + render_with_scratch_and_buffer(std::slice::from_ref(&entry), viewport, 0, None); + let link = result + .link_overlay + .links() + .iter() + .find(|link| matches!(&link.target, crate::render::osc8::LinkTarget::File(_))) + .expect("scanned file target"); + + assert!((0..viewport.height).any(|row| buffer_row_text(&buf, row).contains(path))); + assert_eq!(link.presentation, LinkPresentation::SelfResolvingPath); + assert_eq!( + file_link_policy(link, &official_vscode_remote_context()), + crate::render::osc8::ResolvedLinkTarget { + osc8_url: None, + open_target: None, + } + ); + } + + #[test] + fn official_vscode_remote_tool_headers_delegate_only_self_resolving_paint() { + let cwd = std::path::PathBuf::from("/worktree"); + let target = "/worktree/src/nested/main.rs"; + let terminal = official_vscode_remote_context(); + + for (name, block) in [ + ("Read", RenderBlock::read(target, None)), + ("Edit", RenderBlock::edit(target, None)), + ] { + for (mode, width, expected_paint, expected_presentation) in [ + ( + DisplayMode::Collapsed, + 80, + "main.rs", + LinkPresentation::Opaque, + ), + ( + DisplayMode::Collapsed, + 16, + "\u{2026}", + LinkPresentation::Opaque, + ), + ( + DisplayMode::Expanded, + 80, + "src/nested/main.rs", + LinkPresentation::SelfResolvingPath, + ), + ] { + let mut entry = ScrollbackEntry::new(block.clone()); + entry.display_mode = mode; + let viewport = Rect::new(0, 0, width, 8); + let (result, buf) = render_with_scratch_and_buffer_with_cwd( + std::slice::from_ref(&entry), + viewport, + 0, + None, + Some(&cwd), + ); + let path_links: Vec<_> = result + .link_overlay + .links() + .iter() + .filter(|link| matches!(&link.target, crate::render::osc8::LinkTarget::File(_))) + .collect(); + let painted_rows = (0..viewport.height) + .map(|row| buffer_row_text(&buf, row).trim_end().to_owned()) + .filter(|row| !row.is_empty()) + .collect::>(); + + assert!( + painted_rows.iter().any(|row| row.contains(expected_paint)), + "{name} {mode:?} width={width}: {painted_rows:?}" + ); + assert!(!path_links.is_empty(), "{name} {mode:?} width={width}"); + assert!( + path_links + .iter() + .all(|link| link.presentation == expected_presentation), + "{name} {mode:?} width={width}: {path_links:?}" + ); + let expected_owned = expected_presentation == LinkPresentation::Opaque; + assert!(path_links.iter().all(|link| { + assert_eq!( + link.target, + crate::render::osc8::LinkTarget::File(Arc::from(std::path::Path::new( + target + ))) + ); + let policy = file_link_policy(link, &terminal); + policy.osc8_url.is_some() == expected_owned + && policy.open_target.is_some() == expected_owned + })); + } + + let mut entry = ScrollbackEntry::new(block); + entry.display_mode = DisplayMode::Expanded; + let viewport = Rect::new(0, 0, 16, 8); + let (result, _) = render_with_scratch_and_buffer_with_cwd( + std::slice::from_ref(&entry), + viewport, + 0, + None, + Some(&cwd), + ); + let path_links: Vec<_> = result + .link_overlay + .links() + .iter() + .filter(|link| matches!(&link.target, crate::render::osc8::LinkTarget::File(_))) + .collect(); + assert!(!path_links.is_empty(), "{name} narrow expanded header"); + assert!( + path_links + .iter() + .all(|link| link.presentation == LinkPresentation::Opaque) + ); + assert!(path_links.iter().all(|link| { + let policy = file_link_policy(link, &terminal); + policy.osc8_url.is_some() && policy.open_target.is_some() + })); + } + } + + #[test] + fn basename_headers_stay_grok_owned_for_duplicate_and_outside_targets() { + let cwd = std::path::PathBuf::from("/worktree"); + let terminal = official_vscode_remote_context(); + let cases = [ + ("duplicate-a", "/worktree/src/a/main.rs"), + ("duplicate-b", "/worktree/src/b/main.rs"), + ("outside", "/opt/service/main.rs"), + ]; + + for (name, target) in cases { + for (tool, block) in [ + ("Read", RenderBlock::read(target, None)), + ("Edit", RenderBlock::edit(target, None)), + ] { + let entry = ScrollbackEntry::new(block); + let viewport = Rect::new(0, 0, 80, 5); + let (result, buf) = render_with_scratch_and_buffer_with_cwd( + std::slice::from_ref(&entry), + viewport, + 0, + None, + Some(&cwd), + ); + let link = result + .link_overlay + .links() + .iter() + .find(|link| matches!(&link.target, crate::render::osc8::LinkTarget::File(_))) + .unwrap_or_else(|| panic!("{tool} {name} file target")); + + let painted = (0..viewport.height) + .map(|row| buffer_row_text(&buf, row).trim_end().to_owned()) + .find(|row| row.contains("main.rs")) + .unwrap_or_else(|| panic!("{tool} {name} painted basename")); + assert!(!painted.contains('/'), "{tool} {name}: {painted}"); + assert_eq!( + link.target, + crate::render::osc8::LinkTarget::File(Arc::from(std::path::Path::new(target))), + "{tool} {name} semantic target" + ); + assert_eq!(link.presentation, LinkPresentation::Opaque, "{tool} {name}"); + let policy = file_link_policy(link, &terminal); + assert!(policy.osc8_url.is_some(), "{tool} {name}"); + assert!(policy.open_target.is_some(), "{tool} {name}"); + } + } + } + #[test] fn long_read_header_link_is_clipped_to_offset_content_area() { let path = "/outside/a/very/long/path/that/is/clipped/main.rs"; @@ -3520,7 +3834,11 @@ mod tests { .link_overlay .links() .iter() - .find(|link| link.url.contains("main.rs")) + .find(|link| { + resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.contains("main.rs")) + }) .expect("read header file link"); let content = HorizontalLayout::new(viewport, &AppearanceConfig::default().scrollback.layout).content; @@ -3540,7 +3858,11 @@ mod tests { .link_overlay .links() .iter() - .find(|link| link.url.ends_with(".rs")) + .find(|link| { + resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.ends_with(".rs")) + }) .expect("long Read header file link"); let content = HorizontalLayout::new(viewport, &AppearanceConfig::default().scrollback.layout).content; @@ -3595,11 +3917,11 @@ mod tests { let result = render_with_scratch(std::slice::from_ref(&entry), viewport, 0, None); assert!( - result - .link_overlay - .links() - .iter() - .all(|link| !link.url.contains("/outside/long-file-name.rs")), + result.link_overlay.links().iter().all(|link| { + !resolve_link_target(&link.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.contains("/outside/long-file-name.rs")) + }), "off-row path cells must not be clickable: {:?}", result.link_overlay.links() ); @@ -3628,7 +3950,9 @@ mod tests { let mut by_id: std::collections::BTreeMap> = std::collections::BTreeMap::new(); for l in result.link_overlay.links() { - if &*l.url == url + if resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|target| target.as_ref() == url) && let Some(id) = l.id { by_id.entry(id).or_default().push(l); @@ -3641,7 +3965,15 @@ mod tests { .link_overlay .links() .iter() - .map(|l| (l.screen_row, l.col_start, l.col_end, &*l.url, l.id)) + .map(|l| ( + l.screen_row, + l.col_start, + l.col_end, + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .expect("url"), + l.id + )) .collect::>(), ); } @@ -4023,7 +4355,11 @@ mod tests { .link_overlay .links() .iter() - .filter(|l| &*l.url == url_a || &*l.url == url_b) + .filter(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .is_some_and(|url| url.as_ref() == url_a || url.as_ref() == url_b) + }) .filter_map(|l| l.id) .collect(); assert!( diff --git a/crates/codegen/xai-grok-pager/src/scrollback/state/mod.rs b/crates/codegen/xai-grok-pager/src/scrollback/state/mod.rs index 594ed35..b497a81 100644 --- a/crates/codegen/xai-grok-pager/src/scrollback/state/mod.rs +++ b/crates/codegen/xai-grok-pager/src/scrollback/state/mod.rs @@ -181,8 +181,8 @@ pub struct ScrollbackState { expanded_groups: HashSet, // Link map - /// Monotonically increasing counter, bumped on scroll, viewport, or - /// content changes. Used by `VisibleLinkMap::is_stale()` to skip rebuilds. + /// Monotonically increasing counter, bumped when visible link positions or + /// policy inputs change. Used by `VisibleLinkMap::is_stale()` to skip rebuilds. generation: u64, /// Bumped only when entries are added/removed or an entry's content changes @@ -255,7 +255,7 @@ impl ScrollbackState { self.cwd.as_deref() } - /// Update session cwd; invalidates entry paint caches when it changes. + /// Update session cwd; invalidates cwd-dependent paint, layout, and link maps. pub fn set_cwd(&mut self, cwd: Option) { if self.cwd == cwd { return; @@ -267,6 +267,7 @@ impl ScrollbackState { self.dirty_heights = self.entries.keys().copied().collect(); self.layout_cache = None; self.gaps_may_be_dirty = true; + self.bump_generation(); } /// Create an empty state that continues this one's identity: same @@ -502,8 +503,8 @@ impl ScrollbackState { // Link map generation - /// Current link-map generation. Incremented whenever content, scroll, - /// or viewport changes invalidate the visible link positions. + /// Current link-map generation. Incremented when positions or link-policy + /// inputs change and invalidate the visible link map. pub fn generation(&self) -> u64 { self.generation } diff --git a/crates/codegen/xai-grok-pager/src/scrollback/types.rs b/crates/codegen/xai-grok-pager/src/scrollback/types.rs index eeff948..eb1e62e 100644 --- a/crates/codegen/xai-grok-pager/src/scrollback/types.rs +++ b/crates/codegen/xai-grok-pager/src/scrollback/types.rs @@ -167,8 +167,8 @@ pub struct BlockLine { /// /// The first line of a block should always have `None`. pub joiner: Option, - /// OSC 8 URL when paint text is not a scannable absolute path (tool headers). - pub link_url: Option>, + /// Semantic link target when paint text cannot recover it (tool headers). + pub link_target: Option, } impl Default for BlockLine { @@ -183,7 +183,7 @@ impl Default for BlockLine { selection_range: None, selection_text: None, joiner: None, - link_url: None, + link_target: None, } } } @@ -273,11 +273,6 @@ impl BlockLine { self.joiner = joiner; self } - - pub fn with_link_url(mut self, url: Option>) -> Self { - self.link_url = url; - self - } } /// Flatten a rendered line's spans into the plain text drawn on that row. @@ -610,7 +605,7 @@ mod tests { selection_range: None, selection_text: None, joiner: None, - link_url: None, + link_target: None, }; } diff --git a/crates/codegen/xai-grok-pager/src/slash/registry.rs b/crates/codegen/xai-grok-pager/src/slash/registry.rs index 3829b02..ac1c61b 100644 --- a/crates/codegen/xai-grok-pager/src/slash/registry.rs +++ b/crates/codegen/xai-grok-pager/src/slash/registry.rs @@ -10,9 +10,23 @@ use std::collections::{HashMap, HashSet}; use std::sync::Arc; +use xai_grok_tools::implementations::skills::types::SkillScope; + use super::acp_command::AcpSlashCommand; use super::command::SlashCommand; +fn client_collision_qualified_name( + cmd: &agent_client_protocol::AvailableCommand, +) -> Option { + let meta = cmd.meta.as_ref()?; + meta.get("path").and_then(|v| v.as_str())?; + let scope: SkillScope = serde_json::from_value(meta.get("scope")?.clone()).ok()?; + if scope == SkillScope::Plugin { + return None; + } + Some(format!("{}:{}", scope.as_ref(), cmd.name)) +} + /// Source of a command in the registry. Used for precedence and replacement. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CommandSource { @@ -487,16 +501,20 @@ impl CommandRegistry { "reload-plugins", ]; - // Add new ACP commands, skipping collisions with builtins and blocked names. for acp_cmd in commands { let name_lower = acp_cmd.name.to_lowercase(); - if builtin_keys.contains(&name_lower) { - continue; - } - if BLOCKED_NAMES - .iter() - .any(|b| b.eq_ignore_ascii_case(&name_lower)) - { + let name_reserved = builtin_keys.contains(&name_lower) + || BLOCKED_NAMES + .iter() + .any(|b| b.eq_ignore_ascii_case(&name_lower)); + if name_reserved { + if let Some(qualified) = client_collision_qualified_name(acp_cmd) { + let mut renamed = acp_cmd.clone(); + renamed.name = qualified; + self.commands + .push(Arc::new(AcpSlashCommand::from(&renamed))); + self.sources.push(CommandSource::Acp); + } continue; } self.commands.push(Arc::new(AcpSlashCommand::from(acp_cmd))); @@ -945,6 +963,108 @@ mod tests { assert_eq!(registry.command_count(), 1); } + fn acp_skill(name: &str, scope: &str) -> agent_client_protocol::AvailableCommand { + let meta = serde_json::json!({ "scope": scope, "path": "/x/SKILL.md" }) + .as_object() + .cloned() + .unwrap(); + agent_client_protocol::AvailableCommand::new(name.to_string(), format!("{name} skill")) + .meta(meta) + } + + #[test] + fn acp_nonplugin_skill_colliding_with_builtin_is_requalified() { + let builtin: Arc = Arc::new(DummyCommand { + name: "login", + aliases: &[], + }); + let mut registry = CommandRegistry::new(vec![builtin]); + registry.set_acp_commands(&[acp_skill("login", "local")]); + + assert!(registry.get("login").is_some()); + assert!(registry.is_builtin("login")); + assert!(registry.get("local:login").is_some()); + assert!(!registry.is_builtin("local:login")); + assert_eq!(registry.command_count(), 2, "builtin + re-homed skill"); + assert!( + registry + .triggers() + .iter() + .any(|t| t.canonical == "local:login"), + "re-homed skill should have a dropdown trigger" + ); + } + + #[test] + fn acp_malformed_skill_meta_colliding_with_builtin_is_dropped() { + let builtin: Arc = Arc::new(DummyCommand { + name: "login", + aliases: &[], + }); + let mut registry = CommandRegistry::new(vec![builtin]); + let meta = serde_json::json!({ "scope": "local" }) + .as_object() + .cloned() + .unwrap(); + let cmd = agent_client_protocol::AvailableCommand::new( + "login".to_string(), + "malformed".to_string(), + ) + .meta(meta); + registry.set_acp_commands(&[cmd]); + assert_eq!( + registry.command_count(), + 1, + "malformed-meta collision drops" + ); + assert!(registry.get("local:login").is_none()); + } + + #[test] + fn acp_skill_named_after_blocked_name_is_requalified() { + let builtin: Arc = Arc::new(DummyCommand { + name: "exit", + aliases: &[], + }); + let mut registry = CommandRegistry::new(vec![builtin]); + registry.set_acp_commands(&[acp_skill("hooks-add", "local")]); + assert!(registry.get("local:hooks-add").is_some()); + assert!(registry.get("hooks-add").is_none()); + } + + #[test] + fn acp_plugin_skill_colliding_with_builtin_is_dropped_not_requalified() { + let builtin: Arc = Arc::new(DummyCommand { + name: "login", + aliases: &[], + }); + let mut registry = CommandRegistry::new(vec![builtin]); + registry.set_acp_commands(&[acp_skill("login", "plugin")]); + + assert!(registry.get("login").is_some()); + assert!(registry.is_builtin("login")); + assert!( + registry.get("plugin:login").is_none(), + "pager must not fabricate a plugin-qualified name" + ); + assert_eq!(registry.command_count(), 1, "only the builtin remains"); + } + + #[test] + fn acp_nonskill_colliding_with_builtin_is_dropped() { + let builtin: Arc = Arc::new(DummyCommand { + name: "login", + aliases: &[], + }); + let mut registry = CommandRegistry::new(vec![builtin]); + registry.set_acp_commands(&[agent_client_protocol::AvailableCommand::new( + "login".to_string(), + "shell login".to_string(), + )]); + assert_eq!(registry.command_count(), 1); + assert!(registry.is_builtin("login")); + } + #[test] fn command_without_required_tools_is_always_visible() { let plain: Arc = Arc::new(DummyCommand { diff --git a/crates/codegen/xai-grok-pager/src/tracing.rs b/crates/codegen/xai-grok-pager/src/tracing.rs index 626d26d..ff4cf86 100644 --- a/crates/codegen/xai-grok-pager/src/tracing.rs +++ b/crates/codegen/xai-grok-pager/src/tracing.rs @@ -412,7 +412,7 @@ pub fn init_tracing() -> TracingHandle { }; use xai_grok_telemetry::debug_log::RMCP_SSE_NOISE_TARGET; let (make_writer, rx) = TracingChannelMakeWriter::new(); - let payload_level = if false { "debug" } else { "off" }; + let payload_level = "off"; let directives = format!( "xai_grok_shell=info,xai_grok_pager=trace,xai_grok_tools=info,xai_acp_lib=info,{RMCP_SSE_NOISE_TARGET}=error,sampling_log=off,{ACP_UPDATE_TARGET}=debug,{ACP_UPDATE_PAYLOAD_TARGET}={payload_level}" ); diff --git a/crates/codegen/xai-grok-pager/src/views/btw_overlay.rs b/crates/codegen/xai-grok-pager/src/views/btw_overlay.rs index 8ff1ffe..4a41e76 100644 --- a/crates/codegen/xai-grok-pager/src/views/btw_overlay.rs +++ b/crates/codegen/xai-grok-pager/src/views/btw_overlay.rs @@ -463,6 +463,7 @@ pub fn render_btw_panel( #[cfg(test)] mod tests { use super::*; + use crate::render::osc8::resolve_link_target; fn render_with_model( state: &BtwOverlayState, @@ -693,14 +694,22 @@ mod tests { !overlay.is_empty(), "expected at least one overlay link for markdown href" ); - let found = overlay.links().iter().any(|l| l.url.as_ref() == url); + let found = overlay.links().iter().any(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .as_deref() + .unwrap_or("") + == url + }); assert!( found, "overlay should contain {url}, got: {:?}", overlay .links() .iter() - .map(|l| l.url.as_ref()) + .filter_map( + |l| resolve_link_target(&l.target).and_then(|resolved| resolved.osc8_url) + ) .collect::>() ); // Links live in the body (row >= 1), not the title border. @@ -720,12 +729,21 @@ mod tests { let state = BtwOverlayState::done("q".to_string(), format!("Visit {url} please.")); let (_model, overlay) = render_with_links(&state, 60, 8); assert!( - overlay.links().iter().any(|l| l.url.as_ref() == url), + overlay + .links() + .iter() + .any(|l| resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .as_deref() + .unwrap_or("") + == url), "plain URL should become an overlay link, got: {:?}", overlay .links() .iter() - .map(|l| l.url.as_ref()) + .filter_map( + |l| resolve_link_target(&l.target).and_then(|resolved| resolved.osc8_url) + ) .collect::>() ); } @@ -737,7 +755,11 @@ mod tests { let path = "/Users/test/project/src/main.rs"; let state = BtwOverlayState::done("q".to_string(), format!("See {path} for details.")); let (_model, overlay) = render_with_links(&state, 80, 8); - let urls: Vec<&str> = overlay.links().iter().map(|l| l.url.as_ref()).collect(); + let urls: Vec<_> = overlay + .links() + .iter() + .filter_map(|l| resolve_link_target(&l.target).and_then(|resolved| resolved.osc8_url)) + .collect(); assert!( urls.iter() .any(|u| u.contains("main.rs") && u.starts_with("file://")), @@ -767,7 +789,13 @@ mod tests { let link = overlay .links() .iter() - .find(|l| l.url.as_ref() == url) + .find(|l| { + resolve_link_target(&l.target) + .and_then(|resolved| resolved.osc8_url) + .as_deref() + .unwrap_or("") + == url + }) .expect("scrolled link should still map when visible"); // Body starts at row 1; clamped offset 17 + 4 visible rows → link at // visible index 3 → screen_row = 1 + 3 = 4. diff --git a/crates/codegen/xai-grok-pager/src/views/settings_modal.rs b/crates/codegen/xai-grok-pager/src/views/settings_modal.rs deleted file mode 100644 index a889231..0000000 --- a/crates/codegen/xai-grok-pager/src/views/settings_modal.rs +++ /dev/null @@ -1,12619 +0,0 @@ -//! Settings modal — opens via F2, `/settings`, command palette, and -//! shortcuts-help. -//! -//! ## State machine -//! -//! `SettingsModalState` carries a `UiConfig` snapshot plus a mode machine: -//! -//! - `Browse` — j/k navigates rows; Space toggles Bool; Enter opens -//! a chooser/editor for Enum/String/Int. -//! - `FilterFocused` — `/` enters filter mode; `invalidate_filter` -//! recomputes `filtered_cache` on every mutation. -//! - `PickingEnum { ... }` — enum chooser sub-pane. -//! - `EditingValue { ... }` — inline string/int editor. -//! -//! ## Keyboard ↔ mouse parity -//! -//! Every keyboard interaction has a mouse equivalent via `handle_mouse`. -//! -//! ## Close-key interception -//! -//! F2/Ctrl+,/Cmd+, are intercepted before mode-specific routing. -//! Esc-in-Browse is handled by the `ModalWindow` chrome (so -//! `is_close_key` does NOT match Esc); Esc-in-FilterFocused exits -//! filter mode without closing. - -use std::sync::Arc; - -use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers, MouseEventKind}; -use ratatui::buffer::Buffer; -use ratatui::layout::Rect; -use ratatui::style::{Color, Modifier, Style}; -use ratatui::text::{Line, Span}; -use unicode_width::UnicodeWidthStr; - -use crate::app::actions::Action; -use crate::render::line_utils::truncate_str; -use crate::settings::{ - EnumChoice, OwnedEnumChoice, PagerLocalSnapshot, SettingCategory, SettingKey, SettingKind, - SettingMeta, SettingValue, SettingsRegistry, StringValidator, current_value_for, - dynamic_enum_choices, -}; -use crate::theme::Theme; -use crate::views::modal_window::{ - self, ModalContentArea, ModalSizing, ModalWindowConfig, ModalWindowState, Shortcut, -}; - -use xai_grok_shell::agent::config::UiConfig; - -// --------------------------------------------------------------------------- -// Public constants -// --------------------------------------------------------------------------- - -/// Public display title of the modal — also used by -/// `views/modal.rs::ActiveModal::message` so renames stay in one place. -pub const MODAL_TITLE: &str = "Settings"; - -/// Width of the `"─ "` leading decoration before the title in the -/// modal's top border. Used to compute the breadcrumb hit-rect x offset. -const TITLE_LEADING_DECORATION_W: u16 = 2; // `─ `: 1 cell box-drawing + 1 cell space. - -// Descriptions are now expand-on-demand via Right/Left arrows; -// see `render_expanded_description`. - -/// Below this width the row list is skipped (chrome renders empty). -const CONTENT_MIN_WIDTH: u16 = 10; - -/// Default max width for the modal. Keeps the row list compact on wide terminals. -const STANDARD_MAX_WIDTH: u16 = 110; - -/// Per-side margin when editing `max_thoughts_width` (modal widens -/// to `terminal_width - 2*margin` so the wrap preview is useful). -const MAX_THOUGHTS_WIDTH_WIDENED_MARGIN: u16 = 8; - -/// Outcome of a key or mouse event. Separate from `InputOutcome` -/// because the modal doesn't own `agent.active_modal` — close is -/// the caller's responsibility. -#[derive(Debug)] -#[allow(clippy::large_enum_variant)] -pub enum SettingsKeyOutcome { - /// Close the modal. - Close, - /// Forward to dispatch. - Action(Action), - /// Forward two actions in order (first must resolve before second). - /// Used by `d`-reset-in-picker to revert preview before opening - /// the reset-confirm overlay. - ActionPair(Action, Action), - /// Internal state mutation, no action. - Changed, - /// No-op. - Unchanged, -} - -// --------------------------------------------------------------------------- -// Types -// --------------------------------------------------------------------------- - -/// One row in the visible flat list — either a category header (non- -/// selectable) or a setting row (selectable, dispatchable). -#[derive(Debug, Clone)] -pub enum RowEntry { - Header { category: SettingCategory }, - Setting { key: SettingKey, meta_index: usize }, -} - -/// Mode state for the modal. -#[derive(Debug, Clone)] -pub enum SettingsModalMode { - Browse, - /// `/` was pressed; chars filter the visible rows. - FilterFocused, - /// Enum chooser sub-pane. `supports_preview` is cached at open - /// time to avoid per-keystroke registry lookups. - PickingEnum { - key: SettingKey, - choices_idx: usize, - original_value: SettingValue, - supports_preview: bool, - }, - /// Group sub-sheet: a list of the group's child Bool toggles. `child_idx` - /// is the focused child within the group. Space/Enter toggles in place - /// (the sheet stays open); Esc returns to Browse. Mirrors `PickingEnum`'s - /// open/render/commit flow but for independent toggles. - PickingGroup { - key: SettingKey, - child_idx: usize, - }, - /// Inline string/int editor. `cursor_byte` is always on a char - /// boundary. `validation_error` shows live feedback; commit - /// re-validates before dispatching. No `original_value` — these - /// settings have no live preview, so Esc is a pure cancel. - EditingValue { - key: SettingKey, - buffer: String, - cursor_byte: usize, - validation_error: Option, - }, -} - -/// Settings modal state. Boxed inside `ActiveModal::Settings` to -/// avoid clippy `large_enum_variant`. -pub struct SettingsModalState { - pub window: ModalWindowState, - pub registry: Arc, - /// `UiConfig` snapshot, refreshed by the dispatcher on mutations. - pub ui_snapshot: UiConfig, - pub pager_snapshot: PagerLocalSnapshot, - /// Computed row layout (headers + settings, in render order). - pub rows: Vec, - /// Index into `rows` of the focused row. - pub selected: usize, - /// Vertical scroll offset (line-granular). - pub scroll_offset: usize, - pub mode: SettingsModalMode, - /// Filter query. Persists across FilterFocused→Browse on Enter; cleared by Esc. - pub query: String, - /// Byte offset of the editing cursor within `query`. - pub query_cursor: usize, - /// Row indices matching `query`, recomputed per mutation (not per frame). - filtered_cache: Vec, - - // -- Mouse hit-test rects (populated by render) -- - pub list_area: Rect, - /// Click-hit rect per row, parallel to `rows`. - pub row_rects: Vec, - /// Click-hit rect for the value column on each row. Bool rows - /// toggle on click; Enum/String/Int rows open the sub-pane. - pub value_hit_rects: Vec, - /// `(decrement_rect, increment_rect)` for the Int stepper's - /// `‹`/`›` glyphs. Zero-sized when not in Int editing mode. - pub editor_adornment_rects: (Rect, Rect), - /// Click-hit rect per choice in `PickingEnum`. Each rect spans the - /// full height of a choice (including wrapped description lines). - pub picker_choice_rects: Vec, - /// Hit-rect for the breadcrumb title in sub-pane modes - /// (`PickingEnum`/`EditingValue`). Clicking anywhere on - /// `Settings ›