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. |
ThomasK33
added this pull request to stack #4340
September 22, 2026 15:16
![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: 21c770f36e
ℹ️ 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".
…ws cannot brick compaction An already-committed row with a deep dynamic-tool input/output is re-serialized by every later compaction (boundary + tail copies) and by partial promotion; V8's JSON.stringify/structuredClone and the SDK clone overflow the stack, so the workspace can never compact again. - toolPayloadDepth: explicit-stack `valueExceedsDepth` and `boundToolPayloadDepth(row)` replace over-deep `input`/`output` of dynamic-tool parts with the existing invalid-JSON diagnostic; same reference when nothing exceeds the bound, source row never mutated. - normalizePersistedMessage composes the legacy metadata migration with that bound and replaces normalizeLegacyMuxMetadata at every full-row read in HistoryService and historyScanner (chat, archive, rewrite rows, partial.json). Untouched rows keep identity, so rewrites emit their raw bytes and sealed epochs still archive the original row byte-for-byte; a rewrite that targets the deep row itself, and partial promotion (which deletes partial.json), persist the flattened row. - Node/Jest regression (persisted half): depth 2101 and 6000 rows load flattened with metadata intact and survive two consecutive persistBoundaryWithTailCopies; the archive retains the original deep raw line; a deep partial is recovered flattened, promoted, and partial.json is removed. Bun unit tests for the walker and the row bound. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh --> Signed-off-by: Thomas Kosiewski <tk@coder.com>
…ol payload depth guard Review finding (PR #4337): boundToolPayloadDepth only bounded a dynamic-tool part's own input/output. code_execution persists its sub-calls under part.nestedCalls[*] (NestedToolCallSchema) with their own input/output, so an over-deep nested payload still reached JSON.stringify/structuredClone on partial promotion and compaction rewrites. - Bound the same two known payload keys on each persisted nested call. Every other nested field (toolCallId, toolName, state, failed, timestamp, workflowRun) is retained; nothing else on the row is walked or coerced. Same-reference semantics are kept at every level (row, part, nestedCalls array, healthy call) so untouched rows still rewrite byte-for-byte. - Tests (red on the C2 base): unit — deep nested input/output replaced, healthy sibling call and shallow rows keep identity; Jest with the real HistoryService — a code_execution partial.json whose nested call carries a depth-6000 input reads bounded (parent input, healthy sibling and nested metadata intact), commitPartial promotes the bounded row and deletes partial.json, and a later boundary commit archives the promoted line byte-for-byte. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh --> Signed-off-by: Thomas Kosiewski <tk@coder.com>
ThomasK33
force-pushed
the
ThomasK33/plan-review-depth-history
branch
from
September 22, 2026 15:55
21c770f to
04b834f
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Recover persisted dynamic-tool inputs and outputs, including
nestedCallsinputs and outputs whose JSON nesting exceeds the safety bound introduced in #4336. Reads substitute a shallow diagnostic in memory, so request building, partial promotion, and compaction do not recursively clone or serialize the offending payload.This is the persisted-history prerequisite for #4317. It builds on #4336's limit and rejection diagnostic; the live-input guard remains unchanged.
Review focus
valueExceedsDepthwalks iteratively. Shallow payloads and ordinary strings remain unchanged, including reference identity.normalizePersistedMessagecovers complete-message reads from history, archives, rewrite rows, andpartial.json, including the provider-history scanner.Validation
Native-Node tests load depth-2101 and depth-6000 rows, preserve their metadata, commit two consecutive compaction boundaries with tail copies, and compare archived raw bytes. A separate case verifies partial promotion and deletion of the raw partial. A real
code_executionnested-call partial first reproduced a native stack overflow on the previous head; it now promotes and compacts with bounded payloads while preserving metadata and healthy sibling calls. Depth-256 data remains a control.On
04b834f2774d2dc1d3174aa9ccb0837e193eb7ad:make static-checkpassed, 193 focused Bun tests passed, and all 17 native-Node integration cases passed. Bun 1.3.5, Node 22.19.0, and main's frozen AI SDK 7.0.107 dependencies were used. Combined remote UAT, code/security reviews, and required CI remain merge gates.Compatibility
This is explicit recovery for over-deep dynamic-tool and nested-call
input/output, not a general repair of arbitrary corrupt history. Ordinary payloads are not truncated. The original bytes remain available only where the existing history lifecycle retains them, as described above.Generated with
xum• Model:coder:openai/gpt-6-astra• Thinking:xhigh• Cost:$760.34