fix(tasks): address v2 review follow-ups - #1338
Merged
Merged
Conversation
…1335) #1331 — Stale soft-stop handler after disk-path view exit Clear `setSoftStopHandler` to null before returning in the disk-only path of `enterTaskViewMode`. Previously `wireEscapeToExit` installed a handler that was never cleaned up when the function returned immediately (no tail loop), leaving a stale closure wired to the surface. #1332 — viewingTaskId tracking inoperative Add `ictxRef` module-level ref and `setTasksIctx(ictx)` export in tasks.ts. Call it from bootstrap.ts after `ctx` is constructed. Thread `ictx: ictxRef` into both `TaskViewEntry` constructions (list→Enter path and direct `/tasks:view` path) so `viewingTaskId` is now set and cleared correctly during task view mode. #1333 — Add live tail tests Add three new test cases in task-view.test.ts under "enterTaskViewMode — live tail": 1. Natural stream completion → exitTaskViewMode called, FOOTER_COMPLETE emitted, viewingTaskId cleared. 2. Esc/abort fires → abort handler invoked, viewingTaskId cleared via wireEscapeToExit path. 3. setSoftStopHandler override wired correctly — final call passes null to clear the handler on natural completion. #1334 — Silent .catch() swallows errors Change `.catch(() => resolve())` → `.catch((e) => { ctx.out.error?.(…); resolve(); })` so task view errors are surfaced via the OutputSink error channel instead of silently discarded. #1335 — renderEventsView is dead code Remove the `renderEventsView` export from task-view.ts (zero callers confirmed by grep). Also remove the now-unused `TaskViewOptions` interface, `DEFAULT_MAX_EVENTS` constant, and `formatOutputEvent` / `OutputEvent` imports that existed solely to support it. Fixes #1331, #1332, #1333, #1334, #1335.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses all five follow-up issues from PR #1313 review.
Closes #1331, #1332, #1333, #1334, #1335.
Changes
#1331 — Stale soft-stop handler after disk-path view exit (medium bug)
src/cli/commands/interactive/task-view-mode.tsAdded
entry.ctx.setSoftStopHandler?.(null)before the earlyreturnin the disk-only path ofenterTaskViewMode. PreviouslywireEscapeToExitinstalled a closure that was never cleaned up — the disk path returns immediately without entering a tail loop, leaving a stale handler wired to the surface.#1332 — viewingTaskId tracking inoperative (medium bug)
src/cli/slash/commands/tasks.ts,src/cli/commands/interactive/bootstrap.tsAdded a
setTasksIctx(ictx: InteractiveCtx)export andictxRefmodule-level variable totasks.ts. Called frombootstrap.tsafterctxis constructed. Threadedictx: ictxRefinto bothTaskViewEntryconstructions (list→Enter path and direct/tasks:viewpath) soviewingTaskIdis correctly set and cleared during task view mode.#1333 — Add live tail tests (low)
src/cli/commands/interactive/task-view.test.tsAdded three test cases under
enterTaskViewMode — live tail:exitTaskViewModecalled,FOOTER_COMPLETEemitted,viewingTaskIdclearedviewingTaskIdcleared viawireEscapeToExitpathsetSoftStopHandleroverride is wired correctly — final call passesnullto clear handler on natural completion#1334 — Silent .catch() swallows errors (nit bug)
src/cli/slash/commands/tasks.tsChanged
.catch(() => resolve())to.catch((e) => { ctx.out.error?.(\task view error: ${String(e)}`); resolve(); })so errors are surfaced via theOutputSink.error` channel instead of being silently discarded.#1335 — renderEventsView is dead code (low)
src/cli/commands/interactive/task-view.tsRemoved the
renderEventsViewexport (confirmed zero callers with grep). Also removed the now-unusedTaskViewOptionsinterface,DEFAULT_MAX_EVENTSconstant, andformatOutputEvent/OutputEventimports that existed solely to support it. Updated module doc comment accordingly.Test Results
pnpm lintpasses (tsc --noEmit clean). All touched files are within the 350 code-line ceiling.