fix(tasks): log errors from enterTaskViewMode instead of swallowing (#1334) - #1357
fix(tasks): log errors from enterTaskViewMode instead of swallowing (#1334)#1357griffinwork40 wants to merge 1 commit into
Conversation
…1334) - The fix itself (in src/cli/slash/commands/tasks.ts) was already applied in ed882c3 as part of the #1332 restore commit. - This commit adds the missing test coverage: a new case in tasks.test.ts verifies that when enterTaskViewMode rejects, the error is forwarded to ctx.out.error() ('task view error: ...') rather than silently dropped. - Adds a vi.mock wrapper for task-view-mode.js so tests control enterTaskViewMode without touching disk or real TUI state. Updates the pre-existing 'active handle with history' assertion to match the mocked boundary (call-site verification instead of rendered-output check).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96ed4d1673
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await new Promise<void>((r) => setTimeout(r, 50)); | ||
| process.stdin.emit('data', '\r'); |
There was a problem hiding this comment.
On a slow or loaded filesystem, collectAllTasks() may take longer than 50 ms because it awaits SubagentLogReader.list() before registering the stdin listener. In that case this timer fires first, the simulated Enter event is lost, and handlerPromise hangs until the 10-second test timeout. Synchronize on an observable registration signal, such as the setSoftStopHandler mock being called, rather than relying on a fixed delay.
Useful? React with 👍 / 👎.
1 participant
Fixes #1334.
The
.catch(() => resolve())onenterTaskViewModesilently swallowed all errors with no diagnostic signal. Now logs the error viactx.out.errorbefore resolving.Note: the production fix was already present in
ed882c31(restore hunks dropped in #1315 squash merge). This PR adds the missing test coverage.Changes
src/cli/slash/commands/tasks.test.ts-- new test assertingenterTaskViewModerejection logs the error; updated mock infrastructureVerification
pnpm lintclean