Repository navigation
fix(progress): reconcile live and saved parent updates - #1596
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef72f5516a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0292771af
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6ff0605e1
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59d5e75102
ℹ️ 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".
alandelong-oai
left a comment
There was a problem hiding this comment.
Reviewed 963a775 against the new dd42caa base, including every changed API phase expectation and the current-base integration. The two existing progress-replay issues still reproduce with actual current-source session files: local-shell receipts and missing message projections leave the observer on validation after a distinct later discovery update. All three comparison controls pass. These issues still need attention; the existing inline discussions cover the fixes.
SDK types and changed-test formatting passed. The tracker, complete cost test file, supporting fixtures, dependency locks and instructions are identical to the head reviewed earlier today, so its 282 passing cost tests and built-tracker validation remain applicable. The API production feature is unchanged; the new test expectations consistently retain the parent phase. I did not repeat the previously blocked API integration runs or claim a new full-suite/platform pass.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for fixing the two replay defects and sharing the scenario runner. The original local-shell and missing-projection reproductions now preserve the later discovery update in both source and production builds. The refactor retains all 1,572 expanded scenario names and data, and I found no remaining actionable issue in the current seven-file change.
Fresh checks passed all 304 cost tests on Bun 1.3.14, source/built replay probes, builds, types, formatting and portable checks. Exact-head hosted Linux logs show all 1,572 affected API cases passing, including the delayed custom-validation regressions elsewhere in the suite; Linux and macOS SDK shards passed. Full local API/custom-validation execution remains limited by the existing directory-ownership fixtures, so those known failures were not repeated or bypassed. No live model or complete platform run is claimed; the PR remains a draft.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for the update. The replay integration preserves authoritative progress and usage state, and the final bounded read captures worker usage appended during shutdown. The original missing-projection and local-shell replay controls still pass; I found no remaining actionable issue in this seven-file change.
Fresh checks passed all 307 cost tests, source and built replay controls, builds, types, formatting and portable source checks. Current-head hosted Linux, macOS and Windows results pass the API, cost and custom-validation modules; their relevant inputs match this head, with later main changes elsewhere in the tested merge. Full local API/custom-validation execution remains limited by the existing directory-ownership fixtures.
alandelong-oai
left a comment
There was a problem hiding this comment.
Reviewed the six-file change and current integration. Live/saved event correlation, inventory ownership and validation-phase handling remain sound. The shared inventory/signature helpers and replay-fixture cleanup preserve the reviewed behavior and assertions; I found no material issue.
Validation: 25 focused synthetic-record/accounting and portable checks passed, along with builds, types/models and formatting. Exact-source CI passed 2,894 API, cost and custom-validation tests with four skips. API/custom-validation orchestration was not run locally; no scans, models, workers or commands represented in saved records were executed.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for the simplification. This owner-level merge-safety pass found no material issue in the six-file change. The shared tracker helpers preserve source order and monotonic progress, and the consolidated replay fixtures retain their polling, stop points and assertions.
Validation: 25 fresh focused local checks passed, along with builds, types/models and formatting. Two verified reports from current-head CI pass all 307 cost tests and all 444 cases in the four affected API groups. The overall CI run has completed successfully. API/custom-validation orchestration was not run locally; saved event records were exercised as data only.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for tightening parent progress reconciliation. No material findings in this owner-level merge-safety review. The shared source frontier and replay fixtures preserve occurrence ordering, cumulative resume coverage and validation-phase ownership. I verified that all six feature patches are unchanged from the prior reviewed head and checked their integration with the incoming main changes.
Validation: 25 focused local suite cases, source/built saved-record replay probes, portable checks, SDK builds, types/generated models and formatting passed. Current PR merge CI reports 307 cost and 50 custom-validation cases passing; its tree adds Windows runtime changes beyond this head. The API shard and remaining CI are pending. Full API/SDK, installed-package and cross-platform runtime suites were not run locally.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for isolating the policy-preview fixture build. This owner-level merge-safety pass found no material issue. The six progress feature files are unchanged from the previous review, and I checked their current-base integration and the new subprocess build.
Fresh dependency installs, SDK compilation, types, formatting, 16 focused cost tests, 9 portable compatibility tests, source/built replay controls and resume characterizations passed. Verified reports from current exact-tree CI pass all 307 cost and 73 security-policy tests on both Linux and macOS, including the changed preview test.
The local preview test stopped before bundling because the sandbox exposes an invalid enclosing Git directory. Full local orchestration, installed-package and live-scan checks were not run; remaining hosted checks were still running.
Summary
Scan observers can miss parent updates after a reconnect or count the same command twice when its saved output arrives. Reconcile live and saved events so progress stays in source order.
Changes
Testing
Both
fdf621797band37e54053c2passed all 444 cases in the four suites affected by the fixture change. Their case names, 5,432 replay steps, persisted writes, live events and 2,028 assertion values matched before and after. The broader replay run on37e54053c2passed 1,904 tests and 11,768 assertions. SDK and MCP type checks and SDK formatting passed.At
fdf621797b, all 307 cost tracker tests and all 2,312 cases in the 64 API test groups touched by this PR passed, including resumed, repeated, delayed and truncated events. Together they ran 13,641 assertions. SDK and MCP type checks, generated schema checks and SDK formatting passed.The previous main refresh at
9192a9f87bpassed 427 SDK cases, both incoming Python regression cases, the plugin bundle and all five portable source checks. The earlier test-fixture cleanup preserved the inputs and expectations of all 1,572 generated replay cases. Local validation ran on Linux.The macOS policy-preview test now builds its fixture in a fresh Bun process and runs the preview in Node with Python unavailable. It passed 12 consecutive local runs. All 73 security-policy tests, SDK and MCP type checks, and formatting passed. The hosted macOS shard passed 1,657 tests with 35 skips.
Risk and rollout
An incorrect match in an incomplete log can skip or repeat a progress update. The replay cases cover ambiguous, delayed and truncated output and turn boundaries.
Public disclosure review