Skip to content

🤖 refactor: centralize prepared history publication and origin - #4187

Open
ThomasK33 wants to merge 1 commit into
codex/effect-replacement-acceptancefrom
codex/effect-prepared-history-publication
Open

🤖 refactor: centralize prepared history publication and origin#4187
ThomasK33 wants to merge 1 commit into
codex/effect-replacement-acceptancefrom
codex/effect-prepared-history-publication

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Sep 9, 2026

Copy link
Copy Markdown
Member

Centralizes prepared history publication so manual triggers and their prefixes use one publication boundary. Origin remains independent of visibility and billing, and later runtime layers consume durable receipts and rollback facts instead of interpreting a successful send as acceptance.

This preparation layer exposes #4182's guarded publication without activating cancellation policy; that activation belongs to #4191/#4209 in the same phase. Both Result errors and unexpectedly rejected publication promises reach the existing rollback checkpoints. Only the publication service promise is normalized: acceptance receipts and subsequent callback failures retain their distinct semantics.

Real HistoryService tests inject both failure forms after each prepared prefix and assert rollback preserves concurrent foreign history. A CI-only goal fixture now waits for publication entry and settles its held promise before teardown; observing an atomic rename no longer substitutes for write completion. Production goal behavior is unchanged. Final integrated validation passes on #4191 052fc084517c1a6fd8cfbecdf08c05b635358a32 (4026 tests / 36442 assertions) and #4209 02651d365f63c9dc04d95744207f42f8065d6f35 (4095 tests / 36847 assertions), across 54 affected suites each. Full source/test TypeScript and make static-check pass on both exact commits.

Risk: publication callbacks govern queue and budget accounting. The real-disk assertions protect against lost or duplicated input and incorrect refunds.

This is one layer of the cancellation phase: #4214#4215#4219#4221#4182#4187#4191#4209. All eight PRs merge together after every member has current-head approval and passing CI.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T19:33:57.016884Z 1ce1db1 Manual request
🔒 Security Review Completed 2026-09-11T19:33:53.121043Z 1ce1db1 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33
ThomasK33 added this pull request to stack #4188 September 9, 2026 14:03
@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch 3 times, most recently from 1512246 to 41f5ab3 Compare September 9, 2026 14:52
@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from 41f5ab3 to 04977de Compare September 9, 2026 17:46
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review current head 04977dee3b07940f4fda84da1bcce3f5c64d2f26. The preceding attempt failed while resolving the Git ref; a newer code review in the companion phase has now completed, so this is a retry after that recovery evidence.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 04977dee3b

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

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: 04977dee3b

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.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review current head 2f180b0c3d9a066cdf9196c51b13a0fa4e5bd0f7. The complete description records the final validated layer and integration; addressed findings have replies and are resolved.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 2f180b0c3d

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

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: 2f180b0c3d

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.

@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from 2f180b0 to 7235c68 Compare September 10, 2026 15:48
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review current head 7235c68bc0e902a68eaf73aebe143737933ab156. The complete phase has been reconciled with current main and passes 1,884 focused tests and full static checks. The PR description records the final layer scope and validation; the runtime malformed-partial finding has a published fix and reply.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: 7235c68bc0

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: 7235c68bc0

ℹ️ 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 src/node/services/agentSession.preparedHistory.test.ts Outdated
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed PRRT_kwDOPxxmWM6hM7oo (finding) in a54cba5f5ba57429ac2e3cac092102ee5a2f5bab. The fixture now tracks the real HistoryService append without replacing its implementation. A selected underlying filesystem append uses mockRejectedValueOnce, so the real HistoryService catches the I/O error and returns Err to the rollback branch. Rejecting appendToHistory itself would test a different exception path.

The foreign row still lands after the first owned prefix and before the failing write. Exact call order and persisted sequence assertions prove interleaving; rollback must preserve that row and start no provider. All 11 isolated middle tests and 15 integrated runtime tests pass, as do canonical static checks. No production change was needed for this finding.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review current head a54cba5f5ba57429ac2e3cac092102ee5a2f5bab. The phase has been reconciled with the actual merged pending-state base, all known findings have published fixes or a tracked phase-layer split and replies, and the split passed independent review. The new V1 intermediate tree passed 257 tests and canonical static checks. The final top exactly preserves the complete implementation with its prior static/backend/IPC/UI evidence, plus main’s documentation-only update; see the PR bodies for the precise validation scope.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: a54cba5f5b

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

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: a54cba5f5b

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.

@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from a54cba5 to 2c8c781 Compare September 11, 2026 11:51
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review current head 2c8c7818a68ba91fcfc1df3ea187c77c7b37aa7f. The latest review fixes are published in their owning layers, with replies and qualified validation in the PR descriptions. Both integrated runtime candidates passed canonical static checks. The whole cancellation phase will remain unmerged until every prerequisite and member is ready.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@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: 2c8c7818a6

ℹ️ 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 src/node/services/agentSession.preparedHistory.test.ts Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: 1a5231b629

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

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: 1a5231b629

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.

@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from 1a5231b to fe7f0e7 Compare September 11, 2026 14:23
@ThomasK33
ThomasK33 removed this pull request from stack #4188 September 11, 2026 14:24
@ThomasK33
ThomasK33 added this pull request to stack #4222 September 11, 2026 14:24
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the current head after the complete phase was composed on current main. All known findings have fixes and replies in their owning layers; the eight-member phase stays held until all members are approved and green.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: fe7f0e7618

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: fe7f0e7618

ℹ️ 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 src/node/services/agentSession.preparedHistory.test.ts Outdated
Comment thread src/node/services/agentSession.ts
@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from fe7f0e7 to c3cb906 Compare September 11, 2026 16:27
@ThomasK33

Copy link
Copy Markdown
Member Author

Addressed 3990173176: retained the existing real-service Result Err controls and added mockRejectedValueOnce variants for skill, prompt and trigger publication. Those three variants exposed orphaned prepared prefixes; normalizing the rejected publication promise into Result Err now runs the existing rollback checkpoints while preserving foreign writes. All 14 preparation tests / 85 assertions and canonical static checks pass. The final batch-based runtime carries equivalent rejection coverage without restoring obsolete per-prefix writes.

For 3990173187, activation belongs to the next runtime layers. This preparation PR neither creates nor enforces retained cancellation; its lower API is explicitly inactive (the reviewed preparation head). The production producer, manual capture and guarded send/Resume calls are already in #4191: manual trigger, Resume. #4209 extends the settled-Stop automatic path. The eight PRs are one coordinated phase merge; this PR intentionally supplies the preparation contract while those later PRs own activation.

The goal admission CI failure was also reproduced as an existing test race in files identical to main. Its fixture now awaits publication entry and drains the outstanding write before cleanup; all 189 goal tests pass, with no production goal change.

Combined phase validation: 3,715 tests across 50 affected service suites passed (34,595 assertions). Full make static-check passed on the final publication tree. The only change after the behavioral run was one verified test-fixture formatting correction.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the updated head c3cb90699b165c6e8003b97b19b190a39d769c5e, including the addressed findings and the exact reset/retirement integration carried through this phase. Combined phase validation: 3,715 tests across 50 affected service suites passed (34,595 assertions). Full make static-check passed on the final publication tree. The only change after the behavioral run was one verified test-fixture formatting correction.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: c3cb90699b

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.

@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from c3cb906 to 00397fa Compare September 11, 2026 17:49
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the updated head 00397fa9547bafa81f8d7ae3e1a2f18b08216bf9 and its addressed findings. The cancellation layer passed 3,996 tests across 54 suites; the settled-Stop layer passed 4,064 across the same 54 suites. Its final test-only assertion adjustment also passed the complete affected 52-test suite. Both final runtime branches passed canonical static checks. The complete eight-member cancellation phase remains held until every member is approved and CI passes.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 00397fa954

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

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: 00397fa954

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.

@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from 00397fa to ab882a4 Compare September 11, 2026 18:38
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review the updated head ab882a4f5d7873afe943f2987b3955fc2b23eb60 against its stack base. This layer’s preparation behavior is unchanged by the prerequisite review fixes.

Final integrated validation passes on #4191 266cf35aac (4,009 tests) and #4209 d8c34d12ce (4,078 tests), each across 54 affected suites. Full source/test TypeScript and make static-check pass on both exact commits. The eight-member cancellation phase remains held until every member has current-head approval and passing CI.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: ab882a4f5d

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: ab882a4f5d

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

---

_Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_

<!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
@ThomasK33
ThomasK33 force-pushed the codex/effect-prepared-history-publication branch from ab882a4 to 1ce1db1 Compare September 11, 2026 19:25
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Please review updated head 1ce1db19068201a274c1d4d95f6d1d01195d2853 against its stack base. This layer’s own source delta is unchanged; it inherits the reviewed lower witness fixes.

Final integrated validation passes on #4191 052fc084517c1a6fd8cfbecdf08c05b635358a32 (4026 tests / 36442 assertions) and #4209 02651d365f63c9dc04d95744207f42f8065d6f35 (4095 tests / 36847 assertions), across 54 affected suites each. Full source/test TypeScript and make static-check pass on both exact commits.

All eight members remain held until current-head review approval and required CI are complete.


Generated with xum • Model: unavailable • Thinking: unavailable • Cost: $unavailable

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

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

Reviewed commit: 1ce1db1906

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. You're on a roll.

Reviewed commit: 1ce1db1906

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

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