Repository navigation
Fix snip metadata leaks and TUI corruption - #1600
Conversation
Replace visible snip ID tags with internal snip_id metadata and update SnipTool guidance/tests so the model can request pruning without echoing user-visible IDs. Add compatibility parsing for legacy [id:...] markers, clarify synthetic OpenAI shim tool-result messages, and tighten test cleanup around env/session state. Mitigate Konsole+tmux rendering corruption by disabling the scroll fast path in that environment while preserving bottom-follow behavior and clearing culled cached output. Validation: bun run build; bun run smoke; ANTHROPIC_API_KEY=test-key bun run check; ANTHROPIC_API_KEY=test-key bun run test:full; python -m pytest -q python/tests; bun run security:pr-scan -- --base upstream/main.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughConverts visible snip tags to internal ChangesSnip ID Tagging Migration to Internal Metadata
ScrollBox Fast-Path Disable and Stickiness Improvements
OpenAI Shim and Semantic Assistant Marker
Test Infrastructure and Environment Management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/services/compact/snipCompact.ts`:
- Line 19: Static analysis flagged the regex literals as command-injection risks
but these are false positives; add an inline suppression and brief explanatory
comment above the regex usages to silence SAST. For example, above the
snipMetadataMatch assignment (/\bsnip_id=([a-z0-9]{6})\b/i) and the
/^\[id:...\]$/ usage add a one-line comment like "SAST false positive: regex
literal, not executing user input" plus the project’s suppression token (e.g. //
nosemgrep or // nosec or // eslint-disable-next-line
security/detect-unsafe-regex) so scanners ignore these lines while keeping the
regex literals (snipMetadataMatch and the id regex) unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8f4a2128-c2c6-4f97-802b-774eaebfbd2f
📒 Files selected for processing (15)
docs/windows-aliases-and-launchers.mdscripts/windows/openclaude-aliases.ps1src/commands/install-github-app/setupGitHubActions.test.tssrc/ink/render-node-to-output.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/compact/snipCompact.test.tssrc/services/compact/snipCompact.tssrc/tools/SnipTool/SnipTool.test.tssrc/tools/SnipTool/SnipTool.tssrc/tools/SnipTool/prompt.tssrc/utils/analyzeContext.mcp.test.tssrc/utils/messages.snipTag.test.tssrc/utils/messages.tssrc/utils/sessionStorage.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/tools/SnipTool/SnipTool.test.tssrc/services/compact/snipCompact.test.tssrc/utils/analyzeContext.mcp.test.tssrc/services/api/openaiShim.test.tssrc/utils/messages.snipTag.test.tssrc/commands/install-github-app/setupGitHubActions.test.tssrc/utils/sessionStorage.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/tools/SnipTool/SnipTool.test.tssrc/services/compact/snipCompact.test.tssrc/utils/analyzeContext.mcp.test.tssrc/tools/SnipTool/prompt.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.tssrc/tools/SnipTool/SnipTool.tssrc/utils/messages.snipTag.test.tssrc/commands/install-github-app/setupGitHubActions.test.tssrc/utils/sessionStorage.test.tssrc/ink/render-node-to-output.tssrc/services/compact/snipCompact.tssrc/utils/messages.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/SnipTool/SnipTool.test.tssrc/services/compact/snipCompact.test.tssrc/utils/analyzeContext.mcp.test.tsdocs/windows-aliases-and-launchers.mdsrc/tools/SnipTool/prompt.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.tssrc/tools/SnipTool/SnipTool.tssrc/utils/messages.snipTag.test.tsscripts/windows/openclaude-aliases.ps1src/commands/install-github-app/setupGitHubActions.test.tssrc/utils/sessionStorage.test.tssrc/ink/render-node-to-output.tssrc/services/compact/snipCompact.tssrc/utils/messages.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/SnipTool/SnipTool.test.tssrc/tools/SnipTool/prompt.tssrc/tools/SnipTool/SnipTool.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/SnipTool/SnipTool.test.tssrc/services/compact/snipCompact.test.tssrc/utils/analyzeContext.mcp.test.tssrc/services/api/openaiShim.test.tssrc/utils/messages.snipTag.test.tssrc/commands/install-github-app/setupGitHubActions.test.tssrc/utils/sessionStorage.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/tools/SnipTool/SnipTool.test.tssrc/services/compact/snipCompact.test.tssrc/utils/analyzeContext.mcp.test.tsdocs/windows-aliases-and-launchers.mdsrc/tools/SnipTool/prompt.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.tssrc/tools/SnipTool/SnipTool.tssrc/utils/messages.snipTag.test.tsscripts/windows/openclaude-aliases.ps1src/commands/install-github-app/setupGitHubActions.test.tssrc/utils/sessionStorage.test.tssrc/ink/render-node-to-output.tssrc/services/compact/snipCompact.tssrc/utils/messages.ts
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update docs when setup, commands, or user-facing behavior changes
Files:
docs/windows-aliases-and-launchers.md
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}
⚙️ CodeRabbit configuration file
{README.md,CONTRIBUTING.md,docs/**,.github/pull_request_template.md}: Review docs for accuracy against current code behavior. Flag security or provider claims that overpromise, stale install commands, missing setup caveats, and instructions that could push users toward unsafe credential handling. Keep purely wording-level suggestions non-blocking.
Files:
docs/windows-aliases-and-launchers.md
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}
⚙️ CodeRabbit configuration file
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.
Files:
scripts/windows/openclaude-aliases.ps1
🪛 OpenGrep (1.22.0)
src/services/compact/snipCompact.ts
[ERROR] 19-19: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 23-23: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (37)
docs/windows-aliases-and-launchers.md (1)
162-163: LGTM!scripts/windows/openclaude-aliases.ps1 (1)
206-207: LGTM!src/commands/install-github-app/setupGitHubActions.test.ts (4)
2-2: LGTM!
28-28: LGTM!
170-174: LGTM!
207-207: LGTM!Also applies to: 250-250, 278-278
src/utils/sessionStorage.test.ts (3)
18-18: LGTM!Also applies to: 27-31
173-182: LGTM!Also applies to: 192-207
580-580: LGTM!src/utils/analyzeContext.mcp.test.ts (1)
81-134: LGTM!src/ink/render-node-to-output.ts (5)
52-62: LGTM!
790-798: LGTM!
812-817: LGTM!
940-944: LGTM!
1482-1502: LGTM!src/utils/messages.ts (6)
200-201: LGTM!
1625-1721: LGTM!The migration from visible
[id:...]tags to internalsnip_id=...metadata wrapped in<system-reminder>is correctly implemented:
- Tag format aligns with
normalizeSnipShortIdregex in snipCompact.ts (context snippet 2)- Idempotency check correctly detects existing
snip_id=${idToken}markers via substring match- tool_result fallback appends a dedicated text block preserving the tool_result block intact
<system-reminder>wrapping ensures markers are filtered from user-visible exports (context snippet 3)
2048-2051: LGTM!
2214-2215: LGTM!Also applies to: 2251-2251, 2262-2262
2428-2433: LGTM!
2495-2495: LGTM!Also applies to: 2519-2520
src/services/api/openaiShim.ts (1)
766-773: LGTM!src/services/api/openaiShim.test.ts (2)
5242-5242: LGTM!
5325-5327: LGTM!src/utils/messages.snipTag.test.ts (5)
13-15: LGTM!
17-21: LGTM!
24-32: LGTM!
34-73: LGTM!
88-155: LGTM!src/services/compact/snipCompact.ts (3)
16-28: LGTM!
43-44: LGTM!
57-63: LGTM!src/tools/SnipTool/SnipTool.ts (2)
13-13: LGTM!
64-66: LGTM!src/tools/SnipTool/prompt.ts (1)
6-6: LGTM!src/services/compact/snipCompact.test.ts (1)
289-320: LGTM!src/tools/SnipTool/SnipTool.test.ts (1)
36-45: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ink/render-node-to-output.ts (1)
795-804:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBase the bottom-follow branch on
maxScroll, notscrollHeight.This branch detaches sticky follow when the viewport shrinks more than the content does. In that case
scrollHeightdecreased, so the code takes the "shrank" path, butmaxScrollstill increased. A user who was exactly at the previous bottom then failsscrollTopBeforeFollow >= maxScrolland stops following after a resize/layout change.Suggested fix
- const grew = scrollHeight >= prevScrollHeight - // Growth follow must compare against the previous max: a user who was - // exactly at bottom before streaming adds a row is now below the new - // maxScroll, but should still follow. When content shrinks, use the - // current maxScroll to avoid treating virtualization height artifacts - // as a real "was at bottom" signal. - const positionallyAtBottom = grew + const bottomMovedDown = maxScroll >= prevMaxScroll + // If the bottom moved down (content growth or viewport shrink), compare + // against the previous max so a user who was already at bottom keeps + // following. If the bottom moved up, compare against the current max so + // shrink/virtualization doesn't falsely re-enable follow. + const positionallyAtBottom = bottomMovedDown ? scrollTopBeforeFollow >= prevMaxScroll : scrollTopBeforeFollow >= maxScroll🤖 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/ink/render-node-to-output.ts` around lines 795 - 804, The follow-detachment branch is using scrollHeight comparison (grew = scrollHeight >= prevScrollHeight) which misclassifies shrinks when maxScroll actually increased; change the growth test to compare maxScroll against prevMaxScroll (e.g., grew = maxScroll >= prevMaxScroll) so positionallyAtBottom uses the correct "previous max" logic; update the logic around positionallyAtBottom and the comment to reference maxScroll/prevMaxScroll (symbols: grew, positionallyAtBottom, prevMaxScroll, maxScroll, scrollTopBeforeFollow) so a user exactly at the prior bottom still follows after layout changes.
🤖 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/services/api/openaiShim.ts`:
- Around line 2831-2833: The comment misleadingly states that response is
guaranteed after the try/catch because throwClassifiedTransportError returns
never, yet the code still has a defensive guard `if (!response) continue` inside
the retry loop; update the comment near the `response` usage (and the
surrounding retry loop) to clarify that while `throwClassifiedTransportError` is
typed as never, the `if (!response) continue` check is an intentional defensive
runtime guard for unexpected cases (or remove the incorrect absolute guarantee
claim), referencing the `throwClassifiedTransportError` call and the `response`
variable so future readers understand why the guard remains.
- Line 2879: The local variable responsesResponse currently uses a definite
assignment assertion ("responsesResponse!") which hides control flow; remove the
"!" and either let TypeScript infer the Response type (declare "let
responsesResponse: Response" and ensure all control flows assign it) or declare
"let responsesResponse: Response | undefined" and add a null/undefined check
before any use (or rely on the catch path that calls
throwClassifiedTransportError to exit). Update any subsequent uses of
responsesResponse to handle the possible undefined case or to occur only after
an assignment, referencing the responsesResponse variable in this scope.
---
Outside diff comments:
In `@src/ink/render-node-to-output.ts`:
- Around line 795-804: The follow-detachment branch is using scrollHeight
comparison (grew = scrollHeight >= prevScrollHeight) which misclassifies shrinks
when maxScroll actually increased; change the growth test to compare maxScroll
against prevMaxScroll (e.g., grew = maxScroll >= prevMaxScroll) so
positionallyAtBottom uses the correct "previous max" logic; update the logic
around positionallyAtBottom and the comment to reference maxScroll/prevMaxScroll
(symbols: grew, positionallyAtBottom, prevMaxScroll, maxScroll,
scrollTopBeforeFollow) so a user exactly at the prior bottom still follows after
layout changes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 68764d67-5d2b-4aba-94ef-97fa35f933c4
📒 Files selected for processing (4)
src/ink/render-node-to-output.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/utils/messages.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/services/api/openaiShim.tssrc/services/api/openaiShim.test.tssrc/ink/render-node-to-output.tssrc/utils/messages.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiShim.tssrc/services/api/openaiShim.test.tssrc/ink/render-node-to-output.tssrc/utils/messages.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/services/api/openaiShim.tssrc/services/api/openaiShim.test.tssrc/ink/render-node-to-output.tssrc/utils/messages.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/openaiShim.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiShim.test.ts
🔇 Additional comments (7)
src/ink/render-node-to-output.ts (1)
52-62: LGTM!Also applies to: 404-410, 946-955, 1496-1507
src/utils/messages.ts (1)
481-484: LGTM!Also applies to: 865-871, 885-888, 906-907, 942-943, 960-967, 986-989, 1811-1817, 2868-2869
src/services/api/openaiShim.ts (2)
1059-1061: LGTM!Also applies to: 1066-1077, 1122-1123, 1135-1142, 1338-1340, 1355-1389
770-774: Confirm: no code relies on the old tool-boundary marker string
- No occurrences of
"[Tool execution interrupted by user]"remain in the codebase."[Tool results received]"is only referenced insrc/services/api/openaiShim.tsandsrc/services/api/openaiShim.test.ts.src/utils/exportRenderer.tsx’sisSyntheticContentdoes not special-case either marker string (it filters different synthetic markers /SYNTHETIC_TOOL_RESULT_PLACEHOLDERblocks).src/services/api/openaiShim.test.ts (3)
1-1: LGTM!Also applies to: 3-3, 132-132, 259-259, 322-322, 380-380, 416-416, 479-479, 506-506, 546-546, 587-587, 621-621, 655-655, 689-689, 723-723, 768-768, 851-851, 943-943, 978-978, 1025-1025, 1107-1107, 1169-1169, 1214-1214, 1262-1262, 1307-1307, 1398-1398, 1475-1475, 1558-1558, 1639-1639, 1724-1724, 1774-1774, 1848-1848, 1893-1893, 1992-1992, 2068-2068, 2184-2184, 2244-2244, 2294-2294, 2343-2343, 2391-2391, 2644-2644, 2731-2731, 2811-2811, 2885-2885, 2972-2972, 3037-3037, 3099-3099, 3165-3165, 3226-3226, 3293-3293, 3391-3391, 3489-3489, 3565-3565, 3641-3641, 3739-3739, 3815-3815, 3891-3891, 3967-3967, 4036-4036, 4096-4096, 4151-4151, 4212-4212, 4274-4274, 4303-4303, 4356-4356, 4401-4401, 4446-4446, 4465-4488, 4558-4558, 4597-4630, 4655-4715, 4743-4776, 4807-4811, 4842-4844, 4877-4879, 4909-4911, 4934-4940, 4994-4994, 5054-5054, 5116-5116, 5183-5183, 5261-5261, 5298-5298, 5351-5351, 5385-5385, 5417-5417, 5449-5449, 5488-5488, 5555-5555, 5613-5613, 5672-5672, 5738-5738, 5770-5770, 5804-5804, 5871-5871, 5910-5910, 5943-5943, 5991-5991, 6068-6068, 6126-6126, 6199-6199, 6240-6240, 6277-6277, 6329-6329, 6375-6375, 6413-6413, 6449-6449
2198-2201: LGTM!
5328-5330: LGTM!
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ink/render-node-to-output.ts (1)
795-804:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBase the bottom-follow branch on
maxScroll, notscrollHeight.This branch detaches sticky follow when the viewport shrinks more than the content does. In that case
scrollHeightdecreased, so the code takes the "shrank" path, butmaxScrollstill increased. A user who was exactly at the previous bottom then failsscrollTopBeforeFollow >= maxScrolland stops following after a resize/layout change.Suggested fix
- const grew = scrollHeight >= prevScrollHeight - // Growth follow must compare against the previous max: a user who was - // exactly at bottom before streaming adds a row is now below the new - // maxScroll, but should still follow. When content shrinks, use the - // current maxScroll to avoid treating virtualization height artifacts - // as a real "was at bottom" signal. - const positionallyAtBottom = grew + const bottomMovedDown = maxScroll >= prevMaxScroll + // If the bottom moved down (content growth or viewport shrink), compare + // against the previous max so a user who was already at bottom keeps + // following. If the bottom moved up, compare against the current max so + // shrink/virtualization doesn't falsely re-enable follow. + const positionallyAtBottom = bottomMovedDown ? scrollTopBeforeFollow >= prevMaxScroll : scrollTopBeforeFollow >= maxScroll🤖 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/ink/render-node-to-output.ts` around lines 795 - 804, The follow-detachment branch is using scrollHeight comparison (grew = scrollHeight >= prevScrollHeight) which misclassifies shrinks when maxScroll actually increased; change the growth test to compare maxScroll against prevMaxScroll (e.g., grew = maxScroll >= prevMaxScroll) so positionallyAtBottom uses the correct "previous max" logic; update the logic around positionallyAtBottom and the comment to reference maxScroll/prevMaxScroll (symbols: grew, positionallyAtBottom, prevMaxScroll, maxScroll, scrollTopBeforeFollow) so a user exactly at the prior bottom still follows after layout changes.
🤖 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/services/api/openaiShim.ts`:
- Around line 2831-2833: The comment misleadingly states that response is
guaranteed after the try/catch because throwClassifiedTransportError returns
never, yet the code still has a defensive guard `if (!response) continue` inside
the retry loop; update the comment near the `response` usage (and the
surrounding retry loop) to clarify that while `throwClassifiedTransportError` is
typed as never, the `if (!response) continue` check is an intentional defensive
runtime guard for unexpected cases (or remove the incorrect absolute guarantee
claim), referencing the `throwClassifiedTransportError` call and the `response`
variable so future readers understand why the guard remains.
- Line 2879: The local variable responsesResponse currently uses a definite
assignment assertion ("responsesResponse!") which hides control flow; remove the
"!" and either let TypeScript infer the Response type (declare "let
responsesResponse: Response" and ensure all control flows assign it) or declare
"let responsesResponse: Response | undefined" and add a null/undefined check
before any use (or rely on the catch path that calls
throwClassifiedTransportError to exit). Update any subsequent uses of
responsesResponse to handle the possible undefined case or to occur only after
an assignment, referencing the responsesResponse variable in this scope.
---
Outside diff comments:
In `@src/ink/render-node-to-output.ts`:
- Around line 795-804: The follow-detachment branch is using scrollHeight
comparison (grew = scrollHeight >= prevScrollHeight) which misclassifies shrinks
when maxScroll actually increased; change the growth test to compare maxScroll
against prevMaxScroll (e.g., grew = maxScroll >= prevMaxScroll) so
positionallyAtBottom uses the correct "previous max" logic; update the logic
around positionallyAtBottom and the comment to reference maxScroll/prevMaxScroll
(symbols: grew, positionallyAtBottom, prevMaxScroll, maxScroll,
scrollTopBeforeFollow) so a user exactly at the prior bottom still follows after
layout changes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 68764d67-5d2b-4aba-94ef-97fa35f933c4
📒 Files selected for processing (4)
src/ink/render-node-to-output.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/utils/messages.ts
📜 Review details
🔇 Additional comments (7)
src/ink/render-node-to-output.ts (1)
52-62: LGTM!Also applies to: 404-410, 946-955, 1496-1507
src/utils/messages.ts (1)
481-484: LGTM!Also applies to: 865-871, 885-888, 906-907, 942-943, 960-967, 986-989, 1811-1817, 2868-2869
src/services/api/openaiShim.ts (2)
1059-1061: LGTM!Also applies to: 1066-1077, 1122-1123, 1135-1142, 1338-1340, 1355-1389
770-774: Confirm: no code relies on the old tool-boundary marker string
- No occurrences of
"[Tool execution interrupted by user]"remain in the codebase."[Tool results received]"is only referenced insrc/services/api/openaiShim.tsandsrc/services/api/openaiShim.test.ts.src/utils/exportRenderer.tsx’sisSyntheticContentdoes not special-case either marker string (it filters different synthetic markers /SYNTHETIC_TOOL_RESULT_PLACEHOLDERblocks).src/services/api/openaiShim.test.ts (3)
1-1: LGTM!Also applies to: 3-3, 132-132, 259-259, 322-322, 380-380, 416-416, 479-479, 506-506, 546-546, 587-587, 621-621, 655-655, 689-689, 723-723, 768-768, 851-851, 943-943, 978-978, 1025-1025, 1107-1107, 1169-1169, 1214-1214, 1262-1262, 1307-1307, 1398-1398, 1475-1475, 1558-1558, 1639-1639, 1724-1724, 1774-1774, 1848-1848, 1893-1893, 1992-1992, 2068-2068, 2184-2184, 2244-2244, 2294-2294, 2343-2343, 2391-2391, 2644-2644, 2731-2731, 2811-2811, 2885-2885, 2972-2972, 3037-3037, 3099-3099, 3165-3165, 3226-3226, 3293-3293, 3391-3391, 3489-3489, 3565-3565, 3641-3641, 3739-3739, 3815-3815, 3891-3891, 3967-3967, 4036-4036, 4096-4096, 4151-4151, 4212-4212, 4274-4274, 4303-4303, 4356-4356, 4401-4401, 4446-4446, 4465-4488, 4558-4558, 4597-4630, 4655-4715, 4743-4776, 4807-4811, 4842-4844, 4877-4879, 4909-4911, 4934-4940, 4994-4994, 5054-5054, 5116-5116, 5183-5183, 5261-5261, 5298-5298, 5351-5351, 5385-5385, 5417-5417, 5449-5449, 5488-5488, 5555-5555, 5613-5613, 5672-5672, 5738-5738, 5770-5770, 5804-5804, 5871-5871, 5910-5910, 5943-5943, 5991-5991, 6068-6068, 6126-6126, 6199-6199, 6240-6240, 6277-6277, 6329-6329, 6375-6375, 6413-6413, 6449-6449
2198-2201: LGTM!
5328-5330: LGTM!
🛑 Comments failed to post (2)
src/services/api/openaiShim.ts (2)
2831-2833: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Clarify comment vs runtime guard.
The comment states
responseis guaranteed after thetry/catchbecause the catch always throws, but the code still checksif (!response) continue. WhilethrowClassifiedTransportErrordoes returnnever, the defensive guard in a retry loop is reasonable. Consider either removing the comment or noting that the guard is defensive.🤖 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/services/api/openaiShim.ts` around lines 2831 - 2833, The comment misleadingly states that response is guaranteed after the try/catch because throwClassifiedTransportError returns never, yet the code still has a defensive guard `if (!response) continue` inside the retry loop; update the comment near the `response` usage (and the surrounding retry loop) to clarify that while `throwClassifiedTransportError` is typed as never, the `if (!response) continue` check is an intentional defensive runtime guard for unexpected cases (or remove the incorrect absolute guarantee claim), referencing the `throwClassifiedTransportError` call and the `response` variable so future readers understand why the guard remains.
2879-2879: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Remove definite assignment assertion for clarity.
The
!assertion onresponsesResponseis technically safe (the catch block throws viathrowClassifiedTransportError), but it obscures the control flow. Consider removing it and letting TypeScript infer the type, or use an explicit| undefinedtype with a proper check.♻️ Suggested refactor
- let responsesResponse!: Response + let responsesResponse: Response try { responsesResponse = await fetchWithProxyRetry(responsesUrl, {🤖 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/services/api/openaiShim.ts` at line 2879, The local variable responsesResponse currently uses a definite assignment assertion ("responsesResponse!") which hides control flow; remove the "!" and either let TypeScript infer the Response type (declare "let responsesResponse: Response" and ensure all control flows assign it) or declare "let responsesResponse: Response | undefined" and add a null/undefined check before any use (or rely on the catch path that calls throwClassifiedTransportError to exit). Update any subsequent uses of responsesResponse to handle the possible undefined case or to occur only after an assignment, referencing the responsesResponse variable in this scope.
Use usage-bearing high-context fixtures in autoCompactCooldown tests so the cooldown assertions do not depend on process-global threshold overrides surviving full-suite order. Add CodeRabbit-requested SAST suppression comments for snip regex literals.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/services/compact/snipCompact.ts`:
- Line 19: Update the invalid inline nosemgrep suppressions in
src/services/compact/snipCompact.ts by moving the explanatory text into a
separate comment line and using a proper nosemgrep rule-id on the suppression
line; specifically, for both places where a regex call uses .exec(trimmed)
(locate the .exec(trimmed) usages in the file), replace the single-line "//
nosemgrep: regex literal only; this does not execute user input." with two
lines: one comment containing the explanation ("// Regex literal only; this does
not execute user input.") followed by a pure suppression line using a rule id
(e.g. "// nosemgrep: coderabbit.command-injection.exec-js").
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6beb7b31-e906-444b-bee9-038b2a7b80dd
📒 Files selected for processing (2)
src/query/autoCompactCooldown.test.tssrc/services/compact/snipCompact.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/query/autoCompactCooldown.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/query/autoCompactCooldown.test.tssrc/services/compact/snipCompact.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/query/autoCompactCooldown.test.tssrc/services/compact/snipCompact.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/query/autoCompactCooldown.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/query/autoCompactCooldown.test.tssrc/services/compact/snipCompact.ts
🪛 OpenGrep (1.22.0)
src/services/compact/snipCompact.ts
[ERROR] 20-20: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (2)
src/query/autoCompactCooldown.test.ts (2)
75-95: LGTM!
167-167: LGTM!Also applies to: 222-222, 365-365
Update the snip regex suppressions to CodeRabbit's requested nosemgrep rule-id form, with the explanatory text kept in a separate comment.
Set and restore CLAUDE_CODE_AUTO_COMPACT_WINDOW in autoCompactCooldown tests so the high-context fixture lands above the auto-compact threshold but below the hard prompt limit on both local Windows and Linux CI.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/query/autoCompactCooldown.test.ts (1)
78-98:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFixture content is tiny but claims 170k tokens — test fails because token counting re-counts content, not usage metadata.
The
highContextMessages()helper setsusage.input_tokens: 170_000, but the actual message content is "previous response" (~15 tokens) + "continue" (~5 tokens). Token counting in production code re-counts the actual content, not historical usage metadata, so the fixture registers as ~100 tokens instead of 170k.With
CLAUDE_CODE_AUTO_COMPACT_WINDOW='200000', the auto-compact threshold is ~153k-163k (80-85% of the effective 192k window). The ~100-token fixture falls far below this threshold, soisAboveAutoCompactThresholdreturns false, the blocking check is skipped, andcallModelis invoked — causing the test at line 211 to fail.🛠️ Recommended fix: generate realistic large content
Replace the tiny text with a string large enough to actually represent 170k tokens (roughly 500k+ characters):
function highContextMessages(): Message[] { + // Generate ~170k tokens of content (roughly 500k characters) + const largeText = 'A'.repeat(500_000) return [ { type: 'assistant', message: { id: 'msg-high-context', role: 'assistant', - content: [{ type: 'text', text: 'previous response' }], + content: [{ type: 'text', text: largeText }], usage: { input_tokens: 170_000, output_tokens: 1_000,Alternatively, mock the token counting function to return the expected value, or adjust
CLAUDE_CODE_AUTO_COMPACT_WINDOWto a much lower value so the tiny content still exceeds the threshold.🤖 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/query/autoCompactCooldown.test.ts` around lines 78 - 98, The test fixture highContextMessages() claims 170k input tokens but its content is tiny, so production token counting (used by isAboveAutoCompactThreshold and the caller that may invoke callModel) re-counts and the message doesn't exceed the auto-compact threshold; fix by making the fixture produce realistic large content (e.g., replace the short "previous response" text with a generated string large enough to approximate 170k tokens / ~500k+ characters), or alternatively adjust the test to stub/mock the token counting function used by isAboveAutoCompactThreshold or set CLAUDE_CODE_AUTO_COMPACT_WINDOW to a much smaller value so the fixture legitimately triggers the auto-compact path. Ensure changes reference highContextMessages, isAboveAutoCompactThreshold, and callModel when updating the test.
🤖 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.
Outside diff comments:
In `@src/query/autoCompactCooldown.test.ts`:
- Around line 78-98: The test fixture highContextMessages() claims 170k input
tokens but its content is tiny, so production token counting (used by
isAboveAutoCompactThreshold and the caller that may invoke callModel) re-counts
and the message doesn't exceed the auto-compact threshold; fix by making the
fixture produce realistic large content (e.g., replace the short "previous
response" text with a generated string large enough to approximate 170k tokens /
~500k+ characters), or alternatively adjust the test to stub/mock the token
counting function used by isAboveAutoCompactThreshold or set
CLAUDE_CODE_AUTO_COMPACT_WINDOW to a much smaller value so the fixture
legitimately triggers the auto-compact path. Ensure changes reference
highContextMessages, isAboveAutoCompactThreshold, and callModel when updating
the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9168f025-fe96-40b5-9026-0893cc4cb88f
📒 Files selected for processing (1)
src/query/autoCompactCooldown.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/query/autoCompactCooldown.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise
Files:
src/query/autoCompactCooldown.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/query/autoCompactCooldown.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/query/autoCompactCooldown.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/query/autoCompactCooldown.test.ts
🪛 GitHub Actions: PR Checks / 0_smoke-and-tests.txt
src/query/autoCompactCooldown.test.ts
[error] 211-211: Jest assertion failed: expect(callModel).not.toHaveBeenCalled(). Expected number of calls: 0, Received number of calls: 1.
🪛 GitHub Actions: PR Checks / smoke-and-tests
src/query/autoCompactCooldown.test.ts
[error] 211-211: Test assertion failed (Jest). expect(callModel).not.toHaveBeenCalled() expected 0 calls, but received 1.
🔇 Additional comments (1)
src/query/autoCompactCooldown.test.ts (1)
20-21: LGTM!Also applies to: 36-36
Block oversized requests when autocompact reports an active or tripped breaker, even if a later auto-compact config read is stale. This keeps cooldown protection active and stabilizes the CI smoke path.
* Fix snip metadata leaks and TUI corruption Replace visible snip ID tags with internal snip_id metadata and update SnipTool guidance/tests so the model can request pruning without echoing user-visible IDs. Add compatibility parsing for legacy [id:...] markers, clarify synthetic OpenAI shim tool-result messages, and tighten test cleanup around env/session state. Mitigate Konsole+tmux rendering corruption by disabling the scroll fast path in that environment while preserving bottom-follow behavior and clearing culled cached output. Validation: bun run build; bun run smoke; ANTHROPIC_API_KEY=test-key bun run check; ANTHROPIC_API_KEY=test-key bun run test:full; python -m pytest -q python/tests; bun run security:pr-scan -- --base upstream/main. * Stabilize cooldown smoke test Use usage-bearing high-context fixtures in autoCompactCooldown tests so the cooldown assertions do not depend on process-global threshold overrides surviving full-suite order. Add CodeRabbit-requested SAST suppression comments for snip regex literals. * Use rule-id semgrep suppressions Update the snip regex suppressions to CodeRabbit's requested nosemgrep rule-id form, with the explanatory text kept in a separate comment. * Pin cooldown test context window Set and restore CLAUDE_CODE_AUTO_COMPACT_WINDOW in autoCompactCooldown tests so the high-context fixture lands above the auto-compact threshold but below the hard prompt limit on both local Windows and Linux CI. * Honor autocompact breaker metadata Block oversized requests when autocompact reports an active or tripped breaker, even if a later auto-compact config read is stale. This keeps cooldown protection active and stabilizes the CI smoke path.
Cherry-picked from upstream 9c74231. Replace visible snip ID tags with internal <system-reminder>snip_id=...</system-reminder> metadata and update SnipTool guidance/tests so the model can request pruning without echoing user-visible IDs. Adds compatibility parsing for legacy [id:...] markers, clarifies synthetic OpenAI shim tool-result messages, tightens test cleanup around env/session state. Mitigates Konsole+tmux rendering corruption by disabling the scroll fast path while preserving bottom-follow behavior and clearing culled cached output. OC-specific changes (upstream HEAD files ported verbatim): - src/utils/messages.ts: full UP HEAD replacement - src/utils/sessionStorage.test.ts: full UP HEAD replacement - src/utils/messages.snipTag.test.ts: new file from UP OC-specific stubs: - src/utils/sessionStorage.ts: NO-OP recordGoalState stub (OC lacks Project.insertGoalState + GoalStateEntry — pending port of PR Twigpine#1293 session-scoped /goal continuation) - src/services/api/errors.ts: getVisionNotSupportedErrorMessages + getVisionNotSupportedErrorMessage + VISION_NOT_SUPPORTED_MESSAGE_PREFIX - src/utils/permissions/PermissionMode.ts: isDangerousPermissionMode (OC uses 'bypassPermissions' only, not 'fullAccess') Includes 8 new tests in messages.snipTag.test.ts. 4 goal-related sessionStorage tests fail (stubbed out, see above). Also includes UP autoCompactCooldown test changes (1 test fails due to global fixture ordering — pre-existing flakiness, not port regression).
2 participants
Summary
Fixes #1578.
Mitigates #1584.
[id:...]message tags with internalsnip_id=...metadata wrapped in<system-reminder>so snip targets remain available without leaking into user-visible transcript text or model responses[id:...]markers while accepting bare/fullsnip_id=...metadataUser / Developer Impact
OPENCLAUDE_KONSOLE_TMUX_FAST_SCROLLavailable as an escape hatch.Validation
Ran locally on Windows / PowerShell:
bun run buildbun run smoke$env:ANTHROPIC_API_KEY='test-key'; bun run check3691 pass, 0 fail$env:ANTHROPIC_API_KEY='test-key'; bun run test:full3691 pass, 0 failpython -m pytest -q python/tests44 passedbun run security:pr-scan -- --base upstream/mainSummary by CodeRabbit
Bug Fixes
UX / Messaging
Tests