fix(cli-rs): show permission-blocked tools outside their group

2026-11-14 20:21:39 +03:00 · 2026-11-06 23:42:02 +01:00
parent 2bd8f22e83
commit 7df6b414dd
4 changed files with 154 additions and 27 deletions
@@ -7,7 +7,7 @@
 //! is open or not grouped, its children.

 use std::cell::{Cell, RefCell};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::rc::Rc;

 use opencode_api::generated::{
@@ -54,7 +54,7 @@ pub enum Node {
    Group(Group),
 }

-/// A group row (`SessionGroup` without the permission partition, which E.9a's pending tools need).
+/// A group row (`SessionGroup`).
 #[derive(Clone, Debug, PartialEq)]
 pub struct Group {
    pub kind: Kind,
@@ -63,6 +63,8 @@ pub struct Group {
    pub size: usize,
    /// A later row closed the group, or every thought in it finished.
    pub completed: bool,
+    /// The tool rows below that wait on a permission request, which show outside the group ([`partition_pending`]).
+    pub pending: Vec<Spec>,
 }

 impl Group {
@@ -276,6 +278,26 @@ pub fn project(rows: Vec<Spec>, messages: &[SessionMessageInfo], verbosity: Verb
    out
 }

+/// `partitionPending`: moves the exploration tools waiting on a permission request (by tool call ID) to the end
+/// of their group, keeping the order otherwise, and records the waiting rows of exploration and activity groups.
+pub fn partition_pending(specs: &mut [Spec], pending: &HashSet<&str>) {
+    let blocked =
+        |spec: &Spec| matches!(spec, Spec::Part { part: Part::Tool(id), .. } if pending.contains(id.as_str()));
+    for spec in specs {
+        let Spec::Group(group) = spec else { continue };
+        if !matches!(group.kind, Kind::Exploration | Kind::Activity) {
+            continue;
+        }
+        if group.kind == Kind::Exploration {
+            let (waiting, rest): (Vec<Node>, Vec<Node>) = std::mem::take(&mut group.children)
+                .into_iter()
+                .partition(|node| matches!(node, Node::Entry(spec) if blocked(spec)));
+            group.children = rest.into_iter().chain(waiting).collect();
+        }
+        group.pending = group.entries().into_iter().filter(|spec| blocked(spec)).cloned().collect();
+    }
+}
+
 /// Appends a row along its path (`appendEntry`). The flag is `closesPrevious`.
 fn append(nodes: &mut Vec<(Node, bool)>, spec: Spec, path: &[Kind], closes: bool) {
    let Some((&kind, rest)) = path.split_first() else {
@@ -291,7 +313,7 @@ fn append(nodes: &mut Vec<(Node, bool)>, spec: Spec, path: &[Kind], closes: bool
    }
    let mut children = Vec::new();
    append_child(&mut children, spec, rest);
-    nodes.push((Node::Group(Group { kind, children, size: 1, completed: false }), closes));
+    nodes.push((Node::Group(Group { kind, children, size: 1, completed: false, pending: Vec::new() }), closes));
 }

 fn append_child(nodes: &mut Vec<Node>, spec: Spec, path: &[Kind]) {
@@ -308,7 +330,7 @@ fn append_child(nodes: &mut Vec<Node>, spec: Spec, path: &[Kind]) {
    }
    let mut children = Vec::new();
    append_child(&mut children, spec, rest);
-    nodes.push(Node::Group(Group { kind, children, size: 1, completed: false }));
+    nodes.push(Node::Group(Group { kind, children, size: 1, completed: false, pending: Vec::new() }));
 }

 /// A row's grouping path (`partPath`, `messagePath`) and whether it closes the group before it.
@@ -554,6 +576,8 @@ struct Inner {
    root: View,
    header: View,
    body: View,
+    /// The root group's permission-blocked rows, below it even when it is closed (`PendingEntries`).
+    outside: View,
    level: usize,
    id: RefCell<Option<String>>,
    kind: Cell<Option<Kind>>,
@@ -561,6 +585,7 @@ struct Inner {
    /// The header that is mounted, so a hover only recolors it.
    painted: RefCell<Option<(Header, Painted)>>,
    children: RefCell<Vec<(ChildKey, Child)>>,
+    outside_children: RefCell<Vec<(ChildKey, Child)>>,
    /// The latest thought's title (`latest`), kept while the next thought has none yet.
    latest: RefCell<Option<String>>,
 }
@@ -626,7 +651,8 @@ impl GroupView {
    pub fn new(cx: &Context<'_>, settings: &Rc<Settings>, level: usize) -> Self {
        let header = View::new(cx.app).padding_edge(Edge::Left, 3);
        let body = View::new(cx.app).column().shrink(0.0);
-        let root = View::new(cx.app).column().shrink(0.0).child(&header).child(&body);
+        let outside = View::new(cx.app).column().shrink(0.0);
+        let root = View::new(cx.app).column().shrink(0.0).child(&header).child(&body).child(&outside);
        let inner = Rc::new(Inner {
            app: cx.app.clone(),
            theme: cx.theme.clone(),
@@ -634,12 +660,14 @@ impl GroupView {
            root,
            header,
            body,
+            outside,
            level,
            id: RefCell::default(),
            kind: Cell::default(),
            hover: Cell::new(false),
            painted: RefCell::default(),
            children: RefCell::default(),
+            outside_children: RefCell::default(),
            latest: RefCell::default(),
        });
        inner.header.mouse_pointer(MousePointer::POINTER).on_click(bind!(inner, |inner, cx, _| {
@@ -668,8 +696,14 @@ impl GroupView {
        &self.0.root
    }

-    /// Renders `group`; `completed` is the group's own flag or what its parent passes down.
+    /// Renders a root `group`; `completed` is the group's own flag.
    pub fn update(&self, cx: &Context<'_>, group: &Group, completed: bool) {
+        self.render(cx, group, completed, &group.pending, true);
+    }
+
+    /// Renders `group` without the rows in `pending`; `completed` is what its parent passes down. The root group
+    /// shows the pending rows after it (`outside`).
+    fn render(&self, cx: &Context<'_>, group: &Group, completed: bool, pending: &[Spec], outside: bool) {
        let inner = &self.0;
        let id = group.id(inner.level);
        inner.kind.set(Some(group.kind));
@@ -684,8 +718,8 @@ impl GroupView {
        let grouped = inner.settings.grouped(group.kind, cx.hide_thinking);
        let (header, mode) = match group.kind {
            Kind::Reasoning => self.reasoning(cx, group, completed, expanded, grouped),
-            Kind::Exploration => self.exploration(cx, group, completed, expanded, grouped),
-            Kind::Activity => self.activity(cx, group, completed, expanded),
+            Kind::Exploration => self.exploration(cx, group, completed, expanded, grouped, pending),
+            Kind::Activity => self.activity(cx, group, completed, expanded, pending),
            Kind::Instructions => self.instructions(cx, group, expanded),
        };
        inner.paint(header);
@@ -693,7 +727,10 @@ impl GroupView {
        inner.body.margin_edge(Edge::Top, if group.kind == Kind::Activity && mode.is_some() { 1 } else { 0 });
        inner.body.gap(if group.kind == Kind::Activity { 1 } else { 0 });
        inner.body.padding_edge(Edge::Left, if mode == Some(Mode::Thought) { 3 } else { 0 });
-        inner.children(cx, children, mode.unwrap_or(Mode::Normal), completed);
+        inner.children(&inner.body, &inner.children, cx, children, mode.unwrap_or(Mode::Normal), completed, pending);
+        let waiting: Vec<Node> =
+            if outside { pending.iter().map(|spec| Node::Entry(spec.clone())).collect() } else { Vec::new() };
+        inner.children(&inner.outside, &inner.outside_children, cx, &waiting, Mode::Normal, completed, &[]);
    }

    fn reasoning(
@@ -769,6 +806,7 @@ impl GroupView {
        completed: bool,
        expanded: bool,
        grouped: bool,
+        pending: &[Spec],
    ) -> (Option<Header>, Option<Mode>) {
        if !grouped {
            return (None, Some(Mode::Normal));
@@ -776,6 +814,7 @@ impl GroupView {
        let tools: Vec<_> = group
            .entries()
            .into_iter()
+            .filter(|spec| !pending.contains(spec))
            .filter_map(|spec| match spec {
                Spec::Part { message, part } => match part.resolve(cx.assistant(message)?)? {
                    SessionMessageAssistantContentItem::Tool(tool) => Some(tool),
@@ -803,10 +842,12 @@ impl GroupView {
        group: &Group,
        completed: bool,
        expanded: bool,
+        pending: &[Spec],
    ) -> (Option<Header>, Option<Mode>) {
        let entries = group.entries();
        let items: Vec<_> = entries
            .iter()
+            .filter(|spec| !pending.contains(spec))
            .filter_map(|spec| {
                let Spec::Part { message, part } = spec else {
                    return None;
@@ -845,7 +886,8 @@ impl GroupView {

    /// The content y of each mounted row inside the group, with its message, for navigation.
    pub fn anchors(&self, out: &mut Vec<(String, f64)>) {
-        for (key, child) in self.0.children.borrow().iter() {
+        let (children, outside) = (self.0.children.borrow(), self.0.outside_children.borrow());
+        for (key, child) in children.iter().chain(outside.iter()) {
            match (key, child) {
                (ChildKey::Entry(_, message, _), Child::Row(row)) => {
                    out.push((message.clone(), row.wrapper.rect().screen_y))
@@ -943,11 +985,26 @@ impl Inner {
        }
    }

-    /// Mounts `nodes` in order, reusing the children that are still there (`Children`).
-    fn children(&self, cx: &Context<'_>, nodes: &[Node], mode: Mode, completed: bool) {
-        let mut children = self.children.borrow_mut();
+    /// Mounts `nodes` in `target` in order, reusing the children in `store` that are still there, and leaves out
+    /// the rows in `pending` (`Children`).
+    #[allow(clippy::too_many_arguments)]
+    fn children(
+        &self,
+        target: &View,
+        store: &RefCell<Vec<(ChildKey, Child)>>,
+        cx: &Context<'_>,
+        nodes: &[Node],
+        mode: Mode,
+        completed: bool,
+        pending: &[Spec],
+    ) {
+        let waiting = |node: &Node| matches!(node, Node::Entry(spec) if pending.contains(spec));
+        let mut children = store.borrow_mut();
        let mut old = std::mem::take(&mut *children);
        for (index, node) in nodes.iter().enumerate() {
+            if waiting(node) {
+                continue;
+            }
            let level = self.level + 1;
            let (key, child) = match old.iter().position(|(other, _)| other.is(node, mode, level)) {
                Some(position) => old.remove(position),
@@ -973,17 +1030,17 @@ impl Inner {
                    }
                }
                (Child::Group(view), Node::Group(group)) => {
-                    // A nested group closes when its parent did, when anything follows it, or when every
-                    // thought in it finished.
+                    // A nested group closes when its parent did, when anything but a pending row follows it, or
+                    // when every thought in it finished.
                    let completed = completed
-                        || index + 1 < nodes.len()
+                        || nodes[index + 1..].iter().any(|next| !waiting(next))
                        || (group.kind == Kind::Reasoning
                            && group.entries().iter().all(|spec| {
                                thought(cx, spec).is_some_and(|(_, part)| {
                                    part.time.as_ref().and_then(|time| time.completed).is_some()
                                })
                            }));
-                    view.update(cx, group, completed);
+                    view.render(cx, group, completed, pending, false);
                }
                _ => {}
            }
@@ -992,9 +1049,9 @@ impl Inner {
        for (_, child) in &old {
            child_root(child).detach();
        }
-        self.body.clear();
+        target.clear();
        for (_, child) in children.iter() {
-            self.body.add(child_root(child));
+            target.add(child_root(child));
        }
    }
 }
@@ -1130,6 +1187,7 @@ mod tests {
            ],
            size: 2,
            completed: true,
+            pending: Vec::new(),
        });
        assert_eq!(settings.weight(&group, true), 1);
        let Spec::Group(inner) = &group else { unreachable!() };
@@ -1154,4 +1212,28 @@ mod tests {
        let items: Vec<_> = tools.iter().map(Activity::Tool).collect();
        assert_eq!(activity_summary(&items, 1, true), ("2 reads, 3 tools, 1 instruction".to_owned(), false));
    }
+
+    #[test]
+    fn pending_tools_go_last_in_exploration_and_are_recorded_in_activity() {
+        let entry = |id: &str| Node::Entry(part("msg_1", Part::Tool(id.into())));
+        let group = |kind, ids: &[&str]| {
+            Spec::Group(Group {
+                kind,
+                children: ids.iter().map(|id| entry(id)).collect(),
+                size: ids.len(),
+                completed: false,
+                pending: Vec::new(),
+            })
+        };
+        let mut specs = vec![group(Kind::Exploration, &["a", "b", "c"]), group(Kind::Activity, &["a", "b", "c"])];
+        partition_pending(&mut specs, &HashSet::from(["a", "b"]));
+        let Spec::Group(exploration) = &specs[0] else { unreachable!() };
+        assert_eq!(exploration.children, [entry("c"), entry("a"), entry("b")]);
+        let pending = vec![part("msg_1", Part::Tool("a".into())), part("msg_1", Part::Tool("b".into()))];
+        assert_eq!(exploration.pending, pending);
+        // Activity groups keep their order.
+        let Spec::Group(activity) = &specs[1] else { unreachable!() };
+        assert_eq!(activity.children, [entry("a"), entry("b"), entry("c")]);
+        assert_eq!(activity.pending, pending);
+    }
 }
@@ -553,6 +553,14 @@ impl Inner {
            let mut rows = reduce(messages, self.view.get().turn_tokens.is_some());
            rows.retain(|spec| !matches!(spec, Spec::Message(id) if queued.contains(id.as_str())));
            let mut specs = grouping::project(rows, messages, self.groups.verbosity.get());
+            let blocked: HashSet<&str> = state
+                .permissions(&session)
+                .unwrap_or_default()
+                .iter()
+                .filter_map(|request| request.source.as_ref().filter(|source| source.r#type == "tool"))
+                .map(|source| source.id.as_str())
+                .collect();
+            grouping::partition_pending(&mut specs, &blocked);
            // Compactions waiting in the inbox go last (`createSessionRows`).
            specs.extend(state.pending(&session).iter().filter_map(|item| match item {
                SessionInboxInfo::Compaction(compaction) => Some(Spec::CompactionQueued(compaction.id.0.clone())),
@@ -101,6 +101,29 @@ fn grouped() -> Value {
    ])
 }

+/// A running answer whose second read waits on a permission request.
+fn blocked() -> Value {
+    json!([
+        {
+            "id": "msg_2", "type": "assistant", "agent": "build", "model": { "providerID": "demo", "id": "other" },
+            "content": [
+                {
+                    "type": "tool", "id": "call_a", "name": "read",
+                    "state": { "status": "completed", "input": { "filePath": "src/a.rs" }, "content": [] },
+                    "time": { "created": 1100, "completed": 1200 },
+                },
+                {
+                    "type": "tool", "id": "call_b", "name": "read",
+                    "state": { "status": "running", "input": { "filePath": "src/b.rs" }, "metadata": {} },
+                    "time": { "created": 1300 },
+                },
+            ],
+            "time": { "created": 1000 },
+        },
+        { "id": "msg_1", "type": "user", "text": "Read both", "time": { "created": 1 } },
+    ])
+}
+
 fn routes() -> Vec<MemoryRoute> {
    // The worktree's catalogs answer for the worktree, so the store files them there.
    let mut routes: Vec<MemoryRoute> = [(DIRECTORY, None), (WORKTREE, Some(WORKTREE))]
@@ -141,12 +164,23 @@ fn sessions() -> Vec<MemoryRoute> {
        MemoryRoute::json("GET", "/api/config", 200, &json!([])),
        MemoryRoute::json("GET", "/api/session", 200, &json!({ "data": [], "cursor": {} })),
        MemoryRoute::json("GET", "/api/session/ses_g/message", 200, &json!({ "data": grouped(), "cursor": {} })),
+        MemoryRoute::json("GET", "/api/session/ses_p/message", 200, &json!({ "data": blocked(), "cursor": {} })),
+        MemoryRoute::json(
+            "GET",
+            "/api/session/ses_p/permission",
+            200,
+            &json!({ "data": [{
+                "id": "per_1", "sessionID": "ses_p", "action": "read", "resources": ["elsewhere"],
+                "source": { "type": "tool", "messageID": "msg_2", "id": "call_b" },
+            }] }),
+        ),
        MemoryRoute::json("GET", "/api/session/:id/message", 200, &json!({ "data": [], "cursor": {} })),
    ];
    for (id, directory, title, agent) in [
        ("ses_a", DIRECTORY, "Fix the parser", None),
        ("ses_b", DIRECTORY, "Write the docs", Some("build")),
        ("ses_g", DIRECTORY, "Grouped reads", None),
+        ("ses_p", DIRECTORY, "Blocked read", None),
        ("ses_wt", WORKTREE, "Worktree work", None),
    ] {
        routes.push(MemoryRoute::json(
@@ -352,3 +386,14 @@ fn an_opened_group_stays_open_when_the_session_shows_again() {
    h.wait_for("Explored");
    h.wait_for("Reading file");
 }
+
+#[test]
+fn a_tool_waiting_on_a_permission_shows_outside_its_closed_group() {
+    let h = mount(session_route("ses_p"), Args::default(), animations_off());
+    // `partitionPending`: the header counts the finished read only, and the blocked one shows below it.
+    h.wait_for("Explored: 1 read");
+    let screen = h.app.text();
+    assert!(!screen.contains("src/a.rs"), "{screen}");
+    let header = screen.find("Explored: 1 read").unwrap();
+    assert!(screen[header..].contains("src/b.rs"), "{screen}");
+}
@@ -1,8 +0,0 @@
-# V15: permission-blocked tools stay in their group
-
-E.8's scrolling, navigation, groups, and per-tab scroll anchors match the TypeScript cells (`E.8/navigate` and
-`E.8/tab-anchor`: every checkpoint has zero differing cells). One difference remains; no checkpoint shows it.
-
-Permission-blocked tools in exploration and activity groups (`partitionPending`, `grouping/session.ts:163-176`) are
-not split out: a pending tool stays inside its collapsed group. The permission prompt (E.9a) shows the request below
-the transcript, so nothing is hidden. Reason: E.9a and E.7a own the pending tool rows.