Skip to content

fix(deep-scan): settle completed workers without cancellation races - #341

Merged
mldangelo-oai merged 7 commits into
mainfrom
mdangelo/codex/fix-deep-worker-shutdown-20260810
Aug 11, 2026
Merged

fix(deep-scan): settle completed workers without cancellation races#341
mldangelo-oai merged 7 commits into
mainfrom
mdangelo/codex/fix-deep-worker-shutdown-20260810

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Settle completed Deep Scan workers immediately instead of waiting for subprocess streams to close.
  • Stop forwarding coordinator cancellation once a worker has completed, including cancellation during event-iterator cleanup.
  • Preserve current main's reducer recovery and artifact-tool behavior without unrelated runtime changes.

Verification

  • Focused worker-shutdown and reducer-recovery coverage: 7 passed.
  • TypeScript/generated-model checks passed.
  • Decompressed bundle comparison confirms the runtime change remains limited to the worker executor.

Release sequencing

This changes bundled plugin code. Merge it before the plugin-version bump in release #343 to include it in 0.1.10; if it lands afterward, the next release must increment both bundled plugin version declarations beyond 0.1.17.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

@github-actions github-actions Bot added the bug Something isn't working label Aug 10, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: b850474d08

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: b850474d08

ℹ️ 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".

…ep-worker-shutdown-20260810

# Conflicts:
#	sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-000
#	sdk/typescript/_bundled_plugin/mcp/server.mjs.br.part-001
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review the exact current head b8f24f9. Verify the additive main merge preserves reducer-worker recovery and short artifact tool names while completed workers settle immediately and detach parent cancellation.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: b8f24f912d

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: b8f24f912d

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review the exact current head 44a5d64. Verify completion-time cancellation isolation, active and pre-aborted worker cancellation, terminal-failure cleanup, and preservation of reducer recovery from #342.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 44a5d64ec0

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 44a5d64ec0

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@zcrab-oai zcrab-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Validated review findings are attached inline after three independent reviews and a separate source-level verification.

Comment thread sdk/typescript/tests-ts/deep-scan-worker-shutdown.test.ts
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review the exact current head 5509880. Verify completed workers settle without cancellation races, active workers still receive cancellation, the bundled change remains limited to the worker executor, and current reducer behavior is preserved.

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 5509880e0a

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 5509880e0a

ℹ️ 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".

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review the exact current head ffebbe9. Verify completed Deep Scan workers settle immediately, active and pre-aborted workers still receive cancellation, terminal failures detach coordinator cancellation, and current main remains unchanged outside the bundled worker executor.

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: ffebbe9154

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ffebbe9154

ℹ️ 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".

Comment thread sdk/typescript/tests-ts/deep-scan-worker-shutdown.test.ts
@mldangelo-oai
mldangelo-oai merged commit 71e3d0e into main Aug 11, 2026
17 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-deep-worker-shutdown-20260810 branch August 11, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants