Skip to content

feat: retain provider response observations in native history - #5512

Merged
markstuart-oai merged 5 commits into
feat/codex-harnessfrom
codex/native-response-history
Nov 5, 2026
Merged

markstuart-oai merged 5 commits into
feat/codex-harnessfrom
codex/native-response-history

Conversation

@markstuart-oai

@markstuart-oai markstuart-oai commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Requirement: REQ421 — retain received response identity, terminal status and supplied usage through history and persistence; broader route coverage and accounting acceptance remain separate.

Native history stores the final SDK result and total usage, but can lose details from earlier provider responses. A later success can hide an incomplete response that triggered a retry.

Store received terminal response observations with a stable occurrence ID, provider response ID, status, reason, and supplied usage counters. Link usage records to those observations and preserve them through save, restore, and fork. Fail the operation when required recording fails, without issuing another provider request; preserve cancellation. Existing snapshots still load through normalization.

Missing counters remain unknown, so retained observations alone cannot establish complete billing history. Older SDKs may reject newly saved snapshots.

The merged passive metadata slice supports REQ742: direct-root dispatch facts link to the retained response occurrence without exposing request bodies, headers or credentials.

Test plan

  • Current head 58d072c8 includes saved-login v1 and the approved passive metadata changes merged from feat: expose passive request metadata and response links #5519. All 47 Core patches replay unchanged. Normal source preparation, API generation, Ruff and 40 pure history/metadata tests passed.
  • Its authenticated Linux wheel passed archive provenance, pristine-input, wheel-budget, installed import and RECORD checks. The fresh outside-checkout suite passed 188 history, metadata, AG-UI and cancellation cases, with one documented platform skip.
  • The old macOS log confirmed one failure: the worker README test expected one directory under TMPDIR and found none (3,855 passed, 174 skipped). The upstream fix now records the shell's actual directory. The complete replacement macOS job and other current-head CI remain pending.
  • Earlier history-only qualification passed the 1,500-case integrity-rejection probe, three Core ordering/drop tests and the full 3,672-test wrapper (150 skips). Those results remain historical; the full local wrapper has not rerun on this combined head.

Issue number

None linked.

Checks

  • I've added new tests, if relevant
  • I've run the applicable checks in AGENTS.md, including .agents/skills/code-change-verification/scripts/run.sh when required
  • I've confirmed all required checks pass
  • I've completed the review required by AGENTS.md

@markstuart-oai
markstuart-oai requested review from a team, rm-openai and seratch as code owners November 4, 2026 20:13
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 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-05T07:19:40.601414Z 58d072c New commits
🔒 Security Review ✅ Completed 2026-11-05T07:20:02.809640Z 58d072c 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.

@jbeckwith-oai jbeckwith-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.

Approved at 71ae120 after coordinator and two independent static reviews, including the saved-state security boundary.

Terminal observations preserve supplied status/reason/counters before existing completion/error handling, with native occurrence IDs distinct from provider IDs. Unknown counters remain unknown; normalized legacy accounting is labeled separately. Restore, fork, rollback and compaction retain reporting identities without making observations model input or execution authority. Failed required observation append propagates Fatal through sampling and local/remote compaction without issuing a replacement provider request; cancellation and ephemeral behavior remain explicit. Reviewed the regression coverage and patch manifest checksum. No actionable findings.

Hosted native CI is still in progress; change selection, lint and docs have passed. No candidate code or local tests executed. Retained observations do not establish complete billing history; older readers may reject new snapshot records as documented.

@jbeckwith-oai jbeckwith-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.

New hosted evidence exposes a deterministic failure in the response-history test introduced here, inherited by #5508. Requesting the focused test-contract correction below; this does not establish a new runtime status regression. Coordinator and independent static investigation; no candidate code or local tests executed.

Comment thread tests/test_native_response_history.py Resolved
@jbeckwith-oai

Copy link
Copy Markdown
Contributor

Hosted CI follow-up at 71ae120fc4a393c613a81a882c34b54b9b76ed22: the macOS native job failed test_original_callback_failure_stops_further_repair_work[opening] after its three-minute timeout, then the overall test step reached its ten-minute limit at 87%. The log does not contain a final pytest failure traceback/summary, so it does not yet identify the underlying cause. Please investigate that callback/cleanup hang and obtain a completed macOS run. This is separate from the already-reported failed-versus-cancelled response-history assertion; the existing request-changes decision remains. No candidate code or local tests executed.

@markstuart-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: 7964d538ed

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@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: 7964d538ed

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.

@jbeckwith-oai jbeckwith-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.

The regression now accepts both existing SDK settlement outcomes (failed or cancelled) while still requiring NativeError, provider-completed history, received usage, and exactly one request. This resolves the previous assertion finding; the separate hosted timeout remains a CI follow-up. Coordinator and independent static review complete. Current-head hosted checks: {'SKIPPED': 4, 'SUCCESS': 3, 'IN_PROGRESS': 2}. Native/SDK checks remain pending. No candidate code or local tests executed.

@jbeckwith-oai jbeckwith-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.

New hosted evidence reveals missing provider response history in the rejection path. Requesting changes on this distinct finding; the prior status-assertion correction remains valid. Coordinator and independent static review; no candidate code or local tests executed.

Comment thread tests/test_native_response_history.py Resolved
@markstuart-oai
markstuart-oai force-pushed the codex/native-cache-write-usage branch from 230f9ee to 0013cdf Compare November 5, 2026 00:45
@markstuart-oai
markstuart-oai force-pushed the codex/native-response-history branch from 7964d53 to efa3e9a Compare November 5, 2026 03:11

@jbeckwith-oai jbeckwith-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.

The observation acknowledgment resolves my earlier rejection-path history finding: the stream mapper cannot process the terminal interceptor rejection until Core has persisted the preceding provider observation. The acknowledgment is one-shot across pending polls, and consumer drop releases a waiting producer. Persistence failure cancels and joins this sample's tools without cancelling the owning turn or replacing the fatal retention error. Source review also verified retained receipts, response facts and late execution rows coexist after base integration. Coordinator and independent static re-review found no new actionable issue. Hosted native/SDK CI remains pending; no candidate code or local tests executed.

@markstuart-oai

Copy link
Copy Markdown
Contributor Author

The macOS follow-up is now verified at efa3e9a4. Both the native package and sandbox jobs passed. The completed macOS suite passed the previously timed-out opening case and the invalid-correction and active-repair callback-failure cases, with 3,613 parallel tests and 41 serial tests passing.

The test cleanup guard also releases the held callback before the session closes when an assertion fails, preserving the original error instead of masking it with a cleanup timeout. All reported current-head CI checks are terminal and green.

Base automatically changed from codex/native-cache-write-usage to feat/codex-harness November 5, 2026 05:17
@markstuart-oai
markstuart-oai force-pushed the codex/native-response-history branch from efa3e9a to a776c72 Compare November 5, 2026 05:35

@jbeckwith-oai jbeckwith-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.

Approved after coordinator and independent static re-review of the base integration. Direct tree comparison shows 97 of 102 changed files equal the current base. The remaining overlap preserves the existing response-history projection and test registration; the native observation patch changes only rebased context/whitespace, retaining the previously reviewed receipt-ordering fix.

Hosted CI remains pending. Static review only; no candidate code or local tests ran.

@jbeckwith-oai jbeckwith-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.

Approved after static re-review. Saved-login and previously reviewed passive-request metadata integration preserves response-observation routing and the existing feature patch. Hosted CI/native builds remain pending. No candidate code or tests executed.

@markstuart-oai
markstuart-oai merged commit 7c766a8 into feat/codex-harness Nov 5, 2026
28 checks passed
@markstuart-oai
markstuart-oai deleted the codex/native-response-history branch November 5, 2026 07:45
@jbeckwith-oai

jbeckwith-oai commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Hosted CI at merged 58d072c: macOS sandbox job failed test_stop_before_selected_result_returns_settles_without_stale_approval[True] at tests/test_web_starter.py:569 while waiting for initial pending approval, before the stop assertions. The test is byte-identical to base; logs do not establish a candidate-specific cause. 3916 passed, one failed. Other CI remains pending. No local tests or CI reruns performed.

Update 2026-11-05 09:03 UTC:
Hosted CI follow-up: Resolved CI follow-up at merged head 58d072c: all 46 hosted checks are complete, with only SUCCESS/SKIPPED conclusions. Linux, Windows and macOS native packages all passed. The earlier macOS startup timeout is no longer present on the current checks; closing this CI follow-up.

Read hosted results only; no local candidate code/tests or CI reruns.

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.

3 participants