Skip to content

fix(internal): close final Pi broker review gaps - #2

Merged
malkovitc merged 9 commits into
mainfrom
fix/pi-review-followup
Aug 25, 2026
Merged

malkovitc merged 9 commits into
mainfrom
fix/pi-review-followup

Conversation

@malkovitc

Copy link
Copy Markdown
Owner

Resolves both P1 findings from the post-merge review of fork PR #1.

  • Brokered human thread replies now use the same resolved root anchor as prompt rendering, preserving flat layer-1 threads.
  • Every prompt owns an AbortController; cancel, failure, settlement, and shutdown abort in-flight broker commands so no late publication can occur.
  • Bumps the minimum adapter to pi-acp 0.2.1, failing closed on unsafe 0.2.0.

Checks: Node 21/21; Biome; Rust metadata/anchor tests; desktop version tests; rustfmt; file-size policy; diff-check — PASS.

Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

@codex review

@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: 8d166071d8

ℹ️ 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 crates/buzz-acp/src/prompt_metadata.rs
Comment thread tools/pi-acp/src/adapter.mjs
Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Addressed both findings in exact head $(git rev-parse HEAD):

  • Added authenticated allowedReplyEventIds; resolved thread roots are authorized without mislabeling them as triggering events.
  • Broker runner registers cancellation before spawn, checks pre-aborted state, closes the listener/spawn race, and has a no-spawn regression test.

Node 22/22, Rust anchor/metadata tests, Biome, rustfmt, file-size policy and diff-check PASS.

@codex review

@malkovitc

Copy link
Copy Markdown
Owner Author

Review follow-up exact head: ce8303197c26a41255d3a250f1083968e7245488.

@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: ce8303197c

ℹ️ 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 tools/pi-acp/src/buzz-tools.mjs Outdated
Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Resolved the process-group finding in exact head 47058b7b866c9b60d5c255153fdd0558295230ee: broker commands are detached into their own group on Unix; abort, timeout, and output-limit termination signal the entire group. Added a wrapper-plus-descendant regression proving no late marker/publication. Node 23/23 and Biome PASS. @codex review

@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: 47058b7b86

ℹ️ 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 tools/pi-acp/src/buzz-tools.mjs Outdated
Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Resolved Windows descendant termination in exact head ecf1f45178ac1d4b86d925930cd5e5d537c16547: cancellation uses synchronous taskkill /PID <pid> /T /F before fallback, while Unix keeps detached process-group signals. Node 23/23 and Biome PASS. @codex review

@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: ecf1f45178

ℹ️ 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 tools/pi-acp/src/buzz-tools.mjs
Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Resolved cancellation escalation in exact head 097ce2a18cef01a0b27b9847f8f446b7861e0b86: cancellation now immediately SIGKILLs the entire Unix publication process group (Windows already uses forced taskkill). Regression descendant explicitly traps SIGTERM and still cannot publish. Node 23/23, Biome PASS. @codex review

@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: 097ce2a18c

ℹ️ 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 tools/pi-acp/src/buzz-tools.mjs
Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Resolved detached-wrapper exit in exact head 52f43582914607b61a1d4c403d7a7b2eed4f517a: every broker leader exit forcibly reaps its entire process tree before the result is accepted. Added a wrapper-that-backgrounds-and-exits regression; no delayed publication survives. Node 24/24, Biome PASS. @codex review

@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: 52f4358291

ℹ️ 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 tools/pi-acp/src/buzz-tools.mjs
Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Resolved safe reservation cleanup in exact head bfc3c5d342c553d29d805cfcb91a6b4ea09a756c: pre-spawn aborts carry a distinct code and atomically remove the unstarted reservation; any abort after spawn retains the ambiguous reservation. Added cleanup/retryability regression. Node 25/25, Biome PASS. @codex review

@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: bfc3c5d342

ℹ️ 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 tools/pi-acp/src/buzz-tools.mjs
Comment thread tools/pi-acp/src/buzz-tools.mjs
Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Resolved both findings in exact head 9a474040c4d3294f1f7ca94c61751c1f6bb14801: the internal Pi pilot is now explicitly macOS/Linux-only and omitted from the Windows Desktop catalog; direct Windows adapter launch fails closed. Spawn errors before a child starts are classified safe-unstarted and release their reservation, with regression coverage. Node 26/26, Desktop Pi tests, Biome and file-size policy PASS. @codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 9a474040c4

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

@malkovitc

Copy link
Copy Markdown
Owner Author

@codex review exact head 9a47404

Signed-off-by: evgeniy.chernomortsev <j.malkovitc@gmail.com>
@malkovitc

Copy link
Copy Markdown
Owner Author

Exact head 00ec3c82ed5a125dfc90a1e91ab0db765bf2b7d5 also satisfies the per-commit Desktop file-size ratchet. Windows remains fail-closed and safe-unstarted spawn failures release reservations. @codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: 00ec3c82ed

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

@malkovitc
malkovitc merged commit 6b7da4e into main Aug 25, 2026
47 of 53 checks passed
@malkovitc
malkovitc deleted the fix/pi-review-followup branch August 25, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant