main
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Faych
|
9bd7a37a77 |
fix: _extract_retry_after returns None for non-retryable errors (#394)
* fix: _extract_retry_after returns None for non-retryable errors
* fix: update _extract_retry_after to handle generic transient errors with default retry delay
* fix: extend base _non_retryable_patterns in channel subclasses
* fix(channels): merge structured SDK error check into non-retryable step
* fix(channels): decouple status code and SDK error code extraction in retry logic
- Independently evaluate HTTP status codes and structured SDK error codes
- Fix misleading doc comments for Feishu and DingTalk patterns
- Remove redundant try-except AttributeError on getattr with default
- Expand test coverage for dual-signal matrix and header parsing
* refactor(channels): simplify status code and SDK error extraction via channel overrides
- Handle httpx and aiohttp exceptions in base Channel class
- Override _extract_status_code and _extract_sdk_error_code in SlackChannel and DiscordChannel
- Replace mock exception types in comprehensive test suite with real httpx and aiohttp errors
- Add dedicated Slack and Discord retry error extraction test suites
* fix(channels): clean up Slack and Discord error code extraction
- Remove defensive string checks and attribute guards in SlackChannel
- Directly access exc.response.status_code and exc.response.get('error') in SlackChannel
- Remove unnecessary _extract_sdk_error_code override in DiscordChannel
- Use real SlackApiError, SlackResponse, and discord.HTTPException in unit tests
* refactor: reorder retry logic to prioritize non-retryable checks, remove aiohttp dependency, and clean up exception handling in base and channel modules.
* test(channels): skip Slack/Discord retry tests when the SDK extra is absent
The retry-extraction tests build real SlackApiError / discord.HTTPException
objects, but slack-sdk and discord.py are optional extras that the dev
dependency group does not install. Under CI's `uv sync --dev` all nine
tests failed with ModuleNotFoundError raised from the channel override.
Gate both test classes with skipif(find_spec(...) is None) so the suite is
green without the extras and the tests still run wherever they are installed.
* refactor(channels): replace retry-delay lookup with _extract_retry_delay
_extract_retry_after still read the server-supplied delay by probing
exc.retry_after and exc.response.headers via getattr/hasattr, the last
remnant of the pattern the extractors moved away from. Replace both steps
with one overridable hook, _extract_retry_delay, implemented against the
real exception types:
- base: httpx.HTTPStatusError -> Retry-After header (httpx.Headers is
case-insensitive; HTTP-date form remains unsupported)
- SlackChannel: SlackApiError -> Retry-After, matched case-insensitively
because SlackResponse.headers is a plain dict whose casing depends on the
HTTP client (same approach as slack_sdk's RateLimitErrorRetryHandler)
- TelegramChannel: telegram.error.RetryAfter.retry_after (int, or timedelta
under PTB_TIMEDELTA)
- DiscordChannel: discord.RateLimited.retry_after, which the old duck-typed
getattr matched and would otherwise have been lost
Drop the isinstance(retry, bool) and val >= 0 guards; no SDK produces those.
Delete the test that asserted the duck-typed attribute; add real-object tests
for each override, guarded like the existing SDK-dependent classes.
* ci: install the all-channels extra so SDK-dependent channel tests run
The Slack, Discord, and Telegram retry tests build real SDK exception
objects and are skipped when the SDK is absent. CI only ran `uv sync --dev`,
so those tests never executed there. Install the existing all-channels
extra alongside the dev group; the skipif guards remain for lean local runs.
* fix(channels): honor HTTP-date Retry-After and tolerate malformed values
RFC 9110 allows Retry-After as either delay-seconds or an HTTP-date. The
httpx path treated a date as unparseable and fell back to the 1.0 s default,
so a 503 asking for a specific wait was retried too early. Add
Channel._parse_retry_after, which returns delay-seconds as-is and converts
an HTTP-date to the non-negative seconds until it (tz-less dates read as
UTC).
SlackChannel used a bare float() on the header. A non-numeric value raised
inside the retry predicate, which escapes retry_async and drops the chunk
instead of retrying. Route Slack through the same helper so a bad header
falls back to _rate_limit_delay.
Addresses CodeRabbit review comments on base.py:866 and slack/channel.py:229.
* fix(channels): treat HTTP 400 and 404 as non-retryable
Both are permanent for a given request, so retrying burns the attempt
budget for nothing. Add them to _non_retryable_status_codes alongside
401/403.
Deliberately not a 4xx range check: 408 and 425 are retryable by
definition and 429 is handled by the rate-limit path. A test pins 408 as
still retryable so the range shortcut is not reintroduced later.
Partially addresses CodeRabbit's outside-diff comment on base.py:749-750.
* fix(channels): guard Slack retry extractors against raw aiohttp responses
slack_sdk attaches the bare aiohttp.ClientResponse to SlackApiError when a
JSON-declared body fails to parse. That object has neither status_code nor
get(), so _extract_status_code raised AttributeError inside should_retry,
replacing the original error and skipping the remaining attempts. Narrow
both extractors to SlackResponse/AsyncSlackResponse so such errors fall
through to the message patterns and retry as before. Add a wire-level
regression test against a local aiohttp server.
---------
Co-authored-by: Dinos Papakostas <dinospk1999@gmail.com>
Co-authored-by: X-iZhang <zacharyzhang2022@gmail.com>
|
||
|
Xiaohui Yan
|
3c5cc831c0 |
Feat/configurable bind host (#402)
* feat: configurable bind host for WebUI and langgraph dev (refs #400) WebUI mode was only reachable from the machine running it: the front-end got no bind interface, and `start_langgraph_dev(...)` was called without a host, so both servers stayed on loopback with no way to widen them. Adds two config fields with deliberately different defaults: webui_host = 0.0.0.0 front-end serves the app shell, no secrets langgraph_dev_host = 127.0.0.1 unauthenticated API, agent can run shell The design hinges on separating bind address from client address. Only bind() uses the configured interface; every consumer that *connects* (health probes, occupancy checks, async sub-agent self-dispatch) goes through the new `_probe_host`, which maps a wildcard bind back to loopback and honors a pinned interface verbatim. `_can_bind_port` is the one exception and binds the literal host, since it must replicate the bind the server itself will attempt. - manager.py: `_probe_host`, `_is_loopback_host`, `_format_hostport`; host kwarg threaded through the probes and `start_langgraph_dev`, which now emits `--host` and propagates EVOSCIENTIST_LANGGRAPH_DEV_HOST to the subprocess - sdk.py: `langgraph_dev_url` tracks host as well as port; EvoScientist.py reuses it instead of an inline f-string - server.py: `--host` flag mirroring `--port`, plus a red PUBLIC BIND banner whenever the bind is not provably loopback - webui.py: forwards both hosts; the front-end is widened via HOSTNAME because @evoscientist/webui ships no --host flag — its bin launcher does `HOSTNAME: process.env.HOSTNAME || "127.0.0.1"`. The warning is gated on the backend host only, so the shipped front-end default doesn't print a banner on every launch Verified end to end against a live server: requesting 0.0.0.0 yields a socket listening on 0.0.0.0 with the health probe correctly resolved to 127.0.0.1, while the default still binds 127.0.0.1 only. Note: webui_host defaulting to 0.0.0.0 is a behavior change — upgrading users will find the front-end reachable from the LAN. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: default both bind hosts to 0.0.0.0, add --host and wizard host rendering (closes #400) Completes the remaining items from #400. - `langgraph_dev_host` now defaults to 0.0.0.0, matching `webui_host`. Remote WebUI use needs both anyway (the UI reaches the backend from the browser, not server-side), so a loopback backend default just meant every remote user hit a silently failing UI. `_DEFAULT_HOST` and sdk's `DEFAULT_LANGGRAPH_DEV_HOST` follow, so there is one story about where these servers listen. SECURITY: this exposes an unauthenticated API whose agent can run shell commands. The red PUBLIC BIND banner consequently fires on every launch while exposed — kept deliberately, since the exposure is real and the escape hatch (`--host 127.0.0.1` / `config set langgraph_dev_host`) is only discoverable if we say so. READMEs now lead with the warning and document the SSH-tunnel alternative. - `EvoSci --host <ip>` on the WebUI launch path, driving both servers. In WebUI mode they are two halves of one surface; moving only one leaves the UI loading but unable to reach the agent. Blank values are dropped rather than written as an empty override that would beat the config file. - Onboarding wizard no longer prints hard-coded `http://127.0.0.1:{port}` / `http://localhost:{port}` (steps.py:160, :223) — both render the configured bind through `_base_url` / `_format_hostport`, so a pinned interface is reported honestly and a wildcard still shows loopback. Verified against a live server: with no host argument at all, resolution through EvoScientistConfig yields a socket listening on 0.0.0.0, a client URL of http://127.0.0.1, and the warning gate returning True. Still open and tracked separately: the front-end takes its backend URL from browser input: `@evoscientist/webui` reads only HOSTNAME, PORT and EVOSCIENTIST_LANGGRAPH_DEV_PORT, so advertising a backend URL needs a change in that repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: bump setup-uv v6 -> v9.0.0 to drop the deprecated node20 runtime GitHub now warns that setup-uv@v6 targets Node.js 20 and is being forced onto Node.js 24. v7.0.0 is the release that made that switch, so anything >= v7 clears the warning; v9.0.0 is current. Pinned to the full tag deliberately: setup-uv stopped publishing major and minor tags in v8.0.0 as supply-chain hardening, so `@v9` and `@v8` return 404 and would fail the job outright. Releases are immutable from v8 on, so the full tag is as tamper-proof as a SHA. Comment left in lint.yml because "simplifying" this back to `@v9` is an easy and CI-breaking mistake. actions/checkout@v5 is already node24 and needs no change. Note: v9.0.0 flips the `prune-cache` default to false (upstream did this to ease load on PyPI infrastructure). None of these workflows set it, so they follow the new default and Actions cache usage may grow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cli): correct --host help text and warn on public bind in non-WebUI modes The --host help claimed "WebUI mode only", which is wrong in a way that matters for security. `--host` writes `langgraph_dev_host` unconditionally, and `_ensure_async_subagent_server` auto-starts that backend for tui / cli / serve as well — the langgraph dev server is shared across UI modes. So the flag narrows or widens the agent API in every mode, and only `webui_host` is actually WebUI-specific. Reported against cli/commands.py. The documentation error hid a real gap: the PUBLIC BIND banner lived only in deploy/server.py and deploy/webui.py, so a plain `EvoSci` session bound 0.0.0.0 with no runtime signal whatsoever — and `--help` is opt-in, so fixing the text alone would not surface it. Added the same banner to the shared CLI path, gated on `is_async_subagents_available()`: ensure_langgraph_dev fails soft (async degrades to in-process delegation), and warning about a bind that never happened would be worse than staying quiet. READMEs (EN + zh-CN) get the same correction — the warning block sat inside the Desktop WebUI section and read as WebUI-scoped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): strip the config-derived bind host, not just the CLI one `deploy()` only stripped the `--host` branch. When the flag was omitted, `getattr(config, "langgraph_dev_host", ...)` flowed unstripped into `_is_port_occupied`, `is_langgraph_dev_running`, `start_langgraph_dev` and the banner. `run_webui` already strips unconditionally; this aligns the two. Reachable because `deploy()` reads through `getattr` and is routinely handed duck-typed config objects (tests, embedders) that never run `EvoScientistConfig.__post_init__`, which is what normally normalizes these fields. Worst case was not just a bad bind: `_is_loopback_host(" 127.0.0.1 ")` is False, so a padded loopback value would print a false PUBLIC BIND warning while binding a string socket.bind() rejects outright — a security banner saying the opposite of the truth. Three regression tests added, each verified to fail against the old code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style: apply ruff format to the bind-host changes The Lint workflow runs both `ruff check` and `ruff format --check`; I had only been running the former locally, so five files landed unformatted and failed CI. Whitespace and line-wrapping only — no semantic change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(security): keep the langgraph dev backend on loopback by default The backend is an unauthenticated API whose agent can run shell commands, and it is auto-started in every UI mode (tui/cli/webui/serve/deploy) — so a 0.0.0.0 default put it on the network for users who never asked. Restore 127.0.0.1 as the default and make 0.0.0.0 an explicit opt-in. webui_host keeps its 0.0.0.0 default: the front-end serves the app shell only and holds no credentials. run_webui already prints a remote-backend hint when the front-end is exposed and the backend is not. Help text and both READMEs are reframed around widening rather than narrowing; the escape-hatch tests are inverted to assert the public-bind opt-in survives into argv. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
houren Antony
|
d4f1fbd110 |
ci: add windows-latest to test matrix + fix 11 cross-platform test bugs (#271)
* ci: add windows-latest to test matrix + fix 11 cross-platform test bugs The test workflow ran on ``ubuntu-latest`` only. Per the issue's first bullet — the maintainer's explicit #1 priority — add ``windows-latest`` to the matrix so the manager and related modules are exercised on Windows on every PR. The matrix addition surfaces 18 pre-existing Windows-only test failures. Without fixes the new leg would be 18+ reds from day one and the matrix would just produce a wall of ``fail-fast`` noise. This PR fixes 11 of them; each fix is a real (cross-platform) bug, not a Windows-specific hack — most were already flagged by CodeRabbit on PR #236 but never acted on. The remaining 4 failures need code refactors (``os.killpg`` → ``psutil`` in ``background.py``, ``convert_virtual_paths_in_command`` Windows-aware quoting, tilde expansion) that are documented as out-of-scope follow-ups below. ## What changed * ``.github/workflows/test.yml`` - ``os: [ubuntu-latest, windows-latest]`` → 2 OS × 2 Python = 4 cells. - ``fail-fast: false`` so one bad cell doesn't cancel the rest while the Windows leg is being brought up. Removable in a future PR once the suite is fully green. * ``tests/test_backends.py`` - Hard-coded ``"python3"`` → ``{sys.executable}`` in 7 test commands. Windows has no ``python3`` on PATH; using ``sys.executable`` is portable and matches what CodeRabbit flagged on PR #236. - Strict string comparisons → ``shlex.split`` round-trip in 5 resolver tests. ``shlex.quote`` adds single quotes around backslash paths on Windows, which broke the direct ``==`` compare. - Cross-platform suffix checks in 2 path-resolution tests (``Path(resolved).parts[-2:]`` instead of ``str(resolved).endswith("src/main.py")``). - ``mkdir -p`` → ``sys.executable -c "import os; os.makedirs(...)"`` in the cwd-sanitization test. - ``skipif(sys.platform == "win32")`` on 3 e2e tests that hit the underlying ``shlex.quote`` + ``cmd.exe`` quoting bug (real, separate issue). * ``tests/test_sessions.py`` - ``test_uses_data_dir``: check ``.evoscientist`` in the long path form (via ``Path.resolve()``) rather than the short-path form ``get_db_path`` returns on Windows. * ``tests/test_mcp_client.py`` - ``endswith("python")`` → ``Path(result).stem.lower()`` so ``python.EXE`` matches on Windows. - ``endswith("npx")`` also accepts ``npx.cmd`` so the npm shim on Windows matches. ## Out of scope (follow-up issues to file) * ``os.killpg`` doesn't exist on Windows (``EvoScientist/background.py:248``) — 3 background tests fail. Real fix is the same ``psutil`` walk pattern PR #200 shipped in ``langgraph_dev/manager.py``. * Tilde expansion in file mentions. * Windows-aware shell quoting in ``convert_virtual_paths_in_command``. * Path conventions (``~/.config/evoscientist/`` vs ``%APPDATA%\EvoScientist``) — needs design discussion + ``platformdirs`` migration. * Cross-module audit of ``EvoScientist/tools/execute.py``, ``EvoScientist/ccproxy_manager.py``, ``EvoScientist/config/onboard.py``. Closes #207 (step 1 only — CI matrix + the easy test fixes; remaining bullets tracked separately). * fix: cross-platform compatibility for Windows CI runners - background.py: replace POSIX-only os.killpg/os.getpgid with cross-platform _kill_process_tree() helper. On Windows falls back to Popen.terminate()/Popen.kill() (TerminateProcess); on POSIX keeps existing os.killpg logic. - test_backends.py: replace mkdir -p shell execution in test_literal_workspace_path_replaced with preprocessing-boundary assertion (patch LocalShellBackend.execute, capture command, assert workspace path was rewritten to ./). Avoids POSIX-only mkdir -p on Windows runners. - test_file_mentions.py: monkeypatch USERPROFILE on Windows so ntpath.expanduser() resolves ~ to tmp_path even when HOME is unset on CI runners. * fix(test): cross-platform sleep/true commands for Windows CI Replace POSIX-only sleep/true with module-level helpers that use ping -n / cmd /c on Windows. Also fix python3 -> sys.executable in the non-timeout recovery test. - test_background.py: 7 sleep/true fixes - test_background_middleware.py: 6 sleep/true fixes - test_backends.py: 4 sleep fixes + 1 python3 fix 2318 passed, 0 failed on Windows. * fix(test): use shell-portable double quotes for python -c on Windows cmd.exe does not treat single quotes as string delimiters, so -c 'raise SystemExit(1)' was passed with literal quotes on Windows. Switch to double quotes which work on both cmd.exe and POSIX sh. * fix: use psutil for Windows process tree kill + avoid sys.executable under uv - background.py: replace Popen.terminate()/kill() with psutil-based process tree walking on Windows. TerminateProcess does NOT cascade to grandchildren; psutil.Process.children(recursive=True) ensures the entire tree is signaled. - test_backends.py: replace sys.executable with 'python' in sandbox execute() calls. Under uv, sys.executable is under the workspace and gets rewritten to ./ by prepare_sandbox_command, breaking Linux CI. The plain 'python' command resolves correctly in any activated venv. * fix: broaden try/except in _kill_process_tree to cover proc.children() If the process exits between Process(popen.pid) and children(recursive=True), the children call raises an uncaught exception escaping stop(). Move it inside the existing try/except block. * fix: narrow exception to ProcessLookupError in POSIX _kill_process_tree OSError is too broad — would silently swallow EPERM on SIGKILL, leaving the process alive when we report it as stopped. Match original behavior which only caught ProcessLookupError (process already gone). * style: ruff format test_backends.py * ci: trigger re-run for flaky prompt_toolkit test * style: fix ruff check (import order + RUF005 unpacking) --------- Co-authored-by: Xi Zhang <106144707+X-iZhang@users.noreply.github.com> |
||
|
X-iZhang
|
d226aabe01 | update GitHub Actions workflows to use latest action versions | ||
|
X-iZhang
|
c96d984869 | feat(workflows): add cache-dependency-glob for improved dependency caching in build, lint, and test workflows | ||
|
Dinos Papakostas
|
6faad4bb98 | chore(workflows): update gh workflows to use uv | ||
|
X-iZhang
|
7dedf2b426 | fix: improve cancellation handling in middleware and mixins for Python 3.12+ | ||
|
X-iZhang
|
e725fb08e2 | add CI workflows |