Skip to content

feat(jobs): /jobs panel + status-bar slot + output view + kill confirm#3

Open
ssjoleary wants to merge 3 commits into
mainfrom
phase-11b-jobs
Open

feat(jobs): /jobs panel + status-bar slot + output view + kill confirm#3
ssjoleary wants to merge 3 commits into
mainfrom
phase-11b-jobs

Conversation

@ssjoleary
Copy link
Copy Markdown
Contributor

Summary

  • New src/eca_cli/jobs.clj:jobs state slice, jobs/updated handler, /jobs panel, on-demand jobs/readOutput popup, and d-key kill confirm flow that dispatches jobs/kill on y.
  • Status-bar indicator [N jobs] (wide) / [Nj] (narrow); hidden when no jobs.
  • Panel groups rows by chatLabel, shows <emoji> <summary> · <elapsed> per row using server-supplied status emoji (🟡 running · ✅ completed · 🔴 failed · ⚫ killed) and server-supplied elapsed string (no client clock).
  • Push-only refresh: server emits jobs/updated on every lifecycle change. No polling timer.

Tracks Phase 11b in docs/roadmap.md.

Decisions worth flagging

  • Kill UX: d on row → confirm modal Kill <summary>? [y/n]jobs/kill on y. Matches eca-emacs (eca-jobs.el:122-139) and aligns with the existing :approving mode for destructive actions.
  • Output view: on-demand snapshot via jobs/readOutput, rendered in a transient popup with [stderr] prefix on stderr lines. Mirrors eca-emacs (eca-jobs.el:141-183).
  • Circular require avoided between jobs.cljview.clj via requiring-resolve in view.clj; jobs depends on view for divider/rebuild-lines, view calls jobs lazily for status-bar + panel renderers.
  • Picker filtering deliberately not wired for the jobs panel — only navigation keys (up/down/pgup/pgdn) fall through to cl/list-update. d/y/n/Esc are handled by jobs/handle-key before the generic dispatcher.
  • Empty /jobs shows a system message ("No background jobs") rather than an empty picker.

Status-bar slot order

Frozen as part of Wave 1 architecture decision:
```
workspace · loading · model · agent · variant · [MCPs:n/m] · [N jobs] · tokens · cost · ctx% · title · trust
```
This PR owns the `[N jobs]` slot. The `[MCPs:n/m]` slot is delivered by the Phase 7 PR (#2).

Test plan

  • `bb test` — 98 tests, 444 assertions, 0 failures
  • New `test/eca_cli/jobs_test.clj`: handler-replaces-map, status-bar widths, panel render grouping, kill confirm flow, kill cancel branches, output fetch on Enter, output popup render, panel/popup Escape, commands registration
  • clj-kondo: 0 warnings on `jobs.clj` / `jobs_test.clj`

brepl smoke

```
width=160 -> "[2 jobs]"
width=120 -> "[2 jobs]"
width=80 -> "[2j]"
empty -> nil
```

Panel:
```
Background Jobs (Enter: output · d: kill · Esc: close)
───
── Chat A ──
▸ 🟡 build 5s
🔴 lint 30s exit:2

── Chat B ──
✅ test 1m
```

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a /jobs panel for background shell jobs: a new eca-cli.jobs namespace owns the :jobs state slice, the jobs/updated notification handler, a [N jobs] status-bar fragment, a chat-grouped picker panel with on-demand output popup (jobs/readOutput) and a d/y kill-confirm flow (jobs/kill). The jobs ↔ view circular require is broken by requiring-resolve in view.clj; the panel keys are intercepted in state.clj before the generic picker dispatcher.

Changes:

  • New src/eca_cli/jobs.clj plus test/eca_cli/jobs_test.clj and bb.edn test wiring.
  • view.clj lazy-resolves jobs renderers (panel/output/confirm) and adds the [N jobs] status-bar slot.
  • state.clj/commands.clj/protocol.clj register the jobs/updated notification, :eca-jobs-output runtime msg, /jobs command, and jobs/list|kill|readOutput request helpers.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/eca_cli/jobs.clj New namespace: state slice, handlers, renderers, key dispatch, kill/output cmds.
src/eca_cli/view.clj Lazy jobs-fn resolver, picker delegation for :kind :jobs, [N jobs] status-bar slot.
src/eca_cli/state.clj Wires jobs/updated, :eca-jobs-output, :jobs initial state, picker-key routing.
src/eca_cli/commands.clj Registers /jobs and cmd-open-jobs-panel thin wrapper.
src/eca_cli/protocol.clj Adds jobs-list!, jobs-kill!, jobs-read-output! helpers.
test/eca_cli/jobs_test.clj New tests for handler, status bar, panel, kill confirm, output popup, escape, command registry.
bb.edn Registers eca-cli.jobs-test in the bb test task.
Comments suppressed due to low confidence (6)

src/eca_cli/jobs.clj:119

  • Selection index can desynchronize from the rendered rows for jobs whose :chatLabel is missing/nil. jobs-vec (used to populate :filtered) sorts using #(or (:chatLabel %) "") (empty string), but render-jobs-panel-lines groups by #(or (:chatLabel %) "Unknown Chat") and then sorts groups by key. Empty string sorts before all real labels, while "Unknown Chat" generally sorts after them — so the visual flat order can differ from the :filtered order. As a result, the highlighted row () and the job acted on by Enter/d (which uses :filtered via selected-job) can refer to different jobs. Use the same fallback string in both places.
        groups       (->> filtered
                          (group-by #(or (:chatLabel %) "Unknown Chat"))
                          (sort-by key))

src/eca_cli/jobs.clj:164

  • The output popup renders the entire :lines collection as a single newline-joined string with no clipping or scrolling. For long-running shell jobs the buffered output can easily exceed the terminal height, pushing the header and Esc: back to panel footer off-screen and making the popup unusable. Consider truncating to (:height state) (e.g. tail N lines) or wiring a scrollable list component.
(defn render-output-popup-lines
  "Returns the output popup view: header + buffered lines (stderr highlighted)."
  [state]
  (let [{:keys [job-id data]} (:jobs-view state)
        job     (get-in state [:jobs job-id])
        status  (or (:status data) (:status job) "unknown")
        exit    (:exitCode data)
        label   (truncate-label (or (:summary job) (:label job) job-id) 80)
        header  (str label "  ·  status=" status
                     (when (some? exit) (str "  ·  exit=" exit)))
        lines   (:lines data)
        body    (if (seq lines)
                  (mapv (fn [{:keys [stream text]}]
                          (if (= "stderr" stream)
                            (str "[stderr] " text)
                            text))
                        lines)
                  ["(no output)"])
        footer  "Esc: back to panel"]
    (str/join "\n"
              (into [header (view/divider (:width state))]
                    (conj body (view/divider (:width state)) footer)))))

src/eca_cli/jobs.clj:105

  • The picker is built with (cl/item-list labels :height 8) from compact panel-row-label strings, but render-jobs-panel-lines ignores those rendered labels entirely and re-renders its own grouped view. The list component's labels are dead state: they're never displayed, never re-synced when :jobs is updated by a subsequent jobs/updated notification, and only the selected-index is used. This is confusing and means the panel rows shown to the user will go stale (still showing the original job set) until the user reopens /jobs, even though :jobs and the status-bar [N jobs] will update. Consider either re-deriving rows from :jobs on each render or refreshing the picker on jobs/updated while the panel is open.
      (let [labels (mapv panel-row-label jobs)]
        [(-> state
             (assoc :mode :picking
                    :picker {:kind     :jobs
                             :list     (cl/item-list labels :height 8)
                             :all      jobs
                             :filtered jobs
                             :query    ""})
             (update :input ti/reset))
         nil]))))

src/eca_cli/jobs.clj:231

  • handle-panel-key dispatches all unhandled messages to cl/list-update, including non-key-press messages and printable character key-presses. As a result, typing characters such as y, letters, or pasted text while the panel is open are forwarded into the list component (and silently swallowed) instead of being treated as no-ops. If filtering is "deliberately not wired", consider only forwarding the navigation keys (up/down/pgup/pgdn) explicitly, as stated in the PR description, rather than passing every message through.
    :else
    (let [[new-list _] (cl/list-update (get-in state [:picker :list]) msg)]
      [(assoc-in state [:picker :list] new-list) nil])))

src/eca_cli/jobs.clj:199

  • handle-output-key swallows all non-Escape messages with [state nil]. While the output popup is open the user has no way to dismiss it via q, Enter, or anything other than Escape; more importantly, regular runtime/tick messages won't be reachable here because state.clj only routes to jobs/handle-key for key presses dispatched from the input pipeline, but any future routing of additional message types (e.g. resize) would also be dropped if it ever lands here. At minimum it would be friendlier to also accept q to close the popup, mirroring common TUI conventions.
(defn- handle-output-key [state msg]
  (cond
    (and (msg/key-press? msg) (msg/key-match? msg :escape))
    [(close-overlay state) nil]

    :else [state nil]))

src/eca_cli/jobs.clj:131

  • The exit-code badge is only shown for jobs whose status is exactly "failed". Jobs that are "killed" or that "completed" with a non-zero exit code (depending on the server's status taxonomy) won't surface their exit code, even though the row label still suggests something went wrong. Consider showing exit:N whenever :exitCode is present and non-zero (or for any terminal status), rather than gating on "failed" only.
                             exit    (when (and (= "failed" (:status job)) (:exitCode job))
                                       (str "  exit:" (:exitCode job)))]
                         (str marker emoji " " summary "  " elapsed (or exit ""))))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/eca_cli/jobs.clj Outdated
Comment on lines +69 to +74
(let [p (promise)]
(protocol/jobs-read-output! srv job-id
(fn [r] (deliver p (or (:result r) {}))))
{:type :eca-jobs-output
:job-id job-id
:data (deref p 10000 {:lines [] :status "unknown" :exitCode nil})}))))
ssjoleary and others added 2 commits May 15, 2026 19:42
Addresses PR #3 review: read-output-cmd and 6 other cmd fns blocked
the command-executor thread on (deref p 10000 ...) waiting for the
server response, stalling the TUI for up to 10s per call and
blocking jobs/updated processing. All 7 sites now return immediately
and dispatch results via runtime messages on the existing queue,
mirroring the :eca-jobs-output pattern already in use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse the 4-site truncate-label + (or :summary :label id) duplication
into job-summary; name the 60/4 magic numbers in the chat-group header.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants