Skip to content

fix(logs): bound JSON and error response inspection - #177

Closed
luvs01 wants to merge 1 commit into
devfrom
codex/propose-fix-for-unbounded-response-buffering
Closed

luvs01 wants to merge 1 commit into
devfrom
codex/propose-fix-for-unbounded-response-buffering

Conversation

@luvs01

@luvs01 luvs01 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

This PR owns the combined work from #177, #106. The existing main proposal already includes the bounded non-JSON prefix and pull-driven forwarding targeted by #106. This consolidation also carries its error/cancel cases; current-dev adaptation is still pending.

Remaining implementation and validation

  • Keep one pull-driven delivery path for JSON and non-JSON errors; bound retained inspection bytes without truncating client delivery.
  • Finalize logging exactly once on EOF, error and cancellation, preserving status and current log provenance.
  • Preserve existing SSE frame/output-item safeguards; reconcile inspection detachment with tee cancellation and terminal ownership.
  • Validate large JSON/error bodies, slow readers and cancellation against current dev before upstream submission.

Personal work priority

Combined task: 79.3/100 under the shared personal-value rubric (impact 40%, exposure 20%, evidence 15%, total affordability 10%, applicability 15%). Raw components: [17, 13, 14, 9, 8]. This is a single scope assessment, not the sum or maximum of the source PR scores. Individual source proposals are not separate execution tasks.

Review state

This is a tracking consolidation, not a claim of a completed code merge or current-dev test pass. The existing main-branch code remains at f00eaae5c3447355de736cc7850133010266bb00. Source PRs and their review histories remain reachable from the references above; the unresolved requirements are carried by this checklist. Old proposal test results do not establish review readiness for the combined work.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 00c933a0-e92f-454a-bdac-01023d222c65


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot changed the title fix(logs): bound response metadata inspection [WRONG BRANCH] fix(logs): bound response metadata inspection Aug 8, 2026
@github-actions
github-actions Bot marked this pull request as draft August 8, 2026 14:59
@luvs01 luvs01 closed this Aug 19, 2026
@luvs01 luvs01 reopened this Aug 19, 2026
@github-actions

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

✅ READY

  • all PR quality gates passed.

Hygiene

Deterministic PR hygiene checks passed.

@luvs01 luvs01 changed the title [WRONG BRANCH] fix(logs): bound response metadata inspection fix(logs): bound response metadata inspection Sep 3, 2026
@luvs01
luvs01 changed the base branch from main to dev September 3, 2026 06:25
@github-actions github-actions Bot added the bug Something isn't working label Sep 3, 2026
@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 06:34
@luvs01 luvs01 changed the title fix(logs): bound response metadata inspection fix(logs): bound JSON and error response inspection Sep 15, 2026
github-actions Bot pushed a commit that referenced this pull request Sep 16, 2026
Adapt #177 and #106 onto current dev. Preserve terminal ownership with bounded tee read-ahead, stream non-SSE bodies, and cover cancellation races.

luvs01 commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Implemented the consolidated #177 / #106 scope and opened the upstream draft PR: lidge-jun#4775

Current implementation head: 7adf8c6de8dc2df0a472ebde9fc2a1775dd1229f, including upstream dev at b3035fe292168bc598b5d67e77203e2b65404578.

The lifetime SSE cutoff was replaced with bounded inspection read-ahead so terminal/usage/continuation ownership is not detached. JSON/non-JSON logging now uses bounded diagnostic retention and pull-driven raw delivery; the hard-cancel/partial-terminal race is also covered. Registered integration tests and owning/public documentation are included.

Exact-head Bun 1.4.2 typecheck and targeted regressions passed on Ubuntu, Windows and macOS in https://github.com/luvs01/opencodex/actions/runs/35048973972 . Linux additionally passed related relay/log suites, test layout, structure, privacy and documentation build. The upstream PR body records the exact command/result scope.

This source PR and its code are left unchanged. Upstream lidge-jun#4775 remains draft: the complete repository test run on the earlier implementation is still in progress, current-head full-suite success is not claimed, and upstream required CI needs maintainer execution approval. Review was requested; no merge or readiness attestation was made.

@luvs01

luvs01 commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Closing this tracking PR. The bounded JSON and error response inspection work was reimplemented against current upstream dev and is being carried forward there, so keeping a second copy of the same change open here only splits review and rebase effort. Nothing in this branch is lost: the implementation, its regression coverage and the documentation all moved with it, and the remaining verification is happening on the branch that is actually up for review.

@luvs01 luvs01 closed this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant