Skip to content

Fix exact-cap app-server capReached reporting#22609

Open
etraut-openai wants to merge 1 commit into
mainfrom
etraut/app-server-capreached-exact-cap
Open

Fix exact-cap app-server capReached reporting#22609
etraut-openai wants to merge 1 commit into
mainfrom
etraut/app-server-capreached-exact-cap

Conversation

@etraut-openai
Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai commented May 14, 2026

Addresses #22379

Why

capReached is meant to signal that outputBytesCap actually truncated a stream. The app-server output collectors instead treated "captured byte count equals the cap" as truncation, so output that landed exactly on the cap boundary could be reported as clipped even when no bytes were dropped.

What changed

  • Derive capReached from whether the emitted chunk was shortened in both command/exec and process/spawn.
  • Add exact-cap coverage for streamed command/exec output.
  • Retarget the buffered process/spawn cap regression to cover the exact-boundary case from the issue.

Verification

  • cargo test -p codex-app-server cap_reached

@etraut-openai etraut-openai force-pushed the etraut/app-server-capreached-exact-cap branch from 5b7e810 to 78d5376 Compare May 14, 2026 05:00
@etraut-openai etraut-openai marked this pull request as ready for review May 14, 2026 05:00
@etraut-openai etraut-openai changed the title [codex] Fix exact-cap capReached reporting Fix exact-cap capReached reporting May 14, 2026
@etraut-openai etraut-openai changed the title Fix exact-cap capReached reporting Fix exact-cap app-server capReached reporting May 14, 2026
@etraut-openai etraut-openai force-pushed the etraut/app-server-capreached-exact-cap branch from 78d5376 to 61a5865 Compare May 20, 2026 03:59
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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: 61a586599c

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

None => chunk.as_slice(),
};
cap_reached = Some(observed_num_bytes) == output_bytes_cap;
cap_reached = chunk.len() > capped_chunk.len();
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.

P2 Badge Update process capReached docs for exact-cap outputs

This changes process/spawn to report stdoutCapReached/stderrCapReached as false when a stream's output length exactly equals outputBytesCap, but the public process API docs/schema still describe these fields as reporting whether the stream “reached” the cap (for example app-server/README.md:1090 and the protocol comments that generate the schema). In the exact-cap case, clients following those docs would expect true while the server now sends false; the app-server API guidance in AGENTS.md also requires updating docs/examples when API behavior changes.

Useful? React with 👍 / 👎.

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.

1 participant