Fix patches for Claude Code 2.1.146-2.1.158#761
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughBroad updates across patchers to match multiple minified Claude Code variants, add native binary repack staging and startup validation, introduce new helpers/sub-patches (e.g., print tools filter, bundled remember skill), and add/refine unit tests and graceful-failure behaviors. ChangesClaude Code Patch Robustness & Native Binary Support
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Patcher
participant TempStaging
participant Repacker
CLI->>Patcher: applyCustomization(nativeInstallationPath?)
Patcher->>TempStaging: copy binary
Patcher->>Repacker: repack staged binary
Repacker->>Patcher: repack result
Patcher->>TempStaging: assertNativeBinaryStarts(--version)
TempStaging-->>Patcher: ok / fail
Patcher->>CLI: replace original if ok, else throw
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
src/patches/tokenCountRounding.test.ts (1)
17-27: ⚡ Quick winAdd coverage for the omitted-threshold fallback.
This only proves the explicit
{ threshold: 1000 }path. The new branch ingetRoundingBase()that defaults{}to1000is still untested, so a regression there would slip through even though object input is now part of the contract.Proposed test
it('accepts the current config object shape without emitting object strings', () => { const input = 'let FH=$?ZH:SH.current,lH=H7(zH),QH=J8(lH),aH=L&&!L.isIdle?L.progress?.tokenCount??0:AH+X,M$=M9(aH),dH=J?`${M$} tokens`:`${$$.arrowDown} ${M$} tokens`,xH=J8(dH),B$=[J9.createElement(k,{key:"tokens"},M$," tokens")];'; const result = writeTokenCountRounding(input, { threshold: 1000, }); expect(result).toContain('M$=M9(Math.round((aH)/1000)*1000)'); expect(result).not.toContain('[object Object]'); }); + + it('defaults object config to 1000 when threshold is omitted', () => { + const input = + 'let FH=$?ZH:SH.current,lH=H7(zH),QH=J8(lH),aH=L&&!L.isIdle?L.progress?.tokenCount??0:AH+X,M$=M9(aH),dH=J?`${M$} tokens`:`${$$.arrowDown} ${M$} tokens`,xH=J8(dH),B$=[J9.createElement(k,{key:"tokens"},M$," tokens")];'; + + const result = writeTokenCountRounding(input, {}); + + expect(result).toContain('M$=M9(Math.round((aH)/1000)*1000)'); + });As per coding guidelines, "Test edge cases and error conditions in test files".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/tokenCountRounding.test.ts` around lines 17 - 27, Add a test that exercises the omitted-threshold fallback in getRoundingBase by calling writeTokenCountRounding with an empty config (e.g., {} or undefined) instead of { threshold: 1000 } and asserting the output still contains the rounding expression using 1000 (e.g., "Math.round((aH)/1000)*1000") and does not contain "[object Object]"; update or add the new case near the existing test that references writeTokenCountRounding so the branch that defaults `{}` to 1000 is covered.src/patches/rememberSkill.test.ts (1)
5-27: ⚡ Quick winCover the legacy and no-match paths too.
This only exercises the bundled-path happy case, but
writeRememberSkill()now has two detection branches plus anullfailure mode. Please add at least one legacy-shape fixture and one “no anchor found” assertion so regressions in either path are visible. As per coding guidelines: "Test edge cases and error conditions in test files".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/rememberSkill.test.ts` around lines 5 - 27, The test only covers the bundled-path happy case for writeRememberSkill; add two more assertions: (1) a legacy-shape fixture: supply an input string matching the old/legacy initializer shape and assert the returned result is not null and contains the inserted remember skill (look for 'name:"remember"' and the legacy insertion context), and (2) a no-anchor fixture: supply an input string that lacks any recognizable anchor/initializer and assert writeRememberSkill returns null to cover the failure/no-match branch. Reference writeRememberSkill and reuse the existing describe('writeRememberSkill') block to add these two it() cases so both detection branches and the null path are exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/patches/autoAcceptPlanMode.ts`:
- Around line 17-47: The helper findEnclosingFunctionReturn is searching for
"return ..." only after readyIdx because functionTail is sliced from readyIdx,
so it misses a return that begins before readyIdx; change functionTail to search
the function body before readyIdx (e.g., const functionTail =
oldFile.slice(openBrace, readyIdx)) and adjust the returned index to openBrace +
lastMatch.index so the returned position points to the start of that earlier
return. Also tighten the regex to allow whitespace (e.g., /return\s+([$\w]+)/g)
if needed.
In `@src/patches/conversationTitle.ts`:
- Around line 500-503: The modern-path regex in writeModernTitleCommand is too
loose and can match unrelated VAR=FN(()=>[ arrays; update
writeModernTitleCommand to either reuse the existing slash-command-array finder
(the same pattern/function used elsewhere to locate the exact slash command
array) or add a strict validation step after commandListMatch that ensures the
matched array is the slash-command array (e.g., verify nearby minified
identifiers or array contents include the slash-command marker like "/title" or
the specific command signature used by the bundle). Tighten commandListPattern
or add this post-match check so the modern insertion only proceeds when the
match is the actual slash-command array; otherwise return null so the legacy
fallback runs. Ensure you reference commandListPattern and
writeModernTitleCommand when making the change.
In `@src/patches/helpers.ts`:
- Around line 315-320: The two regexes (rawAliasPattern and wrapperPattern) are
too strict by requiring `=[A-Z]` for the wrapper factory helper; update both
patterns so they accept multi-character helper names and member-access forms
(e.g. `t.Z` or `_interop.x`) instead of a single uppercase letter. Concretely,
in the template strings for rawAliasPattern and wrapperPattern replace the
`=[A-Z]` portion with a pattern that matches an identifier possibly with dots or
dollar/underscore prefixes (for example use a leading identifier class like
`[A-Za-z_$][\\w$]*(?:\\.[A-Za-z_$][\\w$]*)*`), so findThemedBoxWrapper can
detect wrappers when the helper is multi-character or a member-access; keep
using rawBoxComponent and the existing surrounding anchors/capture groups.
In `@src/patches/index.ts`:
- Around line 671-674: The patch entries for opusplan1m and conversation-title
only use modelCustomizationsEnabled as their condition and must also be guarded
against native installs; update their condition expressions (the opusplan1m
entry that calls writeOpusplan1m and the conversation-title entry that calls
writeConversationTitle) to include the native-install guard (e.g. change
condition: modelCustomizationsEnabled to condition: modelCustomizationsEnabled
&& !nativeInstall or the existing native-install flag used elsewhere) so these
binary-unsafe patches are skipped on native installs.
In `@src/patches/inputPatternHighlighters.ts`:
- Around line 247-261: writeInputPatternHighlighters currently calls new
RegExp(regexSource, flags) without guarding so a malformed pattern/flags will
throw and abort the patch run; wrap the RegExp construction in a try/catch (in
writeInputPatternHighlighters around the new RegExp(regexSource, flags) usage),
validate/normalize flags first (ensure only valid flag characters and append 'g'
if missing) and on error log a clear message including highlighter.name,
regexSource and regexFlags, then skip that highlighter (continue) so the patch
run proceeds; keep using stringifyRegex(regex) only for successfully constructed
regex instances.
In `@src/patches/patchesAppliedIndication.ts`:
- Around line 348-353: The regex in functionPattern is too permissive—drop the
`)` and `,` alternatives so it only matches minified boundaries `}` or `;`;
update the declaration functionPattern to use a stricter pattern like
/[};]\s*function\s+([$\w]+)\(/g (keep the global flag) so it only matches
`}function` or `;function` forms and avoids false positives from
`)function`/`,function`.
In `@src/patches/rememberSkill.ts`:
- Around line 24-27: The bundled-skill detector's patterns array (patterns in
rememberSkill.ts) hardcodes minifier-local names like H and $; update those
regexes to capture arbitrary JS identifiers instead of literal letters so the
detector matches the same minified structure regardless of local names. Replace
the first pattern with a regex that matches a function name and then captures
any valid identifier for the parameter and the inner files alias (use identifier
class [A-Za-z_$][\w$]* and backreferences to ensure the same local name is
used), and replace the second pattern to capture the caller identifier similarly
(e.g. /\{([A-Za-z_$][\w$]*)\(\{name:"claude-in-chrome"/) so both patterns match
minified identifier variants while still enforcing the exact minified structure.
In `@src/patches/sessionMemory.ts`:
- Around line 228-233: The current logic treats presence of
querySource:"extract_memories" as sufficient when patchPastSessions(newFile)
returns null, which can let writeSessionMemory succeed without the past-session
UI/gate; change the flow to require a successful patchPastSessions result before
proceeding: call patchPastSessions(newFile), if it returns a truthy value assign
it to newFile and continue, otherwise return null immediately; remove the
fallback branch that checks newFile.includes('querySource:"extract_memories"')
so the patch is only considered applied when patchPastSessions actually enabled
the past-sessions UI/gate.
In `@src/patches/slashCommands.ts`:
- Around line 68-86: findSlashCommandListEndPosition currently picks the largest
=>[ array by item count which can match unrelated large arrays; update it to
require a strict slash-command-specific anchor in addition to the itemCount
check: modify findSlashCommandListEndPosition so each candidate match from
arrowPattern is validated against a second, strict anchor regex that mirrors the
minified bundle pattern used for slash commands (e.g., the exact surrounding
minified token/assignment that precedes the arrow-return array in the bundle)
before calling analyzeArrayFromOpenBracket, and return null if no candidate
matches both the anchor and itemCount criteria; use the existing function name
findSlashCommandListEndPosition and ensure the added anchor check is documented
in the function comment.
In `@src/patches/suppressNativeInstallerWarning.ts`:
- Around line 3-15: The WARNING_PATTERNS array currently contains loose
fragments (e.g., `/ is not in your PATH/`, `/ && source /`) that can match
unrelated minified code; replace those fragments with strict, full-warning
regexes anchored to the native-installer message format used by Claude Code so
they only match the exact warning text (respecting minified layout by allowing
optional whitespace but not arbitrary context). Update the entries in
WARNING_PATTERNS (the array symbol) to combine surrounding text into single,
specific patterns for each variant (for example, include neighboring tokens like
"Native installation exists but", "Run: echo 'export
PATH=\"$HOME/.local/bin:$PATH\"' >>", or the full "installMethod is native, but
... not found" phrases) and remove or tighten any standalone fragment entries so
no isolated short substrings remain.
In `@src/patches/tableFormat.ts`:
- Around line 277-285: The current replace calls in src/patches/tableFormat.ts
mutate any literal `" \\u2502"` / `" │"` anywhere in the bundle; narrow the
regexes so they only match the compact-table output by the table renderer.
Update the two replace calls that operate on newFile to use stricter patterns
that include minified renderer context (e.g., require the specific surrounding
tokens emitted by the table renderer in the bundle) instead of the bare
literal—keep using newFile.replace(...), and leave patchCount++ and
debug('Patched compact table vertical separators') as-is so the patch only
increments when a genuine table-renderer substitution occurred.
In `@src/patches/toolsets.ts`:
- Around line 1162-1177: The current code treats a failure from
writePrintToolsFilter(result, toolsets, defaultToolset) as fatal and returns
null, aborting later patches; change it to be non-fatal like the
writeToolsetAwareErrors handling: call writePrintToolsFilter and if it returns a
falsy value, log a warning/error (e.g., console.error('patch: toolsets: step 2c
failed (writePrintToolsFilter) — continuing without print-only patch')) and
continue with the existing result instead of returning null; if it succeeds,
assign result = returnedValue and proceed to call
writeToolsetAwareErrors(result, toolsets, defaultToolset) as before.
---
Nitpick comments:
In `@src/patches/rememberSkill.test.ts`:
- Around line 5-27: The test only covers the bundled-path happy case for
writeRememberSkill; add two more assertions: (1) a legacy-shape fixture: supply
an input string matching the old/legacy initializer shape and assert the
returned result is not null and contains the inserted remember skill (look for
'name:"remember"' and the legacy insertion context), and (2) a no-anchor
fixture: supply an input string that lacks any recognizable anchor/initializer
and assert writeRememberSkill returns null to cover the failure/no-match branch.
Reference writeRememberSkill and reuse the existing
describe('writeRememberSkill') block to add these two it() cases so both
detection branches and the null path are exercised.
In `@src/patches/tokenCountRounding.test.ts`:
- Around line 17-27: Add a test that exercises the omitted-threshold fallback in
getRoundingBase by calling writeTokenCountRounding with an empty config (e.g.,
{} or undefined) instead of { threshold: 1000 } and asserting the output still
contains the rounding expression using 1000 (e.g., "Math.round((aH)/1000)*1000")
and does not contain "[object Object]"; update or add the new case near the
existing test that references writeTokenCountRounding so the branch that
defaults `{}` to 1000 is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ebbd778a-991e-40f7-a6f0-a4f2baaa62f8
📒 Files selected for processing (35)
src/patches/agentsMd.tssrc/patches/allowBypassPermsInSudo.tssrc/patches/autoAcceptPlanMode.tssrc/patches/contextLimit.tssrc/patches/conversationTitle.tssrc/patches/fixLspSupport.tssrc/patches/helpers.tssrc/patches/hideStartupBanner.tssrc/patches/increaseFileReadLimit.tssrc/patches/index.tssrc/patches/inputPatternHighlighters.tssrc/patches/mcpStartup.tssrc/patches/modelSelector.tssrc/patches/opusplan1m.tssrc/patches/patchesAppliedIndication.tssrc/patches/rememberSkill.test.tssrc/patches/rememberSkill.tssrc/patches/sessionMemory.tssrc/patches/showMoreItemsInSelectMenus.tssrc/patches/slashCommands.tssrc/patches/statuslineUpdateThrottle.tssrc/patches/suppressLineNumbers.tssrc/patches/suppressNativeInstallerWarning.tssrc/patches/suppressRateLimitOptions.tssrc/patches/tableFormat.tssrc/patches/themes.tssrc/patches/thinkerFormat.tssrc/patches/tokenCountRounding.test.tssrc/patches/tokenCountRounding.tssrc/patches/toolsets.tssrc/patches/userMessageDisplay.tssrc/patches/verboseProperty.tssrc/patches/voiceMode.tssrc/patches/worktreeMode.tssrc/tests/tableFormat.test.ts
basekevin
force-pushed
the
fix-patches-for-2.1.146
branch
from
bacfd63 to
232eee5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/patches/inputPatternHighlighters.ts (1)
13-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHex colors are lost once every highlighter gets a
stylecallback.
buildChalkChain()only extracts decimal components, so#ffffffproduces no color chain and#123456becomes a bogus single RGB argument. Because line 273 always emitsstyle:(x)=>..., the renderer takes the style branch and suppressescolor/backgroundColor, so hex-configured highlighters render unstyled. Either handle hex colors in the generated style path or omitstylewhen the chain does not actually encode the color.Also applies to: 216-273
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/inputPatternHighlighters.ts` around lines 13 - 24, buildChalkChain currently only extracts decimal RGB components from highlighter.foregroundColor / backgroundColor, losing hex colors (e.g. "`#ffffff`") and producing invalid chains; because a style callback is always emitted later, hex-configured highlighters end up unstyled. Fix buildChalkChain to detect hex color strings (e.g. /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/), and when a hex is detected either append a .hex('`#rrggbb`') / .bgHex('`#rrggbb`') segment to chain or convert the hex to proper numeric RGB components and include them in .rgb(...) / .bgRgb(...); alternatively, if no valid color was encoded at all, avoid emitting the unconditional style:(x)=>... branch so you don't suppress color/backgroundColor. Update references in buildChalkChain, the variables chain and highlighter.foregroundColor/backgroundColor, and the code that always emits the style callback to respect whether color segments were actually appended.
♻️ Duplicate comments (2)
src/patches/autoAcceptPlanMode.ts (1)
34-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConstrain the fallback return search to returns before
readyIdx.This still picks the last
return ...createElementin the whole function. If the component has another top-level render return after the"Ready to code?"branch, the fallback injects into that later branch instead of the ready dialog. Track the last top-levelreturnbeforereadyIdxinstead of scanning to function end.Suggested fix
const findEnclosingFunctionReturn = ( oldFile: string, readyIdx: number ): number | null => { const functionStart = oldFile.lastIndexOf('function ', readyIdx); if (functionStart === -1) return null; const openBrace = oldFile.indexOf('{', functionStart); if (openBrace === -1 || openBrace > readyIdx) return null; let depth = 0; - for (let index = openBrace; index < oldFile.length; index++) { + let lastTopLevelReturn: number | null = null; + for (let index = openBrace; index < readyIdx; index++) { const char = oldFile[index]; if (char === '{') depth++; - else if (char === '}') { + else if (char === '}') { depth--; - if (depth === 0) { - const functionTail = oldFile.slice(openBrace, index + 1); - const returnPattern = /return [$\w]+(?:\.default)?\.createElement/g; - let match: RegExpExecArray | null; - let lastMatch: RegExpExecArray | null = null; - while ((match = returnPattern.exec(functionTail)) !== null) { - lastMatch = match; - } - return lastMatch ? openBrace + lastMatch.index : null; - } + } else if ( + depth === 1 && + oldFile.startsWith('return ', index) + ) { + lastTopLevelReturn = index; } } - return null; + return lastTopLevelReturn; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/autoAcceptPlanMode.ts` around lines 34 - 41, The current fallback finds the last return matching returnPattern by scanning functionTail to the end, which can pick a later top-level render; instead restrict the search to only the portion before readyIdx: compute a substring from openBrace up to readyIdx (e.g., const functionTailBeforeReady = oldFile.slice(openBrace, readyIdx)), run returnPattern.exec loop on that substring to set lastMatch, and then return openBrace + lastMatch.index (or null) so the injected fallback is guaranteed to be the last top-level return before readyIdx; update references to functionTail, returnPattern, lastMatch, openBrace and readyIdx accordingly.src/patches/helpers.ts (1)
315-320:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBroaden the wrapper-factory matcher beyond a single uppercase symbol.
Both regexes still require
=[A-Z](...), so Method 5 drops back to the raw Box component on builds where the wrapper factory is a multi-character helper or member access liket.Z/_interop.x. That defeats the new themed-wrapper path on exactly the minified variants this helper is meant to support.Suggested fix
const rawAliasPattern = new RegExp( - `var [^;]{0,120};var [$\\w]+=[A-Z]\\(\\(\\)=>\\{[^}]{0,500}([$\\w]+)=${escapeIdent(rawBoxComponent)}\\}\\)` + `var [^;]{0,120};var [$\\w]+=[$\\w]+(?:\\.[$\\w]+)?\\(\\(\\)=>\\{[^}]{0,500}([$\\w]+)=${escapeIdent(rawBoxComponent)}\\}\\)` ); @@ const wrapperPattern = new RegExp( - `function ([$\\w]+)\\([^)]+\\)\\{(?=[\\s\\S]{0,2500}createElement\\(${escapeIdent(rawAlias)},\\{\\.\\.\\.[$\\w]+,borderColor:)[\\s\\S]{0,3000}?return [$\\w]+\\}var [^;]{0,160};var [$\\w]+=[A-Z]\\(\\(\\)=>\\{[^}]{0,600}([$\\w]+)=\\1\\}\\)` + `function ([$\\w]+)\\([^)]+\\)\\{(?=[\\s\\S]{0,2500}createElement\\(${escapeIdent(rawAlias)},\\{\\.\\.\\.[$\\w]+,borderColor:)[\\s\\S]{0,3000}?return [$\\w]+\\}var [^;]{0,160};var [$\\w]+=[$\\w]+(?:\\.[$\\w]+)?\\(\\(\\)=>\\{[^}]{0,600}([$\\w]+)=\\1\\}\\)` );Based on learnings: In patches under
src/patches/, assume code works with minified, original Claude Code installations. Patch regexes must be strict and match the minified format exactly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/helpers.ts` around lines 315 - 320, The two regexes (rawAliasPattern and wrapperPattern) are too narrow because they require the factory to be a single uppercase symbol; update both patterns so the RHS of the "=" that identifies the wrapper factory accepts multi-character identifiers and dotted member access (e.g. t.Z or _interop.x) instead of only =[A-Z]; keep matching logic around rawBoxComponent, rawAlias and the wrapper function intact but change the factory matcher to permit identifier characters and dot separators (so multi-char/minified helpers are matched).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/patches/autoAcceptPlanMode.ts`:
- Around line 226-227: The insertion offset is computed relative to the sliced
buffer (beforeReady) but the code uses readyIdx - 500, causing wrong absolute
offsets; change the calculation that currently uses readyIdx - 500 to add
returnSearchStart to returnMatch.index (i.e., use returnSearchStart +
returnMatch.index) so the match index is converted to the absolute position in
oldFile; apply the same fix to the second occurrence mentioned (lines 274-275)
where returnMatch.index is combined with readyIdx.
In `@src/patches/rememberSkill.ts`:
- Around line 44-46: The regex built for markerPattern embeds the variable
skillRegistrationFn literally, which lets characters like $ be treated as regex
metacharacters; update the construction in rememberSkill.ts so
skillRegistrationFn is first escaped (e.g., via an escapeRegExp helper that
replaces regex-special chars with escaped versions) before interpolation into
the new RegExp for markerPattern; ensure markerPattern uses the escaped value
when creating the RegExp so matches aren’t broken when findSkillRegistrationFn
returns identifiers containing $ or other metacharacters.
In `@src/patches/sessionMemory.ts`:
- Around line 127-134: The current early-return in writeSessionMemory (checking
file.includes('tengu_session_search_toggled') ||
file.includes('tengu_session_all_projects_toggled')) is too weak and can produce
false positives; replace it with a strict structural regex that matches the
minified enabled-path signature (i.e., the exact sequence of tokens the 2.1.152+
bundles produce around the session-search UI/gate removal) instead of simple
substring checks for 'tengu_session_search_toggled' or
'tengu_session_all_projects_toggled'; if the strict regex does not match, do not
return file (fail closed) so writeSessionMemory proceeds to apply the patch.
Ensure the regex is tolerant of minification (no whitespace/newline reliance)
and anchored to the unique surrounding tokens in the enabled path so only true
2.1.152+ bundles are treated as already enabled.
---
Outside diff comments:
In `@src/patches/inputPatternHighlighters.ts`:
- Around line 13-24: buildChalkChain currently only extracts decimal RGB
components from highlighter.foregroundColor / backgroundColor, losing hex colors
(e.g. "`#ffffff`") and producing invalid chains; because a style callback is
always emitted later, hex-configured highlighters end up unstyled. Fix
buildChalkChain to detect hex color strings (e.g.
/^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/), and when a hex is detected either append
a .hex('`#rrggbb`') / .bgHex('`#rrggbb`') segment to chain or convert the hex to
proper numeric RGB components and include them in .rgb(...) / .bgRgb(...);
alternatively, if no valid color was encoded at all, avoid emitting the
unconditional style:(x)=>... branch so you don't suppress color/backgroundColor.
Update references in buildChalkChain, the variables chain and
highlighter.foregroundColor/backgroundColor, and the code that always emits the
style callback to respect whether color segments were actually appended.
---
Duplicate comments:
In `@src/patches/autoAcceptPlanMode.ts`:
- Around line 34-41: The current fallback finds the last return matching
returnPattern by scanning functionTail to the end, which can pick a later
top-level render; instead restrict the search to only the portion before
readyIdx: compute a substring from openBrace up to readyIdx (e.g., const
functionTailBeforeReady = oldFile.slice(openBrace, readyIdx)), run
returnPattern.exec loop on that substring to set lastMatch, and then return
openBrace + lastMatch.index (or null) so the injected fallback is guaranteed to
be the last top-level return before readyIdx; update references to functionTail,
returnPattern, lastMatch, openBrace and readyIdx accordingly.
In `@src/patches/helpers.ts`:
- Around line 315-320: The two regexes (rawAliasPattern and wrapperPattern) are
too narrow because they require the factory to be a single uppercase symbol;
update both patterns so the RHS of the "=" that identifies the wrapper factory
accepts multi-character identifiers and dotted member access (e.g. t.Z or
_interop.x) instead of only =[A-Z]; keep matching logic around rawBoxComponent,
rawAlias and the wrapper function intact but change the factory matcher to
permit identifier characters and dot separators (so multi-char/minified helpers
are matched).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80ba42fd-eb9c-4b1c-916e-4afde0646a92
📒 Files selected for processing (31)
src/patches/agentsMd.tssrc/patches/allowBypassPermsInSudo.tssrc/patches/autoAcceptPlanMode.test.tssrc/patches/autoAcceptPlanMode.tssrc/patches/contextLimit.tssrc/patches/conversationTitle.tssrc/patches/fixLspSupport.tssrc/patches/helpers.tssrc/patches/hideStartupBanner.tssrc/patches/index.tssrc/patches/inputPatternHighlighters.test.tssrc/patches/inputPatternHighlighters.tssrc/patches/modelCustomizationsToggle.test.tssrc/patches/modelSelector.tssrc/patches/patchesAppliedIndication.tssrc/patches/rememberSkill.test.tssrc/patches/rememberSkill.tssrc/patches/sessionMemory.tssrc/patches/showMoreItemsInSelectMenus.tssrc/patches/suppressLineNumbers.tssrc/patches/suppressNativeInstallerWarning.tssrc/patches/suppressRateLimitOptions.tssrc/patches/tableFormat.tssrc/patches/thinkerFormat.tssrc/patches/tokenCountRounding.test.tssrc/patches/tokenCountRounding.tssrc/patches/toolsets.tssrc/patches/userMessageDisplay.tssrc/patches/verboseProperty.tssrc/patches/worktreeMode.tssrc/tests/tableFormat.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/tests/tableFormat.test.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- src/patches/allowBypassPermsInSudo.ts
- src/patches/rememberSkill.test.ts
- src/patches/fixLspSupport.ts
- src/patches/contextLimit.ts
- src/patches/tableFormat.ts
- src/patches/verboseProperty.ts
- src/patches/worktreeMode.ts
- src/patches/patchesAppliedIndication.ts
- src/patches/suppressRateLimitOptions.ts
- src/patches/hideStartupBanner.ts
- src/patches/suppressLineNumbers.ts
- src/patches/suppressNativeInstallerWarning.ts
- src/patches/tokenCountRounding.test.ts
- src/patches/userMessageDisplay.ts
- src/patches/agentsMd.ts
- src/patches/index.ts
basekevin
force-pushed
the
fix-patches-for-2.1.146
branch
from
232eee5 to
098c38f
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/patches/rememberSkill.ts (1)
47-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEscape
skillRegistrationFnbefore interpolating intomarkerPattern.
skillRegistrationFncan contain$(captured via[A-Za-z_$][\w$]*), which will be interpreted as a regex end-of-line anchor when interpolated directly. UseescapeIdent(already imported from./index) to escape the identifier.Proposed fix
const writeBundledRememberSkill = ( oldFile: string, skillRegistrationFn: string ): string | null => { const markerPattern = new RegExp( - `function\\s+[$\\w]+\\(\\)\\{${skillRegistrationFn}\\(\\{name:"update-config"` + `function\\s+[$\\w]+\\(\\)\\{${escapeIdent(skillRegistrationFn)}\\(\\{name:"update-config"` );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/rememberSkill.ts` around lines 47 - 49, The regex construction for markerPattern currently interpolates skillRegistrationFn directly, which lets any $ in the identifier be treated as a regex anchor; update the code to call the imported escapeIdent on skillRegistrationFn before interpolation (e.g., const escaped = escapeIdent(skillRegistrationFn)) and use that escaped value when building markerPattern so the identifier is safely treated as a literal in the RegExp; keep the same RegExp structure and variable name markerPattern and ensure escapeIdent from ./index is used.src/patches/autoAcceptPlanMode.ts (1)
274-275:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the absolute insertion offset calculation.
Line 274 computes the offset using
readyIdx - 500, butreturnSearchStarton line 226 isMath.max(0, readyIdx - 2500). SincereturnMatch.indexis relative tobeforeReadywhich starts atreturnSearchStart, the calculation should usereturnSearchStartinstead.Suggested fix
- const absoluteStart = Math.max(0, readyIdx - 500) + returnMatch.index!; + const absoluteStart = returnSearchStart + returnMatch.index!;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/autoAcceptPlanMode.ts` around lines 274 - 275, The calculation of absoluteStart is wrong: it uses a hardcoded 500 offset (readyIdx - 500) but returnMatch.index is relative to beforeReady which begins at returnSearchStart (computed as Math.max(0, readyIdx - 2500)); update the absolute offset computation in the block that defines absoluteStart so it uses returnSearchStart (i.e., Math.max(0, readyIdx - 2500)) plus returnMatch.index instead of readyIdx - 500, keeping the existing Math.max(0, ...) guard and leaving acceptFuncName/insertion unchanged.src/patches/toolsets.ts (1)
1163-1168:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the
Step 2c is an extra compatibility patch for one execution path, but it currently aborts the entire toolsets patch when that shape isn't present. This turns a print-only drift into a full failure for interactive filtering,
/toolset, and the statusline updates that already patched successfully in Steps 1-2b. This should degrade gracefully like Step 2d.Suggested fix
// Step 2c: Patch the non-interactive --print tool context - result = writePrintToolsFilter(result, toolsets, defaultToolset); - if (!result) { - console.error('patch: toolsets: step 2c failed (writePrintToolsFilter)'); - return null; - } + const printResult = writePrintToolsFilter(result, toolsets, defaultToolset); + if (!printResult) { + console.error( + 'patch: toolsets: step 2c failed (writePrintToolsFilter) — continuing without --print filtering' + ); + } else { + result = printResult; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/toolsets.ts` around lines 1163 - 1168, The writePrintToolsFilter patch (called as result = writePrintToolsFilter(result, toolsets, defaultToolset)) should be treated as non-fatal: if writePrintToolsFilter returns a falsy value, do not return null; instead log a warning and continue using the existing result so earlier successful patches remain. Update the code path around writePrintToolsFilter so it checks the return value and on failure calls console.warn (or similar) with a clear message including "writePrintToolsFilter" and then leaves result unchanged (do not abort); ensure subsequent steps (e.g., Step 2d) still run normally.
🧹 Nitpick comments (1)
src/patches/toolsets.ts (1)
198-199: ⚡ Quick winAvoid
\bin regex patterns for better V8 performance.Per coding guidelines,
\bword boundaries can cause V8 performance issues. Consider using literal character alternatives.Suggested fix
- const newInputPattern = /\binputValue:([$\w]+),/g; - const oldInputPattern = /\binput:([$\w]+),/g; + const newInputPattern = /[,;{]inputValue:([$\w]+),/g; + const oldInputPattern = /[,;{]input:([$\w]+),/g;Note: Adjust the captured group index usage if the pattern changes add a leading character class.
As per coding guidelines: "Avoid
\bin regex patterns due to V8 performance issues; use literal character alternatives".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/patches/toolsets.ts` around lines 198 - 199, The regex near variable "i" in toolsets.ts uses \b word-boundary which should be replaced with an explicit literal character alternative for V8 performance; update the pattern to use a leading character class (e.g., a boundary match like start-or-non-word character) instead of \b and then adjust any captured-group indexing that assumed the old boundary (update uses of the capture groups where the regex is consumed). Locate the regex around the variable "i" and replace the \b occurrence with the character-class based boundary and verify all references to the capture groups in the surrounding code are updated to the new group indices.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/patches/autoAcceptPlanMode.ts`:
- Around line 274-275: The calculation of absoluteStart is wrong: it uses a
hardcoded 500 offset (readyIdx - 500) but returnMatch.index is relative to
beforeReady which begins at returnSearchStart (computed as Math.max(0, readyIdx
- 2500)); update the absolute offset computation in the block that defines
absoluteStart so it uses returnSearchStart (i.e., Math.max(0, readyIdx - 2500))
plus returnMatch.index instead of readyIdx - 500, keeping the existing
Math.max(0, ...) guard and leaving acceptFuncName/insertion unchanged.
In `@src/patches/rememberSkill.ts`:
- Around line 47-49: The regex construction for markerPattern currently
interpolates skillRegistrationFn directly, which lets any $ in the identifier be
treated as a regex anchor; update the code to call the imported escapeIdent on
skillRegistrationFn before interpolation (e.g., const escaped =
escapeIdent(skillRegistrationFn)) and use that escaped value when building
markerPattern so the identifier is safely treated as a literal in the RegExp;
keep the same RegExp structure and variable name markerPattern and ensure
escapeIdent from ./index is used.
In `@src/patches/toolsets.ts`:
- Around line 1163-1168: The writePrintToolsFilter patch (called as result =
writePrintToolsFilter(result, toolsets, defaultToolset)) should be treated as
non-fatal: if writePrintToolsFilter returns a falsy value, do not return null;
instead log a warning and continue using the existing result so earlier
successful patches remain. Update the code path around writePrintToolsFilter so
it checks the return value and on failure calls console.warn (or similar) with a
clear message including "writePrintToolsFilter" and then leaves result unchanged
(do not abort); ensure subsequent steps (e.g., Step 2d) still run normally.
---
Nitpick comments:
In `@src/patches/toolsets.ts`:
- Around line 198-199: The regex near variable "i" in toolsets.ts uses \b
word-boundary which should be replaced with an explicit literal character
alternative for V8 performance; update the pattern to use a leading character
class (e.g., a boundary match like start-or-non-word character) instead of \b
and then adjust any captured-group indexing that assumed the old boundary
(update uses of the capture groups where the regex is consumed). Locate the
regex around the variable "i" and replace the \b occurrence with the
character-class based boundary and verify all references to the capture groups
in the surrounding code are updated to the new group indices.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a1fef16-08a9-43da-9ec8-7ae5e7a24d5a
📒 Files selected for processing (32)
src/patches/agentsMd.tssrc/patches/allowBypassPermsInSudo.tssrc/patches/autoAcceptPlanMode.test.tssrc/patches/autoAcceptPlanMode.tssrc/patches/contextLimit.tssrc/patches/conversationTitle.tssrc/patches/fixLspSupport.tssrc/patches/helpers.tssrc/patches/hideStartupBanner.tssrc/patches/index.tssrc/patches/inputPatternHighlighters.test.tssrc/patches/inputPatternHighlighters.tssrc/patches/modelCustomizationsToggle.test.tssrc/patches/modelSelector.tssrc/patches/patchesAppliedIndication.tssrc/patches/rememberSkill.test.tssrc/patches/rememberSkill.tssrc/patches/sessionMemory.tssrc/patches/showMoreItemsInSelectMenus.tssrc/patches/slashCommands.tssrc/patches/suppressLineNumbers.tssrc/patches/suppressNativeInstallerWarning.tssrc/patches/suppressRateLimitOptions.tssrc/patches/tableFormat.tssrc/patches/thinkerFormat.tssrc/patches/tokenCountRounding.test.tssrc/patches/tokenCountRounding.tssrc/patches/toolsets.tssrc/patches/userMessageDisplay.tssrc/patches/verboseProperty.tssrc/patches/worktreeMode.tssrc/tests/tableFormat.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/tests/tableFormat.test.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- src/patches/fixLspSupport.ts
- src/patches/autoAcceptPlanMode.test.ts
- src/patches/suppressRateLimitOptions.ts
- src/patches/tokenCountRounding.test.ts
- src/patches/verboseProperty.ts
- src/patches/contextLimit.ts
- src/patches/inputPatternHighlighters.test.ts
- src/patches/suppressLineNumbers.ts
- src/patches/patchesAppliedIndication.ts
- src/patches/agentsMd.ts
- src/patches/modelSelector.ts
- src/patches/tableFormat.ts
- src/patches/conversationTitle.ts
- src/patches/index.ts
- src/patches/hideStartupBanner.ts
- src/patches/userMessageDisplay.ts
- src/patches/thinkerFormat.ts
- src/patches/slashCommands.ts
- src/patches/suppressNativeInstallerWarning.ts
- src/patches/sessionMemory.ts
- src/patches/modelCustomizationsToggle.test.ts
mike1858
changed the title
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
2 participants
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.
Closes #635
Closes #643
Closes #663
Closes #665
Closes #671
Closes #674
Closes #676
Closes #681
Closes #694
Closes #701
Closes #716
Closes #734
Closes #741
Closes #752
Closes #756
Closes #758
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Tests