Repository navigation
Preserve managed recovery authority and prepare SDK 0.1.1280 - #5260
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. |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
|
@codex review |
|
Score: 74/100 Sound hardening and good regression tests, but the lease-lifetime change leaves paths where renewal never stops.
Generated by Claude Code |
|
Assessed the lease-cleanup concern against the actual producer and session retirement contract. The concrete regression tests reproduce premature renewal loss on client disconnect and post-invocation setup failure against the previous implementation, and pass with the published repair. The six deferred-producer fixtures now forward their actual completion and pass with explicit operation/resource sanitizers. The real packed-runtime HTTP/PostgreSQL SIGKILL-and-replacement proof also passes with one acknowledged effect execution. I am retaining the fence rather than applying the suggested unconditional stop. Duplicate parsing is intentional to keep public parsing free of private provider provenance; the formatting suggestion is optional. Hosted review/CI and actual staging acceptance remain pending. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0106c19edc
ℹ️ 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".
…etimes Capture mutable Promise, Map and filesystem operations on managed recovery paths, keep leases through producer settlement, separate public parsing from provider provenance and validate initial checkpoints before publishing. Update custom test producers to report actual completion without relaxing unknown-producer fencing. Related: SDK #5213 review findings 4304903774, 4304903777, 4304903781, 4304903784 Tested: Pinned Deno 2.9.7 production check; 170 integration tests and 137 steps; six explicit resource-sanitized deferred producer cases; format and lint Tested: Required uncommitted review; its sibling test completion finding addressed with red/green coverage Not-tested: Fresh packed crash qualification, hosted CI and staging acceptance remain Confidence: high Scope-risk: moderate
Prepare SDK 0.1.1280 through the existing version helpers and generator. Keep package metadata, workspace lock, runtime constant and embedded hydration runtime synchronized so the normal main release workflow publishes the repaired cohort. Tested: Required uncommitted review; its runtime-version finding addressed through generated artifacts Tested: Supported generation task, release-version tests, whitespace checks Not-tested: Public publication and staging acceptance remain Scope-risk: narrow
Tested: Required uncommitted review clear; all 22 focused writer tests; full formatting and lint checks Scope-risk: narrow
The built-in runtime could reject transport, model-readiness or memory preflight before reporting its producer lifetime. Managed recovery then treated a known empty startup as an unknown producer and retained session capacity. Reserve completion before preflight and settle it on failure only while no producer has started. Mark runtime invocation at the actual dispatch boundary. Rejected: Stop renewal whenever completion is missing | an invoked override or failed sandbox cleanup can still leave effects running Confidence: high Scope-risk: moderate Tested: Original real-owner startup regression fails before repair; focused cancellation/recovery tests and positive invocation-boundary cases pass; production typecheck, formatting, lint and required review Not-tested: Final normal publication and staging lifecycle acceptance remain
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
0106c19 to
a26043e
Compare
|
@codex review |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
Merged through the normal queue at Post-publication qualification checked the actual registry tarball integrity and all 9,477 installed API-consumer files. The real HTTP/PostgreSQL checkpoint → owned process SIGKILL → replacement → authenticated full restore → native completion test passed against the published package, with one acknowledged effect, one replacement model call and two original tool starts. All 28 observed production modules came from the installed package, with no source fallback. The Node 22 prototype-poisoning probe also passed. This qualifies the published package for consumer integration. Compatible API/server publication and actual staging lifecycle, email comparison and OKF acceptance remain; this is not a staging completion claim. |
2 participants
Managed recovery must preserve ownership and durable acknowledgements after application code runs. This follow-up to #5213 captures mutable Promise, Map and filesystem operations; keeps lease renewal active until the actual producer settles; keeps public snapshot parsing free of provider provenance; and validates initial snapshots before committing their external cursor. Built-in transport, model-readiness and memory failures report their lifetime before preflight, allowing known empty startups to retire while unknown producers and failed cleanup remain fenced.
Prepares SDK 0.1.1280 through synchronized package, runtime and generated hydration versions so the normal release pipeline can publish the fixes.
Validation: pinned Deno 2.9.7 production checks; 173 recovery/session/stream tests and 137 steps; six deferred-producer tests with explicit operation/resource sanitizers; regressions reproduced against the previous implementation; required current-base review. Exact-head packed-runtime qualification passed: supported Linux npm build, shipped-module graph without source fallback, 19 API provenance tests, API typecheck across 4,862 canonical source files, and real HTTP/PostgreSQL SIGKILL recovery with one execution of the acknowledged effect. Normal publication and staging acceptance remain separate gates.