Skip to content

fix(progress): reconcile live and saved parent updates - #1596

Merged
mldangelo-oai merged 104 commits into
mainfrom
mdangelo/codex/scan-parent-progress
Nov 3, 2026
Merged

mldangelo-oai merged 104 commits into
mainfrom
mdangelo/codex/scan-parent-progress

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

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

  • Match parent command completions across the live stream and saved log, including truncated output. Keep worker receipts and separate turns distinct.
  • Recover missing file and review counts from the parent inventory. Preserve the validation phase when older progress arrives during custom validation.
  • Advance the parent source position through one shared operation for live, saved and duplicate completions. Reuse each saved marker’s position and observation order when reconciling inventory.
  • Run the source-partition and eligibility tests through the same replay fixture as the other progress suites. Each suite still defines its own inputs, expectations and stop points.

Testing

Both fdf621797b and 37e54053c2 passed 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 on 37e54053c2 passed 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 9192a9f87b passed 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

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@github-actions github-actions Bot added the bug Something isn't working label Oct 11, 2026
@mldangelo-oai
mldangelo-oai marked this pull request as ready for review October 11, 2026 08:48
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 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-11-03T01:14:35.347871Z a52f005 New commits
🔒 Security Review ✅ Completed 2026-11-03T01:14:15.115913Z a52f005 New commits
ℹ️ 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.

@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: 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".

Comment thread sdk/typescript/src/cost.ts Resolved
Comment thread sdk/typescript/src/cost.ts Resolved

@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: 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".

Comment thread sdk/typescript/src/api.ts Resolved
Comment thread sdk/typescript/src/cost.ts Resolved

@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: 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".

Comment thread sdk/typescript/src/cost.ts Resolved
Comment thread sdk/typescript/src/cost.ts Resolved

@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: 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".

Comment thread sdk/typescript/src/cost.ts Resolved

@alandelong-oai alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review November 2, 2026 19:40

@alandelong-oai alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@mldangelo-oai
mldangelo-oai merged commit 5befcdf into main Nov 3, 2026
177 of 179 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/scan-parent-progress branch November 3, 2026 02:14
@github-actions github-actions Bot mentioned this pull request Nov 3, 2026
3 tasks
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.

5 participants