refactor: use Promise.withResolvers() (Node 22+) by marcoscaceres · Pull Request #503 · speced/respec-web-services

marcoscaceres · 2026-04-25T11:32:16Z

Summary

Replace new Promise((resolve, reject) => { ... }) with Promise.withResolvers() in 3 call sites:

utils/background-task-queue.ts — worker message listener and module registration
utils/sh.ts — shell exec wrapper

Cleaner code, same behavior. Node 22+ native API.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors internal async flows to use Promise.withResolvers() instead of new Promise((resolve, reject) => ...), aiming for cleaner promise wiring while keeping behavior consistent on Node 22+.

Changes:

Refactor sh() exec wrapper to use Promise.withResolvers() for completion signaling.
Refactor BackgroundTaskQueue worker message listeners and module registration to use Promise.withResolvers().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
utils/sh.ts	Replaces manual Promise constructor with `Promise.withResolvers()` around child process lifecycle.
utils/background-task-queue.ts	Replaces Promise constructor usages in worker response handling and init registration with `Promise.withResolvers()`.

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

Copilot

Pull request overview

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

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

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

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

Copilot

Pull request overview

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

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

Replace new Promise((resolve, reject) => { ... }) with the cleaner Promise.withResolvers() pattern in background-task-queue.ts (2 sites) and sh.ts (1 site).

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

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

Copilot

Pull request overview

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

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

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

marcoscaceres · 2026-04-27T08:12:12Z

@copilot Can you fix and resolve unaddressed comments and get this happy again?

…e.ts - sh.ts: Remove broken duplicate Promise.withResolvers + new Promise code; use Promise.withResolvers() properly; switch exit->close for complete stdio flush; add error event handler for spawn failures; make rejection shapes consistent (both reject with Error instances with command/stdout/stderr/code fields) - background-task-queue: Store split2 stream refs so off() targets the right stream and unpipe on cleanup; add worker error/exit handling in run() to prevent deadlocks on unexpected worker failures; add worker error/exit handling in activate() to prevent indefinite hangs during initialization Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/4640ebb0-d6ac-4269-b7bf-e563038f39e1

…bugging Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/4640ebb0-d6ac-4269-b7bf-e563038f39e1

Copilot · 2026-04-27T08:18:09Z

@copilot Can you fix and resolve unaddressed comments and get this happy again?

Fixed in eef7301. Here's what was addressed:

utils/sh.ts (commit 220d437):

Removed the broken duplicate Promise.withResolvers + new Promise code
Switched from exit to close event so stdio streams are fully flushed before settling
Added error event handler for spawn failures (promise no longer hangs indefinitely)
Made rejection shapes consistent — both error and non-zero exit paths now reject with an Error instance carrying { command, stdout, stderr, code } fields

utils/background-task-queue.ts (commits 220d437, eef7301):

Stored split2 stream references so off() and unpipe() target the correct transform stream (not worker.stdout/stderr directly)
Added worker error/exit listeners in run() — if the worker dies mid-job, the lock is released and the promise rejects with the job ID in the error message instead of deadlocking the queue
Added worker error/exit listeners in activate() — if the worker crashes during module initialization, the promise rejects rather than hanging indefinitely

marcoscaceres · 2026-04-27T08:51:54Z

@copilot can you review this and fix any issues that come up?

…k release Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5a7c6559-a65f-44e5-865b-b8a2f0b8c955

marcoscaceres · 2026-04-27T09:08:41Z

@copilot can you review this again and fix any issues

marcoscaceres · 2026-04-27T20:01:21Z

@copilot Can you confirm that all your comments have been addressed? Please resolve them if you’re happy. Can you fix anything outstanding from your feedback that hasn’t been addressed?

Copilot · 2026-04-27T20:05:19Z

@copilot Can you confirm that all your comments have been addressed? Please resolve them if you’re happy. Can you fix anything outstanding from your feedback that hasn’t been addressed?

All reviewer comments have been addressed. Here's the final status:

utils/sh.ts: close event (complete stdio flush), error handler (no more indefinite hangs on spawn failure), consistent Error-instance rejection shape with { command, stdout, stderr, code } on all paths.

utils/background-task-queue.ts: Split2 streams stored in variables so off()/unpipe() target the correct transform; run() has cross-removing error/exit handlers that release the lock and reject on worker death; activate() has the same pattern to prevent init hangs.

CI tests are passing on the latest commit (a3bd437).

* refactor: use Promise.withResolvers() (Node 22+) Replace new Promise((resolve, reject) => { ... }) with the cleaner Promise.withResolvers() pattern in background-task-queue.ts (2 sites) and sh.ts (1 site). * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: address all PR review feedback on sh.ts and background-task-queue.ts - sh.ts: Remove broken duplicate Promise.withResolvers + new Promise code; use Promise.withResolvers() properly; switch exit->close for complete stdio flush; add error event handler for spawn failures; make rejection shapes consistent (both reject with Error instances with command/stdout/stderr/code fields) - background-task-queue: Store split2 stream refs so off() targets the right stream and unpipe on cleanup; add worker error/exit handling in run() to prevent deadlocks on unexpected worker failures; add worker error/exit handling in activate() to prevent indefinite hangs during initialization Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/4640ebb0-d6ac-4269-b7bf-e563038f39e1 * fix: add job ID to worker unexpected-exit error message for easier debugging Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/4640ebb0-d6ac-4269-b7bf-e563038f39e1 * fix: cross-remove error/exit listeners in run() to prevent double lock release Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5a7c6559-a65f-44e5-865b-b8a2f0b8c955 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

marcoscaceres requested a review from Copilot April 25, 2026 12:26

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread utils/sh.ts

Comment thread utils/sh.ts Outdated

Comment thread utils/sh.ts Outdated

marcoscaceres force-pushed the chore/promise-withresolvers branch from 9df2706 to 889eecb Compare April 25, 2026 12:42

marcoscaceres requested a review from Copilot April 25, 2026 13:08

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 13:08 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread utils/background-task-queue.ts Outdated

marcoscaceres requested a review from Copilot April 25, 2026 13:13

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 13:13 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread utils/background-task-queue.ts Outdated

Comment thread utils/sh.ts

marcoscaceres requested a review from Copilot April 25, 2026 13:38

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 13:38 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread utils/sh.ts

refactor: use Promise.withResolvers() (Node 22+)

87d7f76

Replace new Promise((resolve, reject) => { ... }) with the cleaner Promise.withResolvers() pattern in background-task-queue.ts (2 sites) and sh.ts (1 site).

marcoscaceres force-pushed the chore/promise-withresolvers branch from 889eecb to 87d7f76 Compare April 26, 2026 00:17

marcoscaceres requested a review from Copilot April 26, 2026 01:20

Copilot started reviewing on behalf of marcoscaceres April 26, 2026 01:20 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread utils/sh.ts

Comment thread utils/background-task-queue.ts

Comment thread utils/background-task-queue.ts

marcoscaceres requested a review from Copilot April 26, 2026 03:38

Copilot started reviewing on behalf of marcoscaceres April 26, 2026 03:39 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread utils/sh.ts

marcoscaceres requested a review from Copilot April 26, 2026 03:59

Copilot started reviewing on behalf of marcoscaceres April 26, 2026 03:59 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Apply suggestion from @Copilot

787d3c0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of marcoscaceres April 27, 2026 08:12 View session

Copilot AI added 2 commits April 27, 2026 08:16

fix: add job ID to worker unexpected-exit error message for easier de…

eef7301

…bugging Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/4640ebb0-d6ac-4269-b7bf-e563038f39e1

Copilot finished work on behalf of marcoscaceres April 27, 2026 08:20

Copilot started work on behalf of marcoscaceres April 27, 2026 08:55 View session

fix: cross-remove error/exit listeners in run() to prevent double loc…

5e4dd8a

…k release Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/5a7c6559-a65f-44e5-865b-b8a2f0b8c955

Copilot finished work on behalf of marcoscaceres April 27, 2026 09:02

Copilot started work on behalf of marcoscaceres April 27, 2026 09:08 View session

Merge branch 'main' into chore/promise-withresolvers

a3bd437

Copilot started work on behalf of marcoscaceres April 27, 2026 20:01 View session

Copilot finished work on behalf of marcoscaceres April 27, 2026 20:06

marcoscaceres marked this pull request as ready for review April 27, 2026 20:08

marcoscaceres requested a review from sidvishnoi April 27, 2026 20:11

sidvishnoi approved these changes Apr 28, 2026

View reviewed changes

marcoscaceres merged commit befebf4 into main Apr 28, 2026
4 checks passed

marcoscaceres deleted the chore/promise-withresolvers branch April 28, 2026 12:12

Uh oh!

Conversation

marcoscaceres commented Apr 25, 2026

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

marcoscaceres commented Apr 27, 2026

Uh oh!

Copilot AI commented Apr 27, 2026

Uh oh!

marcoscaceres commented Apr 27, 2026

Uh oh!

marcoscaceres commented Apr 27, 2026

Uh oh!

marcoscaceres commented Apr 27, 2026

Uh oh!

Copilot AI commented Apr 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants