Commit Graph

14 Commits

Author SHA1 Message Date
ZeyuFu 2bd7946001 fix(non-interactive-env): use powershell syntax on Windows regardless of SHELL/MSYSTEM
Closes #3607.

## Root cause

On Windows, `detectCommandShellType()` fell through to `detectShellType()`
for two common environments and incorrectly returned `"unix"`:

1. **SHELL points at a Unix-shaped path** (e.g. Git Bash sets
   `SHELL=/usr/bin/bash` on a fresh Windows install).
   The `detectWindowsShellType(process.env.SHELL)` probe didn't recognize
   `bash` as a Windows shell, so the function fell through and
   `detectShellType()` returned `"unix"`.

2. **MSYSTEM is set but SHELL is not** (Git Bash leaves MSYSTEM permanently
   set system-wide even when the active shell is PowerShell).
   The fall-through path returned `"unix"` via the MSYSTEM check.

In both cases, the hook then prepended `export KEY=val;` to git commands,
which PowerShell rejects with:

  `export : 无法将"export"项识别为 cmdlet...`

OpenCode on Windows runs the bash tool through a Windows shell
(PowerShell by default, cmd as the user-overridable fallback), regardless
of MSYSTEM or a Unix-shaped SHELL set by Git Bash — so the env prefix
must use Windows-compatible syntax.

## Fix

`detectCommandShellType()` now short-circuits on `process.platform === "win32"`:

- If `SHELL` points at a recognized Windows shell (`cmd.exe`, `powershell.exe`,
  `pwsh.exe`), return that.
- If `SHELL` and `MSYSTEM` are both unset, fall back to `ComSpec` then to cmd.
- Otherwise, default to PowerShell — matching what OpenCode actually spawns.

`detectShellType()` is unchanged; other callers (including non-Windows
platforms) are unaffected.

## Test changes

Three pre-existing tests encoded the buggy behavior as expected behavior
and have been updated to assert the new PowerShell syntax with a
`(#3607)` marker and a comment explaining why a Unix-shaped SHELL on
win32 must still resolve to PowerShell. WSL is not affected because in
WSL `process.platform === "linux"`, not `"win32"`.

- `src/hooks/non-interactive-env/`: 24 tests pass / 0 fail
- `bunx tsc --noEmit`: clean

## Note on issue thread

The sisyphus-bot triage comment on #3607 framed this as a policy choice
between (A) forcing Windows env-prefix syntax and (B) resolving against
the OpenCode-configured shell. This PR implements option (A) as the
minimal surgical fix; option (B) remains a follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 10:59:29 -04:00
YeonGyu-Kim 47d60a74d3 fix(non-interactive-env): honor Windows ComSpec shell
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-05-13 22:47:37 +09:00
YeonGyu-Kim fef1d4530b fix(non-interactive-env): respect Windows command shell
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-05-13 20:07:46 +09:00
YeonGyu-Kim 0dab3116b7 fix: resolve 3 bugs (#3366, #3272, #3222)
- shell-env: detect Git Bash via MSYSTEM env var when SHELL is unset (#3366)
  On some Git Bash installations SHELL is not set but MSYSTEM (MINGW64/MSYS)
  is always present. Check MSYSTEM before PSModulePath to avoid emitting
  PowerShell syntax in bash shells.

- session-state: resolve legacy agent names in resolveRegisteredAgentName (#3272)
  Historical sessions stored agent names like 'Sisyphus (Ultraworker)' which
  don't match the current registered format. Fall back to getAgentConfigKey
  for legacy/parenthesized name resolution before returning the raw name.

- config-migration: skip backup when file content is unchanged (#3222)
  Compare serialized config with existing file content before creating a
  timestamped .bak file. Only create backup when the on-disk content
  actually differs from the migrated content.
2026-04-15 10:43:44 +09:00
YeonGyu-Kim 796a646c63 fix(non-interactive-env): use detectShellType instead of hardcoded win32 check (#3310, #3338)
Hardcoded platform check forced PowerShell env syntax on Windows
regardless of the actual shell. Replaced with detectShellType() so
Git Bash, WSL, and similar environments use unix prefixes while
native PowerShell continues to get powershell prefixes.

🤖 Generated with OhMyOpenCode assistance
https://github.com/code-yeongyu/oh-my-opencode
2026-04-12 02:29:28 +09:00
YeonGyu-Kim 706640ace3 fix: use detectShellType() instead of hardcoded 'unix' in non-interactive-env hook
The non-interactive-env hook hardcoded shellType as 'unix', causing
'export' syntax to be used on Windows PowerShell where it doesn't work.
This caused sub-agent infinite loops on Windows as git commands would
fail with 'export: The term export is not recognized' and retry forever.

Fix: use the existing detectShellType() function which correctly detects
PowerShell (via PSModulePath), csh, cmd (win32 fallback), and unix shells.

Updated tests to verify platform-aware shell syntax selection.

Closes #3000
2026-04-03 18:26:24 +09:00
Strocs e5b7fd40bb test(non-interactive-env): add idempotency test for env prefix injection 2026-02-13 21:51:38 -03:00
YeonGyu-Kim f146aeff0f refactor: major codebase cleanup - BDD comments, file splitting, bug fixes (#1350)
* style(tests): normalize BDD comments from '// #given' to '// given'

- Replace 4,668 Python-style BDD comments across 107 test files
- Patterns changed: // #given -> // given, // #when -> // when, // #then -> // then
- Also handles no-space variants: //#given -> // given

* fix(rules-injector): prefer output.metadata.filePath over output.title

- Extract file path resolution to dedicated output-path.ts module
- Prefer metadata.filePath which contains actual file path
- Fall back to output.title only when metadata unavailable
- Fixes issue where rules weren't injected when tool output title was a label

* feat(slashcommand): add optional user_message parameter

- Add user_message optional parameter for command arguments
- Model can now call: command='publish' user_message='patch'
- Improves error messages with clearer format guidance
- Helps LLMs understand correct parameter usage

* feat(hooks): restore compaction-context-injector hook

- Restore hook deleted in cbbc7bd0 for session compaction context
- Injects 7 mandatory sections: User Requests, Final Goal, Work Completed,
  Remaining Tasks, Active Working Context, MUST NOT Do, Agent Verification State
- Re-register in hooks/index.ts and main plugin entry

* refactor(background-agent): split manager.ts into focused modules

- Extract constants.ts for TTL values and internal types (52 lines)
- Extract state.ts for TaskStateManager class (204 lines)
- Extract spawner.ts for task creation logic (244 lines)
- Extract result-handler.ts for completion handling (265 lines)
- Reduce manager.ts from 1377 to 755 lines (45% reduction)
- Maintain backward compatible exports

* refactor(agents): split prometheus-prompt.ts into subdirectory

- Move 1196-line prometheus-prompt.ts to prometheus/ subdirectory
- Organize prompt sections into separate files for maintainability
- Update agents/index.ts exports

* refactor(delegate-task): split tools.ts into focused modules

- Extract categories.ts for category definitions and routing
- Extract executor.ts for task execution logic
- Extract helpers.ts for utility functions
- Extract prompt-builder.ts for prompt construction
- Reduce tools.ts complexity with cleaner separation of concerns

* refactor(builtin-skills): split skills.ts into individual skill files

- Move each skill to dedicated file in skills/ subdirectory
- Create barrel export for backward compatibility
- Improve maintainability with focused skill modules

* chore: update import paths and lockfile

- Update prometheus import path after refactor
- Update bun.lock

* fix(tests): complete BDD comment normalization

- Fix remaining #when/#then patterns missed by initial sed
- Affected: state.test.ts, events.test.ts

---------

Co-authored-by: justsisyphus <justsisyphus@users.noreply.github.com>
2026-02-01 16:47:50 +09:00
YeonGyu-Kim 15c4637e0a fix(hooks): use unix shell syntax for bash tool on all platforms (#1015)
The bash tool always runs in a Unix-like shell (bash/sh), even on Windows (via Git Bash, WSL, etc.), so we should always use unix export syntax instead of detecting the shell type dynamically.

Fixes #983

Fixes #889

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: justsisyphus <justsisyphus@users.noreply.github.com>
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-01-23 16:24:58 +09:00
Victor Sumner 4e8106b019 test: stabilize non-interactive env hook
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-01-18 20:39:02 -05:00
Jeon Suyeol c38b078c12 fix(test): isolate environment in non-interactive-env hook tests (#822)
- Delete PSModulePath in beforeEach() to prevent CI cross-platform detection
- Set SHELL=/bin/bash to ensure tests start with clean Unix-like environment
- Fixes flaky test failures on GitHub Actions CI runners
- Tests can still override these values for PowerShell-specific behavior
2026-01-16 00:54:53 +09:00
Sisyphus d6499cbe31 fix(non-interactive-env): add Windows/PowerShell support (#573)
* fix(non-interactive-env): add Windows/PowerShell support

- Create shared shell-env utility with cross-platform shell detection
- Detect shell type via PSModulePath, SHELL env vars, platform fallback
- Support Unix (export), PowerShell ($env:), and cmd.exe (set) syntax
- Add 41 comprehensive unit tests for shell-env utilities
- Add 5 cross-platform integration tests for hook behavior
- All 696 tests pass, type checking passes, build succeeds

Closes #566

* fix: address review feedback - add isNonInteractive check and cmd.exe % escaping

- Add isNonInteractive() check to only apply env vars in CI/non-interactive contexts (Issue #566)
- Fix cmd.exe percent sign escaping to prevent environment variable expansion
- Update test expectations for correct % escaping behavior

Resolves feedback from @greptile-apps and @cubic-dev-ai

---------

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
2026-01-15 16:04:06 +09:00
YeonGyu-Kim f61e1a5f2b fix(non-interactive-env): use export for env vars to apply to all chained commands
Previous `VAR=val cmd` format only applied to first command in chains.
New `export VAR=val; cmd` format ensures variables persist for all commands.

Also increased test timeouts for todo-continuation-enforcer stability.

🤖 GENERATED WITH ASSISTANCE OF [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
2026-01-03 15:36:01 +09:00
YeonGyu-Kim 4a9bdc89aa fix(non-interactive-env): prepend env vars directly to git command string
OpenCode's bash tool ignores args.env and uses hardcoded process.env in spawn().
Work around this by prepending GIT_EDITOR, EDITOR, VISUAL, and PAGER env vars
directly to the command string. Only applies to git commands to avoid bloating
non-git commands.

Added shellEscape() and buildEnvPrefix() helper functions to properly escape
env var values and construct the prefix string.

🤖 GENERATED WITH ASSISTANCE OF [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode)
2026-01-02 22:30:55 +09:00