[1135] feature: unified-shell-resolution (3/4) - #54
Closed
myk1yt wants to merge 31 commits into
Closed
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.
…add ignoreBinaries
…nterface - optional lifecycle broke TS18048 across TerminalRegistry, ExecuteCommandTool, and ExecaTerminalProcess
…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
…lation in terminal-lifecycle test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Upstream PR: Zoo-Code-Org#1135