fix: stripEnvelopeMetadata multiline wrapper regression#591
fix: stripEnvelopeMetadata multiline wrapper regression#591jlin53882 wants to merge 2 commits intoCortexReach:masterfrom
Conversation
- FIX 1 (MAJOR): encounteredWrapperYet flag replaces global hasLeadingWrapper Boilerplate is now only stripped when a wrapper has ALREADY appeared on a previous line, not just because a wrapper exists somewhere in the leading zone. This fixes the false positive where boilerplate text like 'Do not use any memory tools.' appearing BEFORE a wrapper in the input would incorrectly be deleted. - FIX 2 (MINOR): preserve inline content after wrapper prefix Old implementation stripped only the wrapper prefix, preserving inline payload. New implementation initially dropped the entire line (regression). This fix restores inline content preservation by: 1. Stripping wrapper prefix 2. Removing boilerplate phrases from remainder using INLINE_BOILERPLATE_RE 3. Keeping remainder if non-empty (non-boilerplate content preserved) - Add 3 new test cases covering both fixes Closes: (PR 531 review fixes)
…er bug - MR2: INLINE_BOILERPLATE_RE now anchored with ^ to avoid stripping quoted boilerplate later in payload - Hidden bug: JSON stripper regex only worked when message_id came before sender_id; now uses order-independent lookahead - Added 4 new test cases covering the fixes
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4bca34f4d
ℹ️ 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".
| result.push(remainder); | ||
| continue; |
There was a problem hiding this comment.
Exit leading-zone after preserving inline wrapper payload
When a wrapper line has non-boilerplate inline content, that content is real user text and should end leading-wrapper stripping. This branch keeps stillInLeadingZone true after result.push(remainder), so the very next line is still treated as lead-in and can be dropped if it matches a boilerplate phrase (for example, [Subagent Context] User content first. followed by Do not use any memory tools. loses the second user line). The previous implementation stopped stripping once inline payload was kept, so this is a content-loss regression.
Useful? React with 👍 / 👎.
| /```json\s*(?=\{[\s\S]*?"message_id"\s*:)(?=\{[\s\S]*?"sender_id"\s*:)\{[\s\S]*?\}\s*```/g, | ||
| "", |
There was a problem hiding this comment.
Constrain message/sender JSON match to current fence
The new order-independent JSON regex can overmatch because its lookaheads search past the current fenced block. If one ```json ... ``` block lacks message_id/sender_id but a later block has them, the first block is still removed, which strips legitimate user JSON whenever metadata appears later in the same text. The key checks need to be limited to the same fenced block being removed.
Useful? React with 👍 / 👎.
說明
保留所有原始 commit:
Commits
測試
✅ 27/27 tests pass
Based on PR #531, creating new PR with preserved commits.