feat(jobs): /jobs panel + status-bar slot + output view + kill confirm#3
feat(jobs): /jobs panel + status-bar slot + output view + kill confirm#3ssjoleary wants to merge 3 commits into
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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.cljplustest/eca_cli/jobs_test.cljandbb.edntest wiring. view.cljlazy-resolves jobs renderers (panel/output/confirm) and adds the[N jobs]status-bar slot.state.clj/commands.clj/protocol.cljregister thejobs/updatednotification,:eca-jobs-outputruntime msg,/jobscommand, andjobs/list|kill|readOutputrequest 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
:chatLabelis missing/nil.jobs-vec(used to populate:filtered) sorts using#(or (:chatLabel %) "")(empty string), butrender-jobs-panel-linesgroups 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:filteredorder. As a result, the highlighted row (▸) and the job acted on byEnter/d(which uses:filteredviaselected-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
:linescollection 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 andEsc: back to panelfooter 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 compactpanel-row-labelstrings, butrender-jobs-panel-linesignores 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:jobsis updated by a subsequentjobs/updatednotification, and only theselected-indexis 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:jobsand the status-bar[N jobs]will update. Consider either re-deriving rows from:jobson each render or refreshing the picker onjobs/updatedwhile 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-keydispatches all unhandled messages tocl/list-update, including non-key-press messages and printable character key-presses. As a result, typing characters such asy, 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-keyswallows all non-Escape messages with[state nil]. While the output popup is open the user has no way to dismiss it viaq,Enter, or anything other than Escape; more importantly, regular runtime/tick messages won't be reachable here becausestate.cljonly routes tojobs/handle-keyfor 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 acceptqto 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 showingexit:Nwhenever:exitCodeis 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.
| (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})})))) |
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>
Summary
src/eca_cli/jobs.clj—:jobsstate slice,jobs/updatedhandler,/jobspanel, on-demandjobs/readOutputpopup, andd-key kill confirm flow that dispatchesjobs/killony.[N jobs](wide) /[Nj](narrow); hidden when no jobs.chatLabel, shows<emoji> <summary> · <elapsed>per row using server-supplied status emoji (🟡 running · ✅ completed · 🔴 failed · ⚫ killed) and server-supplied elapsed string (no client clock).jobs/updatedon every lifecycle change. No polling timer.Tracks Phase 11b in
docs/roadmap.md.Decisions worth flagging
don row → confirm modalKill <summary>? [y/n]→jobs/killony. Matches eca-emacs (eca-jobs.el:122-139) and aligns with the existing:approvingmode for destructive actions.jobs/readOutput, rendered in a transient popup with[stderr]prefix on stderr lines. Mirrors eca-emacs (eca-jobs.el:141-183).jobs.clj↔view.cljviarequiring-resolvein view.clj; jobs depends on view fordivider/rebuild-lines, view calls jobs lazily for status-bar + panel renderers.up/down/pgup/pgdn) fall through tocl/list-update.d/y/n/Esc are handled byjobs/handle-keybefore the generic dispatcher./jobsshows 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
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
```