//! End-to-end integration test for the workspace manager TUI.
//! Drives `manager::handle_key` with a scripted key stream — no live
//! terminal.

use anyhow::Result;
use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers};
use jackin::{
    config::{AppConfig, ConfigEditor},
    console::{
        ConsoleStage, ConsoleState,
        manager::{
            ManagerStage, ManagerState,
            auth_kind::AuthKind,
            handle_key,
            render::editor::{AuthRow, auth_flat_rows},
            state::{EditorState, EditorTab, FieldFocus, Modal, TextInputTarget},
        },
    },
    paths::JackinPaths,
    workspace::{MountConfig, WorkspaceConfig, WorkspaceRoleOverride},
};
use ratatui::Terminal;
use ratatui::backend::TestBackend;
use tempfile::tempdir;

const fn key(code: KeyCode) -> KeyEvent {
    KeyEvent {
        code,
        modifiers: KeyModifiers::NONE,
        kind: KeyEventKind::Press,
        state: KeyEventState::NONE,
    }
}

fn seed_config(paths: &JackinPaths, temp_dir: &std::path::Path) -> Result<AppConfig> {
    seed_config_with_env(paths, temp_dir, vec![])
}

/// Seed a workspace with pre-populated `env` keys. Used by several Secrets
/// integration tests that need existing env rows to navigate over. Pass
/// an empty `Vec` (or call `seed_config`) for the no-env case.
///
/// # Drop requirement
/// The caller must hold the `TempDir` that owns `temp_dir` alive for the
/// entire duration of the test. Dropping `TempDir` before the test assertions
/// removes the on-disk workspace paths that `JackinPaths` references.
fn seed_config_with_env(
    paths: &JackinPaths,
    temp_dir: &std::path::Path,
    env: Vec<(&str, &str)>,
) -> Result<AppConfig> {
    paths.ensure_base_dirs()?;
    // Use a host path that actually exists (the tempdir) so
    // WorkspaceConfig's workdir-must-equal-or-be-covered-by-mount-dst
    // validation passes.
    let host_path = temp_dir.display().to_string();
    let env_map: std::collections::BTreeMap<String, jackin::operator_env::EnvValue> = env
        .into_iter()
        .map(|(k, v)| {
            (
                k.to_string(),
                jackin::operator_env::EnvValue::Plain(v.to_string()),
            )
        })
        .collect();
    let ws = WorkspaceConfig {
        workdir: host_path.clone(),
        mounts: vec![MountConfig {
            src: host_path.clone(),
            dst: host_path,
            readonly: false,
            isolation: jackin::isolation::MountIsolation::Shared,
        }],
        env: env_map,
        ..Default::default()
    };
    let mut ce = ConfigEditor::open(paths)?;
    ce.create_workspace("big-monorepo", ws)?;
    ce.save()
}

/// Helper to build a `ManagerState` already sitting on the Secrets tab
/// for the "big-monorepo" workspace. Shortcut around navigating the list
/// → editor → tab-cycle key sequence in every test.
fn manager_on_secrets_tab<'a>(config: &AppConfig, cwd: &std::path::Path) -> ManagerState<'a> {
    let mut state = ManagerState::from_config(config, cwd);
    let ws = config
        .workspaces
        .get("big-monorepo")
        .expect("seed must create big-monorepo")
        .clone();
    let mut editor = EditorState::new_edit("big-monorepo".into(), ws);
    editor.active_tab = EditorTab::Secrets;
    editor.tab_bar_focused = false;
    editor.active_field = FieldFocus::Row(0);
    state.stage = ManagerStage::Editor(editor);
    state
}

/// Borrow the active `EditorState` from a `ManagerState` — panics if the
/// stage isn't Editor. Every Secrets integration test sets `stage =
/// Editor(...)` before asserting, so this is safe in-test.
fn editor_mut<'s, 'a>(state: &'s mut ManagerState<'a>) -> &'s mut EditorState<'a> {
    let ManagerStage::Editor(editor) = &mut state.stage else {
        panic!("expected Editor stage");
    };
    editor
}

fn editor<'s, 'a>(state: &'s ManagerState<'a>) -> &'s EditorState<'a> {
    let ManagerStage::Editor(editor) = &state.stage else {
        panic!("expected Editor stage");
    };
    editor
}

/// Render the manager state to a 100x30 `TestBackend` and return the raw
/// buffer as newline-delimited rows. Used to assert that a given glyph
/// appears on screen.
fn render_to_dump(
    state: &mut ManagerState<'_>,
    config: &AppConfig,
    cwd: &std::path::Path,
) -> String {
    let backend = TestBackend::new(100, 30);
    let mut term = Terminal::new(backend).unwrap();
    term.draw(|f| {
        jackin::console::manager::render(f, state, config, cwd);
    })
    .unwrap();
    let buf = term.backend().buffer();
    let mut out = String::new();
    for y in 0..buf.area.height {
        for x in 0..buf.area.width {
            out.push_str(buf[(x, y)].symbol());
        }
        out.push('\n');
    }
    out
}

#[test]
fn delete_workspace_via_manager() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;

    assert!(
        config.workspaces.contains_key("big-monorepo"),
        "seed failed"
    );

    let cwd = temp.path();
    let mut state = ManagerState::from_config(&config, cwd);
    assert_eq!(state.workspaces.len(), 1);
    // Preselect lands on the saved workspace (selected=1) because the
    // cwd matches — press `d` to delete.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('d')),
    )?;
    assert!(
        matches!(state.stage, ManagerStage::ConfirmDelete { .. }),
        "expected ConfirmDelete stage after 'd', got {:?}",
        state.stage
    );

    // Press 'y' — commits the delete.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('y')),
    )?;

    // Config on disk should no longer have big-monorepo.
    let reloaded = AppConfig::load_or_init(&paths)?;
    assert!(
        !reloaded.workspaces.contains_key("big-monorepo"),
        "workspace should be deleted from disk"
    );

    // In-memory state: returned to List, empty workspace list.
    assert!(matches!(state.stage, ManagerStage::List));
    assert!(
        state.workspaces.is_empty(),
        "in-memory list should be empty"
    );

    Ok(())
}

// ── Secrets tab integration tests ─────────────────────────────────

#[test]
fn secrets_tab_shows_existing_env() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let config = seed_config_with_env(
        &paths,
        temp.path(),
        vec![("DB_URL", "postgres://localhost")],
    )?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Unmask so the literal value reaches the dump.
    editor_mut(&mut state).unmasked_rows.insert((
        jackin::console::manager::state::SecretsScopeTag::Workspace,
        "DB_URL".into(),
    ));

    let dump = render_to_dump(&mut state, &config, cwd);
    assert!(
        dump.contains("DB_URL"),
        "existing env key must be visible on Secrets tab; got:\n{dump}"
    );
    assert!(
        dump.contains("postgres://localhost"),
        "unmasked value must be visible; got:\n{dump}"
    );
    Ok(())
}

/// Edit an existing env value via the `TextInput` modal, save, and verify
/// the on-disk TOML reflects the new value. The cursor opens directly on
/// the key row (the Secrets tab no longer renders a preamble label —
/// row 0 is the first navigable row).
#[test]
fn secrets_edit_value_saves_to_disk() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("DB_URL", "old-value")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Secrets flat rows on this fixture (no focusable header):
    //   0 WorkspaceKeyRow("DB_URL")
    //   1 WorkspaceAddSentinel
    // Cursor is already on row 0 — no nav needed.
    assert!(
        matches!(editor(&state).active_field, FieldFocus::Row(0)),
        "cursor must open on the first key row"
    );

    // Enter opens the EnvValue modal pre-filled with the old value.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Clear the pre-filled "old-value" (9 chars) and type "new-value".
    for _ in 0..9 {
        handle_key(
            &mut state,
            &mut config,
            &paths,
            cwd,
            key(KeyCode::Backspace),
        )?;
    }
    for ch in "new-value".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    // Commit the TextInput.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("DB_URL")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("new-value"),
        "pending.env must reflect the edit"
    );

    // Kick off the save: `S` opens ConfirmSave, Enter commits.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('S')),
    )?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    let reloaded = AppConfig::load_or_init(&paths)?;
    let ws = reloaded
        .workspaces
        .get("big-monorepo")
        .expect("workspace must still exist");
    assert_eq!(
        ws.env
            .get("DB_URL")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("new-value"),
        "on-disk env must reflect the edit"
    );
    Ok(())
}

/// Delete an existing env key via `D` → `Y`, save, and verify the key is
/// absent on disk.
#[test]
fn secrets_delete_key_saves_to_disk() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("DB_URL", "doomed")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Cursor opens on row 0 (the key row — no focusable header above it).
    // `D` opens the DeleteEnvVar Confirm modal.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('D')),
    )?;
    // `Y` commits the delete.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('y')),
    )?;

    assert!(
        !editor(&state).pending.env.contains_key("DB_URL"),
        "pending.env must no longer contain the deleted key"
    );

    // `S` → Enter writes through to disk.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('S')),
    )?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    let reloaded = AppConfig::load_or_init(&paths)?;
    let ws = reloaded
        .workspaces
        .get("big-monorepo")
        .expect("workspace must still exist");
    assert!(
        !ws.env.contains_key("DB_URL"),
        "on-disk env must no longer contain the deleted key"
    );
    Ok(())
}

/// `M` toggles per-row mask state. The cursor opens on the first key
/// row (`DB_URL`) — pressing M adds it to `unmasked_rows`; pressing M
/// again removes it.
#[test]
fn secrets_masking_m_toggle() -> Result<()> {
    use jackin::console::manager::state::SecretsScopeTag;
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("DB_URL", "v")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    assert!(
        editor(&state).unmasked_rows.is_empty(),
        "default must have unmasked_rows empty"
    );
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('m')),
    )?;
    assert!(
        editor(&state)
            .unmasked_rows
            .contains(&(SecretsScopeTag::Workspace, "DB_URL".into())),
        "M must insert focused row into unmasked_rows"
    );
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('m')),
    )?;
    assert!(
        editor(&state).unmasked_rows.is_empty(),
        "second M must remove the focused row from unmasked_rows"
    );
    Ok(())
}

/// Role-override section starts collapsed; `→` on the header expands it
/// and the role's env key becomes visible; `←` collapses it back and
/// the key disappears from the buffer.
#[test]
fn secrets_agent_section_expand_collapse() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    paths.ensure_base_dirs()?;
    let host_path = temp.path().display().to_string();

    // Seed a workspace with one role override. Using `ConfigEditor`
    // keeps the test aligned with the real save path.
    let mut role_env = std::collections::BTreeMap::new();
    role_env.insert(
        "LOG_LEVEL".into(),
        jackin::operator_env::EnvValue::Plain("debug".into()),
    );
    let mut roles = std::collections::BTreeMap::new();
    roles.insert(
        "agent-smith".into(),
        WorkspaceRoleOverride {
            env: role_env,
            claude: None,
            codex: None,
            amp: None,
            kimi: None,
            opencode: None,
            github: None,
        },
    );
    let ws = WorkspaceConfig {
        workdir: host_path.clone(),
        mounts: vec![MountConfig {
            src: host_path.clone(),
            dst: host_path,
            readonly: false,
            isolation: jackin::isolation::MountIsolation::Shared,
        }],
        roles,
        ..Default::default()
    };
    let mut ce = ConfigEditor::open(&paths)?;
    ce.create_workspace("big-monorepo", ws)?;
    let mut config = ce.save()?;

    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    // Unmask so the key value (`debug`) would show up in the dump if
    // the section were expanded.
    editor_mut(&mut state).unmasked_rows.insert((
        jackin::console::manager::state::SecretsScopeTag::Role("agent-smith".into()),
        "LOG_LEVEL".into(),
    ));

    // Secrets flat rows on this fixture (no workspace-level keys, one
    // collapsed role section; no preamble rows are rendered):
    //   0 WorkspaceAddSentinel
    //   1 SectionSpacer
    //   2 AgentHeader { role: "agent-smith", expanded: false }
    // Pressing `↓` once skips the SectionSpacer at row 1 and lands on
    // the role header at row 2.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    assert!(
        matches!(editor(&state).active_field, FieldFocus::Row(2)),
        "cursor must skip SectionSpacer and land on the role header row; \
         got {:?}",
        editor(&state).active_field
    );

    // Before expansion: LOG_LEVEL must not appear in the render.
    let dump_collapsed = render_to_dump(&mut state, &config, cwd);
    assert!(
        !dump_collapsed.contains("LOG_LEVEL"),
        "collapsed section must not render role keys; got:\n{dump_collapsed}"
    );

    // `→` on a collapsed role header expands the section, symmetric with
    // `←` collapsing an expanded header (verified below). Enter on the
    // header also expands; Right is exercised here because it's the binding
    // most likely to be eaten by the tab-advance handler if the guard ever
    // regresses.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    assert!(
        editor(&state).secrets_expanded.contains("agent-smith"),
        "→ on collapsed header must expand the agent-smith section"
    );
    let dump_expanded = render_to_dump(&mut state, &config, cwd);
    assert!(
        dump_expanded.contains("LOG_LEVEL"),
        "expanded section must render role keys; got:\n{dump_expanded}"
    );

    // Cursor is still on the AgentHeader row (expanded). `←` collapses.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Left))?;
    assert!(
        !editor(&state).secrets_expanded.contains("agent-smith"),
        "← on expanded header must collapse the section"
    );
    let dump_recollapsed = render_to_dump(&mut state, &config, cwd);
    assert!(
        !dump_recollapsed.contains("LOG_LEVEL"),
        "recollapsed section must hide role keys; got:\n{dump_recollapsed}"
    );
    Ok(())
}

/// Adding a workspace-level env key via direct mutation flips
/// `is_dirty()` and bumps `change_count()` by 1. Exercises the
/// diff-based state layer that `change_count` relies on (covered here
/// end-to-end alongside the editor plumbing).
#[test]
fn secrets_dirty_detection_and_change_count() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    assert!(!editor(&state).is_dirty());
    assert_eq!(editor(&state).change_count(), 0);

    editor_mut(&mut state).pending.env.insert(
        "NEW_KEY".into(),
        jackin::operator_env::EnvValue::Plain("v".into()),
    );

    assert!(editor(&state).is_dirty(), "env add must flip is_dirty");
    assert!(
        editor(&state).change_count() >= 1,
        "env add must bump change_count"
    );
    Ok(())
}

/// Three-step Add flow: `A` opens the `EnvKey` modal, typing + Enter
/// stashes the key and opens the `SourcePicker`; Enter on the (default)
/// Plain choice opens the `EnvValue` modal; typing + Enter commits the
/// value into `pending.env`.
#[test]
fn secrets_add_new_key_flow() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // `A` opens the EnvKey modal. The default cursor is on row 0
    // (the WorkspaceAddSentinel — a Workspace-scope row), exactly the
    // scope we want for this assertion.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('A')),
    )?;

    // Type "API_KEY" into the EnvKey TextInput.
    for ch in "API_KEY".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    // Commit — stashes the key and opens the SourcePicker.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::SourcePicker { .. })),
        "EnvKey commit must open SourcePicker; got {:?}",
        editor(&state).modal
    );
    // Default focus is Plain — Enter commits Plain and opens EnvValue.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Type "s3cret" into the EnvValue TextInput and commit.
    for ch in "s3cret".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("API_KEY")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("s3cret"),
        "pending.env must contain the new key after the three-step add"
    );
    Ok(())
}

// ── 1Password picker integration tests ────────────────────────────

/// Picker may load or fall into a fatal state depending on `op` on
/// `$PATH`; either way the modal variant must be `OpPicker` and
/// `pending_picker_target` must record (scope, key).
#[test]
fn op_picker_opens_on_p_from_secrets_key_row() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("DB_URL", "postgres")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    state.op_available = true;

    // Cursor opens directly on row 0 (the WorkspaceKeyRow for DB_URL —
    // no focusable header above it).
    assert!(matches!(editor(&state).active_field, FieldFocus::Row(0)));

    // Press P directly on the key row — no Enter into the text modal first.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('p')),
    )?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::OpPicker { .. })),
        "P on a key row must open the OpPicker modal directly"
    );
    // `pending_picker_target` records the focused key so the commit
    // handler can write straight into pending.env.
    match &editor(&state).pending_picker_target {
        Some((_, Some(key))) => {
            assert_eq!(key, "DB_URL", "key-row P must stash the focused key");
        }
        other => panic!(
            "expected pending_picker_target = Some((scope, Some(\"DB_URL\"))), got {other:?}"
        ),
    }
    Ok(())
}

/// Esc on the `OpPicker` (vault pane / fatal state / loading) closes
/// the modal entirely — the picker is no longer a sub-mode of the
/// `EnvValue` text modal, so cancel returns the operator to the editor
/// list view with `pending.env` unchanged.
#[test]
fn op_picker_cancel_closes_modal() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("DB_URL", "untouched")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    state.op_available = true;

    // Cursor opens on the key row (row 0, no focusable header above);
    // press P to open the picker.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('p')),
    )?;
    assert!(matches!(editor(&state).modal, Some(Modal::OpPicker { .. })));

    // Esc on the OpPicker closes the modal and clears the picker target.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        editor(&state).modal.is_none(),
        "Esc-cancel must close the picker entirely; got {:?}",
        editor(&state).modal
    );
    assert!(
        editor(&state).pending_picker_target.is_none(),
        "Esc-cancel must clear pending_picker_target"
    );
    assert!(
        editor(&state).pending_picker_value.is_none(),
        "Esc-cancel must clear pending_picker_value"
    );
    // Cancel is a pure UI action — the on-pending env value is unchanged.
    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("DB_URL")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("untouched"),
        "Esc-cancel must not mutate pending.env"
    );
    Ok(())
}

/// `P` on an existing key row, picker drives to a Field commit — the
/// `op://Vault/Item/Field` reference must land directly in
/// `pending.env[key]` and the modal must close. No follow-up text modal.
#[test]
fn op_picker_commit_writes_value_directly_to_pending() -> Result<()> {
    use jackin::console::widgets::op_picker::{OpLoadState, OpPickerStage};
    use jackin::operator_env::{OpField, OpItem, OpVault};

    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("DB_URL", "old-value")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    state.op_available = true;

    // Cursor opens on row 0 (DB_URL key row — no focusable header above);
    // P opens the picker.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('p')),
    )?;

    // Drive the picker straight to the Field stage with seeded vault +
    // item + fields, mirroring what `poll_load` would do after the
    // background loads complete. Same Option-Z direct-field approach
    // the prior commit-7 picker tests use.
    {
        let editor_state = editor_mut(&mut state);
        match &mut editor_state.modal {
            Some(Modal::OpPicker { state: picker }) => {
                picker.vaults = vec![OpVault {
                    id: "v1".into(),
                    name: "Personal".into(),
                }];
                picker.selected_vault = Some(OpVault {
                    id: "v1".into(),
                    name: "Personal".into(),
                });
                picker.items = vec![OpItem {
                    id: "i1".into(),
                    name: "Database".into(),
                    subtitle: String::new(),
                }];
                picker.selected_item = Some(OpItem {
                    id: "i1".into(),
                    name: "Database".into(),
                    subtitle: String::new(),
                });
                picker.fields = vec![OpField {
                    id: "password".into(),
                    label: "password".into(),
                    field_type: "concealed".into(),
                    concealed: true,
                    reference: String::new(),
                }];
                picker.field_list_state.select(Some(0));
                picker.stage = OpPickerStage::Field;
                picker.load_state = OpLoadState::Ready;
                // Drop any in-flight probe result so handle_key's
                // leading poll_load doesn't race the test by draining
                // a stale `Err(...)` from the constructor's account
                // probe and routing into the Fatal-error guard.
                picker.cancel_in_flight_load();
            }
            other => panic!("expected OpPicker modal; got {other:?}"),
        }
    }

    // Enter on the Field pane commits the `op://...` reference.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // The reference must land directly in pending.env (no text modal
    // intermediate). `as_persisted_str()` on an `OpRef` returns the
    // UUID-form `op` field (vault uuid=v1, item uuid=i1, field=password).
    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("DB_URL")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("op://v1/i1/password"),
        "picker commit must write the UUID-form op:// reference straight into pending.env[key]"
    );
    assert!(
        editor(&state).modal.is_none(),
        "modal must close after key-row picker commit; got {:?}",
        editor(&state).modal
    );
    assert!(editor(&state).pending_picker_target.is_none());
    assert!(editor(&state).pending_picker_value.is_none());
    Ok(())
}

/// `P` on the `+ Add environment variable` sentinel: the picker commits
/// a path before the operator has named the key. The `EnvKey` modal opens
/// next, the path is held on `pending_picker_value`, and committing the
/// key name writes both into pending.env at once.
#[test]
fn op_picker_sentinel_p_flow() -> Result<()> {
    use jackin::console::widgets::op_picker::{OpLoadState, OpPickerStage};
    use jackin::operator_env::{OpField, OpItem, OpVault};

    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    // Empty env so the only navigable row is WorkspaceAddSentinel at
    // index 0 (no preamble label is rendered).
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    state.op_available = true;

    // Cursor opens on row 0 (WorkspaceAddSentinel).
    assert!(matches!(editor(&state).active_field, FieldFocus::Row(0)));

    // P on the sentinel opens the picker with `pending_picker_target =
    // Some((Workspace, None))`.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('p')),
    )?;
    assert!(matches!(editor(&state).modal, Some(Modal::OpPicker { .. })));
    match &editor(&state).pending_picker_target {
        Some((_, None)) => {}
        other => panic!("sentinel P must stash (scope, None); got pending_picker_target={other:?}"),
    }

    // Drive the picker to a Field commit.
    {
        let editor_state = editor_mut(&mut state);
        match &mut editor_state.modal {
            Some(Modal::OpPicker { state: picker }) => {
                picker.vaults = vec![OpVault {
                    id: "v1".into(),
                    name: "Personal".into(),
                }];
                picker.selected_vault = Some(OpVault {
                    id: "v1".into(),
                    name: "Personal".into(),
                });
                picker.items = vec![OpItem {
                    id: "i1".into(),
                    name: "API Keys".into(),
                    subtitle: String::new(),
                }];
                picker.selected_item = Some(OpItem {
                    id: "i1".into(),
                    name: "API Keys".into(),
                    subtitle: String::new(),
                });
                picker.fields = vec![OpField {
                    id: "credential".into(),
                    label: "credential".into(),
                    field_type: "concealed".into(),
                    concealed: true,
                    reference: String::new(),
                }];
                picker.field_list_state.select(Some(0));
                picker.stage = OpPickerStage::Field;
                picker.load_state = OpLoadState::Ready;
                // Drop any in-flight probe result so handle_key's
                // leading poll_load doesn't race the test by draining
                // a stale `Err(...)` from the constructor's account
                // probe and routing into the Fatal-error guard.
                picker.cancel_in_flight_load();
            }
            other => panic!("expected OpPicker modal; got {other:?}"),
        }
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // After the picker commits: EnvKey modal is open and the OpRef is
    // stashed on pending_picker_value.
    match &editor(&state).modal {
        Some(Modal::TextInput {
            target: TextInputTarget::EnvKey { .. },
            ..
        }) => {}
        other => panic!("expected TextInput(EnvKey) modal; got {other:?}"),
    }
    // pending_picker_value holds an EnvValue::OpRef; check the `op` field
    // via as_persisted_str(). UUID form: vault=v1, item=i1, field=credential.
    assert_eq!(
        editor(&state)
            .pending_picker_value
            .as_ref()
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("op://v1/i1/credential"),
        "picker commit must stash the UUID-form op:// reference for the EnvKey commit"
    );

    // Type the new key name and Enter — the EnvKey commit handler must
    // consume `pending_picker_value` and write the pair into pending.env.
    for ch in "API_KEY".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("API_KEY")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("op://v1/i1/credential"),
        "EnvKey commit must write the stashed UUID-form OpRef into pending.env"
    );
    assert!(
        editor(&state).pending_picker_value.is_none(),
        "EnvKey commit must clear pending_picker_value"
    );
    assert!(
        editor(&state).modal.is_none(),
        "EnvKey commit on the picker fast-path must close the modal; got {:?}",
        editor(&state).modal
    );
    Ok(())
}

// ── SourcePicker integration tests ────────────────────────────────

/// Drives Enter-on-sentinel → ScopePicker(All roles) → `EnvKey`
/// commit, leaving `SourcePicker` open. Default `op_available = false`.
fn drive_to_source_picker<'a>(
    config: &mut AppConfig,
    paths: &JackinPaths,
    cwd: &std::path::Path,
    key_name: &str,
) -> Result<ManagerState<'a>> {
    // Build a manager state on the Secrets tab with op_available
    // baked in via the test-only constructor (false by default).
    let mut state = manager_on_secrets_tab(config, cwd);
    // Enter on the workspace sentinel opens the ScopePicker modal —
    // default focus is `AllAgents`, so a second Enter commits the
    // workspace path and opens the EnvKey modal.
    handle_key(&mut state, config, paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, config, paths, cwd, key(KeyCode::Enter))?;
    // Type the key.
    for ch in key_name.chars() {
        handle_key(&mut state, config, paths, cwd, key(KeyCode::Char(ch)))?;
    }
    // Commit the EnvKey — opens SourcePicker.
    handle_key(&mut state, config, paths, cwd, key(KeyCode::Enter))?;
    Ok(state)
}

/// `Enter` on `+ Add` walks: `EnvKey` → `SourcePicker` → `EnvValue` → commit.
/// The `SourcePicker` is the new step between the existing two text
/// modals. Verifies the `Plain` branch lands the typed value in
/// `pending.env`.
#[test]
fn enter_on_sentinel_opens_envkey_then_sourcepicker_then_value_modal() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = drive_to_source_picker(&mut config, &paths, cwd, "API_KEY")?;
    // SourcePicker is the active modal.
    assert!(
        matches!(editor(&state).modal, Some(Modal::SourcePicker { .. })),
        "EnvKey commit on a sentinel must open SourcePicker; got {:?}",
        editor(&state).modal
    );

    // Default focus is Plain — Enter opens the EnvValue text modal.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    match &editor(&state).modal {
        Some(Modal::TextInput {
            target: TextInputTarget::EnvValue { key, .. },
            ..
        }) => {
            assert_eq!(key, "API_KEY", "EnvValue modal must carry the typed key");
        }
        other => panic!("expected TextInput(EnvValue); got {other:?}"),
    }

    // Type the value and commit.
    for ch in "s3cret".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("API_KEY")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("s3cret"),
        "Plain-text source path must land the typed value in pending.env"
    );
    Ok(())
}

/// POSIX `VAR=""` differs from `unset VAR`, so `EnvValue` must commit
/// the empty string into `pending.env`.
#[test]
fn env_value_modal_allows_empty_commit() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = drive_to_source_picker(&mut config, &paths, cwd, "EMPTY_OK")?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::SourcePicker { .. })),
        "expected SourcePicker; got {:?}",
        editor(&state).modal
    );

    // Plain branch → EnvValue modal opens.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(
            editor(&state).modal,
            Some(Modal::TextInput {
                target: TextInputTarget::EnvValue { .. },
                ..
            })
        ),
        "expected EnvValue modal; got {:?}",
        editor(&state).modal
    );

    // Press Enter immediately on an empty textarea — must commit `""`
    // into pending.env. With the previous global non-empty validity
    // rule this Enter was swallowed.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("EMPTY_OK")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some(""),
        "EnvValue modal must allow committing an empty string \
         (POSIX VAR=\"\" semantics)"
    );
    assert!(
        editor(&state).modal.is_none(),
        "EnvValue commit must close the modal; got {:?}",
        editor(&state).modal
    );
    Ok(())
}

/// `SourcePicker` → 1Password branch: when op is available and the
/// operator picks the Op choice, the `OpPicker` modal opens with
/// `pending_picker_target = (scope, Some(key))` so its commit handler
/// can write the `op://...` reference straight into the named key.
#[test]
fn enter_on_sentinel_path_to_op_picker() -> Result<()> {
    use jackin::console::widgets::source_picker::SourcePickerState;
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    // Build the state and force `op_available = true` on the
    // SourcePicker so the Op branch is selectable. (The default
    // `from_config` constructor used by `manager_on_secrets_tab` gives
    // op_available=false; this test fakes the available state by
    // injecting the modal directly with op_available=true.)
    let mut state = manager_on_secrets_tab(&config, cwd);
    // Pretend the operator already typed "API_KEY" and committed via
    // the sentinel-add path: stash pending_env_key + open SourcePicker
    // with op_available = true.
    {
        let editor_state = editor_mut(&mut state);
        editor_state.pending_env_key = Some((
            jackin::console::manager::state::SecretsScopeTag::Workspace,
            "API_KEY".into(),
        ));
        editor_state.modal = Some(Modal::SourcePicker {
            state: SourcePickerState::new("API_KEY".into(), true),
        });
    }

    // Right arrow → focus moves to Op. Enter commits Op and opens the
    // OpPicker modal.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    assert!(
        matches!(editor(&state).modal, Some(Modal::OpPicker { .. })),
        "Op branch on SourcePicker must open the OpPicker; got {:?}",
        editor(&state).modal
    );
    match &editor(&state).pending_picker_target {
        Some((_, Some(name))) => {
            assert_eq!(name, "API_KEY", "OpPicker target must carry the typed key");
        }
        other => {
            panic!("expected pending_picker_target = (scope, Some(\"API_KEY\")); got {other:?}")
        }
    }
    // pending_env_key is consumed once OpPicker takes ownership of the
    // (scope, key) pair via pending_picker_target.
    assert!(
        editor(&state).pending_env_key.is_none(),
        "Op branch must consume pending_env_key into pending_picker_target"
    );
    Ok(())
}

/// When `op_available = false`, the Op button on the `SourcePicker` is
/// disabled: `→`/`Tab` cycling skips it, focus stays on Plain, and the
/// `O` direct hotkey is inert. The picker commits Plain regardless of
/// key flailing.
#[test]
fn source_picker_op_disabled_when_op_missing() -> Result<()> {
    use jackin::console::widgets::source_picker::{SourceChoice, SourcePickerState};
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = manager_on_secrets_tab(&config, cwd);
    {
        let editor_state = editor_mut(&mut state);
        editor_state.pending_env_key = Some((
            jackin::console::manager::state::SecretsScopeTag::Workspace,
            "API_KEY".into(),
        ));
        editor_state.modal = Some(Modal::SourcePicker {
            state: SourcePickerState::new("API_KEY".into(), false),
        });
    }

    // Press Right arrow + Tab — neither must advance focus to Op.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('O')),
    )?;
    match &editor(&state).modal {
        Some(Modal::SourcePicker { state }) => {
            assert_eq!(
                state.focused,
                SourceChoice::Plain,
                "focus must stay on Plain when op_available is false"
            );
        }
        other => panic!("expected SourcePicker still active; got {other:?}"),
    }
    // Enter on the still-Plain focus commits Plain and opens EnvValue.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(
            editor(&state).modal,
            Some(Modal::TextInput {
                target: TextInputTarget::EnvValue { .. },
                ..
            })
        ),
        "Enter on Plain (only available choice) must commit and open EnvValue; got {:?}",
        editor(&state).modal
    );
    Ok(())
}

/// `Esc` on the `SourcePicker` closes the modal and clears
/// `pending_env_key`, `pending_picker_value`. Operator returns to the
/// Secrets tab with no entry added.
#[test]
fn source_picker_esc_clears_pending_state() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = drive_to_source_picker(&mut config, &paths, cwd, "API_KEY")?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::SourcePicker { .. })
    ));
    assert!(
        editor(&state).pending_env_key.is_some(),
        "EnvKey commit must stash (scope, key)"
    );

    // Esc cancels.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        editor(&state).modal.is_none(),
        "Esc on SourcePicker must close the modal; got {:?}",
        editor(&state).modal
    );
    assert!(
        editor(&state).pending_env_key.is_none(),
        "Esc on SourcePicker must clear pending_env_key"
    );
    assert!(
        editor(&state).pending_picker_value.is_none(),
        "Esc on SourcePicker must clear pending_picker_value"
    );
    assert!(
        !editor(&state).pending.env.contains_key("API_KEY"),
        "Esc on SourcePicker must not write any env entry; got {:?}",
        editor(&state).pending.env
    );
    Ok(())
}

/// Drives Account → Vault → Item → Field. Verifies committed `op://`
/// is the bare `op://Vault/Item/Field` form (account scope is not
/// encoded in the path).
#[test]
#[allow(clippy::too_many_lines)]
fn op_picker_multi_account_flow() -> Result<()> {
    use jackin::console::widgets::op_picker::{OpLoadState, OpPickerStage};
    use jackin::operator_env::{OpAccount, OpField, OpItem, OpVault};

    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("DB_URL", "old")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    state.op_available = true;

    // Cursor opens on row 0 (DB_URL key row); P opens the picker.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('p')),
    )?;

    // Seed the picker as if the constructor's probe had returned two
    // accounts: stage = Account, accounts populated, none selected yet.
    {
        let editor_state = editor_mut(&mut state);
        match &mut editor_state.modal {
            Some(Modal::OpPicker { state: picker }) => {
                picker.accounts = vec![
                    OpAccount {
                        id: "ACCT_A".into(),
                        email: "alice@example.com".into(),
                        url: "alpha.1password.com".into(),
                    },
                    OpAccount {
                        id: "ACCT_B".into(),
                        email: "bob@example.com".into(),
                        url: "beta.1password.com".into(),
                    },
                ];
                picker.account_list_state.select(Some(1)); // Bob
                picker.selected_account = None;
                picker.stage = OpPickerStage::Account;
                picker.load_state = OpLoadState::Ready;
                // Drop any in-flight probe result so handle_key's
                // leading poll_load doesn't race the test by draining
                // a stale `Err(...)` from the constructor's account
                // probe and routing into the Fatal-error guard.
                picker.cancel_in_flight_load();
            }
            other => panic!("expected OpPicker modal; got {other:?}"),
        }
    }

    // Enter on the Account pane — selects Bob, advances to Vault.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    match &editor(&state).modal {
        Some(Modal::OpPicker { state: picker }) => {
            assert_eq!(picker.stage, OpPickerStage::Vault);
            assert_eq!(
                picker.selected_account.as_ref().map(|a| a.id.as_str()),
                Some("ACCT_B"),
                "Enter on Account must capture the selection"
            );
        }
        other => panic!("expected OpPicker modal; got {other:?}"),
    }

    // Seed Vault → Item → Field straight to the commit point.
    {
        let editor_state = editor_mut(&mut state);
        match &mut editor_state.modal {
            Some(Modal::OpPicker { state: picker }) => {
                picker.vaults = vec![OpVault {
                    id: "v1".into(),
                    name: "Shared".into(),
                }];
                picker.selected_vault = Some(OpVault {
                    id: "v1".into(),
                    name: "Shared".into(),
                });
                picker.items = vec![OpItem {
                    id: "i1".into(),
                    name: "Database".into(),
                    subtitle: String::new(),
                }];
                picker.selected_item = Some(OpItem {
                    id: "i1".into(),
                    name: "Database".into(),
                    subtitle: String::new(),
                });
                picker.fields = vec![OpField {
                    id: "password".into(),
                    label: "password".into(),
                    field_type: "concealed".into(),
                    concealed: true,
                    reference: String::new(),
                }];
                picker.field_list_state.select(Some(0));
                picker.stage = OpPickerStage::Field;
                picker.load_state = OpLoadState::Ready;
                // Drop any in-flight probe result so handle_key's
                // leading poll_load doesn't race the test by draining
                // a stale `Err(...)` from the constructor's account
                // probe and routing into the Fatal-error guard.
                picker.cancel_in_flight_load();
            }
            other => panic!("expected OpPicker modal; got {other:?}"),
        }
    }

    // Enter on the Field pane commits the OpRef. The `op` field uses
    // UUID-form identifiers (vault=v1, item=i1, field=password).
    // Account scoping is not embedded in the URL; the launch-time
    // resolver uses the operator's default `op` account context.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("DB_URL")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("op://v1/i1/password"),
        "multi-account picker commit must produce a UUID-form op:// reference"
    );
    assert!(editor(&state).modal.is_none());
    Ok(())
}

// ── Modal::RolePicker dispatch tests ─────────────────────────────

/// Seed a config with a workspace that has `agent_keys.len()` allowed
/// roles (each registered as `config.roles`), optionally a named
/// `default_role`. Returns the saved `AppConfig`.
fn seed_config_with_agents(
    paths: &JackinPaths,
    temp_dir: &std::path::Path,
    agent_keys: &[&str],
    default_role: Option<&str>,
) -> Result<AppConfig> {
    paths.ensure_base_dirs()?;
    let host_path = temp_dir.display().to_string();
    let mut config = AppConfig::default();
    for key in agent_keys {
        config.roles.insert(
            (*key).to_string(),
            jackin::config::RoleSource {
                git: format!("https://example.invalid/jackin-{key}.git"),
                trusted: true,
                env: std::collections::BTreeMap::new(),
            },
        );
    }
    let toml = toml::to_string(&config)?;
    std::fs::write(&paths.config_file, toml)?;

    let ws = WorkspaceConfig {
        workdir: host_path.clone(),
        mounts: vec![MountConfig {
            src: host_path.clone(),
            dst: host_path,
            readonly: false,
            isolation: jackin::isolation::MountIsolation::Shared,
        }],
        allowed_roles: agent_keys.iter().map(|s| (*s).to_string()).collect(),
        default_role: default_role.map(String::from),
        ..Default::default()
    };
    let mut ce = ConfigEditor::open(paths)?;
    ce.create_workspace("multi-role-ws", ws)?;
    ce.save()
}

/// `Enter` on a workspace row with two eligible roles and no default
/// must open `Modal::RolePicker` overlaid on the manager list — not
/// short-circuit to a launch outcome.
#[test]
fn agent_picker_opens_when_multiple_agents_available() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let config = seed_config_with_agents(
        &paths,
        temp.path(),
        &["chainargos/agent-smith", "chainargos/agent-brown"],
        None,
    )?;
    let cwd = temp.path();
    let mut state = ConsoleState::new(&config, cwd)?;

    let outcome = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    assert!(
        outcome.is_none(),
        "multi-role dispatch must stay in the run-loop (Ok(None)); got {outcome:?}"
    );
    let ConsoleStage::Manager(ms) = &state.stage;
    match &ms.inline_role_picker {
        Some(picker) => {
            assert_eq!(picker.roles.len(), 2);
            assert_eq!(picker.filtered.len(), 2);
        }
        other => panic!("expected inline RolePicker; got {other:?}"),
    }
    Ok(())
}

/// `default_role` set on the workspace must preselect that role in the
/// inline picker instead of launching without confirmation.
#[test]
fn agent_picker_opens_with_default_agent_preselected() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let config = seed_config_with_agents(
        &paths,
        temp.path(),
        &["chainargos/agent-smith", "chainargos/agent-brown"],
        Some("chainargos/agent-smith"),
    )?;
    let cwd = temp.path();
    let mut state = ConsoleState::new(&config, cwd)?;

    let outcome = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    assert!(
        outcome.is_none(),
        "default_role dispatch must stay in the run-loop so the operator confirms"
    );
    let ConsoleStage::Manager(ms) = &state.stage;
    let picker = ms
        .inline_role_picker
        .as_ref()
        .expect("default_role dispatch must open the inline picker");
    let selected = picker
        .list_state
        .selected
        .expect("default role should be selected");
    assert_eq!(picker.filtered[selected].key(), "chainargos/agent-smith");
    Ok(())
}

/// Exactly one eligible role skips the picker and returns the role directly.
#[test]
fn agent_picker_opens_when_single_eligible_agent() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let config = seed_config_with_agents(&paths, temp.path(), &["chainargos/agent-smith"], None)?;
    let cwd = temp.path();
    let mut state = ConsoleState::new(&config, cwd)?;

    let outcome = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    let (role, _workspace, agent) = outcome
        .expect("single eligible role must auto-select and return directly, not open picker");
    assert_eq!(role.key(), "chainargos/agent-smith");
    assert!(agent.is_none(), "agent must not be pre-selected");
    let ConsoleStage::Manager(ms) = &state.stage;
    assert!(
        ms.inline_role_picker.is_none(),
        "picker must not be open after single-role auto-select"
    );
    Ok(())
}

/// `Enter` on the picker commits the selected role — `manager::handle_key`
/// returns `InputOutcome::LaunchWithAgent(role)` so `run_console` can
/// resolve the workspace and break the event loop.
#[test]
fn agent_picker_enter_commits_launch() -> Result<()> {
    use jackin::console::manager::InputOutcome;

    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_agents(
        &paths,
        temp.path(),
        &["chainargos/agent-smith", "chainargos/agent-brown"],
        None,
    )?;
    let cwd = temp.path();
    let mut state = ConsoleState::new(&config, cwd)?;

    state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    let ConsoleStage::Manager(ms) = &mut state.stage;
    assert!(ms.inline_role_picker.is_some());

    // Enter on the picker — selection defaults to index 0
    // (BTreeMap ordering of role keys).
    let outcome = handle_key(ms, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    match outcome {
        InputOutcome::LaunchWithAgent(role) => {
            assert!(
                role.key() == "chainargos/agent-brown" || role.key() == "chainargos/agent-smith",
                "picker commit must surface one of the two seeded roles; got {}",
                role.key()
            );
        }
        other => panic!("expected LaunchWithAgent outcome; got {other:?}"),
    }
    assert!(
        ms.inline_role_picker.is_none(),
        "picker commit must close the inline picker"
    );
    Ok(())
}

/// `Esc` on the picker closes the modal and returns `Continue` — the
/// operator stays on the manager list with an unchanged selection.
#[test]
fn agent_picker_esc_closes_modal() -> Result<()> {
    use jackin::console::manager::InputOutcome;

    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_agents(
        &paths,
        temp.path(),
        &["chainargos/agent-smith", "chainargos/agent-brown"],
        None,
    )?;
    let cwd = temp.path();
    let mut state = ConsoleState::new(&config, cwd)?;

    state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    let ConsoleStage::Manager(ms) = &mut state.stage;
    assert!(ms.inline_role_picker.is_some());

    let outcome = handle_key(ms, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        matches!(outcome, InputOutcome::Continue),
        "Esc on the picker must produce Continue; got {outcome:?}"
    );
    assert!(
        ms.inline_role_picker.is_none(),
        "Esc must close the inline picker; got {:?}",
        ms.inline_role_picker
    );
    Ok(())
}

// ── Duplicate-env-key live-validation tests ──────────────────────────

/// Operator opens the `EnvKey` modal on a workspace where `EXISTING`
/// already lives in `pending.env` and types the same name. Enter must
/// be swallowed (modal stays open, `EnvKey` target intact) and the
/// pre-existing value in `pending.env["EXISTING"]` must be unchanged.
#[test]
fn env_key_modal_blocks_duplicate_workspace_key() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("EXISTING", "kept-value")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Rows: 0 WorkspaceKeyRow("EXISTING"), 1 WorkspaceAddSentinel.
    // Navigate to the sentinel; Enter opens the ScopePicker, then a
    // second Enter (default focus = AllAgents) opens the EnvKey modal.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(
            editor(&state).modal,
            Some(Modal::TextInput {
                target: TextInputTarget::EnvKey { .. },
                ..
            })
        ),
        "ScopePicker(AllAgents) commit must open the EnvKey modal; got {:?}",
        editor(&state).modal
    );

    // Type the colliding key.
    for ch in "EXISTING".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    let env_size_before = editor(&state).pending.env.len();
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Modal must still be the EnvKey TextInput (Enter was swallowed).
    assert!(
        matches!(
            editor(&state).modal,
            Some(Modal::TextInput {
                target: TextInputTarget::EnvKey { .. },
                ..
            })
        ),
        "Enter on a duplicate key must leave the EnvKey modal open; got {:?}",
        editor(&state).modal
    );
    assert_eq!(
        editor(&state).pending.env.len(),
        env_size_before,
        "duplicate Enter must not add an env entry"
    );
    assert_eq!(
        editor(&state)
            .pending
            .env
            .get("EXISTING")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("kept-value"),
        "the pre-existing value must remain untouched"
    );
    Ok(())
}

/// Same guard, role-override scope. Seed a workspace with one role
/// override `LOG_LEVEL`, expand the section, navigate to its `+ Add`
/// sentinel, and type the colliding key.
#[test]
fn env_key_modal_blocks_duplicate_agent_key() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    paths.ensure_base_dirs()?;
    let host_path = temp.path().display().to_string();

    let mut role_env = std::collections::BTreeMap::new();
    role_env.insert(
        "LOG_LEVEL".into(),
        jackin::operator_env::EnvValue::Plain("debug".into()),
    );
    let mut roles = std::collections::BTreeMap::new();
    roles.insert(
        "agent-smith".into(),
        WorkspaceRoleOverride {
            env: role_env,
            claude: None,
            codex: None,
            amp: None,
            kimi: None,
            opencode: None,
            github: None,
        },
    );
    let ws = WorkspaceConfig {
        workdir: host_path.clone(),
        mounts: vec![MountConfig {
            src: host_path.clone(),
            dst: host_path,
            readonly: false,
            isolation: jackin::isolation::MountIsolation::Shared,
        }],
        roles,
        ..Default::default()
    };
    let mut ce = ConfigEditor::open(&paths)?;
    ce.create_workspace("big-monorepo", ws)?;
    let mut config = ce.save()?;

    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Rows on this fixture (no workspace keys, one collapsed role section):
    //   0 WorkspaceAddSentinel
    //   1 SectionSpacer  (skipped by ↑/↓)
    //   2 AgentHeader { role: "agent-smith", expanded: false }
    // Expand the role section so the AgentAddSentinel row exists.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    assert!(editor(&state).secrets_expanded.contains("agent-smith"));

    // After expansion:
    //   0 WorkspaceAddSentinel
    //   1 SectionSpacer
    //   2 AgentHeader (expanded)
    //   3 AgentKeyRow { role: "agent-smith", key: "LOG_LEVEL" }
    //   4 AgentAddSentinel("agent-smith")
    // Navigate to row 4.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;

    // Enter to open the EnvKey modal for the role scope.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::TextInput {
            target: TextInputTarget::EnvKey { .. },
            ..
        })
    ));

    for ch in "LOG_LEVEL".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // The modal must still be open, and the role's env unchanged.
    assert!(
        matches!(
            editor(&state).modal,
            Some(Modal::TextInput {
                target: TextInputTarget::EnvKey { .. },
                ..
            })
        ),
        "Enter on a duplicate role key must leave the EnvKey modal open; got {:?}",
        editor(&state).modal
    );
    let agent_entry = editor(&state)
        .pending
        .roles
        .get("agent-smith")
        .expect("role override must survive");
    assert_eq!(agent_entry.env.len(), 1);
    assert_eq!(
        agent_entry
            .env
            .get("LOG_LEVEL")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("debug"),
        "pre-existing role value must remain untouched"
    );
    Ok(())
}

/// Regression: the duplicate guard does not block a unique name. Type
/// `NEW_KEY` at a workspace whose `pending.env` already contains
/// `EXISTING` and confirm the `EnvKey` modal commits (transitions to
/// `SourcePicker`, the next stage in the unified add flow).
#[test]
fn env_key_modal_allows_unique_name() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_env(&paths, temp.path(), vec![("EXISTING", "kept-value")])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Sentinel is row 1 here (row 0 is the existing key row). The
    // first Enter opens the ScopePicker; the second commits AllAgents
    // and opens the EnvKey modal.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    for ch in "NEW_KEY".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Unique name commits — flow advances to the SourcePicker.
    assert!(
        matches!(editor(&state).modal, Some(Modal::SourcePicker { .. })),
        "unique key must commit and open the SourcePicker; got {:?}",
        editor(&state).modal
    );
    Ok(())
}

// ── ScopePicker + AgentOverridePicker integration tests ──────────────

/// Seed the on-disk config + workspace for the override-picker tests.
/// Adds a workspace with the given `allowed_roles` (each also
/// registered as an `RoleSource`), and pre-populates `pending` per-
/// role overrides for any names in `with_overrides`.
fn seed_override_picker_workspace(
    paths: &JackinPaths,
    temp_dir: &std::path::Path,
    allowed: &[&str],
    with_overrides: &[&str],
) -> Result<AppConfig> {
    paths.ensure_base_dirs()?;
    let host_path = temp_dir.display().to_string();
    let mut config = AppConfig::default();
    for name in allowed {
        config.roles.insert(
            (*name).to_string(),
            jackin::config::RoleSource {
                git: format!("https://example.invalid/{name}.git"),
                trusted: true,
                env: std::collections::BTreeMap::new(),
            },
        );
    }
    let toml = toml::to_string(&config)?;
    std::fs::write(&paths.config_file, toml)?;

    let mut roles_map = std::collections::BTreeMap::new();
    for name in with_overrides {
        let mut env = std::collections::BTreeMap::new();
        env.insert(
            "LOG_LEVEL".into(),
            jackin::operator_env::EnvValue::Plain("debug".into()),
        );
        roles_map.insert(
            (*name).into(),
            WorkspaceRoleOverride {
                env,
                claude: None,
                codex: None,
                amp: None,
                kimi: None,
                opencode: None,
                github: None,
            },
        );
    }

    let ws = WorkspaceConfig {
        workdir: host_path.clone(),
        mounts: vec![MountConfig {
            src: host_path.clone(),
            dst: host_path,
            readonly: false,
            isolation: jackin::isolation::MountIsolation::Shared,
        }],
        allowed_roles: allowed.iter().map(|s| (*s).to_string()).collect(),
        roles: roles_map,
        ..Default::default()
    };
    let mut ce = ConfigEditor::open(paths)?;
    ce.create_workspace("big-monorepo", ws)?;
    ce.save()
}

/// Press `Enter` on the workspace-level `+ Add environment variable`
/// sentinel: the `Modal::ScopePicker` opens on the editor stage so the
/// operator can pick "All roles" or "Specific role" before falling
/// into the rest of the add flow.
#[test]
fn sentinel_enter_opens_scope_picker() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Rows: [WorkspaceAddSentinel]. Cursor opens on row 0.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::ScopePicker { .. })),
        "Enter on the workspace sentinel must open the ScopePicker; got {:?}",
        editor(&state).modal
    );
    Ok(())
}

/// `ScopePicker` → `AllAgents` path: pick the default-focused choice and
/// confirm the `EnvKey` modal opens with `Workspace` scope.
#[test]
fn scope_picker_all_path_to_workspace_envkey() -> Result<()> {
    use jackin::console::manager::state::SecretsScopeTag;
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Open the ScopePicker; default focus is AllAgents — Enter commits.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    match &editor(&state).modal {
        Some(Modal::TextInput { target, .. }) => match target {
            TextInputTarget::EnvKey { scope } => {
                assert_eq!(
                    scope,
                    &SecretsScopeTag::Workspace,
                    "AllAgents commit must open the EnvKey modal with Workspace scope"
                );
            }
            other => panic!("expected TextInputTarget::EnvKey; got {other:?}"),
        },
        other => panic!("expected Modal::TextInput; got {other:?}"),
    }
    Ok(())
}

/// `ScopePicker` → `SpecificAgent` → `AgentPicker` → `EnvKey` path. Verifies
/// each transition: `ScopePicker` right-arrow → `SpecificAgent` focus,
/// Enter → `AgentOverridePicker`; picker Enter → `EnvKey` with `Role`
/// scope. `pending.roles` is NOT mutated — the section materialises
/// organically once the first key/value commits.
#[test]
fn scope_picker_specific_path_to_agent_picker_then_envkey() -> Result<()> {
    use jackin::console::manager::state::SecretsScopeTag;
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Open the ScopePicker, advance focus to SpecificAgent, commit.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // The role-override picker is now open with the eligible set.
    match &editor(&state).modal {
        Some(Modal::RoleOverridePicker { state: picker }) => {
            assert_eq!(picker.roles.len(), 1);
            assert_eq!(picker.roles[0].key(), "agent-smith");
        }
        other => panic!("expected Modal::RoleOverridePicker; got {other:?}"),
    }

    // Commit the only eligible role — the EnvKey modal opens with
    // `Role(<name>)` scope and `pending.roles` stays empty.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        editor(&state).pending.roles.is_empty(),
        "picker commit must not create an override entry; got pending.roles keys={:?}",
        editor(&state).pending.roles.keys().collect::<Vec<_>>()
    );
    assert!(
        editor(&state).secrets_expanded.is_empty(),
        "picker commit must not pre-expand any section"
    );
    match &editor(&state).modal {
        Some(Modal::TextInput { target, .. }) => match target {
            TextInputTarget::EnvKey { scope } => {
                assert_eq!(
                    scope,
                    &SecretsScopeTag::Role("agent-smith".into()),
                    "EnvKey modal must scope to the picked role"
                );
            }
            other => panic!("expected TextInputTarget::EnvKey; got {other:?}"),
        },
        other => panic!("expected Modal::TextInput; got {other:?}"),
    }
    Ok(())
}

/// Roles already carrying an override are not filtered out — the
/// operator may want to add more keys.
#[test]
fn agent_picker_lists_all_allowed_agents_not_filtered_by_existing_overrides() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(
        &paths,
        temp.path(),
        &["agent-smith", "agent-brown"],
        &["agent-smith"],
    )?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Workspace sentinel → ScopePicker → SpecificAgent → AgentPicker.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    match &editor(&state).modal {
        Some(Modal::RoleOverridePicker { state: picker }) => {
            let mut keys: Vec<String> = picker
                .roles
                .iter()
                .map(jackin::selector::RoleSelector::key)
                .collect();
            keys.sort();
            assert_eq!(
                keys,
                vec!["agent-brown".to_string(), "agent-smith".to_string()],
                "agent-smith already has an override section but must still appear so the operator can add another key"
            );
        }
        other => panic!("expected Modal::RoleOverridePicker; got {other:?}"),
    }
    Ok(())
}

/// Esc on the `ScopePicker` closes the modal and leaves
/// `pending.roles` and `pending.env` untouched — backing out is a
/// pure no-op.
#[test]
fn cancel_from_scope_picker_returns_to_secrets_tab() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::ScopePicker { .. })
    ));

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        editor(&state).modal.is_none(),
        "Esc on the ScopePicker must close the modal; got {:?}",
        editor(&state).modal
    );
    assert!(
        editor(&state).pending.roles.is_empty(),
        "Esc must not create an override entry"
    );
    assert!(
        editor(&state).pending.env.is_empty(),
        "Esc must not write any env entry"
    );
    Ok(())
}

/// Esc on the `EnvKey` modal that opens after the picker commit must
/// leave `pending.roles` untouched — no orphan empty section.
#[test]
fn cancel_from_envkey_after_agent_pick_does_not_create_section() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Workspace sentinel → ScopePicker → SpecificAgent → AgentPicker
    // → commit → EnvKey modal opens.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::TextInput {
            target: TextInputTarget::EnvKey { .. },
            ..
        })
    ));

    // Esc on the EnvKey modal.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        editor(&state).modal.is_none(),
        "Esc must close the EnvKey modal; got {:?}",
        editor(&state).modal
    );
    assert!(
        editor(&state).pending.roles.is_empty(),
        "Esc on EnvKey must not have created an override entry; got keys={:?}",
        editor(&state).pending.roles.keys().collect::<Vec<_>>()
    );
    assert!(
        editor(&state).secrets_expanded.is_empty(),
        "Esc on EnvKey must not have expanded any section"
    );
    Ok(())
}

/// Esc on the `SourcePicker` modal that opens after a valid `EnvKey` commit
/// must also leave `pending.roles` untouched. Mirrors the `EnvKey`
/// cancel test one step deeper in the chain.
#[test]
fn cancel_from_sourcepicker_after_agent_pick_does_not_create_section() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Workspace sentinel → ScopePicker → SpecificAgent → AgentPicker
    // → commit → EnvKey modal.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Type a valid key name and commit → SourcePicker modal opens.
    for ch in "API_TOKEN".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::SourcePicker { .. })),
        "EnvKey commit must open SourcePicker; got {:?}",
        editor(&state).modal
    );

    // Esc on the SourcePicker.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        editor(&state).modal.is_none(),
        "Esc must close the SourcePicker; got {:?}",
        editor(&state).modal
    );
    assert!(
        editor(&state).pending.roles.is_empty(),
        "Esc on SourcePicker must not have created an override entry; got keys={:?}",
        editor(&state).pending.roles.keys().collect::<Vec<_>>()
    );
    assert!(
        editor(&state).secrets_expanded.is_empty(),
        "Esc on SourcePicker must not have expanded any section"
    );
    Ok(())
}

/// Drive the full chain — workspace sentinel → ScopePicker(Specific) →
/// `AgentPicker` → `EnvKey` → SourcePicker(Plain) → `EnvValue` → commit. The
/// override section materialises only on this final commit, with the
/// key/value present and the section expanded.
#[test]
fn completing_value_after_agent_pick_creates_section_with_one_var() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Workspace sentinel → ScopePicker → SpecificAgent → AgentPicker
    // → commit → EnvKey modal.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Type the key, commit → SourcePicker.
    for ch in "API_TOKEN".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::SourcePicker { .. })
    ));

    // SourcePicker default selection is Plain — Enter commits → EnvValue.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(
            editor(&state).modal,
            Some(Modal::TextInput {
                target: TextInputTarget::EnvValue { .. },
                ..
            })
        ),
        "SourcePicker(Plain) commit must open EnvValue; got {:?}",
        editor(&state).modal
    );

    // Type the value and commit.
    for ch in "secret".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // The override section now exists with the single key/value.
    let roles = &editor(&state).pending.roles;
    assert!(
        roles.contains_key("agent-smith"),
        "pending.roles must contain the chosen role; got keys={:?}",
        roles.keys().collect::<Vec<_>>()
    );
    assert_eq!(
        roles
            .get("agent-smith")
            .unwrap()
            .env
            .get("API_TOKEN")
            .map(jackin::operator_env::EnvValue::as_persisted_str),
        Some("secret"),
        "the committed key/value must land in the role's env map"
    );
    // The section must be auto-expanded.
    assert!(
        editor(&state).secrets_expanded.contains("agent-smith"),
        "value commit must auto-expand the role's section"
    );
    // All modals closed.
    assert!(
        editor(&state).modal.is_none(),
        "value commit must close every modal; got {:?}",
        editor(&state).modal
    );
    Ok(())
}

/// Esc on the override picker (reachable through the `ScopePicker`'s
/// `SpecificAgent` path) closes the modal and leaves `pending.roles`
/// untouched — symmetric with the ScopePicker-cancel test, one step
/// deeper.
#[test]
fn cancel_from_agent_override_picker_after_scope_pick_does_not_create_section() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &[])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);

    // Open the picker via ScopePicker → SpecificAgent.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::RoleOverridePicker { .. })
    ));

    // Esc — the modal closes and pending.roles stays empty.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        editor(&state).modal.is_none(),
        "Esc must close the override picker; got {:?}",
        editor(&state).modal
    );
    assert!(
        editor(&state).pending.roles.is_empty(),
        "Esc must not create an override entry; got pending.roles keys={:?}",
        editor(&state).pending.roles.keys().collect::<Vec<_>>()
    );
    assert!(
        editor(&state).secrets_expanded.is_empty(),
        "Esc must not expand any section"
    );
    Ok(())
}

/// Once a key has landed in an role's section, the in-section
/// `+ Add <role> environment variable` sentinel must remain a direct
/// fast-path to the `EnvKey` modal — the `ScopePicker` only intercedes at
/// the workspace-level sentinel.
#[test]
fn in_section_agent_sentinel_skips_scope_picker() -> Result<()> {
    use jackin::console::manager::state::SecretsScopeTag;
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config =
        seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &["agent-smith"])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    // Pre-expand the role so its in-section sentinel is reachable.
    editor_mut(&mut state)
        .secrets_expanded
        .insert("agent-smith".into());

    // Rows now: [WorkspaceAddSentinel, SectionSpacer,
    //            AgentHeader(expanded), AgentKeyRow(LOG_LEVEL),
    //            AgentAddSentinel]. The first `↓` skips the
    //            non-focusable SectionSpacer at row 1 and lands on the
    //            AgentHeader at row 2; subsequent `↓` presses walk to
    //            the AgentAddSentinel at row 4.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;

    // Enter — the EnvKey modal opens directly with `Role` scope. No
    // ScopePicker intercedes here.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    match &editor(&state).modal {
        Some(Modal::TextInput { target, .. }) => match target {
            TextInputTarget::EnvKey { scope } => {
                assert_eq!(
                    scope,
                    &SecretsScopeTag::Role("agent-smith".into()),
                    "in-section sentinel must open EnvKey with the contextual Role scope"
                );
            }
            other => panic!("expected TextInputTarget::EnvKey; got {other:?}"),
        },
        other => panic!(
            "expected Modal::TextInput on in-section sentinel; got {other:?} (ScopePicker must not intercede)"
        ),
    }
    Ok(())
}

// ── Role-section header absorbs ←/→ regardless of expanded state ──
//
// Bug: pressing → on an *already-expanded* header (or ← on a *collapsed*
// one) used to fall through to the tab-cycle handler — operators saw the
// active editor tab change unexpectedly when arrowing on
// `▼ Role: <name>` / `▶ Role: <name>`. The header now absorbs ←/→ in
// both states; semantics codified in RULES.md
// § "TUI Keybindings → Contextual key absorption".

/// `→` on a collapsed role header expands the section AND leaves the
/// active tab on Secrets. Already covered by
/// `secrets_agent_section_expand_collapse`, but pinned here as a focused
/// regression guard for the contextual-absorption rule.
#[test]
fn right_on_collapsed_agent_header_expands_does_not_change_tab() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config =
        seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &["agent-smith"])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    // ↓ from row 0 (WorkspaceAddSentinel) skips the SectionSpacer and
    // lands on the AgentHeader at row 2.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    assert!(matches!(editor(&state).active_field, FieldFocus::Row(2)));
    assert!(
        !editor(&state).secrets_expanded.contains("agent-smith"),
        "section must start collapsed"
    );

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;

    assert!(
        editor(&state).secrets_expanded.contains("agent-smith"),
        "→ on collapsed header must expand the section"
    );
    assert_eq!(
        editor(&state).active_tab,
        EditorTab::Secrets,
        "→ on header must NOT advance the active tab"
    );
    Ok(())
}

/// `→` on an *already-expanded* role header is a no-op — but it must
/// still be absorbed. The active tab must not change. This is the exact
/// regression the operator reported.
#[test]
fn right_on_expanded_agent_header_does_nothing_does_not_change_tab() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config =
        seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &["agent-smith"])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    // Pre-expand the section so we exercise the "already expanded" path.
    editor_mut(&mut state)
        .secrets_expanded
        .insert("agent-smith".into());

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;

    assert!(
        editor(&state).secrets_expanded.contains("agent-smith"),
        "→ on expanded header must leave the section expanded (no-op on the section)"
    );
    assert_eq!(
        editor(&state).active_tab,
        EditorTab::Secrets,
        "→ on expanded header must NOT advance the active tab"
    );
    Ok(())
}

/// `←` on an expanded role header collapses the section. Active tab
/// stays put.
#[test]
fn left_on_expanded_agent_header_collapses_does_not_change_tab() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config =
        seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &["agent-smith"])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    editor_mut(&mut state)
        .secrets_expanded
        .insert("agent-smith".into());

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Left))?;

    assert!(
        !editor(&state).secrets_expanded.contains("agent-smith"),
        "← on expanded header must collapse the section"
    );
    assert_eq!(
        editor(&state).active_tab,
        EditorTab::Secrets,
        "← on header must NOT rewind the active tab"
    );
    Ok(())
}

/// `←` on an *already-collapsed* role header is a no-op — but it must
/// still be absorbed. The active tab must not change. Mirror of the
/// `→`-on-expanded case.
#[test]
fn left_on_collapsed_agent_header_does_nothing_does_not_change_tab() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config =
        seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &["agent-smith"])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    // Section starts collapsed; don't expand it.
    assert!(!editor(&state).secrets_expanded.contains("agent-smith"));

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Left))?;

    assert!(
        !editor(&state).secrets_expanded.contains("agent-smith"),
        "← on collapsed header must leave the section collapsed"
    );
    assert_eq!(
        editor(&state).active_tab,
        EditorTab::Secrets,
        "← on collapsed header must NOT rewind the active tab"
    );
    Ok(())
}

/// Regression: `Tab` is intentionally *not* absorbed. Even when focused
/// on an `AgentHeader`, pressing `Tab` advances to the next editor tab.
/// Only `←` and `→` are owned by the header — `Tab` is the canonical
/// tab-cycle key and never participates in contextual absorption.
#[test]
fn tab_on_agent_header_advances_tab_normally() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config =
        seed_override_picker_workspace(&paths, temp.path(), &["agent-smith"], &["agent-smith"])?;
    let cwd = temp.path();
    let mut state = manager_on_secrets_tab(&config, cwd);
    // Cursor on AgentHeader (row 2 — ↓ from row 0 skips the spacer).
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Down))?;
    assert!(matches!(editor(&state).active_field, FieldFocus::Row(2)));

    // `Tab` from Secrets must advance to Auth (the next tab) regardless of focus.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    assert_eq!(
        editor(&state).active_tab,
        EditorTab::Auth,
        "Tab on a header must still advance the active tab"
    );
    Ok(())
}

// ── Launch-routing regression tests for commit 53 ────────────────────
//
// PR #171 review (Codex, Section 1: High): `ConsoleState.workspaces`
// was a snapshot built once at console startup. Manager edits
// (create / rename / edit / delete) rebuilt `ManagerState.workspaces`
// after each save but never refreshed the console-level snapshot, so
// the launch dispatcher kept reading stale data.
//
// Fix (Option B): drop the snapshot entirely and have
// `dispatch_launch_for_workspace` build a fresh `WorkspaceChoice` from
// the current `AppConfig` per call (`build_workspace_choice` in
// `console::state`). These tests pin the four operator-visible failure
// modes the bug produced — each would have failed against the
// pre-fix code.

/// A workspace created via `ConfigEditor` after the `ConsoleState` has
/// been built must be resolvable by the launch dispatcher in the same
/// session. Under the stale-snapshot bug, the new name was absent from
/// `ConsoleState.workspaces` and the dispatcher returned `Ok(None)`.
#[test]
fn launch_after_create_workspace_uses_fresh_data() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config =
        seed_config_with_agents(&paths, temp.path(), &["chainargos/agent-smith"], None)?;
    let cwd = temp.path();

    // Build the console state BEFORE the new workspace exists.
    let mut state = ConsoleState::new(&config, cwd)?;

    // Now create a second workspace via ConfigEditor — same code path
    // the manager save flow uses (no CLI subprocess, no UX detour).
    let host_path = cwd.display().to_string();
    let new_ws = WorkspaceConfig {
        workdir: host_path.clone(),
        mounts: vec![MountConfig {
            src: host_path.clone(),
            dst: host_path,
            readonly: false,
            isolation: jackin::isolation::MountIsolation::Shared,
        }],
        allowed_roles: vec!["chainargos/agent-smith".to_string()],
        default_role: Some("chainargos/agent-smith".to_string()),
        ..Default::default()
    };
    {
        let mut ce = ConfigEditor::open(&paths)?;
        ce.create_workspace("freshly-created", new_ws)?;
        config = ce.save()?;
    }

    // Dispatch a launch against the freshly-created name. The dispatcher
    // must build the choice from the current `config` and auto-select the
    // single role, proving it reads fresh data rather than a startup snapshot.
    let outcome = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("freshly-created".into()),
    )?;
    let (role, _workspace, agent) = outcome.expect(
        "freshly-created workspace must auto-select and return directly; under the bug, \
         ConsoleState.workspaces was a startup snapshot and didn't include the new name",
    );
    assert_eq!(role.key(), "chainargos/agent-smith");
    assert!(agent.is_none());
    Ok(())
}

/// A workspace renamed via `ConfigEditor` after the `ConsoleState` has
/// been built must be resolvable by its new name. Under the bug, the
/// dispatcher would look up the OLD name in the snapshot — find a
/// `WorkspaceChoice` whose `input` was `Saved(old)` — and the
/// preview-resolve step would then fail because `config.workspaces[old]`
/// no longer existed on disk.
#[test]
fn launch_after_rename_uses_new_name() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_agents(
        &paths,
        temp.path(),
        &["chainargos/agent-smith"],
        Some("chainargos/agent-smith"),
    )?;
    let cwd = temp.path();

    let mut state = ConsoleState::new(&config, cwd)?;

    // Rename "multi-role-ws" → "renamed-ws" via ConfigEditor.
    {
        let mut ce = ConfigEditor::open(&paths)?;
        ce.rename_workspace("multi-role-ws", "renamed-ws")?;
        config = ce.save()?;
    }

    // Dispatch against the new name — must resolve and auto-select the
    // single eligible role, proving the dispatcher uses the new name.
    let outcome = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("renamed-ws".into()),
    )?;
    let (role, _workspace, agent) = outcome
        .expect("renamed workspace must resolve under the new name and auto-select the role");
    assert_eq!(role.key(), "chainargos/agent-smith");
    assert!(agent.is_none());

    // OLD name must not resolve — under the snapshot bug it did.
    let stale_outcome = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    assert!(
        stale_outcome.is_none(),
        "the old (renamed-away) name must not resolve to a launch outcome; got {stale_outcome:?}"
    );
    Ok(())
}

/// Post-edit `default_role` must preselect the new default in the picker.
#[test]
fn launch_after_default_agent_change_preselects_new_default() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config_with_agents(
        &paths,
        temp.path(),
        &["chainargos/agent-smith", "chainargos/agent-brown"],
        // No default_role — two eligible roles → picker would open.
        None,
    )?;
    let cwd = temp.path();

    let mut state = ConsoleState::new(&config, cwd)?;

    // Confirm baseline: dispatch against the seeded workspace opens the
    // picker (no short-circuit).
    let baseline = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    assert!(
        baseline.is_none(),
        "baseline (no default_role) must open the picker, not direct-launch"
    );
    {
        let ConsoleStage::Manager(ms) = &mut state.stage;
        // Close the picker so the next dispatch can reopen / short-circuit.
        ms.list_modal = None;
    }
    state.pending_launch = None;

    // Now set default_role via ConfigEditor (same path the manager's
    // save flow drives via WorkspaceEdit { default_role: Some(_), .. }).
    {
        let mut ce = ConfigEditor::open(&paths)?;
        let edit = jackin::workspace::WorkspaceEdit {
            default_role: Some(Some("chainargos/agent-smith".to_string())),
            ..jackin::workspace::WorkspaceEdit::default()
        };
        ce.edit_workspace("multi-role-ws", edit)?;
        config = ce.save()?;
    }

    // Dispatch again — with the new default_role in config, the
    // dispatcher must preselect that role in the picker.
    let after = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    assert!(
        after.is_none(),
        "after default_role is set, dispatch must keep the operator in the picker flow"
    );
    let ConsoleStage::Manager(ms) = &state.stage;
    let picker = ms
        .inline_role_picker
        .as_ref()
        .expect("post-default dispatch must open the inline picker");
    let selected = picker
        .list_state
        .selected
        .expect("default role should be selected");
    assert_eq!(picker.filtered[selected].key(), "chainargos/agent-smith");
    Ok(())
}

/// Post-delete dispatch against the old name must return `Ok(None)`.
#[test]
fn launch_after_delete_workspace_does_not_resolve_old_choice() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    seed_config_with_agents(
        &paths,
        temp.path(),
        &["chainargos/agent-smith"],
        Some("chainargos/agent-smith"),
    )?;
    // Survivor so the delete leaves config non-empty.
    let host_path = temp.path().display().to_string();
    let second = WorkspaceConfig {
        workdir: host_path.clone(),
        mounts: vec![MountConfig {
            src: host_path.clone(),
            dst: host_path,
            readonly: false,
            isolation: jackin::isolation::MountIsolation::Shared,
        }],
        allowed_roles: vec!["chainargos/agent-smith".to_string()],
        default_role: Some("chainargos/agent-smith".to_string()),
        ..Default::default()
    };
    let mut config = {
        let mut ce = ConfigEditor::open(&paths)?;
        ce.create_workspace("survivor-ws", second)?;
        ce.save()?
    };
    let cwd = temp.path();

    let mut state = ConsoleState::new(&config, cwd)?;

    // Delete the first workspace via ConfigEditor.
    {
        let mut ce = ConfigEditor::open(&paths)?;
        ce.remove_workspace("multi-role-ws")?;
        config = ce.save()?;
    }

    // Attempt a launch against the deleted name — must no-op.
    let outcome = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("multi-role-ws".into()),
    )?;
    assert!(
        outcome.is_none(),
        "deleted workspace must not resolve to a launch outcome; under the bug, \
         the snapshot retained it and the dispatcher would have launched a ghost; \
         got {outcome:?}"
    );

    // Sanity: the surviving workspace still resolves and auto-selects its single role.
    let alive = state.dispatch_launch_for_workspace(
        &config,
        cwd,
        jackin::workspace::LoadWorkspaceInput::Saved("survivor-ws".into()),
    )?;
    let (role, _workspace, agent) =
        alive.expect("survivor-ws must still resolve and auto-select its role");
    assert_eq!(role.key(), "chainargos/agent-smith");
    assert!(agent.is_none());
    Ok(())
}

// ── Auth tab helpers ──────────────────────────────────────────────────

/// Return the flat-row index of the first `AuthRow` that matches `pred`.
fn auth_row_idx(
    ed: &EditorState<'_>,
    config: &AppConfig,
    pred: impl Fn(&AuthRow) -> bool,
) -> usize {
    auth_flat_rows(ed, config)
        .iter()
        .position(pred)
        .expect("required auth row not found")
}

// ── Auth tab integration test ─────────────────────────────────────────
//
// End-to-end coverage of the auth-form save path: open the form on a
// workspace × Claude row, set mode = api_key + a literal credential,
// commit the form, save the editor, reload from disk, and assert the
// persisted TOML carries BOTH the `auth_forward = "api_key"` block AND
// the `ANTHROPIC_API_KEY` env var.
//
// The bug this guards against: prior to the C1 fixup, the auth-form
// commit only mutated per-agent auth blocks in `editor.pending`, but
// `build_workspace_edit` / `WorkspaceEdit` carried no auth-forward
// field, so `edit_workspace` re-rendered the workspace table from the
// parsed-from-disk in-memory copy — silently overwriting the operator's
// mode change. The credential env var landed (env diff is wired
// separately) but the mode never reached disk; on reload, the resolver
// fell back to the global default and ignored the freshly-written key.
#[test]
#[allow(clippy::too_many_lines)]
fn auth_form_save_persists_mode_and_credential_to_disk() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    // Start the manager on the Auth tab, cursor on the workspace × Claude row.
    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config
        .workspaces
        .get("big-monorepo")
        .expect("seed must create big-monorepo")
        .clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    let ws_claude_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::WorkspaceMode {
                kind: AuthKind::Claude
            }
        )
    });
    ed.active_field = FieldFocus::Row(ws_claude_idx);
    state.stage = ManagerStage::Editor(ed);

    // Enter opens the auth-edit form modal on workspace × Claude.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::AuthForm { .. })),
        "Enter on WorkspaceMode/Claude must open AuthForm; got {:?}",
        editor(&state).modal
    );

    // Cycle mode: None → Sync → ApiKey (two Spaces).
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    // Tab advances to credential row; Enter opens the source picker;
    // Enter picks the default Plain text source.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::AuthSourcePicker { .. })),
        "credential row Enter must open AuthSourcePicker; got {:?}",
        editor(&state).modal
    );
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::TextInput { .. })),
        "Plain source must open credential text input; got {:?}",
        editor(&state).modal
    );
    // Type "sk-ant-test".
    for ch in "sk-ant-test".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    // Enter confirms text input, returning to the auth form Save button.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    // Enter commits the form.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        editor(&state).modal.is_none(),
        "auth-form save must close the modal"
    );

    // pending.claude reflects ApiKey, pending.env carries the credential.
    let pending = &editor(&state).pending;
    assert_eq!(
        pending.claude.as_ref().map(|c| c.auth_forward),
        Some(jackin::config::AuthForwardMode::ApiKey),
        "form commit must set workspace × claude mode in pending"
    );
    assert!(
        pending.env.contains_key("ANTHROPIC_API_KEY"),
        "form commit must set credential env var in pending"
    );

    // Save the editor: `s` opens the ConfirmSave modal (no collapses
    // expected here since the seed has a single mount), Enter commits
    // and bounces back to List.
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('s')),
    )?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Reload AppConfig from disk and assert both halves of the auth
    // change survived the round-trip.
    let reloaded = AppConfig::load_or_init(&paths)?;
    let ws_on_disk = reloaded
        .workspaces
        .get("big-monorepo")
        .expect("workspace must still exist on disk");
    assert_eq!(
        ws_on_disk.claude.as_ref().map(|c| c.auth_forward),
        Some(jackin::config::AuthForwardMode::ApiKey),
        "reload must see [claude] auth_forward = api_key in the workspace file"
    );
    let env_value = ws_on_disk
        .env
        .get("ANTHROPIC_API_KEY")
        .expect("reload must see ANTHROPIC_API_KEY in workspace env");
    match env_value {
        jackin::operator_env::EnvValue::Plain(s) => assert_eq!(s, "sk-ant-test"),
        jackin::operator_env::EnvValue::OpRef(_) => {
            panic!("expected literal credential, got OpRef")
        }
    }

    // Belt-and-braces: read the raw TOML and confirm the literal text is
    // there. Catches a future regression where a typed accessor papered
    // over a missing block (e.g. resolver fall-through).
    let toml = std::fs::read_to_string(paths.workspaces_dir.join("big-monorepo.toml"))?;
    assert!(
        toml.contains("[claude]"),
        "raw TOML must carry the workspace claude block; got:\n{toml}"
    );
    assert!(
        toml.contains(r#"auth_forward = "api_key""#),
        "raw TOML must carry auth_forward = \"api_key\"; got:\n{toml}"
    );
    assert!(
        toml.contains(r#"ANTHROPIC_API_KEY = "sk-ant-test""#),
        "raw TOML must carry the credential env var; got:\n{toml}"
    );
    Ok(())
}

#[test]
fn auth_credential_source_enter_opens_source_picker() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config
        .workspaces
        .get("big-monorepo")
        .expect("seed must create big-monorepo")
        .clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    let ws_claude_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::WorkspaceMode {
                kind: AuthKind::Claude
            }
        )
    });
    ed.active_field = FieldFocus::Row(ws_claude_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    let Some(Modal::AuthSourcePicker { state: picker }) = &editor(&state).modal else {
        panic!(
            "Enter on auth credential source must open AuthSourcePicker; got {:?}",
            editor(&state).modal
        );
    };
    assert_eq!(picker.key, "ANTHROPIC_API_KEY");

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::AuthForm { .. })),
        "Esc from AuthSourcePicker must restore the auth form"
    );

    Ok(())
}

#[test]
fn auth_add_role_override_flow_uses_selected_auth_kind() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config
        .workspaces
        .get("big-monorepo")
        .expect("seed must create big-monorepo")
        .clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);

    let sentinel_idx = auth_row_idx(&ed, &config, |r| matches!(r, AuthRow::AddSentinel { .. }));
    ed.active_field = FieldFocus::Row(sentinel_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::AuthRolePicker { .. })),
        "Enter on AddSentinel must open AuthRolePicker; got {:?}",
        editor(&state).modal
    );

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        match &editor(&state).modal {
            Some(Modal::AuthForm {
                target:
                    jackin::console::manager::state::AuthFormTarget::WorkspaceRole { role, kind },
                ..
            }) => {
                assert_eq!(*kind, AuthKind::Claude);
                assert!(
                    !role.is_empty(),
                    "role must propagate from AuthRolePicker → AuthForm"
                );
                true
            }
            _ => false,
        },
        "Enter on AuthRolePicker must open AuthForm for selected auth kind; got {:?}",
        editor(&state).modal
    );
    Ok(())
}

#[test]
fn auth_role_header_left_right_toggles_expansion() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let over = WorkspaceRoleOverride {
        claude: Some(jackin::config::AgentAuthConfig {
            auth_forward: jackin::config::AuthForwardMode::Ignore,
        }),
        ..Default::default()
    };
    ws.roles.insert("the-architect".into(), over);

    let mut state = ManagerState::from_config(&config, cwd);
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.tab_bar_focused = false;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    let header_idx = auth_row_idx(&ed, &config, |r| matches!(r, AuthRow::RoleHeader { .. }));
    ed.active_field = FieldFocus::Row(header_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Right))?;
    assert!(editor(&state).auth_expanded.contains("the-architect"));
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Left))?;
    assert!(!editor(&state).auth_expanded.contains("the-architect"));
    Ok(())
}

#[test]
fn auth_role_header_d_clears_selected_auth_kind_override() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let over = WorkspaceRoleOverride {
        claude: Some(jackin::config::AgentAuthConfig {
            auth_forward: jackin::config::AuthForwardMode::Ignore,
        }),
        codex: Some(
            jackin::config::CodexAuthConfig::new(jackin::config::AgentAuthConfig {
                auth_forward: jackin::config::AuthForwardMode::ApiKey,
            })
            .unwrap(),
        ),
        ..Default::default()
    };
    ws.roles.insert("the-architect".into(), over);

    let mut state = ManagerState::from_config(&config, cwd);
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    let header_idx = auth_row_idx(&ed, &config, |r| matches!(r, AuthRow::RoleHeader { .. }));
    ed.active_field = FieldFocus::Row(header_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('d')),
    )?;
    assert!(
        editor(&state).modal.is_none(),
        "selected auth-kind clear should not open a confirm modal"
    );
    let role_over = editor(&state)
        .pending
        .roles
        .get("the-architect")
        .expect("override entry stays even after clear");
    assert!(role_over.claude.is_none());
    assert!(
        role_over.codex.is_some(),
        "hidden Codex override must be untouched"
    );
    Ok(())
}

#[test]
fn auth_role_agent_row_d_silently_clears_single_agent() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let over = WorkspaceRoleOverride {
        claude: Some(jackin::config::AgentAuthConfig {
            auth_forward: jackin::config::AuthForwardMode::Ignore,
        }),
        codex: Some(
            jackin::config::CodexAuthConfig::new(jackin::config::AgentAuthConfig {
                auth_forward: jackin::config::AuthForwardMode::ApiKey,
            })
            .unwrap(),
        ),
        ..Default::default()
    };
    ws.roles.insert("the-architect".into(), over);

    let mut state = ManagerState::from_config(&config, cwd);
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(jackin::console::manager::auth_kind::AuthKind::Claude);
    ed.auth_expanded.insert("the-architect".into());
    let claude_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::RoleMode {
                kind: AuthKind::Claude,
                ..
            }
        )
    });
    ed.active_field = FieldFocus::Row(claude_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('d')),
    )?;
    assert!(
        editor(&state).modal.is_none(),
        "single-agent clear must be silent (no modal)"
    );
    let ro = editor(&state).pending.roles.get("the-architect").unwrap();
    assert!(ro.claude.is_none());
    assert!(ro.codex.is_some(), "Codex override must be untouched");
    Ok(())
}

/// Pressing Enter on an `AuthKind` row sets `auth_selected_kind` and
/// resets the cursor to row 0. Drives the kind picker → focused-view
/// transition through the real keystroke dispatcher (every other auth
/// integration test pre-seeds `auth_selected_kind`, which would mask
/// a regression in the input-layer wiring).
#[test]
fn auth_enter_on_auth_kind_focuses_selected_agent() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    // No `auth_selected_kind` set — picker view.
    let codex_kind_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::AuthKindRow {
                kind: AuthKind::Codex
            }
        )
    });
    ed.active_field = FieldFocus::Row(codex_kind_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    let ed = editor(&state);
    assert_eq!(ed.auth_selected_kind, Some(AuthKind::Codex));
    assert!(
        matches!(ed.active_field, FieldFocus::Row(0)),
        "Enter on AuthKind must reset cursor to Row(0); got {:?}",
        ed.active_field
    );
    Ok(())
}

/// Esc on the focused auth view pops back to the kind picker WITHOUT
/// triggering the dirty-modal flow even when `editor.is_dirty()`.
/// Pending edits stay in `editor.pending`; a subsequent Esc on the
/// picker would fall through to the dirty check.
#[test]
fn auth_esc_from_focused_view_pops_to_picker_without_dirty_modal() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    // Mutate `pending` so `is_dirty()` is true — this is what would
    // otherwise route Esc through `Modal::SaveDiscardCancel`.
    ed.pending.git_pull_on_entry = !ed.pending.git_pull_on_entry;
    ed.active_field = FieldFocus::Row(0);
    state.stage = ManagerStage::Editor(ed);
    assert!(editor(&state).is_dirty(), "fixture must be dirty");

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;

    let ed = editor(&state);
    assert!(
        ed.auth_selected_kind.is_none(),
        "Esc on focused view must clear auth_selected_kind"
    );
    assert!(
        ed.modal.is_none(),
        "Esc must not open the dirty SaveDiscardCancel modal mid-tab"
    );
    assert!(
        ed.is_dirty(),
        "pending edits must survive the in-tab navigation pop"
    );
    Ok(())
}

/// Tab/BackTab leaving the Auth tab clears `auth_selected_kind` so
/// re-entering the tab returns to the kind picker rather than a
/// stale focused view.
#[test]
fn auth_tab_cycle_off_auth_clears_selected_agent() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.tab_bar_focused = false;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    state.stage = ManagerStage::Editor(ed);

    // Tab from content returns to tab bar and advances to next tab (General).
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    let ed = editor(&state);
    assert_ne!(ed.active_tab, EditorTab::Auth);
    assert!(
        ed.auth_selected_kind.is_none(),
        "leaving the Auth tab must drop the focused-kind selection"
    );
    Ok(())
}

/// Pressing D on a `WorkspaceSource` row clears the workspace-level
/// auth-forward override for the focused kind. Mirrors the existing
/// `RoleSource` clear test but for the workspace layer.
#[test]
fn auth_workspace_source_d_clears_workspace_mode() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut ws = config.workspaces.get("big-monorepo").unwrap().clone();
    ws.claude = Some(jackin::config::AgentAuthConfig {
        auth_forward: jackin::config::AuthForwardMode::ApiKey,
    });
    ws.env.insert(
        "ANTHROPIC_API_KEY".into(),
        jackin::operator_env::EnvValue::Plain("k".into()),
    );

    let mut state = ManagerState::from_config(&config, cwd);
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    let source_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::WorkspaceSource {
                kind: AuthKind::Claude
            }
        )
    });
    ed.active_field = FieldFocus::Row(source_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('d')),
    )?;

    let ed = editor(&state);
    assert!(
        ed.modal.is_none(),
        "workspace source clear must be silent (no modal)"
    );
    assert!(
        ed.pending.claude.is_none(),
        "D on WorkspaceSource must drop the workspace-level claude override"
    );
    Ok(())
}

/// Cancelling the credential `Modal::TextInput` (the literal-text leg
/// of the source-picker round trip) must restore `Modal::AuthForm` and
/// drain `pending_auth_form_return` — not silently leave the operator
/// on a blank Auth tab.
#[test]
fn auth_credential_text_input_cancel_restores_form() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    let ws_claude_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::WorkspaceMode {
                kind: AuthKind::Claude
            }
        )
    });
    ed.active_field = FieldFocus::Row(ws_claude_idx);
    state.stage = ManagerStage::Editor(ed);

    // Open form → cycle to api_key → Tab to credential row → Enter →
    // SourcePicker → Enter (Plain) → TextInput → Esc.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::AuthSourcePicker { .. })
    ));
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(matches!(
        editor(&state).modal,
        Some(Modal::TextInput { .. })
    ));

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Esc))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::AuthForm { .. })),
        "TextInput cancel must restore AuthForm; got {:?}",
        editor(&state).modal
    );
    assert!(
        editor(&state).pending_auth_form_return.is_none(),
        "TextInput cancel must drain pending_auth_form_return"
    );
    Ok(())
}

/// `op_available = false` must propagate through the auth-form ↔
/// `AuthSourcePicker` handoff, so operators on hosts without `op` see
/// the picker with the 1Password choice disabled.
#[test]
fn auth_source_picker_op_disabled_when_op_missing() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    state.op_available = false;
    let ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Claude);
    let ws_claude_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::WorkspaceMode {
                kind: AuthKind::Claude
            }
        )
    });
    ed.active_field = FieldFocus::Row(ws_claude_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    let Some(Modal::AuthSourcePicker { state: picker }) = &editor(&state).modal else {
        panic!(
            "Enter on credential source must open AuthSourcePicker; got {:?}",
            editor(&state).modal
        );
    };
    assert!(
        !picker.op_available,
        "op_available must propagate from ManagerState through to the picker"
    );
    Ok(())
}

// ── GitHub auth-tab integration tests ────────────────────────────────
//
// End-to-end coverage of the new GitHub kind on the Auth tab.
// Mirror-shape with `auth_form_save_persists_mode_and_credential_to_disk`
// for Claude — open the form on the workspace × Github row, set mode =
// token + a literal `GH_TOKEN`, commit, save, reload from disk, and
// assert the persisted TOML carries BOTH the `[github]`
// auth_forward = "token" block AND the `GH_TOKEN` env var on the
// matching `[github.env]` block.
#[allow(clippy::too_many_lines)]
#[test]
fn github_auth_form_save_persists_token_mode_and_gh_token_to_disk() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut state = ManagerState::from_config(&config, cwd);
    let ws = config
        .workspaces
        .get("big-monorepo")
        .expect("seed must create big-monorepo")
        .clone();
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Github);
    let ws_github_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::WorkspaceMode {
                kind: AuthKind::Github
            }
        )
    });
    ed.active_field = FieldFocus::Row(ws_github_idx);
    state.stage = ManagerStage::Editor(ed);

    // Enter opens the auth-edit form on workspace × Github.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::AuthForm { .. })),
        "Enter on WorkspaceMode/Github must open AuthForm; got {:?}",
        editor(&state).modal
    );

    // Cycle mode: None → Sync → Token (two presses through the
    // [Sync, Token, Ignore] cycle).
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char(' ')),
    )?;
    // Tab → credential row, Enter → source picker, Enter → plain text.
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Tab))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::AuthSourcePicker { .. })),
        "credential row Enter must open AuthSourcePicker; got {:?}",
        editor(&state).modal
    );
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        matches!(editor(&state).modal, Some(Modal::TextInput { .. })),
        "Plain source must open credential text input; got {:?}",
        editor(&state).modal
    );
    for ch in "ghp_round_trip".chars() {
        handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Char(ch)))?;
    }
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    assert!(
        editor(&state).modal.is_none(),
        "auth-form save must close the modal"
    );

    // pending.github reflects Token + GH_TOKEN landed on the github
    // env block (NOT the regular workspace env).
    let pending = &editor(&state).pending;
    let github = pending
        .github
        .as_ref()
        .expect("workspace github block must be set in pending");
    assert_eq!(github.auth_forward, jackin::config::GithubAuthMode::Token);
    let value = github
        .env
        .get("GH_TOKEN")
        .expect("GH_TOKEN must land on the workspace github env block");
    match value {
        jackin::operator_env::EnvValue::Plain(s) => assert_eq!(s, "ghp_round_trip"),
        jackin::operator_env::EnvValue::OpRef(_) => panic!("expected literal credential"),
    }
    assert!(
        !pending.env.contains_key("GH_TOKEN"),
        "GH_TOKEN must not leak into the regular workspace env map"
    );

    // Save the editor (s opens ConfirmSave, Enter commits).
    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('s')),
    )?;
    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;

    // Reload AppConfig from disk and assert the round-trip.
    let reloaded = AppConfig::load_or_init(&paths)?;
    let ws_on_disk = reloaded
        .workspaces
        .get("big-monorepo")
        .expect("workspace must still exist on disk");
    let github_on_disk = ws_on_disk
        .github
        .as_ref()
        .expect("[github] block must be on disk after save");
    assert_eq!(
        github_on_disk.auth_forward,
        jackin::config::GithubAuthMode::Token
    );
    let env_value = github_on_disk
        .env
        .get("GH_TOKEN")
        .expect("reload must see GH_TOKEN on the github env block");
    match env_value {
        jackin::operator_env::EnvValue::Plain(s) => assert_eq!(s, "ghp_round_trip"),
        jackin::operator_env::EnvValue::OpRef(_) => panic!("expected literal credential"),
    }
    // GH_TOKEN must NOT leak into the regular workspace env map after
    // reload (the kind-scoped layer is the only place it should live).
    assert!(
        !ws_on_disk.env.contains_key("GH_TOKEN"),
        "GH_TOKEN must not appear in [env]; only in [github.env]"
    );

    // Belt-and-braces: read the raw workspace TOML and confirm the
    // literal text landed on `[github]` / `[github.env]`, not `[env]`.
    let toml = std::fs::read_to_string(paths.workspaces_dir.join("big-monorepo.toml"))?;
    assert!(
        toml.contains("[github]"),
        "raw TOML must carry the workspace github block; got:\n{toml}"
    );
    assert!(
        toml.contains(r#"auth_forward = "token""#),
        "raw TOML must carry auth_forward = \"token\"; got:\n{toml}"
    );
    assert!(
        toml.contains(r#"GH_TOKEN = "ghp_round_trip""#),
        "raw TOML must carry GH_TOKEN; got:\n{toml}"
    );
    Ok(())
}

/// `D` on a Github `RoleHeader` clears the role's
/// `[roles.<role>.github]` override end-to-end through
/// the input dispatcher (in addition to the unit-level coverage in
/// `src/console/manager/input/auth.rs`).
#[test]
fn github_role_header_d_clears_github_role_override() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    let mut ws = config.workspaces.get("big-monorepo").unwrap().clone();
    let over = WorkspaceRoleOverride {
        github: Some(jackin::config::GithubAuthConfig {
            auth_forward: jackin::config::GithubAuthMode::Ignore,
            ..Default::default()
        }),
        ..Default::default()
    };
    ws.roles.insert("the-architect".into(), over);

    let mut state = ManagerState::from_config(&config, cwd);
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Github);
    let header_idx = auth_row_idx(&ed, &config, |r| {
        matches!(
            r,
            AuthRow::RoleHeader { role, .. } if role == "the-architect"
        )
    });
    ed.active_field = FieldFocus::Row(header_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(
        &mut state,
        &mut config,
        &paths,
        cwd,
        key(KeyCode::Char('d')),
    )?;
    let role_entry = editor(&state)
        .pending
        .roles
        .get("the-architect")
        .expect("override entry must remain after D");
    assert!(
        role_entry.github.is_none(),
        "D on github RoleHeader must clear the role's github override"
    );
    Ok(())
}

/// Integration counterpart for the unit-level
/// `github_role_override_picker_filters_already_overridden_roles` —
/// drives the picker open through the keystroke dispatcher with the
/// github kind selected and asserts the candidate list filters out a
/// role that already carries a `[…github]` override.
#[test]
fn github_role_override_picker_filters_already_overridden_roles_via_dispatcher() -> Result<()> {
    let temp = tempdir()?;
    let paths = JackinPaths::for_tests(temp.path());
    let mut config = seed_config(&paths, temp.path())?;
    let cwd = temp.path();

    // Pre-seed a workspace × role × github override on `the-architect`
    // so the picker should filter it out and only offer other roles.
    let mut ws = config.workspaces.get("big-monorepo").unwrap().clone();
    ws.roles.insert(
        "the-architect".into(),
        WorkspaceRoleOverride {
            github: Some(jackin::config::GithubAuthConfig {
                auth_forward: jackin::config::GithubAuthMode::Ignore,
                ..Default::default()
            }),
            ..Default::default()
        },
    );

    let mut state = ManagerState::from_config(&config, cwd);
    let mut ed = EditorState::new_edit("big-monorepo".into(), ws);
    ed.active_tab = EditorTab::Auth;
    ed.auth_selected_kind = Some(AuthKind::Github);
    let sentinel_idx = auth_row_idx(&ed, &config, |r| matches!(r, AuthRow::AddSentinel { .. }));
    ed.active_field = FieldFocus::Row(sentinel_idx);
    state.stage = ManagerStage::Editor(ed);

    handle_key(&mut state, &mut config, &paths, cwd, key(KeyCode::Enter))?;
    let Some(Modal::AuthRolePicker { state: picker }) = &editor(&state).modal else {
        panic!(
            "Enter on AddSentinel must open AuthRolePicker for github kind; got {:?}",
            editor(&state).modal
        );
    };
    let labels: Vec<String> = picker
        .roles
        .iter()
        .map(jackin::selector::RoleSelector::key)
        .collect();
    assert!(
        !labels.iter().any(|s| s == "the-architect"),
        "the-architect already has a github override and must be filtered out; got {labels:?}"
    );
    Ok(())
}
