Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant enhancement to the task scheduler by integrating an interactive TTY model. The core objective is to provide a robust mechanism for interactive tasks to run without interference from other concurrent processes, while also ensuring that injected sub-tasks correctly inherit and respect these interactive barriers. This change improves the reliability and predictability of task execution, particularly for user-facing interactive commands, and addresses potential deadlock scenarios in complex task dependencies.

Highlights

• Interactive TTY Scheduler Barrier: Implemented a minimal interactive TTY model within the existing DAG scheduler, ensuring interactive tasks acquire a full semaphore to prevent concurrent unrelated runtime tasks. Injected descendants bypass permit acquisition, running under the parent's barrier, and mixed runtime+injected chains are handled to avoid deadlocks.
• Task Schema and CLI Exposure: Added an interactive boolean property to task schemas and exposed this field in tasks info --json, tasks ls --json, and MCP mise://tasks output.
• TTY State Restoration and Ctrl-C Handling: Ensured terminal state is restored after raw/interactive execution on Unix systems. Enhanced Ctrl-C handling to track running PIDs and allow for graceful interruption of tasks.
• Improved Scheduler Logic: Refactored permit acquisition in the scheduler to support ordered acquisition, handle interactive task barriers, and prevent deadlocks in mixed task chains, especially under --jobs=1.
• New E2E Tests: Added several end-to-end tests to validate the new interactive scheduler behavior, including scenarios for mixed parent/injected tasks, deadlock regressions, and interactive PTY usage.

Greptile Summary

This PR introduces a minimal interactive TTY scheduler barrier into mise's existing DAG task scheduler. Interactive tasks (interactive=true) now acquire all available semaphore permits (max_permits) to prevent concurrent unrelated tasks from running, while injected sub-tasks of an interactive parent bypass permit acquisition via env-marker propagation (__MISE_INTERACTIVE_CHAIN, __MISE_PARENT_PERMIT_CHAIN). A ticket-based ordering mechanism (permit_order_next / permit_order_cursor / Notify) is added to prevent start-order inversions under contention. The PR also adds: TTY state save/restore after raw/interactive execution, PID tracking in RUNNING_PIDS for interactive processes so Ctrl-C can kill them, a sticky INTERRUPTED flag to drive is_stopping() without requiring a task failure, a deps.rs sort for deterministic leaf ordering, and schema/JSON/MCP exposure of the interactive field.

Key findings:

• Interactive barrier & ordering logic: The permit-acquisition mechanism and ticket-based ordering are correct and well-tested by the comprehensive e2e suite. The double-subscribe race fix in pump_deps/inject_and_wait correctly eliminates a completion-event hang.
• TTY validation timing issue: The ensure!(stdin.is_terminal()) check in task_executor.rs fires inside the JoinSet task, after max_permits have been acquired. On misconfiguration (non-TTY stdin), all other queued work is briefly blocked while the error propagates. Moving this check earlier would improve latency on this error path.
• Overall correctness: The core semaphore-barrier mechanism is logically sound, permit counting and env-marker propagation work correctly, and the refactored spawn-and-acquire pattern maintains proper ordering invariants.

Confidence Score: 4/5

• Safe to merge; one minor logic issue flagged (TTY validation timing) that improves error-path performance but doesn't affect core functionality.
• The interactive scheduler barrier mechanism is correct and well-tested. The permit-ordering protocol, env-marker propagation, and double-subscribe race fix are all sound. The single flagged issue (TTY check after semaphore acquisition) is a latency concern on misconfiguration, not a correctness bug. The happy path and primary interactive task execution are unaffected.
• src/task/task_executor.rs (TTY validation placement)

Sequence Diagram

sequenceDiagram
    participant Sched as Scheduler Loop
    participant SpawnJob as spawn_sched_job
    participant AcqGuard as acquire_permit_guard
    participant Semaphore as Semaphore (max_permits)
    participant Executor as TaskExecutor
    participant InjectWait as inject_and_wait

    Sched->>SpawnJob: (interactive_task, deps)
    SpawnJob->>SpawnJob: permit_count = max_permits<br/>permit_order = fetch_add(order_next)
    SpawnJob->>SpawnJob: in_flight++
    SpawnJob->>AcqGuard: acquire_permit_guard(permit_count, permit_order)
    AcqGuard->>AcqGuard: wait until cursor == order
    AcqGuard->>Semaphore: acquire_many_owned(max_permits) ← blocks all other tasks
    Semaphore-->>AcqGuard: permit granted
    AcqGuard->>AcqGuard: cursor++, notify_waiters()
    AcqGuard-->>SpawnJob: (permit, should_run=true)
    SpawnJob->>Executor: run_task_sched(interactive_task)
    Executor->>Executor: ensure! stdin is_terminal()
    Executor->>Executor: stdin/stdout/stderr = Stdio::inherit()
    Executor->>InjectWait: inject_and_wait(injected_children,<br/>interactive_barrier=true, parent_holds_permit=false)
    InjectWait->>InjectWait: add __MISE_INTERACTIVE_CHAIN=1 to env
    InjectWait->>Sched: send injected child tasks (permit_count=0, bypass semaphore)
    Sched-->>InjectWait: children complete
    InjectWait-->>Executor: done
    Executor-->>SpawnJob: Ok(())
    SpawnJob->>Semaphore: drop permit (max_permits released)
    SpawnJob->>SpawnJob: in_flight--

    Note over Sched,Semaphore: Regular tasks queued after interactive task<br/>are blocked in ordering wait until permit released

_{Last reviewed commit: c49c340}

Thanks for iterating on this. The PR is still more complex than it needs to be though. The semaphore-draining approach (acquire_many_owned(max_permits)), ticket-lock ordering (permit_order_next/permit_order_cursor/Notify), and env var propagation (__MISE_INTERACTIVE_CHAIN/__MISE_PARENT_PERMIT_CHAIN) are all solving problems that wouldn't exist with the right primitive.

The goal is simple: only one interactive task runs at a time, and no other tasks run concurrently with it. This maps directly to a tokio::sync::RwLock<()>:

• Normal tasks acquire a read lock (many can run concurrently, gated by the existing --jobs semaphore as today)
• Interactive tasks acquire a write lock (exclusive — waits for all running tasks to finish, blocks new ones from starting)

Here's roughly what it should look like in spawn_sched_job:

// Add to SpawnContext:
pub interactive_lock: Arc<tokio::sync::RwLock<()>>,

// In spawn_sched_job, after permit acquisition:
let _interactive_guard = if task.interactive {
    Some(ctx.interactive_lock.write_owned().await)
} else {
    Some(ctx.interactive_lock.read_owned().await)  
};

That's it for the scheduler side. No ticket locks, no env var markers, no semaphore draining. The RwLock semantics naturally handle everything — concurrent readers, exclusive writer, proper ordering.

The parts of this PR that are fine and should be kept:

• Adding interactive field to Task + schema + JSON output
• Terminal state save/restore in cmd.rs
• Forcing stdin/stdout/stderr=inherit for interactive tasks in the executor
• TTY validation (ensure!(stdin.is_terminal()))

The parts that should be removed/replaced:

• permit_order_next/permit_order_cursor/permit_order_notify (ticket lock)
• __MISE_INTERACTIVE_CHAIN/__MISE_PARENT_PERMIT_CHAIN env vars
• acquire_many_owned(max_permits) semaphore draining
• permit_count_for_task / acquire_permit_guard
• validate_injected_interactive_non_tty (50+ lines of graph walking)
• All the helper functions in task_helpers.rs (task_is_interactive_chain, task_is_parent_permit_chain, task_propagates_interactive_barrier, task_uses_runtime_slot)

Could you simplify this down to the RwLock approach?

This comment was generated by Claude Code.

Thanks again for the review and discussion here.

I opened a new PR that supersedes this one: #8473.

I also want to clarify a misunderstanding on my side: I hadn’t understood that the intended RwLock scope was the finest task runtime segment (i.e. only around actual command execution), rather than the entire task lifecycle. The new PR follows that segment-level locking model.

This new PR keeps the implementation intentionally minimal and avoids the extra ticket/env-marker/semaphore-drain complexity discussed here.

Closing #8464 in favor of #8473.