Skip to content

Fix/shell#387

Open
makiroll1125 wants to merge 2 commits into
V1.4.1from
fix/shell
Open

Fix/shell#387
makiroll1125 wants to merge 2 commits into
V1.4.1from
fix/shell

Conversation

@makiroll1125

@makiroll1125 makiroll1125 commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

What

  • run_shell cmd branch: app/data/action/run_shell.py now invokes the Windows cmd shell via subprocess.Popen(args, shell=use_shell, ...) with use_shell = shell_choice == "cmd", passing the raw command string, instead of the old list-form args = ["cmd.exe", "/d", "/s", "/c", command] passed to Popen without shell=True. powershell/pwsh branches are untouched.
  • Chat draft persistence: chatInputSlice.ts gains a drafts: Record<string, string> map (keyed by livingUIId ?? 'main') plus setDraftText/clearDraftText reducers; store/selectors/chatInput.ts gains a selectDraftText(key) selector; Chat.tsx's composer text now reads/writes through Redux instead of a local useState.
  • BytePlus silent failures: agent_core/core/impl/llm/interface.py's _generate_byteplus_with_session and _generate_byteplus_with_prefix_cache now populate error/error_info_obj (via classify_llm_error) on their return dict whenever the underlying call raised, instead of discarding the caught exception and returning empty content.

Why

  • run_shell: Python's automatic list2cmdline() quoting for a list-form Popen call backslash-escapes embedded double quotes, but cmd.exe's own /C//S argument parser doesn't understand backslash-escaped quotes, it expects literal, unescaped ones. Any command containing a quoted argument (e.g. claude -p "<prompt>") got corrupted before reaching the shell, silently breaking every such invocation rather than raising a clear error.
  • Chat draft persistence: the composer's useState lived inside Chat.tsx, which react-router's <Routes> fully unmounts when navigating to another tab (Settings, Tasks & Actions) so a draft in progress was destroyed on every tab switch. Moving it into the chatInput Redux slice (already mounted above the router) lets it survive route unmounts. It's keyed by conversation because Chat.tsx is shared between the main Chat page and every Living UI project's chat panel, so a flat single field would have leaked one conversation's draft into another's.
  • BytePlus silent failures: both functions already caught the underlying exception (e.g. a 429 HTTPError) into a local exc_obj, but the final return discarded it, so a rate-limited or otherwise-failed call looked identical to "success with no content" (_finalize_session_response logs a generic "LLM returned empty response" and can't distinguish it). Consecutive failures still aborted the agent after 5 tries, but with no rate-limit classification attached, so no backoff/retry-aware handling downstream ever fired. Five other provider paths in the same file (including BytePlus's own non-session path) already follow the correct pattern; the session/prefix-cache paths just hadn't been brought in line.
  • Closes [V1.4.1] Silent context overflow bug in the BytePlus LLM provider #384 and [V1.4.1] Text in chat input removed when switch to other tabs #382

How to test

run_shell: run a run_shell action with shell: "cmd" and a command containing an embedded quoted argument (e.g. claude -p "test prompt" or any foo -x "a b" style invocation), in both foreground and background mode. Confirm the child process receives the argument intact (no stray literal quote characters), where previously it would fail to parse or resolve to a mangled path/argument.

Chat draft persistence:
Type a draft in the main Chat input, navigate to a new tab such as Settings or Tasks & Actions, then navigate back and the draft should still be present.

BytePlus silent failures: mock/trigger a BytePlus API call that returns a 429 (or any non-2xx) response and confirm the returned dict now has error and error_info_obj set (error_info_obj.category == RATE_LIMIT for a 429), and that _finalize_session_response logs a classified rate-limit error instead of "LLM returned empty response." If a live BytePlus key with rate-limit headroom is available, reproduce by issuing requests fast enough to hit ModelAccountTpmRateLimitExceeded and confirm the same.

@makiroll1125
makiroll1125 requested review from ahmad-ajmal and zfoong and removed request for ahmad-ajmal July 17, 2026 05:06
@makiroll1125

makiroll1125 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Don't merge yet, I plan to merge some additional fixes from fix/misc-issues.

#329 and #381 fixed, #380 in progress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants