Skip to content

fix(observability): preserve browser and flush guarantees - #3268

Merged
kwakayama merged 3 commits into
mainfrom
codex/observability-browser-flush-followup-20260802
Aug 2, 2026
Merged

kwakayama merged 3 commits into
mainfrom
codex/observability-browser-flush-followup-20260802

Conversation

@kojiwakayama

@kojiwakayama kojiwakayama commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

Description

Follow-up to #3237 that closes late durability and cross-runtime compatibility gaps without adding third-party dependencies.

  • Serialize file-log durability syncs with the write queue so a later flush() waits for writes accepted during an active flush.
  • Capture immutable node:util/types brand checks synchronously at the runtime trust boundary while keeping browser and edge module graphs free of Node builtin imports.
  • Fail closed on supported server runtimes when complete native brand checks are unavailable instead of silently weakening validation.
  • Centralize the minimum Node.js contract for the root and first-party npm packages, reject extension engine overrides below that floor, and align public runtime documentation.
  • Preserve synchronous Node, Deno, Bun, browser, and edge behavior without top-level-await or dynamic-import lifecycle hangs.
  • Add focused regressions for flush interleaving, browser-safe exports, hostile globals, runtime compatibility, npm metadata, and documentation consistency.

Runtime support after this change:

  • Node.js 22.3 or later
  • Deno 2.2 or later
  • Bun 1.1 or later

Review context:

Related Issue(s)

Follow-up to #3237.

Type of Change

  • Bug fix
  • New feature
  • Breaking change: raises the documented and published minimum Node.js and Deno versions
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test update

Checklist

  • Public runtime requirements and API guidance match the implementation
  • Focused regression tests cover the corrected behavior
  • Core remains free of third-party runtime dependencies
  • Browser and edge public exports remain linkable

Verification

  • Full integration suite: 256 tests / 2,595 steps passed
  • Full observability suite: 52 files / 871 steps passed
  • Focused compatibility, telemetry, and file-log tests passed
  • Browser-safe export tests passed
  • npm build and Node 22 install smoke passed
  • Deno 2.2 and Bun 1.1 runtime checks passed
  • deno task verify:quick
  • git diff --check

Copilot AI review requested due to automatic review settings August 2, 2026 15:30
@kojiwakayama
kojiwakayama requested a review from kwakayama as a code owner August 2, 2026 15:30
@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Copilot AI 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.

Pull request overview

This PR is a follow-up observability fix that keeps the public observability surface browser-linkable while tightening FileLogSubscriber.flush() durability guarantees so later flushes wait for writes accepted during an active flush.

Changes:

  • Make src/platform/compat/error-introspection.ts load node:util/types only on detected Node-compatible hosts (via dynamic import) to preserve browser/edge linkability.
  • Serialize file-log durability syncs with the write queue so a later flush() does not resolve while writes accepted during an active flush are still pending.
  • Add targeted regressions: flush interleaving coverage in file-log-subscriber.test.ts and expand browser-safe export checks.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/platform/compat/error-introspection.ts Replaces static node:util/types import with conditional dynamic import to keep browser imports linkable.
src/observability/file-log-subscriber.ts Refactors flush tracking to snapshot the queue and gate new writes behind a flush barrier.
src/observability/file-log-subscriber.test.ts Adds a regression test for flush interleaving (writes queued during an active flush).
scripts/build/browser-safe-exports.test.ts Updates/extends browser bundle assertions for the observability public barrel.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/observability/file-log-subscriber.ts
Comment thread scripts/build/browser-safe-exports.test.ts

@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: 9df06851d0

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

Comment thread src/platform/compat/error-introspection.ts Outdated
Copilot AI review requested due to automatic review settings August 2, 2026 15:38

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 2, 2026 16:13
@kojiwakayama
kojiwakayama force-pushed the codex/observability-browser-flush-followup-20260802 branch from c3d9595 to ef05f85 Compare August 2, 2026 16:13

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 2, 2026 17:11
@kojiwakayama
kojiwakayama force-pushed the codex/observability-browser-flush-followup-20260802 branch from ef05f85 to c39614c Compare August 2, 2026 17:11

Copilot AI 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.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

@kwakayama
kwakayama added this pull request to the merge queue Aug 2, 2026
Merged via the queue into main with commit 26a905c Aug 2, 2026
32 checks passed
@kwakayama
kwakayama deleted the codex/observability-browser-flush-followup-20260802 branch August 2, 2026 17:42
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