diff --git a/.github/actions/hermes-smoke-test/action.yml b/.github/actions/hermes-smoke-test/action.yml index 08b9f93634..8b79c4bf34 100644 --- a/.github/actions/hermes-smoke-test/action.yml +++ b/.github/actions/hermes-smoke-test/action.yml @@ -29,9 +29,13 @@ runs: - name: hermes --help shell: bash run: | + # Use the image's real ENTRYPOINT (/init + main-wrapper.sh) so + # this exercises the actual production startup path. PR #30136 + # review caught that an --entrypoint override here had been + # silently neutered by the s6-overlay migration — stage2-hook + # ignores its CMD args, so the smoke test was a no-op. docker run --rm \ -v /tmp/hermes-test:/opt/data \ - --entrypoint /opt/hermes/docker/entrypoint.sh \ "${{ inputs.image }}" --help - name: hermes dashboard --help @@ -43,5 +47,4 @@ runs: # installed package. docker run --rm \ -v /tmp/hermes-test:/opt/data \ - --entrypoint /opt/hermes/docker/entrypoint.sh \ "${{ inputs.image }}" dashboard --help diff --git a/.github/workflows/docker-lint.yml b/.github/workflows/docker-lint.yml new file mode 100644 index 0000000000..f1673813e9 --- /dev/null +++ b/.github/workflows/docker-lint.yml @@ -0,0 +1,68 @@ +name: Docker / shell lint + +# Lints the container build inputs: Dockerfile (via hadolint) and any shell +# scripts under docker/ (via shellcheck). These catch the class of regression +# the behavioral docker-publish smoke test can't — unquoted variable +# expansions, silently-failing RUN commands, etc. +# +# Rules and ignores are documented in .hadolint.yaml at the repo root. +# shellcheck severity is pinned to `error` so SC1091-style "can't follow +# sourced script" info-level warnings don't fail the job — the .venv +# activate script doesn't exist at lint time. + +on: + push: + branches: [main] + paths: + - Dockerfile + - docker/** + - .hadolint.yaml + - .github/workflows/docker-lint.yml + pull_request: + branches: [main] + paths: + - Dockerfile + - docker/** + - .hadolint.yaml + - .github/workflows/docker-lint.yml + +permissions: + contents: read + +concurrency: + group: docker-lint-${{ github.ref }} + cancel-in-progress: true + +jobs: + hadolint: + name: Lint Dockerfile (hadolint) + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: hadolint + uses: hadolint/hadolint-action@54c9adbab1582c2ef04b2016b760714a4bfde3cf # v3.1.0 + with: + dockerfile: Dockerfile + config: .hadolint.yaml + failure-threshold: warning + + shellcheck: + name: Lint docker/ shell scripts (shellcheck) + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: shellcheck + uses: ludeeus/action-shellcheck@00cae500b08a931fb5698e11e79bfbd38e612a38 # v2.0.0 + env: + # Severity = error: SC1091 (can't follow sourced script) is info- + # level and would otherwise fail when the venv activate script + # doesn't exist at lint time. + SHELLCHECK_OPTS: --severity=error + with: + scandir: ./docker diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index df6fa29d7e..c0e69bcf3d 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -27,9 +27,9 @@ on: permissions: contents: read -# Concurrency: push/release runs are NEVER cancelled so every merge gets its -# own SHA-tagged image; :main and :latest are guarded separately by the -# move-main and move-latest jobs. PR runs reuse a PR-scoped group with +# Concurrency: push/release runs are NEVER cancelled so every merge gets +# its own :main or release-tagged image. :latest is guarded separately +# by the move-latest job. PR runs reuse a PR-scoped group with # cancel-in-progress: true so rapid pushes to the same PR collapse to the # latest commit. concurrency: @@ -80,6 +80,56 @@ jobs: with: image: ${{ env.IMAGE_NAME }}:test + # --------------------------------------------------------------------- + # Run the docker-integration test suite against the freshly-built + # image already loaded into the local daemon (`:test`). These tests + # are excluded from the sharded `tests.yml :: test` matrix on purpose + # (see `_SKIP_PARTS` in scripts/run_tests_parallel.py) because each + # shard would otherwise reach the session-scoped ``built_image`` + # fixture in ``tests/docker/conftest.py`` and start a 3-7min + # ``docker build`` under a 180s pytest-timeout cap — guaranteed to + # die in fixture setup. + # + # Piggybacking here avoids a second image build: the smoke test + # already proved the image loads + runs, so the daemon has it under + # `${IMAGE_NAME}:test` and we just point ``HERMES_TEST_IMAGE`` at + # that. The fixture's ``HERMES_TEST_IMAGE`` branch (see + # tests/docker/conftest.py:62-63) short-circuits the rebuild. + # + # Why this job and not a standalone one: the image is 5GB+; passing + # it between jobs via ``docker save``/``upload-artifact`` is slower + # than the build itself. Reusing the existing daemon state is the + # cheapest path to coverage on every PR that touches docker code. + # --------------------------------------------------------------------- + - name: Install uv (for docker tests) + uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5 + + - name: Set up Python 3.11 (for docker tests) + run: uv python install 3.11 + + - name: Install Python dependencies (for docker tests) + run: | + uv venv .venv --python 3.11 + source .venv/bin/activate + # ``dev`` extra pulls in pytest, pytest-asyncio, pytest-timeout — + # everything tests/docker/ needs. We deliberately avoid ``all`` + # here because the docker tests only drive the container via + # subprocess and don't import hermes_agent's optional deps. + uv pip install -e ".[dev]" + + - name: Run docker integration tests + env: + # Skip rebuild; use the image already loaded by the build step. + HERMES_TEST_IMAGE: ${{ env.IMAGE_NAME }}:test + # Match the policy in tests.yml :: test job — no accidental + # real-API calls from inside the harness. + OPENROUTER_API_KEY: "" + OPENAI_API_KEY: "" + NOUS_API_KEY: "" + run: | + source .venv/bin/activate + python -m pytest tests/docker/ -v --tb=short + - name: Log in to Docker Hub if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release' uses: docker/login-action@4907a6ddec9925e35a0a9e82d7399ccc52663121 # v4.1.0 @@ -92,10 +142,10 @@ jobs: # pattern for multi-runner multi-platform builds. # # We apply the OCI revision label here (and again on arm64) because - # the move-main / move-latest jobs read it off the linux/amd64 - # sub-manifest config of the floating tag to decide whether it's safe - # to advance. The label must be on each per-arch image — manifest - # lists themselves don't carry image config labels. + # the move-latest job reads it off the linux/amd64 sub-manifest + # config of the floating tag to decide whether it's safe to advance. + # The label must be on each per-arch image — manifest lists themselves + # don't carry image config labels. - name: Push amd64 by digest id: push if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release' @@ -208,8 +258,14 @@ jobs: # --------------------------------------------------------------------------- # Stitch both per-arch digests into a single tagged multi-arch manifest. # This is a registry-side operation — no building, no layer re-push — - # so it runs in ~30 seconds. On main pushes it produces :sha-. - # On releases it produces :. + # so it runs in ~30 seconds. On main pushes it produces :main; on + # releases it produces :. + # + # For main pushes the ancestor check runs BEFORE the manifest push so + # we never overwrite :main with an older commit. The top-level + # concurrency group (`docker-${{ github.ref }}` with + # `cancel-in-progress: false`) already serialises runs per ref; the + # ancestor check is defense-in-depth. # --------------------------------------------------------------------------- merge: if: github.repository == 'NousResearch/hermes-agent' && (github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release') @@ -217,10 +273,15 @@ jobs: needs: [build-amd64, build-arm64] timeout-minutes: 10 outputs: - pushed_sha_tag: ${{ steps.mark_pushed.outputs.pushed }} pushed_release_tag: ${{ steps.mark_release_pushed.outputs.pushed }} release_tag: ${{ steps.tag.outputs.tag }} steps: + - name: Checkout code + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 1000 + - name: Download digests uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 with: @@ -237,120 +298,19 @@ jobs: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} - # Compute the tag for this run. Main pushes use sha- (so every - # commit gets its own immutable tag); releases use the release tag name. - - name: Compute tag - id: tag - run: | - if [ "${{ github.event_name }}" = "release" ]; then - echo "tag=${{ github.event.release.tag_name }}" >> "$GITHUB_OUTPUT" - else - echo "tag=sha-${{ github.sha }}" >> "$GITHUB_OUTPUT" - fi - - - name: Create manifest list and push - working-directory: /tmp/digests - run: | - set -euo pipefail - # Build the arg array from each digest file (filename = the digest - # hex, with no sha256: prefix; empty file content, only the name - # matters). Using an array avoids shellcheck SC2046 and keeps - # every digest a single argv token even under pathological names. - args=() - for digest_file in *; do - args+=("${IMAGE_NAME}@sha256:${digest_file}") - done - docker buildx imagetools create \ - -t "${IMAGE_NAME}:${TAG}" \ - "${args[@]}" - env: - IMAGE_NAME: ${{ env.IMAGE_NAME }} - TAG: ${{ steps.tag.outputs.tag }} - - - name: Inspect image - run: | - docker buildx imagetools inspect "${IMAGE_NAME}:${TAG}" - env: - IMAGE_NAME: ${{ env.IMAGE_NAME }} - TAG: ${{ steps.tag.outputs.tag }} - - # Signal to move-main that the SHA tag is live. Only on main pushes; - # releases set pushed_release_tag instead. - - name: Mark SHA tag pushed - id: mark_pushed - if: github.event_name == 'push' && github.ref == 'refs/heads/main' - run: echo "pushed=true" >> "$GITHUB_OUTPUT" - - # Signal to move-latest that the release tag is live. - - name: Mark release tag pushed - id: mark_release_pushed - if: github.event_name == 'release' - run: echo "pushed=true" >> "$GITHUB_OUTPUT" - - # --------------------------------------------------------------------------- - # Move :main to point at the SHA tag the merge job pushed. - # - # :main is the floating tag that tracks the tip of the main branch. Every - # merge to main retags :main forward. Users who want "latest dev build" - # pull :main; users who want stable releases pull :latest. - # - # The real serialization guarantee comes from the top-level concurrency - # group (`docker-${{ github.ref }}` with `cancel-in-progress: false`), - # which ensures at most one workflow run for this ref executes at a time. - # That means two move-main steps for the same ref cannot overlap. - # - # This job has its own concurrency group as defense-in-depth: if the - # top-level group is ever loosened, queued move-mains will run serially - # in arrival order, each one running the ancestor check below and either - # advancing :main or skipping. `cancel-in-progress: false` matches the - # top-level setting — we don't want rapid pushes to cancel a queued - # move-main, because the ancestor check is the real safety mechanism - # and queueing is cheap (move-main is a ~30s registry op). - # - # Combined with the ancestor check, this means :main only ever moves - # forward in git history. - # --------------------------------------------------------------------------- - move-main: - if: | - github.repository == 'NousResearch/hermes-agent' - && github.event_name == 'push' - && github.ref == 'refs/heads/main' - && needs.merge.outputs.pushed_sha_tag == 'true' - needs: merge - runs-on: ubuntu-latest - timeout-minutes: 10 - concurrency: - group: docker-move-main-${{ github.ref }} - cancel-in-progress: false - steps: - - name: Checkout code - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - fetch-depth: 1000 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3 - - - name: Log in to Docker Hub - uses: docker/login-action@4907a6ddec9925e35a0a9e82d7399ccc52663121 # v4.1.0 - with: - username: ${{ secrets.DOCKERHUB_USERNAME }} - password: ${{ secrets.DOCKERHUB_TOKEN }} - # Read the git revision label off the current :main manifest, then - # use `git merge-base --is-ancestor` to check whether our commit is a - # descendant of it. If :main doesn't exist yet, or its label is - # missing, we treat that as "safe to publish". If another run already - # advanced :main past us (or diverged), we skip and leave it alone. + # use `git merge-base --is-ancestor` to check whether our commit is + # a descendant of it. If :main doesn't exist yet, or its label is + # missing, we treat that as "safe to publish". If another run + # already advanced :main past us (or diverged), we skip and leave + # it alone. - name: Decide whether to move :main + if: github.event_name == 'push' && github.ref == 'refs/heads/main' id: main_check run: | set -euo pipefail image=nousresearch/hermes-agent - # Pull the JSON for the linux/amd64 sub-manifest's config and extract - # the OCI revision label with jq — Go template field access can't - # handle dots in map keys, so using json+jq is the robust route. image_json=$( docker buildx imagetools inspect "${image}:main" \ --format '{{ json (index .Image "linux/amd64") }}' \ @@ -383,7 +343,6 @@ jobs: exit 0 fi - # Make sure we have the :main commit locally for merge-base. if ! git cat-file -e "${current_sha}^{commit}" 2>/dev/null; then git fetch --no-tags --prune origin \ "+refs/heads/main:refs/remotes/origin/main" \ @@ -396,7 +355,6 @@ jobs: exit 0 fi - # Our SHA must be a descendant of the current :main to be safe. if git merge-base --is-ancestor "${current_sha}" "${GITHUB_SHA}"; then echo "Our commit is a descendant of :main — safe to advance." echo "push_main=true" >> "$GITHUB_OUTPUT" @@ -405,19 +363,48 @@ jobs: echo "push_main=false" >> "$GITHUB_OUTPUT" fi - # Retag the already-pushed SHA manifest as :main. This is a registry- - # side operation — no rebuild, no layer re-push — so it's quick and - # atomic per-tag. The ancestor check above plus the cancel-in-progress - # concurrency on this job together guarantee we only ever move :main - # forward in git history. - - name: Move :main to this SHA - if: steps.main_check.outputs.push_main == 'true' + # Compute the tag for this run. Main pushes tag directly as :main + # (no per-commit SHA tags); releases use the release tag name. + - name: Compute tag + id: tag + run: | + if [ "${{ github.event_name }}" = "release" ]; then + echo "tag=${{ github.event.release.tag_name }}" >> "$GITHUB_OUTPUT" + else + echo "tag=main" >> "$GITHUB_OUTPUT" + fi + + # Gate the manifest push on the ancestor check for main pushes. + # For releases there is no gate — the check doesn't even run. + - name: Create manifest list and push + if: github.event_name != 'push' || steps.main_check.outputs.push_main == 'true' + working-directory: /tmp/digests run: | set -euo pipefail - image=nousresearch/hermes-agent + args=() + for digest_file in *; do + args+=("${IMAGE_NAME}@sha256:${digest_file}") + done docker buildx imagetools create \ - --tag "${image}:main" \ - "${image}:sha-${GITHUB_SHA}" + -t "${IMAGE_NAME}:${TAG}" \ + "${args[@]}" + env: + IMAGE_NAME: ${{ env.IMAGE_NAME }} + TAG: ${{ steps.tag.outputs.tag }} + + - name: Inspect image + if: github.event_name != 'push' || steps.main_check.outputs.push_main == 'true' + run: | + docker buildx imagetools inspect "${IMAGE_NAME}:${TAG}" + env: + IMAGE_NAME: ${{ env.IMAGE_NAME }} + TAG: ${{ steps.tag.outputs.tag }} + + # Signal to move-latest that the release tag is live. + - name: Mark release tag pushed + id: mark_release_pushed + if: github.event_name == 'release' + run: echo "pushed=true" >> "$GITHUB_OUTPUT" # --------------------------------------------------------------------------- # Move :latest to point at the release tag the merge job pushed. @@ -427,10 +414,10 @@ jobs: # # We still run an ancestor check against the existing :latest so that a # backport release on an older branch (e.g. patching v1.1.5 after v1.2.3 - # is out) doesn't drag :latest backwards. The check is the same shape as - # move-main: read the OCI revision label off the current :latest, look up - # that commit in git, and only advance if our release commit is a strict - # descendant. + # is out) doesn't drag :latest backwards. The check is the same shape + # as the ancestor check in the merge job for :main: read the OCI + # revision label off the current :latest, look up that commit in git, + # and only advance if our release commit is a strict descendant. # --------------------------------------------------------------------------- move-latest: if: | diff --git a/.github/workflows/supply-chain-audit.yml b/.github/workflows/supply-chain-audit.yml index 9eb76e6a5f..2f727e8d25 100644 --- a/.github/workflows/supply-chain-audit.yml +++ b/.github/workflows/supply-chain-audit.yml @@ -47,14 +47,17 @@ jobs: HEAD="${{ github.event.pull_request.head.sha }}" # Added lines only, excluding lockfiles. - DIFF=$(git diff "$BASE".."$HEAD" -- . ':!uv.lock' ':!*.lock' ':!package-lock.json' ':!yarn.lock' || true) + # Three-dot diff (base...head) diffs from the merge base to HEAD, + # so only changes introduced by this PR are included — not changes + # that landed on main after the PR branched off. + DIFF=$(git diff "$BASE"..."$HEAD" -- . ':!uv.lock' ':!*.lock' ':!package-lock.json' ':!yarn.lock' || true) FINDINGS="" # --- .pth files (auto-execute on Python startup) --- # The exact mechanism used in the litellm supply chain attack: # https://github.com/BerriAI/litellm/issues/24512 - PTH_FILES=$(git diff --name-only "$BASE".."$HEAD" | grep '\.pth$' || true) + PTH_FILES=$(git diff --name-only "$BASE"..."$HEAD" | grep '\.pth$' || true) if [ -n "$PTH_FILES" ]; then FINDINGS="${FINDINGS} ### 🚨 CRITICAL: .pth file added or modified @@ -97,7 +100,12 @@ jobs: # --- Install-hook files (setup.py/sitecustomize/usercustomize/__init__.pth) --- # These execute during pip install or interpreter startup. - SETUP_HITS=$(git diff --name-only "$BASE".."$HEAD" | grep -E '(^|/)(setup\.py|setup\.cfg|sitecustomize\.py|usercustomize\.py|__init__\.pth)$' || true) + # Anchored at repo root: only the top-level setup.py/setup.cfg run during + # `pip install`, and only top-level sitecustomize.py/usercustomize.py are + # auto-loaded by the interpreter via site.py. Any nested file with the + # same name (e.g. hermes_cli/setup.py — the CLI setup wizard) is unrelated + # and produced false positives that trained reviewers to ignore the scanner. + SETUP_HITS=$(git diff --name-only "$BASE"..."$HEAD" | grep -E '^(setup\.py|setup\.cfg|sitecustomize\.py|usercustomize\.py|__init__\.pth)$' || true) if [ -n "$SETUP_HITS" ]; then FINDINGS="${FINDINGS} ### 🚨 CRITICAL: Install-hook file added or modified @@ -158,7 +166,7 @@ jobs: HEAD="${{ github.event.pull_request.head.sha }}" # Only check added lines in pyproject.toml - ADDED=$(git diff "$BASE".."$HEAD" -- pyproject.toml | grep '^+' | grep -v '^+++' || true) + ADDED=$(git diff "$BASE"..."$HEAD" -- pyproject.toml | grep '^+' | grep -v '^+++' || true) if [ -z "$ADDED" ]; then echo "found=false" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3ffaa10d00..b48b0bab08 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -23,11 +23,22 @@ concurrency: jobs: test: runs-on: ubuntu-latest - timeout-minutes: 60 + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + slice: [1, 2, 3, 4, 5, 6] steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Restore duration cache + uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + with: + path: test_durations.json + # Single stable key. main always overwrites, PRs always find it. + key: test-durations + - name: Install ripgrep (prebuilt binary) run: | set -euo pipefail @@ -54,7 +65,7 @@ jobs: source .venv/bin/activate uv pip install -e ".[all,dev]" - - name: Run tests + - name: Run tests (slice ${{ matrix.slice }}/6) # Per-file isolation via scripts/run_tests_parallel.py: discovers # every test_*.py file under tests/ (excluding integration/ + e2e/), # then runs `python -m pytest ` in a freshly-spawned subprocess @@ -72,15 +83,61 @@ jobs: # state across files, which is exactly the leakage we wanted to # fix. ThreadPoolExecutor + subprocess.run is ~60 lines and does # the job with cleaner semantics. + # + # Matrix slicing (--slice I/N): files are distributed across 6 + # jobs by cached duration (LPT algorithm) so each job gets + # roughly equal wall time. Without a cache, files default to 2s + # estimate and get split roughly evenly by count — still correct, + # just not perfectly balanced. run: | source .venv/bin/activate - python scripts/run_tests_parallel.py + python scripts/run_tests_parallel.py --slice ${{ matrix.slice }}/6 env: # Ensure tests don't accidentally call real APIs OPENROUTER_API_KEY: "" OPENAI_API_KEY: "" NOUS_API_KEY: "" + - name: Upload per-slice durations + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: test-durations-slice-${{ matrix.slice }} + path: test_durations.json + retention-days: 1 + + # Merge per-slice duration data into a single cache, so future runs + # (including PRs) get balanced slicing. + save-durations: + needs: test + if: always() && github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + steps: + - name: Download all slice durations + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + pattern: test-durations-slice-* + path: durations + merge-multiple: true + + - name: Merge into single durations file + run: | + python3 -c " + import json, glob, os + merged = {} + for f in glob.glob('durations/*test_durations.json'): + with open(f) as fh: + merged.update(json.load(fh)) + with open('test_durations.json', 'w') as fh: + json.dump(merged, fh, indent=2, sort_keys=True) + print(f'Merged {len(merged)} file durations') + " + + - name: Save merged duration cache + uses: actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + with: + path: test_durations.json + key: test-durations + e2e: runs-on: ubuntu-latest timeout-minutes: 15 @@ -121,4 +178,4 @@ jobs: env: OPENROUTER_API_KEY: "" OPENAI_API_KEY: "" - NOUS_API_KEY: "" + NOUS_API_KEY: "" \ No newline at end of file diff --git a/.gitignore b/.gitignore index 42388c9a67..5cf95df692 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ __pycache__/web_tools.cpython-310.pyc logs/ data/ .pytest_cache/ +test_durations.json .pytest-cache/ tmp/ temp_vision_images/ diff --git a/.hadolint.yaml b/.hadolint.yaml new file mode 100644 index 0000000000..81e80c14b6 --- /dev/null +++ b/.hadolint.yaml @@ -0,0 +1,36 @@ +# hadolint configuration for the Hermes Agent Dockerfile. +# See https://github.com/hadolint/hadolint#configure for rules. +# +# We want hadolint to surface NEW Dockerfile lint regressions, but we +# don't want to rewrite the existing image to silence rules that are +# either intentional or pragmatic tradeoffs for this project. Each +# ignore below has a one-line justification. +failure-threshold: warning + +ignored: + # Pin versions in apt get install. We intentionally don't pin common + # tools (curl, git, openssh-client, etc.) — security updates flow in + # via the periodic base-image rebuild, and pinning would lock us to + # superseded patch releases. Same rationale as nearly every distro- + # base official image (python, node, debian). + - DL3008 + # Use WORKDIR to switch to a directory. The image uses `(cd web && …)` + # / `(cd ../ui-tui && …)` inline subshells for one-off build steps + # because they don't affect later RUN commands; promoting them to + # full WORKDIR switches with restores would obscure intent. + - DL3003 + # Multiple consecutive RUN instructions. The `touch README.md` + `uv + # sync` split is intentional — `touch` is cheap, `uv sync` is the + # expensive layer-cached step we want isolated, and merging them + # would invalidate the cache for trivial changes. + - DL3059 + # Last USER should not be root. /init (s6-overlay) runs as root so the + # stage2 hook can usermod/groupmod and chown the data volume per + # HERMES_UID at runtime; each supervised service then drops to the + # hermes user via `s6-setuidgid`. + - DL3002 + +# Require explicit base-image pins (SHA256) — we already do this. +trustedRegistries: + - docker.io + - ghcr.io diff --git a/Dockerfile b/Dockerfile index 6e8f020963..be4e8848bb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,4 @@ FROM ghcr.io/astral-sh/uv:0.11.6-python3.13-trixie@sha256:b3c543b6c4f23a5f2df22866bd7857e5d304b67a564f4feab6ac22044dde719b AS uv_source -FROM tianon/gosu:1.19-trixie@sha256:3b176695959c71e123eb390d427efc665eeb561b1540e82679c15e992006b8b9 AS gosu_source FROM debian:13.4 # Disable Python stdout buffering to ensure logs are printed immediately @@ -9,18 +8,68 @@ ENV PYTHONUNBUFFERED=1 # install survives the /opt/data volume overlay at runtime. ENV PLAYWRIGHT_BROWSERS_PATH=/opt/hermes/.playwright -# Install system dependencies in one layer, clear APT cache -# tini reaps orphaned zombie processes (MCP stdio subprocesses, git, bun, etc.) -# that would otherwise accumulate when hermes runs as PID 1. See #15012. +# Install system dependencies in one layer, clear APT cache. +# tini was previously PID 1 to reap orphaned zombie processes (MCP stdio +# subprocesses, git, bun, etc.) that would otherwise accumulate when hermes +# ran as PID 1. See #15012. Phase 2 of the s6-overlay supervision plan +# replaces tini with s6-overlay's /init (PID 1 = s6-svscan), which reaps +# zombies non-blockingly on SIGCHLD and additionally supervises the main +# hermes process, the dashboard, and per-profile gateways. RUN apt-get update && \ apt-get install -y --no-install-recommends \ - build-essential curl nodejs npm python3 ripgrep ffmpeg gcc python3-dev libffi-dev procps git openssh-client docker-cli tini && \ + build-essential curl nodejs npm python3 ripgrep ffmpeg gcc python3-dev libffi-dev procps git openssh-client docker-cli xz-utils && \ rm -rf /var/lib/apt/lists/* +# ---------- s6-overlay install ---------- +# s6-overlay provides supervision for the main hermes process, the dashboard, +# and per-profile gateways. /init becomes PID 1 below — see ENTRYPOINT. +# +# Multi-arch: BuildKit auto-populates TARGETARCH (amd64 / arm64). s6-overlay +# uses tarball names keyed on the kernel arch string (x86_64 / aarch64), so +# we map between them inline. The noarch + symlinks tarballs are +# architecture-independent and reused as-is. +# +# We use `curl` instead of `ADD` for the per-arch tarball because `ADD` +# evaluates its URL at parse time, before any ARG / TARGETARCH substitution +# — splitting one URL per arch into two ADDs would download both on every +# build and leave dead bytes in the cache. A single curl + arch-keyed URL +# is simpler and cache-friendlier. +# +# Supply-chain integrity: every tarball is checksum-verified against the +# upstream-published SHA256. To bump S6_OVERLAY_VERSION, fetch the four +# `.sha256` files from the corresponding release and update the ARGs. The +# checksum lookup happens during build, so a compromised release artifact +# fails the build loudly instead of silently producing a tampered image. +ARG TARGETARCH +ARG S6_OVERLAY_VERSION=3.2.3.0 +ARG S6_OVERLAY_NOARCH_SHA256=b720f9d9340efc8bb07528b9743813c836e4b02f8693d90241f047998b4c53cf +ARG S6_OVERLAY_X86_64_SHA256=a93f02882c6ed46b21e7adb5c0add86154f01236c93cd82c7d682722e8840563 +ARG S6_OVERLAY_AARCH64_SHA256=0952056ff913482163cc30e35b2e944b507ba1025d78f5becbb89367bf344581 +ARG S6_OVERLAY_SYMLINKS_SHA256=a60dc5235de3ecbcf874b9c1f18d73263ab99b289b9329aa950e8729c4789f0e +ADD https://github.com/just-containers/s6-overlay/releases/download/v${S6_OVERLAY_VERSION}/s6-overlay-noarch.tar.xz /tmp/ +ADD https://github.com/just-containers/s6-overlay/releases/download/v${S6_OVERLAY_VERSION}/s6-overlay-symlinks-noarch.tar.xz /tmp/ +RUN set -eu; \ + case "${TARGETARCH:-amd64}" in \ + amd64) s6_arch="x86_64"; s6_arch_sha="${S6_OVERLAY_X86_64_SHA256}" ;; \ + arm64) s6_arch="aarch64"; s6_arch_sha="${S6_OVERLAY_AARCH64_SHA256}" ;; \ + *) echo "Unsupported TARGETARCH=${TARGETARCH} for s6-overlay" >&2; exit 1 ;; \ + esac; \ + curl -fsSL --retry 3 -o /tmp/s6-overlay-arch.tar.xz \ + "https://github.com/just-containers/s6-overlay/releases/download/v${S6_OVERLAY_VERSION}/s6-overlay-${s6_arch}.tar.xz"; \ + { \ + printf '%s %s\n' "${S6_OVERLAY_NOARCH_SHA256}" /tmp/s6-overlay-noarch.tar.xz; \ + printf '%s %s\n' "${s6_arch_sha}" /tmp/s6-overlay-arch.tar.xz; \ + printf '%s %s\n' "${S6_OVERLAY_SYMLINKS_SHA256}" /tmp/s6-overlay-symlinks-noarch.tar.xz; \ + } > /tmp/s6-overlay.sha256; \ + sha256sum -c /tmp/s6-overlay.sha256; \ + tar -C / -Jxpf /tmp/s6-overlay-noarch.tar.xz; \ + tar -C / -Jxpf /tmp/s6-overlay-arch.tar.xz; \ + tar -C / -Jxpf /tmp/s6-overlay-symlinks-noarch.tar.xz; \ + rm /tmp/s6-overlay-*.tar.xz /tmp/s6-overlay.sha256 + # Non-root user for runtime; UID can be overridden via HERMES_UID at runtime RUN useradd -u 10000 -m -d /opt/data hermes -COPY --chmod=0755 --from=gosu_source /gosu /usr/local/bin/ COPY --chmod=0755 --from=uv_source /usr/local/bin/uv /usr/local/bin/uvx /usr/local/bin/ WORKDIR /opt/hermes @@ -103,18 +152,73 @@ RUN cd web && npm run build && \ USER root RUN chmod -R a+rX /opt/hermes && \ chown -R hermes:hermes /opt/hermes/.venv /opt/hermes/ui-tui /opt/hermes/node_modules -# Start as root so the entrypoint can usermod/groupmod + gosu. -# If HERMES_UID is unset, the entrypoint drops to the default hermes user (10000). +# Start as root so the s6-overlay stage2 hook can usermod/groupmod and chown +# the data volume. Each supervised service then drops to the hermes user via +# `s6-setuidgid hermes` in its run script. If HERMES_UID is unset, services +# run as the default hermes user (UID 10000). # ---------- Link hermes-agent itself (editable) ---------- # Deps are already installed in the cached layer above; `--no-deps` makes # this a fast (~1s) egg-link creation with no resolution or downloads. RUN uv pip install --no-cache-dir --no-deps -e "." +# ---------- s6-overlay service wiring ---------- +# Static services declared at build time: main-hermes + dashboard. +# Per-profile gateway services are registered dynamically at runtime by +# the profile create/delete hooks (Phase 4); they live under +# /run/service/ (tmpfs) and are reconciled on container restart by +# /etc/cont-init.d/02-reconcile-profiles (Phase 4 Task 4.0). +COPY docker/s6-rc.d/ /etc/s6-overlay/s6-rc.d/ + +# stage2-hook handles UID/GID remap, volume chown, config seeding, +# skills sync — all the work the old entrypoint.sh did before +# `exec hermes`. Wired in as cont-init.d/01- so it +# runs before user services start. +# +# 02-reconcile-profiles re-creates per-profile gateway s6 service +# slots from $HERMES_HOME/profiles// after a container restart +# (the /run/service/ scandir is tmpfs and wiped on restart). Phase 4. +RUN mkdir -p /etc/cont-init.d && \ + printf '#!/bin/sh\nexec /opt/hermes/docker/stage2-hook.sh\n' \ + > /etc/cont-init.d/01-hermes-setup && \ + chmod +x /etc/cont-init.d/01-hermes-setup +COPY --chmod=0755 docker/cont-init.d/015-supervise-perms /etc/cont-init.d/015-supervise-perms +COPY --chmod=0755 docker/cont-init.d/02-reconcile-profiles /etc/cont-init.d/02-reconcile-profiles + # ---------- Runtime ---------- ENV HERMES_WEB_DIST=/opt/hermes/hermes_cli/web_dist ENV HERMES_HOME=/opt/data -ENV PATH="/opt/data/.local/bin:${PATH}" +# Pre-s6 entrypoint.sh did `source .venv/bin/activate` which exported +# the venv bin onto PATH; Architecture B's main-wrapper.sh does the +# same for the container's main process, but `docker exec` and our +# cont-init.d scripts don't pass through the wrapper. Expose the venv +# bin globally so `docker exec hermes ...` and any +# subprocess that doesn't activate the venv first still find hermes. +ENV PATH="/opt/hermes/.venv/bin:/opt/data/.local/bin:${PATH}" RUN mkdir -p /opt/data VOLUME [ "/opt/data" ] -ENTRYPOINT [ "/usr/bin/tini", "-g", "--", "/opt/hermes/docker/entrypoint.sh" ] + +# s6-overlay's /init is PID 1. It sets up the supervision tree, runs +# /etc/cont-init.d/* (our stage2 hook), starts s6-rc services +# declared in /etc/s6-overlay/s6-rc.d/, then exec's its remaining +# argv as the container's "main program" with stdin/stdout/stderr +# inherited (this is what makes interactive --tui work). When the +# main program exits, /init begins stage 3 shutdown and the container +# exits with the program's exit code. Replaces tini — see Phase 2 of +# docs/plans/2026-05-07-s6-overlay-dynamic-subagent-gateways.md. +# +# We use the ENTRYPOINT+CMD split rather than CMD alone so the +# wrapper is prepended to user-supplied args automatically: +# +# docker run → /init main-wrapper.sh (CMD default) +# docker run chat -q "hi" → /init main-wrapper.sh chat -q hi +# docker run sleep infinity → /init main-wrapper.sh sleep infinity +# docker run --tui → /init main-wrapper.sh --tui +# +# main-wrapper.sh handles arg routing (bare-exec vs. hermes +# subcommand vs. no-args), drops to the hermes user via s6-setuidgid, +# and exec's the final program so its exit code becomes the container +# exit code. Without the wrapper-as-ENTRYPOINT, leading-dash args +# like `--version` would be intercepted by /init's POSIX shell. +ENTRYPOINT [ "/init", "/opt/hermes/docker/main-wrapper.sh" ] +CMD [ ] diff --git a/README.md b/README.md index b659f56fa5..9b14816429 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,27 @@ hermes doctor # Diagnose any issues 📖 **[Full documentation →](https://hermes-agent.nousresearch.com/docs/)** +--- + +## Skip the API-key collection — Nous Portal + +Hermes works with whatever provider you want — that's not changing. But if you'd rather not collect five separate API keys for the model, web search, image generation, TTS, and a cloud browser, **[Nous Portal](https://portal.nousresearch.com)** covers all of them under one subscription: + +- **300+ models** — pick any of them with `/model ` +- **Tool Gateway** — web search (Firecrawl), image generation (FAL), text-to-speech (OpenAI), cloud browser (Browser Use), all routed through your sub. No extra accounts. + +One command from a fresh install: + +```bash +hermes setup --portal +``` + +That logs you in via OAuth, sets Nous as your provider, and turns on the Tool Gateway. Check what's wired up any time with `hermes portal status`. Full details on the [Tool Gateway docs page](https://hermes-agent.nousresearch.com/docs/user-guide/features/tool-gateway). + +You can still bring your own keys per-tool whenever you want — the gateway is per-backend, not all-or-nothing. + +--- + ## CLI vs Messaging Quick Reference Hermes has two entry points: start the terminal UI with `hermes`, or run the gateway and talk to it from Telegram, Discord, Slack, WhatsApp, Signal, or Email. Once you're in a conversation, many slash commands are shared across both interfaces. diff --git a/README.zh-CN.md b/README.zh-CN.md index 9a96457441..e2228234ce 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -65,6 +65,27 @@ hermes doctor # 诊断问题 📖 **[完整文档 →](https://hermes-agent.nousresearch.com/docs/)** +--- + +## 省去到处收集 API Key — Nous Portal + +Hermes 始终允许你使用任意服务商，这点不会改变。但如果你不想为模型、网页搜索、图像生成、TTS、云浏览器分别去申请五个不同的 API Key，**[Nous Portal](https://portal.nousresearch.com)** 用一个订阅就能覆盖全部： + +- **300+ 模型** — 用 `/model ` 随时切换 +- **Tool Gateway** — 网页搜索（Firecrawl）、图像生成（FAL）、文本转语音（OpenAI）、云浏览器（Browser Use），全部通过订阅托管。无需额外注册任何账户。 + +全新安装时一条命令即可： + +```bash +hermes setup --portal +``` + +它会通过 OAuth 登录、把 Nous 设为推理服务商，并启用 Tool Gateway。随时用 `hermes portal status` 查看路由状态。完整说明见 [Tool Gateway 文档](https://hermes-agent.nousresearch.com/docs/user-guide/features/tool-gateway)。 + +你随时可以按工具单独切回自己的 API Key — Gateway 是按工具粒度生效的，不是一刀切。 + +--- + ## CLI 与消息平台 快速对照 Hermes 有两种入口：用 `hermes` 启动终端 UI，或运行网关从 Telegram、Discord、Slack、WhatsApp、Signal 或 Email 与之对话。进入对话后，许多斜杠命令在两种界面中通用。 diff --git a/acp_adapter/server.py b/acp_adapter/server.py index fbdee70527..81c22c1877 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -1534,7 +1534,11 @@ class HermesACPAgent(acp.Agent): ) except Exception: logger.debug("Failed to auto-title ACP session %s", session_id, exc_info=True) - if final_response and conn and not streamed_message: + if final_response and conn and (not streamed_message or result.get("response_transformed")): + # Deliver the final response when streaming did not already send it, + # or when a plugin hook transformed the response after streaming + # finished (e.g. transform_llm_output) — otherwise the appended / + # rewritten text never reaches the client. update = acp.update_agent_message_text(final_response) await conn.session_update(session_id, update) diff --git a/agent/agent_init.py b/agent/agent_init.py index be9a09dd2f..e20755c509 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -607,6 +607,31 @@ def init_agent( # Falling back would send Anthropic credentials to third-party endpoints (Fixes #1739, #minimax-401). _is_native_anthropic = agent.provider == "anthropic" effective_key = (api_key or resolve_anthropic_token() or "") if _is_native_anthropic else (api_key or "") + + # MiniMax OAuth issues short-lived (~15-min) access tokens. The + # Anthropic SDK caches ``api_key`` as a static string at client + # construction time, so a session that resolves the bearer once + # at startup will keep sending the same token until MiniMax + # returns 401 mid-session. Swap the static string for a callable + # token provider — ``build_anthropic_client`` recognizes the + # callable and installs an httpx event hook that mints a fresh + # bearer per outbound request (re-reading auth.json so a refresh + # persisted by another process is visible immediately). + # The cached refresh path is a no-op when the token still has + # ``MINIMAX_OAUTH_REFRESH_SKEW_SECONDS`` of life left, so steady- + # state cost is one file read + one timestamp compare per request. + if agent.provider == "minimax-oauth" and isinstance(effective_key, str) and effective_key: + try: + from hermes_cli.auth import build_minimax_oauth_token_provider + effective_key = build_minimax_oauth_token_provider() + except Exception as _mm_exc: # noqa: BLE001 — never block startup on this + import logging as _logging + _logging.getLogger(__name__).warning( + "MiniMax OAuth: failed to install per-request token provider " + "(%s); falling back to static bearer that will expire ~15min in.", + _mm_exc, + ) + agent.api_key = effective_key agent._anthropic_api_key = effective_key agent._anthropic_base_url = base_url @@ -618,7 +643,7 @@ def init_agent( # that cause 401/403 on their endpoints. Guards #1739 and # the third-party identity-injection bug. from agent.anthropic_adapter import _is_oauth_token as _is_oat - agent._is_anthropic_oauth = _is_oat(effective_key) if _is_native_anthropic else False + agent._is_anthropic_oauth = _is_oat(effective_key) if (_is_native_anthropic and isinstance(effective_key, str)) else False agent._anthropic_client = build_anthropic_client(effective_key, base_url, timeout=_provider_timeout) # No OpenAI client needed for Anthropic mode agent.client = None @@ -951,16 +976,14 @@ def init_agent( # Expose session ID to tools (terminal, execute_code) so agents can # reference their own session for --resume commands, cross-session - # coordination, and logging. Uses the ContextVar system from - # session_context.py for concurrency safety (gateway runs multiple - # sessions in one process). Also writes os.environ as fallback for - # CLI mode where ContextVars aren't used. - os.environ["HERMES_SESSION_ID"] = agent.session_id + # coordination, and logging. Keep the ContextVar and os.environ + # fallback synchronized because different tool paths still read both. try: - from gateway.session_context import _SESSION_ID - _SESSION_ID.set(agent.session_id) + from gateway.session_context import set_current_session_id + + set_current_session_id(agent.session_id) except Exception: - pass # CLI/test mode — ContextVar not needed + os.environ["HERMES_SESSION_ID"] = agent.session_id # Session logs go into ~/.hermes/sessions/ alongside gateway sessions hermes_home = get_hermes_home() @@ -1125,7 +1148,18 @@ def init_agent( # through _ra().get_tool_definitions()). Duplicate function names cause # 400 errors on providers that enforce unique names (e.g. Xiaomi # MiMo via Nous Portal). - if agent._memory_manager and agent.tools is not None: + # + # Respect the platform's enabled_toolsets configuration (#5544): + # enabled_toolsets is None → no filter, inject (backward compat) + # "memory" in enabled_toolsets → user opted in, inject + # otherwise (incl. []) → user excluded memory, skip injection + # + # Without this gate, `platform_toolsets: telegram: []` still leaks memory + # provider tools (fact_store, etc.) into the tool surface — a 10x latency + # penalty on local models and a frequent trigger of tool-call loops. + if agent._memory_manager and agent.tools is not None and ( + agent.enabled_toolsets is None or "memory" in agent.enabled_toolsets + ): _existing_tool_names = { t.get("function", {}).get("name") for t in agent.tools @@ -1393,6 +1427,7 @@ def init_agent( base_url=agent.base_url, api_key=getattr(agent, "api_key", ""), provider=agent.provider, + api_mode=agent.api_mode, ) if not agent.quiet_mode: _ra().logger.info("Using context engine: %s", _selected_engine.name) @@ -1435,8 +1470,22 @@ def init_agent( # errors. Even with the cache fix, dedup is the right defense # against plugin paths that may register the same schemas via # ctx.register_tool(). Mirrors the memory tools dedup above. + # + # Respect the platform's enabled_toolsets configuration (#5544): + # context engine tools follow the same gating pattern as memory + # provider tools — without the gate, `platform_toolsets: telegram: []` + # would still leak lcm_* tools into the tool surface and incur the + # same local-model latency penalty. agent._context_engine_tool_names: set = set() - if hasattr(agent, "context_compressor") and agent.context_compressor and agent.tools is not None: + if ( + hasattr(agent, "context_compressor") + and agent.context_compressor + and agent.tools is not None + and ( + agent.enabled_toolsets is None + or "context_engine" in agent.enabled_toolsets + ) + ): _existing_tool_names = { t.get("function", {}).get("name") for t in agent.tools diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index b98fe4b44e..f7c8819eb5 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -132,7 +132,7 @@ def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_que except json.JSONDecodeError: # This shouldn't happen since we validate and retry during conversation, # but if it does, log warning and use empty dict - logging.warning(f"Unexpected invalid JSON in trajectory conversion: {tool_call['function']['arguments'][:100]}") + logger.warning(f"Unexpected invalid JSON in trajectory conversion: {tool_call['function']['arguments'][:100]}") arguments = {} tool_call_json = { @@ -617,9 +617,28 @@ def recover_with_credential_pool( # existing entitlement keyword set in ``_is_entitlement_failure``. # Any 403 against ``xai-oauth`` is treated as entitlement here so # the refresh loop can't spin in those cases either. + # + # Exception (#29344): xAI's ``[WKE=unauthenticated:...]`` suffix and + # the ``OAuth2 access token could not be validated`` phrasing are + # xAI's authoritative "this is a stale token, not entitlement" + # signal. When either fires we must NOT apply the catch-all + # override — refresh is the recoverable path for these bodies, and + # blanket-classifying them as entitlement was the bug that left + # long-running TUI sessions stuck on stale tokens until the user + # exited and reopened. is_entitlement = agent._is_entitlement_failure(error_context, status_code) if not is_entitlement and status_code == 403 and (agent.provider or "") == "xai-oauth": - is_entitlement = True + _disambiguator_haystack = " ".join( + str(error_context.get(k) or "").lower() + for k in ("message", "reason", "code", "error") + if isinstance(error_context, dict) + ) + _is_xai_auth_failure = ( + "[wke=unauthenticated:" in _disambiguator_haystack + or "oauth2 access token could not be validated" in _disambiguator_haystack + ) + if not _is_xai_auth_failure: + is_entitlement = True if is_entitlement: _ra().logger.info( "Credential %s — entitlement-shaped 403 from %s; " @@ -728,7 +747,7 @@ def try_recover_primary_transport( time.sleep(wait_time) return True except Exception as e: - logging.warning("Primary transport recovery failed: %s", e) + logger.warning("Primary transport recovery failed: %s", e) return False # ── End provider fallback ────────────────────────────────────────────── @@ -891,19 +910,20 @@ def restore_primary_runtime(agent) -> bool: base_url=rt["compressor_base_url"], api_key=rt["compressor_api_key"], provider=rt["compressor_provider"], + api_mode=rt.get("compressor_api_mode", ""), ) # ── Reset fallback chain for the new turn ── agent._fallback_activated = False agent._fallback_index = 0 - logging.info( + logger.info( "Primary runtime restored for new turn: %s (%s)", agent.model, agent.provider, ) return True except Exception as e: - logging.warning("Failed to restore primary runtime: %s", e) + logger.warning("Failed to restore primary runtime: %s", e) return False # Which error types indicate a transient transport failure worth @@ -1064,10 +1084,7 @@ def dump_api_request_debug( timestamp = datetime.now().strftime("%Y%m%d_%H%M%S_%f") dump_file = agent.logs_dir / f"request_dump_{agent.session_id}_{timestamp}.json" - dump_file.write_text( - json.dumps(dump_payload, ensure_ascii=False, indent=2, default=str), - encoding="utf-8", - ) + atomic_json_write(dump_file, dump_payload, default=str) agent._vprint(f"{agent.log_prefix}🧾 Request debug dump written to: {dump_file}") @@ -1077,7 +1094,7 @@ def dump_api_request_debug( return dump_file except Exception as dump_error: if agent.verbose_logging: - logging.warning(f"Failed to dump API request debug payload: {dump_error}") + logger.warning(f"Failed to dump API request debug payload: {dump_error}") return None @@ -1352,6 +1369,22 @@ def switch_model(agent, new_model, new_provider, api_key='', base_url='', api_mo # API key — falling back would send Anthropic credentials to third-party endpoints. _is_native_anthropic = new_provider == "anthropic" effective_key = (api_key or agent.api_key or resolve_anthropic_token() or "") if _is_native_anthropic else (api_key or agent.api_key or "") + + # MiniMax OAuth: swap static string for a per-request callable token + # provider so the rebuilt client survives 15-min token expiry. See + # the matching block in agent_init.py for the full rationale. + if new_provider == "minimax-oauth" and isinstance(effective_key, str) and effective_key: + try: + from hermes_cli.auth import build_minimax_oauth_token_provider + effective_key = build_minimax_oauth_token_provider() + except Exception as _mm_exc: # noqa: BLE001 + import logging as _logging + _logging.getLogger(__name__).warning( + "MiniMax OAuth: failed to install per-request token provider " + "on switch (%s); using static bearer.", + _mm_exc, + ) + agent.api_key = effective_key agent._anthropic_api_key = effective_key agent._anthropic_base_url = base_url or getattr(agent, "_anthropic_base_url", None) @@ -1359,7 +1392,7 @@ def switch_model(agent, new_model, new_provider, api_key='', base_url='', api_mo effective_key, agent._anthropic_base_url, timeout=get_provider_request_timeout(agent.provider, agent.model), ) - agent._is_anthropic_oauth = _is_oauth_token(effective_key) if _is_native_anthropic else False + agent._is_anthropic_oauth = _is_oauth_token(effective_key) if (_is_native_anthropic and isinstance(effective_key, str)) else False agent.client = None agent._client_kwargs = {} else: @@ -1446,6 +1479,7 @@ def switch_model(agent, new_model, new_provider, api_key='', base_url='', api_mo "compressor_api_key": getattr(_cc, "api_key", "") if _cc else "", "compressor_provider": getattr(_cc, "provider", agent.provider) if _cc else agent.provider, "compressor_context_length": _cc.context_length if _cc else 0, + "compressor_api_mode": getattr(_cc, "api_mode", agent.api_mode) if _cc else agent.api_mode, "compressor_threshold_tokens": _cc.threshold_tokens if _cc else 0, } if api_mode == "anthropic_messages": @@ -1477,7 +1511,7 @@ def switch_model(agent, new_model, new_provider, api_key='', base_url='', api_mo agent._fallback_chain = fallback_chain agent._fallback_model = fallback_chain[0] if fallback_chain else None - logging.info( + logger.info( "Model switched in-place: %s (%s) -> %s (%s)", old_model, old_provider, new_model, new_provider, ) @@ -2116,33 +2150,56 @@ def apply_pending_steer_to_tool_results(agent, messages: list, num_tool_msgs: in def force_close_tcp_sockets(client: Any) -> int: - """Force-close underlying TCP sockets to prevent CLOSE-WAIT accumulation. + """Abort in-flight TCP I/O by shutting down sockets WITHOUT closing FDs. - When a provider drops a connection mid-stream, httpx's ``client.close()`` - performs a graceful shutdown which leaves sockets in CLOSE-WAIT until the - OS times them out (often minutes). This method walks the httpx transport - pool and issues ``socket.shutdown(SHUT_RDWR)`` + ``socket.close()`` to - force an immediate TCP RST, freeing the file descriptors. + When a provider drops a connection mid-stream — or the user issues an + interrupt — we want to unblock httpx's reader/writer immediately rather + than waiting for the kernel's per-connection timeout. ``shutdown(SHUT_RDWR)`` + achieves that: it sends FIN, breaks any pending ``recv``/``send`` with EOF + or ``EPIPE``, but does NOT release the file descriptor. - Returns the number of sockets force-closed. + Historically this helper also called ``socket.close()`` so the FD got + released immediately, but that's unsafe when (as is the case for both the + interrupt-abort path and stale-call kill path) the helper runs on a + different thread than the one driving the request: + + * The Python ``socket.socket`` we close here is the SAME object held by + httpx's pool, so closing it via Python sets its ``_fd`` to -1 and + future operations on that Python object fail safely. + * BUT the SSL wrapper (``ssl.SSLSocket``'s underlying OpenSSL ``BIO``) + caches the raw integer FD. Once ``os.close(fd)`` runs, the kernel may + immediately recycle that integer to the next ``open()`` call — e.g. + the kanban dispatcher opening ``kanban.db``. + * The owning worker thread then unwinds httpx, the SSL layer flushes a + pending TLS record, and the encrypted bytes get written into the + wrong file (issue #29507: 24-byte TLS application-data record + clobbering SQLite header bytes 5..28). + + The fix is to let the owning thread own the close. ``shutdown()`` from any + thread is FD-safe; ``close()`` is not. The httpx connection's own close + path — which runs from the worker thread when it unwinds — will release + the FD via the same ``socket.socket`` object, and because Python's socket + close atomically swaps ``_fd`` to -1 *before* issuing ``os.close``, there + is no FD-aliasing window when only one thread closes. + + Returns the number of sockets shut down. (Field kept as + ``tcp_force_closed=N`` in the log line for backwards-compatible parsing.) """ import socket as _socket - closed = 0 + shutdown_count = 0 try: for sock in _iter_pool_sockets(client): try: sock.shutdown(_socket.SHUT_RDWR) except OSError: + # Already shut down / not connected / FD invalid — all benign. pass - try: - sock.close() - except OSError: - pass - closed += 1 + # IMPORTANT (#29507): do NOT call sock.close() here. See docstring. + shutdown_count += 1 except Exception as exc: _ra().logger.debug("Force-close TCP sockets sweep error: %s", exc) - return closed + return shutdown_count diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index c94d664a43..898df7eb68 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -15,6 +15,8 @@ import json import logging import os import platform +import secrets +import stat import subprocess from pathlib import Path from urllib.parse import urlparse @@ -1040,11 +1042,34 @@ def _write_claude_code_credentials( existing["claudeAiOauth"] = oauth_data cred_path.parent.mkdir(parents=True, exist_ok=True) - _tmp_cred = cred_path.with_suffix(".tmp") - _tmp_cred.write_text(json.dumps(existing, indent=2), encoding="utf-8") - _tmp_cred.replace(cred_path) - # Restrict permissions (credentials file) - cred_path.chmod(0o600) + # Per-process random suffix avoids collisions between concurrent + # writers and stale leftovers from a prior crashed write. + _tmp_cred = cred_path.with_suffix(f".tmp.{os.getpid()}.{secrets.token_hex(4)}") + try: + # Create the temp file atomically at 0o600. The previous + # write_text + post-replace chmod opened a TOCTOU window where + # both the temp file and the destination briefly inherited the + # process umask (commonly 0o644 = world-readable), exposing + # Claude Code OAuth tokens to other local users between create + # and chmod. Mirrors agent/google_oauth.py (#19673) and + # tools/mcp_oauth.py (#21148). Parent dir (~/.claude/) is + # owned by Claude Code itself, so we leave its mode alone. + fd = os.open( + str(_tmp_cred), + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + stat.S_IRUSR | stat.S_IWUSR, + ) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(existing, fh, indent=2) + fh.flush() + os.fsync(fh.fileno()) + os.replace(_tmp_cred, cred_path) + except OSError: + try: + _tmp_cred.unlink(missing_ok=True) + except OSError: + pass + raise except (OSError, IOError) as e: logger.debug("Failed to write refreshed credentials: %s", e) @@ -1606,182 +1631,155 @@ def _content_parts_to_anthropic_blocks(parts: Any) -> List[Dict[str, Any]]: return out -def convert_messages_to_anthropic( - messages: List[Dict], - base_url: str | None = None, - model: str | None = None, -) -> Tuple[Optional[Any], List[Dict]]: - """Convert OpenAI-format messages to Anthropic format. +def _convert_assistant_message(m: Dict[str, Any]) -> Dict[str, Any]: + """Convert an assistant message to Anthropic content blocks. - Returns (system_prompt, anthropic_messages). - System messages are extracted since Anthropic takes them as a separate param. - system_prompt is a string or list of content blocks (when cache_control present). - - When *base_url* is provided and points to a third-party Anthropic-compatible - endpoint, all thinking block signatures are stripped. Signatures are - Anthropic-proprietary — third-party endpoints cannot validate them and will - reject them with HTTP 400 "Invalid signature in thinking block". - - When *model* is provided and matches the Kimi / Moonshot family (or - *base_url* is a Kimi / Moonshot host), unsigned thinking blocks - synthesised from ``reasoning_content`` are preserved on replayed - assistant tool-call messages — Kimi requires the field to exist, even - if empty. + Handles thinking blocks, regular content, tool calls, and + reasoning_content injection for Kimi/DeepSeek endpoints. """ - system = None - result = [] - - for m in messages: - role = m.get("role", "user") - content = m.get("content", "") - - if role == "system": - if isinstance(content, list): - # Preserve cache_control markers on content blocks - has_cache = any( - p.get("cache_control") for p in content if isinstance(p, dict) - ) - if has_cache: - system = [p for p in content if isinstance(p, dict)] - else: - system = "\n".join( - p["text"] for p in content if p.get("type") == "text" - ) - else: - system = content - continue - - if role == "assistant": - blocks = _extract_preserved_thinking_blocks(m) - if content: - if isinstance(content, list): - converted_content = _convert_content_to_anthropic(content) - if isinstance(converted_content, list): - blocks.extend(converted_content) - else: - blocks.append({"type": "text", "text": str(content)}) - for tc in m.get("tool_calls", []): - if not tc or not isinstance(tc, dict): - continue - fn = tc.get("function", {}) - args = fn.get("arguments", "{}") - try: - parsed_args = json.loads(args) if isinstance(args, str) else args - except (json.JSONDecodeError, ValueError): - parsed_args = {} - blocks.append({ - "type": "tool_use", - "id": _sanitize_tool_id(tc.get("id", "")), - "name": fn.get("name", ""), - "input": parsed_args, - }) - # Kimi's /coding endpoint (Anthropic protocol) requires assistant - # tool-call messages to carry reasoning_content when thinking is - # enabled server-side. Preserve it as a thinking block so Kimi - # can validate the message history. See hermes-agent#13848. - # - # Accept empty string "" — _copy_reasoning_content_for_api() - # injects "" as a tier-3 fallback for Kimi tool-call messages - # that had no reasoning. Kimi requires the field to exist, even - # if empty. - # - # Prepend (not append): Anthropic protocol requires thinking - # blocks before text and tool_use blocks. - # - # Guard: only add when reasoning_details didn't already contribute - # thinking blocks. On native Anthropic, reasoning_details produces - # signed thinking blocks — adding another unsigned one from - # reasoning_content would create a duplicate (same text) that gets - # downgraded to a spurious text block on the last assistant message. - reasoning_content = m.get("reasoning_content") - _already_has_thinking = any( - isinstance(b, dict) and b.get("type") in {"thinking", "redacted_thinking"} - for b in blocks - ) - if isinstance(reasoning_content, str) and not _already_has_thinking: - blocks.insert(0, {"type": "thinking", "thinking": reasoning_content}) - # Anthropic rejects empty assistant content - effective = blocks or content - if not effective or effective == "": - effective = [{"type": "text", "text": "(empty)"}] - result.append({"role": "assistant", "content": effective}) - continue - - if role == "tool": - # Sanitize tool_use_id and ensure non-empty content. - # Computer-use (and other multimodal) tool results arrive as - # either a list of OpenAI-style content parts, or a dict - # marked `_multimodal` with an embedded `content` list. Convert - # both into Anthropic `tool_result` inner blocks (text + image). - multimodal_blocks: Optional[List[Dict[str, Any]]] = None - if isinstance(content, dict) and content.get("_multimodal"): - multimodal_blocks = _content_parts_to_anthropic_blocks( - content.get("content") or [] - ) - # Fallback text if the conversion produced nothing usable. - if not multimodal_blocks and content.get("text_summary"): - multimodal_blocks = [ - {"type": "text", "text": str(content["text_summary"])} - ] - elif isinstance(content, list): - converted = _content_parts_to_anthropic_blocks(content) - if any(b.get("type") == "image" for b in converted): - multimodal_blocks = converted - # Back-compat: some callers stash blocks under a private key. - if multimodal_blocks is None: - stashed = m.get("_anthropic_content_blocks") - if isinstance(stashed, list) and stashed: - text_content = content if isinstance(content, str) and content.strip() else None - multimodal_blocks = ( - [{"type": "text", "text": text_content}] + stashed - if text_content else list(stashed) - ) - - if multimodal_blocks: - result_content: Any = multimodal_blocks - elif isinstance(content, str): - result_content = content - else: - result_content = json.dumps(content) if content else "(no output)" - if not result_content: - result_content = "(no output)" - tool_result = { - "type": "tool_result", - "tool_use_id": _sanitize_tool_id(m.get("tool_call_id", "")), - "content": result_content, - } - if isinstance(m.get("cache_control"), dict): - tool_result["cache_control"] = dict(m["cache_control"]) - # Merge consecutive tool results into one user message - if ( - result - and result[-1]["role"] == "user" - and isinstance(result[-1]["content"], list) - and result[-1]["content"] - and result[-1]["content"][0].get("type") == "tool_result" - ): - result[-1]["content"].append(tool_result) - else: - result.append({"role": "user", "content": [tool_result]}) - continue - - # Regular user message — validate non-empty content (Anthropic rejects empty) + content = m.get("content", "") + blocks = _extract_preserved_thinking_blocks(m) + if content: if isinstance(content, list): - converted_blocks = _convert_content_to_anthropic(content) - # Check if all text blocks are empty - if not converted_blocks or all( - b.get("text", "").strip() == "" - for b in converted_blocks - if isinstance(b, dict) and b.get("type") == "text" - ): - converted_blocks = [{"type": "text", "text": "(empty message)"}] - result.append({"role": "user", "content": converted_blocks}) + converted_content = _convert_content_to_anthropic(content) + if isinstance(converted_content, list): + blocks.extend(converted_content) else: - # Validate string content is non-empty - if not content or (isinstance(content, str) and not content.strip()): - content = "(empty message)" - result.append({"role": "user", "content": content}) + blocks.append({"type": "text", "text": str(content)}) + for tc in m.get("tool_calls", []): + if not tc or not isinstance(tc, dict): + continue + fn = tc.get("function", {}) + args = fn.get("arguments", "{}") + try: + parsed_args = json.loads(args) if isinstance(args, str) else args + except (json.JSONDecodeError, ValueError): + parsed_args = {} + blocks.append({ + "type": "tool_use", + "id": _sanitize_tool_id(tc.get("id", "")), + "name": fn.get("name", ""), + "input": parsed_args, + }) + # Kimi's /coding endpoint (Anthropic protocol) requires assistant + # tool-call messages to carry reasoning_content when thinking is + # enabled server-side. Preserve it as a thinking block so Kimi + # can validate the message history. See hermes-agent#13848. + # + # Accept empty string "" — _copy_reasoning_content_for_api() + # injects "" as a tier-3 fallback for Kimi tool-call messages + # that had no reasoning. Kimi requires the field to exist, even + # if empty. + # + # Prepend (not append): Anthropic protocol requires thinking + # blocks before text and tool_use blocks. + # + # Guard: only add when reasoning_details didn't already contribute + # thinking blocks. On native Anthropic, reasoning_details produces + # signed thinking blocks — adding another unsigned one from + # reasoning_content would create a duplicate (same text) that gets + # downgraded to a spurious text block on the last assistant message. + reasoning_content = m.get("reasoning_content") + _already_has_thinking = any( + isinstance(b, dict) and b.get("type") in {"thinking", "redacted_thinking"} + for b in blocks + ) + if isinstance(reasoning_content, str) and not _already_has_thinking: + blocks.insert(0, {"type": "thinking", "thinking": reasoning_content}) + # Anthropic rejects empty assistant content + effective = blocks or content + if not effective or effective == "": + effective = [{"type": "text", "text": "(empty)"}] + return {"role": "assistant", "content": effective} + +def _convert_tool_message_to_result( + result: List[Dict[str, Any]], m: Dict[str, Any] +) -> None: + """Convert a tool message to an Anthropic tool_result, merging consecutive + results into one user message. + + Mutates ``result`` in place — either appends a new user message or extends + the trailing user message's tool_result list. + """ + content = m.get("content", "") + multimodal_blocks: Optional[List[Dict[str, Any]]] = None + if isinstance(content, dict) and content.get("_multimodal"): + multimodal_blocks = _content_parts_to_anthropic_blocks( + content.get("content") or [] + ) + # Fallback text if the conversion produced nothing usable. + if not multimodal_blocks and content.get("text_summary"): + multimodal_blocks = [ + {"type": "text", "text": str(content["text_summary"])} + ] + elif isinstance(content, list): + converted = _content_parts_to_anthropic_blocks(content) + if any(b.get("type") == "image" for b in converted): + multimodal_blocks = converted + # Back-compat: some callers stash blocks under a private key. + if multimodal_blocks is None: + stashed = m.get("_anthropic_content_blocks") + if isinstance(stashed, list) and stashed: + text_content = content if isinstance(content, str) and content.strip() else None + multimodal_blocks = ( + [{"type": "text", "text": text_content}] + stashed + if text_content else list(stashed) + ) + + if multimodal_blocks: + result_content: Any = multimodal_blocks + elif isinstance(content, str): + result_content = content + else: + result_content = json.dumps(content) if content else "(no output)" + if not result_content: + result_content = "(no output)" + tool_result = { + "type": "tool_result", + "tool_use_id": _sanitize_tool_id(m.get("tool_call_id", "")), + "content": result_content, + } + if isinstance(m.get("cache_control"), dict): + tool_result["cache_control"] = dict(m["cache_control"]) + # Merge consecutive tool results into one user message + if ( + result + and result[-1]["role"] == "user" + and isinstance(result[-1]["content"], list) + and result[-1]["content"] + and result[-1]["content"][0].get("type") == "tool_result" + ): + result[-1]["content"].append(tool_result) + else: + result.append({"role": "user", "content": [tool_result]}) + + +def _convert_user_message(content: Any) -> Dict[str, Any]: + """Validate and convert a user message to anthropic format.""" + if isinstance(content, list): + converted_blocks = _convert_content_to_anthropic(content) + if not converted_blocks or all( + b.get("text", "").strip() == "" + for b in converted_blocks + if isinstance(b, dict) and b.get("type") == "text" + ): + converted_blocks = [{"type": "text", "text": "(empty message)"}] + return {"role": "user", "content": converted_blocks} + else: + if not content or (isinstance(content, str) and not content.strip()): + content = "(empty message)" + return {"role": "user", "content": content} + + +def _strip_orphaned_tool_blocks(result: List[Dict[str, Any]]) -> None: + """Strip tool_use blocks with no matching tool_result, and vice versa. + + Context compression or session truncation can remove either side of a + tool-call pair. Anthropic rejects both orphans with HTTP 400. + + Mutates ``result`` in place. + """ # Strip orphaned tool_use blocks (no matching tool_result follows) tool_result_ids = set() for m in result: @@ -1799,10 +1797,7 @@ def convert_messages_to_anthropic( if not m["content"]: m["content"] = [{"type": "text", "text": "(tool call removed)"}] - # Strip orphaned tool_result blocks (no matching tool_use precedes them). - # This is the mirror of the above: context compression or session truncation - # can remove an assistant message containing a tool_use while leaving the - # subsequent tool_result intact. Anthropic rejects these with a 400. + # Strip orphaned tool_result blocks (no matching tool_use precedes them) tool_use_ids = set() for m in result: if m["role"] == "assistant" and isinstance(m["content"], list): @@ -1819,12 +1814,16 @@ def convert_messages_to_anthropic( if not m["content"]: m["content"] = [{"type": "text", "text": "(tool result removed)"}] - # Enforce strict role alternation (Anthropic rejects consecutive same-role messages) + +def _merge_consecutive_roles(result: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + """Merge consecutive same-role messages to enforce Anthropic alternation. + + Returns a new list (caller must rebind ``result``). + """ fixed = [] for m in result: if fixed and fixed[-1]["role"] == m["role"]: if m["role"] == "user": - # Merge consecutive user messages prev_content = fixed[-1]["content"] curr_content = m["content"] if isinstance(prev_content, str) and isinstance(curr_content, str): @@ -1832,7 +1831,6 @@ def convert_messages_to_anthropic( elif isinstance(prev_content, list) and isinstance(curr_content, list): fixed[-1]["content"] = prev_content + curr_content else: - # Mixed types — wrap string in list if isinstance(prev_content, str): prev_content = [{"type": "text", "text": prev_content}] if isinstance(curr_content, str): @@ -1855,7 +1853,6 @@ def convert_messages_to_anthropic( elif isinstance(prev_blocks, str) and isinstance(curr_blocks, str): fixed[-1]["content"] = prev_blocks + "\n" + curr_blocks else: - # Mixed types — normalize both to list and merge if isinstance(prev_blocks, str): prev_blocks = [{"type": "text", "text": prev_blocks}] if isinstance(curr_blocks, str): @@ -1863,37 +1860,34 @@ def convert_messages_to_anthropic( fixed[-1]["content"] = prev_blocks + curr_blocks else: fixed.append(m) - result = fixed + return fixed - # ── Thinking block signature management ────────────────────────── - # Anthropic signs thinking blocks against the full turn content. - # Any upstream mutation (context compression, session truncation, - # orphan stripping, message merging) invalidates the signature, - # causing HTTP 400 "Invalid signature in thinking block". - # - # Signatures are Anthropic-proprietary. Third-party endpoints - # (MiniMax, Microsoft Foundry, self-hosted proxies) cannot validate - # them and will reject them outright. When targeting a third-party - # endpoint, strip ALL thinking/redacted_thinking blocks from every - # assistant message — the third-party will generate its own - # thinking blocks if it supports extended thinking. - # - # For direct Anthropic (strategy following clawdbot/OpenClaw): - # 1. Strip thinking/redacted_thinking from all assistant messages - # EXCEPT the last one — preserves reasoning continuity on the - # current tool-use chain while avoiding stale signature errors. - # 2. Downgrade unsigned thinking blocks (no signature) to text — - # Anthropic can't validate them and will reject them. - # 3. Strip cache_control from thinking/redacted_thinking blocks — - # cache markers can interfere with signature validation. + +def _manage_thinking_signatures( + result: List[Dict[str, Any]], base_url: str | None, model: str | None +) -> None: + """Strip or preserve thinking blocks based on endpoint type. + + Anthropic signs thinking blocks against the full turn content. + Any upstream mutation (context compression, session truncation, orphan + stripping, message merging) invalidates the signature, causing HTTP 400 + "Invalid signature in thinking block". + + Signatures are Anthropic-proprietary. Third-party endpoints (MiniMax, + Azure AI Foundry, AWS Bedrock, self-hosted proxies) cannot validate them + and will reject them outright. Kimi's /coding and DeepSeek's /anthropic + endpoints speak the Anthropic protocol upstream but require unsigned + thinking blocks (synthesised from ``reasoning_content``) to round-trip on + replayed assistant tool-call messages. See hermes-agent#13848 (Kimi) and + hermes-agent#16748 (DeepSeek). + + Mutates ``result`` in place. + """ _THINKING_TYPES = frozenset(("thinking", "redacted_thinking")) _is_third_party = _is_third_party_anthropic_endpoint(base_url) - # Kimi /coding and DeepSeek /anthropic share a contract: both speak the - # Anthropic Messages protocol upstream but require that thinking blocks - # synthesised from reasoning_content round-trip on subsequent turns when - # thinking is enabled. Signed Anthropic blocks still have to be stripped - # (neither endpoint can validate Anthropic's signatures); unsigned blocks - # are preserved. See hermes-agent#13848 (Kimi) and #16748 (DeepSeek). + # Kimi / DeepSeek share a contract: strip signed Anthropic blocks + # (neither upstream can validate Anthropic signatures), preserve unsigned + # ones synthesised from reasoning_content. See #13848, #16748. _preserve_unsigned_thinking = ( _is_kimi_family_endpoint(base_url, model) or _is_deepseek_anthropic_endpoint(base_url) @@ -1910,26 +1904,19 @@ def convert_messages_to_anthropic( continue if _preserve_unsigned_thinking: - # Kimi's /coding and DeepSeek's /anthropic endpoints both enable - # thinking server-side and require unsigned thinking blocks on - # replayed assistant tool-call messages. Strip signed Anthropic - # blocks (neither upstream can validate Anthropic signatures) but - # preserve the unsigned ones we synthesised from reasoning_content. + # Kimi / DeepSeek: strip signed, preserve unsigned. new_content = [] for b in m["content"]: if not isinstance(b, dict) or b.get("type") not in _THINKING_TYPES: new_content.append(b) continue if b.get("signature") or b.get("data"): - # Anthropic-signed block — upstream can't validate, strip + # Signed (or redacted-with-data) — upstream can't validate, strip. continue - # Unsigned thinking (synthesised from reasoning_content) — - # keep it: the upstream needs it for message-history validation. new_content.append(b) m["content"] = new_content or [{"type": "text", "text": "(empty)"}] elif _is_third_party or idx != last_assistant_idx: - # Third-party endpoint: strip ALL thinking blocks from every - # assistant message — signatures are Anthropic-proprietary. + # Third-party: strip ALL thinking blocks (signatures are proprietary). # Direct Anthropic: strip from non-latest assistant messages only. stripped = [ b for b in m["content"] @@ -1937,24 +1924,21 @@ def convert_messages_to_anthropic( ] m["content"] = stripped or [{"type": "text", "text": "(thinking elided)"}] else: - # Latest assistant on direct Anthropic: keep signed thinking - # blocks for reasoning continuity; downgrade unsigned ones to - # plain text. + # Latest assistant on direct Anthropic: keep signed, downgrade unsigned + # to text so the reasoning isn't lost. new_content = [] for b in m["content"]: if not isinstance(b, dict) or b.get("type") not in _THINKING_TYPES: new_content.append(b) continue if b.get("type") == "redacted_thinking": - # Redacted blocks use 'data' for the signature payload + # Redacted blocks use 'data' for the signature payload — + # drop the block when 'data' is missing (can't be validated). if b.get("data"): new_content.append(b) - # else: drop — no data means it can't be validated elif b.get("signature"): - # Signed thinking block — keep it new_content.append(b) else: - # Unsigned thinking — downgrade to text so it's not lost thinking_text = b.get("thinking", "") if thinking_text: new_content.append({"type": "text", "text": thinking_text}) @@ -1966,12 +1950,15 @@ def convert_messages_to_anthropic( if isinstance(b, dict) and b.get("type") in _THINKING_TYPES: b.pop("cache_control", None) - # ── Image eviction: keep only the most recent N screenshots ───── - # computer_use screenshots (base64 images) sit inside tool_result - # blocks: they accumulate and are sent with every API call. Each - # costs ~1,465 tokens; after 10+ the conversation becomes slow - # even for simple text queries. Walk backward, keep the most recent - # _MAX_KEEP_IMAGES, replace older ones with a text placeholder. + +def _evict_old_screenshots(result: List[Dict[str, Any]]) -> None: + """Keep only the most recent ``_MAX_KEEP_IMAGES`` computer-use screenshots. + + Base64 images cost ~1,465 tokens each and accumulate across tool calls. + Walk backward, keep the most recent N, replace older ones with a placeholder. + + Mutates ``result`` in place. + """ _MAX_KEEP_IMAGES = 3 _image_count = 0 for msg in reversed(result): @@ -1998,6 +1985,68 @@ def convert_messages_to_anthropic( for b in inner ] + +def convert_messages_to_anthropic( + messages: List[Dict], + base_url: str | None = None, + model: str | None = None, +) -> Tuple[Optional[Any], List[Dict]]: + """Convert OpenAI-format messages to Anthropic format. + + Returns (system_prompt, anthropic_messages). + System messages are extracted since Anthropic takes them as a separate param. + system_prompt is a string or list of content blocks (when cache_control present). + + When *base_url* is provided and points to a third-party Anthropic-compatible + endpoint, all thinking block signatures are stripped. Signatures are + Anthropic-proprietary — third-party endpoints cannot validate them and will + reject them with HTTP 400 "Invalid signature in thinking block". + + When *model* is provided and matches the Kimi / Moonshot family (or + *base_url* is a Kimi / Moonshot host), unsigned thinking blocks + synthesised from ``reasoning_content`` are preserved on replayed + assistant tool-call messages — Kimi requires the field to exist, even + if empty. + """ + system = None + result: List[Dict[str, Any]] = [] + + for m in messages: + role = m.get("role", "user") + content = m.get("content", "") + + if role == "system": + if isinstance(content, list): + # Preserve cache_control markers on content blocks + has_cache = any( + p.get("cache_control") for p in content if isinstance(p, dict) + ) + if has_cache: + system = [p for p in content if isinstance(p, dict)] + else: + system = "\n".join( + p["text"] for p in content if p.get("type") == "text" + ) + else: + system = content + continue + + if role == "assistant": + result.append(_convert_assistant_message(m)) + continue + + if role == "tool": + _convert_tool_message_to_result(result, m) + continue + + # Regular user message + result.append(_convert_user_message(content)) + + _strip_orphaned_tool_blocks(result) + result = _merge_consecutive_roles(result) + _manage_thinking_signatures(result, base_url, model) + _evict_old_screenshots(result) + return system, result @@ -2098,9 +2147,13 @@ def build_anthropic_kwargs( block["text"] = text # 3. Prefix tool names with mcp_ (Claude Code convention) + # Skip names that already begin with the marker — native MCP server + # tools (from mcp_servers: in config.yaml) are registered under their + # full mcp__ name and would double-prefix otherwise, + # breaking round-trip registry lookup in normalize_response. GH-25255. if anthropic_tools: for tool in anthropic_tools: - if "name" in tool: + if "name" in tool and not tool["name"].startswith(_MCP_TOOL_PREFIX): tool["name"] = _MCP_TOOL_PREFIX + tool["name"] # 4. Prefix tool names in message history (tool_use and tool_result blocks) diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 89dc7d935b..78ec00d3f1 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -3260,7 +3260,7 @@ def resolve_provider_client( if client is None: logger.warning( "resolve_provider_client: xai-oauth requested but no xAI " - "OAuth token found (run: hermes model -> xAI Grok OAuth — SuperGrok Subscription)" + "OAuth token found (run: hermes model -> xAI Grok OAuth — SuperGrok / Premium+)" ) return None, None final_model = _normalize_resolved_model(model or default, provider) @@ -3730,6 +3730,37 @@ _VISION_AUTO_PROVIDER_ORDER = ( ) +def _main_model_supports_vision(provider: str, model: Optional[str]) -> bool: + """Return True when ``provider``/``model`` is known to accept image input. + + Used by the vision auto-detect chain to skip the user's main provider + when it's known to be text-only (e.g. DeepSeek, gpt-oss without vision). + Without this guard, ``resolve_vision_provider_client(provider="auto")`` + would happily return the main-provider client and any subsequent image + payload would surface as a cryptic provider-side error + (``unknown variant `image_url`, expected `text```, #31179). + + Returns True when capability lookup is unknown — preserves the historical + behaviour of attempting the call, so providers we haven't catalogued yet + don't silently regress to text-only. + """ + try: + from agent.image_routing import _lookup_supports_vision + from hermes_cli.config import load_config + except ImportError: + return True + try: + supports = _lookup_supports_vision(provider, model, load_config()) + except Exception: # pragma: no cover - defensive + return True + if supports is None: + # No capability data — keep current behaviour and let the call attempt + # happen rather than silently skipping. This avoids false-positive + # skips for new/custom providers. + return True + return bool(supports) + + def _normalize_vision_provider(provider: Optional[str]) -> str: return _normalize_aux_provider(provider) @@ -3870,6 +3901,23 @@ def resolve_vision_provider_client( "vision support) — falling through to aggregator chain", main_provider, ) + elif not _main_model_supports_vision(main_provider, vision_model): + # The main model is known to be text-only (e.g. DeepSeek V4, + # gpt-oss-120b without vision). Building a client and sending + # an image would produce a cryptic provider-side error like + # ``unknown variant `image_url`, expected `text``` (#31179). + # Fall through to the aggregator chain instead. + # + # Only log the provider name (not the model) — mirrors the + # sibling _PROVIDERS_WITHOUT_VISION branch above, and avoids + # CodeQL py/clear-text-logging-sensitive-data heuristic false + # positives on multi-value interpolations. + logger.debug( + "Vision auto-detect: skipping main provider %s " + "(reports no vision capability) — falling through to " + "aggregator chain", + main_provider, + ) else: rpc_client, rpc_model = resolve_provider_client( main_provider, vision_model, @@ -4281,6 +4329,23 @@ def _get_cached_client( return client, model or default_model +# Aliases that target direct REST APIs not modeled as first-class providers +# in PROVIDER_REGISTRY. Used for ``auxiliary..provider`` so users can +# write the obvious name and have it resolve to a working ``custom`` endpoint +# without needing to know our internal provider IDs. +# +# Why these specifically: PROVIDER_REGISTRY has ``openai-codex`` (OAuth) and +# ``custom`` (manual base_url + OPENAI_API_KEY) but no plain ``openai`` for +# direct API-key access. Users predictably type ``provider: openai`` and +# expect it to use OPENAI_API_KEY against api.openai.com. Previously this +# silently fell back to the user's main provider, sending OpenAI model names +# to e.g. DeepSeek and producing cryptic ``unknown variant 'image_url'`` +# errors (issue #31179). +_AUX_DIRECT_API_BASE_URLS: Dict[str, str] = { + "openai": "https://api.openai.com/v1", +} + + def _resolve_task_provider_model( task: str = None, provider: str = None, @@ -4317,6 +4382,25 @@ def _resolve_task_provider_model( resolved_model = model or cfg_model resolved_api_mode = cfg_api_mode + # Convenience aliases for direct API-key endpoints that aren't first-class + # providers (e.g. ``provider: openai`` → custom + api.openai.com/v1). + # Applied to both explicit args and config-derived values. When the user + # has already supplied a base_url we keep their endpoint but still rewrite + # the provider to ``custom`` so resolution doesn't hit the + # PROVIDER_REGISTRY-only path (which has no ``openai`` entry). + def _expand_direct_api_alias(prov: Optional[str], existing_base: Optional[str]) -> Tuple[Optional[str], Optional[str]]: + if not prov: + return prov, existing_base + target_base = _AUX_DIRECT_API_BASE_URLS.get(prov.strip().lower()) + if target_base is None: + return prov, existing_base + return "custom", existing_base or target_base + + if provider: + provider, base_url = _expand_direct_api_alias(provider, base_url) + if cfg_provider: + cfg_provider, cfg_base_url = _expand_direct_api_alias(cfg_provider, cfg_base_url) + if base_url: return "custom", resolved_model, base_url, api_key, resolved_api_mode if provider: @@ -4344,7 +4428,17 @@ _DEFAULT_AUX_TIMEOUT = 30.0 def _get_auxiliary_task_config(task: str) -> Dict[str, Any]: - """Return the config dict for auxiliary., or {} when unavailable.""" + """Return the config dict for auxiliary., or {} when unavailable. + + For plugin-registered auxiliary tasks (see + :meth:`hermes_cli.plugins.PluginContext.register_auxiliary_task`) the + plugin's declared *defaults* are layered underneath the user's config + so an unconfigured plugin task still works: + + plugin defaults ← config.yaml auxiliary. (user wins) + + Built-in tasks ignore this path (their defaults live in DEFAULT_CONFIG). + """ if not task: return {} try: @@ -4354,7 +4448,27 @@ def _get_auxiliary_task_config(task: str) -> Dict[str, Any]: return {} aux = config.get("auxiliary", {}) if isinstance(config, dict) else {} task_config = aux.get(task, {}) if isinstance(aux, dict) else {} - return task_config if isinstance(task_config, dict) else {} + if not isinstance(task_config, dict): + task_config = {} + + # Layer plugin-declared defaults underneath user config so + # ctx.register_auxiliary_task(defaults={...}) takes effect without + # forcing the user to write config.yaml entries. + try: + from hermes_cli.plugins import get_plugin_auxiliary_tasks + for _entry in get_plugin_auxiliary_tasks(): + if _entry.get("key") == task: + _defaults = _entry.get("defaults") or {} + if isinstance(_defaults, dict): + merged = dict(_defaults) + merged.update(task_config) + return merged + break + except Exception: + # Plugin discovery failure must not break aux task config reads. + pass + + return task_config def _get_task_timeout(task: str, default: float = _DEFAULT_AUX_TIMEOUT) -> float: diff --git a/agent/background_review.py b/agent/background_review.py index ba65b2b1bc..35d3d5191a 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -115,7 +115,10 @@ _SKILL_REVIEW_PROMPT = ( "Protected skills (DO NOT edit these):\n" " • Bundled skills (shipped with Hermes, e.g. 'hermes-agent').\n" " • Hub-installed skills (installed via 'hermes skills install').\n" - " • Pinned skills (marked via 'hermes curator pin').\n" + "Pinned skills (marked via 'hermes curator pin') CAN be improved — " + "pin only blocks deletion/archive/consolidation by the curator, not " + "content updates. Patch them when a pitfall or missing step turns up, " + "same as any other agent-created skill.\n" "If the only skills that need updating are protected, say\n" "'Nothing to save.' and stop.\n\n" "Do NOT capture (these become persistent self-imposed constraints " @@ -198,7 +201,10 @@ _COMBINED_REVIEW_PROMPT = ( "Protected skills (DO NOT edit these):\n" " • Bundled skills (shipped with Hermes, e.g. 'hermes-agent').\n" " • Hub-installed skills (installed via 'hermes skills install').\n" - " • Pinned skills (marked via 'hermes curator pin').\n" + "Pinned skills (marked via 'hermes curator pin') CAN be improved — " + "pin only blocks deletion/archive/consolidation by the curator, not " + "content updates. Patch them when a pitfall or missing step turns up, " + "same as any other agent-created skill.\n" "If the only skills that need updating are protected, say\n" "'Nothing to save.' and stop.\n\n" "Do NOT capture as skills (these become persistent self-imposed " diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index c68f2271f5..b52bd6a1fb 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -91,23 +91,55 @@ def interruptible_api_call(agent, api_kwargs: dict): provider fallback. """ result = {"response": None, "error": None} - request_client_holder = {"client": None} + request_client_holder = {"client": None, "owner_tid": None} request_client_lock = threading.Lock() def _set_request_client(client): with request_client_lock: request_client_holder["client"] = client + # #29507: stamp the owning thread so a stranger-thread interrupt + # only shuts the connection down rather than racing the worker + # for FD ownership during ``client.close()``. + request_client_holder["owner_tid"] = threading.get_ident() return client def _take_request_client(): with request_client_lock: client = request_client_holder.get("client") request_client_holder["client"] = None + request_client_holder["owner_tid"] = None return client def _close_request_client_once(reason: str) -> None: - request_client = _take_request_client() - if request_client is not None: + # #29507: dispatch on the calling thread. + # + # When ``_call`` (the worker) reaches its ``finally`` it owns the + # close and we pop + fully close as before. When a *stranger* thread + # (the interrupt-check loop, the stale-call detector) drives the + # close, only shut the sockets down so the worker's blocked + # ``recv``/``send`` unwinds with an ``EPIPE`` / EOF — and let the + # worker close ``client`` from its own thread on its way out. That + # avoids the FD-recycling race where the kernel reassigned a + # just-closed TLS socket FD to ``kanban.db``, and the still-live SSL + # BIO on the worker thread then wrote a 24-byte TLS application-data + # record into the SQLite header (#29507). + with request_client_lock: + request_client = request_client_holder.get("client") + owner_tid = request_client_holder.get("owner_tid") + stranger_thread = ( + request_client is not None + and owner_tid is not None + and owner_tid != threading.get_ident() + ) + if not stranger_thread: + # Owning thread (or no recorded owner) → pop and fully close. + request_client_holder["client"] = None + request_client_holder["owner_tid"] = None + if request_client is None: + return + if stranger_thread: + agent._abort_request_openai_client(request_client, reason=reason) + else: agent._close_request_openai_client(request_client, reason=reason) def _call(): @@ -549,6 +581,17 @@ def build_assistant_message(agent, assistant_message, finish_reason: str) -> dic if isinstance(_san_content, str) and _san_content: _san_content = agent._strip_think_blocks(_san_content).strip() + # Defence-in-depth: redact credentials (PATs, API keys, Bearer tokens) + # from assistant content BEFORE the message enters conversation history. + # If the model accidentally inlines a secret in its natural-language + # response, catch it here at the persistence boundary so it never + # reaches state.db, session_*.json, gateway delivery, or compression. + # Respects HERMES_REDACT_SECRETS via redact_sensitive_text — no-op + # when disabled. (#19798) + if isinstance(_san_content, str) and _san_content: + from agent.redact import redact_sensitive_text + _san_content = redact_sensitive_text(_san_content) + msg = { "role": "assistant", "content": _san_content, @@ -670,6 +713,18 @@ def build_assistant_message(agent, assistant_message, finish_reason: str) -> dic "arguments": tool_call.function.arguments }, } + # Defence-in-depth: redact credentials from tool call arguments + # before they enter conversation history. Tool execution uses the + # raw API response object, not this dict, so redacting the + # persisted shape is safe and only affects storage. Catches the + # case where a model accidentally inlines a secret into a tool + # call (e.g. `terminal(command="curl -H 'Authorization: Bearer + # sk-...'")`). (#19798) + if isinstance(tc_dict["function"]["arguments"], str): + from agent.redact import redact_sensitive_text + tc_dict["function"]["arguments"] = redact_sensitive_text( + tc_dict["function"]["arguments"] + ) # Preserve extra_content (e.g. Gemini thought_signature) so it # is sent back on subsequent API calls. Without this, Gemini 3 # thinking models reject the request with a 400 error. @@ -725,7 +780,7 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool current_base_url = str(getattr(agent, "base_url", "") or "").rstrip("/").lower() fb_base_url_for_dedup = (fb.get("base_url") or "").strip().rstrip("/").lower() if fb_provider == current_provider and fb_model == current_model: - logging.warning( + logger.warning( "Fallback skip: chain entry %s/%s matches current provider/model", fb_provider, fb_model, ) @@ -736,7 +791,7 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool and fb_base_url_for_dedup == current_base_url and fb_model == current_model ): - logging.warning( + logger.warning( "Fallback skip: chain entry base_url %s matches current backend", fb_base_url_for_dedup, ) @@ -768,7 +823,7 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool explicit_base_url=fb_base_url_hint, explicit_api_key=fb_api_key_hint) if fb_client is None: - logging.warning( + logger.warning( "Fallback to %s failed: provider not configured", fb_provider) return agent._try_activate_fallback() # try next in chain @@ -776,8 +831,11 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool from hermes_cli.model_normalize import normalize_model_for_provider fb_model = normalize_model_for_provider(fb_model, fb_provider) - except Exception: - pass + except Exception as _norm_err: + logger.warning( + "Could not normalize fallback model %r for provider %r: %s", + fb_model, fb_provider, _norm_err, + ) # Determine api_mode from provider / base URL / model fb_api_mode = "chat_completions" @@ -905,19 +963,20 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool base_url=agent.base_url, api_key=getattr(agent, "api_key", ""), # callable preserved → call_llm provider=agent.provider, + api_mode=agent.api_mode, ) agent._emit_status( f"🔄 Primary model failed — switching to fallback: " f"{fb_model} via {fb_provider}" ) - logging.info( + logger.info( "Fallback activated: %s → %s (%s)", old_model, fb_model, fb_provider, ) return True except Exception as e: - logging.error("Failed to activate fallback %s: %s", fb_model, e) + logger.error("Failed to activate fallback %s: %s", fb_model, e) return agent._try_activate_fallback() # try next in chain @@ -1133,7 +1192,7 @@ def handle_max_iterations(agent, messages: list, api_call_count: int) -> str: final_response = "I reached the iteration limit and couldn't generate a summary." except Exception as e: - logging.warning(f"Failed to get summary response: {e}") + logger.warning(f"Failed to get summary response: {e}") final_response = f"I reached the maximum iterations ({agent.max_iterations}) but couldn't summarize. Error: {str(e)}" return final_response @@ -1162,12 +1221,12 @@ def cleanup_task_resources(agent, task_id: str) -> None: _ra().cleanup_vm(task_id) except Exception as e: if agent.verbose_logging: - logging.warning(f"Failed to cleanup VM for task {task_id}: {e}") + logger.warning(f"Failed to cleanup VM for task {task_id}: {e}") try: _ra().cleanup_browser(task_id) except Exception as e: if agent.verbose_logging: - logging.warning(f"Failed to cleanup browser for task {task_id}: {e}") + logger.warning(f"Failed to cleanup browser for task {task_id}: {e}") @@ -1271,23 +1330,44 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= return result["response"] result = {"response": None, "error": None, "partial_tool_names": []} - request_client_holder = {"client": None, "diag": None} + request_client_holder = {"client": None, "diag": None, "owner_tid": None} request_client_lock = threading.Lock() def _set_request_client(client): with request_client_lock: request_client_holder["client"] = client + # See #29507 explanation in the non-streaming variant above. + request_client_holder["owner_tid"] = threading.get_ident() return client def _take_request_client(): with request_client_lock: client = request_client_holder.get("client") request_client_holder["client"] = None + request_client_holder["owner_tid"] = None return client def _close_request_client_once(reason: str) -> None: - request_client = _take_request_client() - if request_client is not None: + # See #29507 explanation in the non-streaming variant above. A + # stranger thread (the interrupt-check / stale-stream detector loop) + # only aborts sockets — never pops, never calls ``client.close()`` — + # so the worker thread retains ownership of the FD release. + with request_client_lock: + request_client = request_client_holder.get("client") + owner_tid = request_client_holder.get("owner_tid") + stranger_thread = ( + request_client is not None + and owner_tid is not None + and owner_tid != threading.get_ident() + ) + if not stranger_thread: + request_client_holder["client"] = None + request_client_holder["owner_tid"] = None + if request_client is None: + return + if stranger_thread: + agent._abort_request_openai_client(request_client, reason=reason) + else: agent._close_request_openai_client(request_client, reason=reason) first_delta_fired = {"done": False} @@ -2020,8 +2100,21 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= # Streaming failed AFTER some tokens were already delivered to # the platform. Re-raising would let the outer retry loop make # a new API call, creating a duplicate message. Return a - # partial "stop" response instead so the outer loop treats this - # turn as complete (no retry, no fallback). + # partial response stub instead and let the outer loop decide: + # + # - text-only partials → finish_reason="length" so the + # conversation loop persists the partial assistant content + # and asks the model to continue from where the stream + # died (issue #30963: partial stop misclassified as a + # clean completion was exiting the loop with budget + # remaining and an unfinished goal). + # + # - partial mid-tool-call → finish_reason="stop" stays. + # The user-visible warning we append says "Ask me to + # retry if you want to continue", so the agent should + # hand control back rather than auto-retry a tool call + # that may have side-effects. + # # Recover whatever content was already streamed to the user. # _current_streamed_assistant_text accumulates text fired # through _fire_stream_delta, so it has exactly what the @@ -2059,14 +2152,17 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= "of text; surfaced warning to user: %s", _partial_names, len(_partial_text or ""), result["error"], ) + _stub_finish_reason = "stop" else: logger.warning( - "Partial stream delivered before error; returning stub " - "response with %s chars of recovered content to prevent " - "duplicate messages: %s", + "Partial stream delivered before error; returning " + "length-truncated stub with %s chars of recovered " + "content so the loop can continue from where the " + "stream died: %s", len(_partial_text or ""), result["error"], ) + _stub_finish_reason = "length" _stub_msg = SimpleNamespace( role="assistant", content=_partial_text, tool_calls=None, reasoning_content=None, @@ -2075,7 +2171,7 @@ def interruptible_streaming_api_call(agent, api_kwargs: dict, *, on_first_delta= id="partial-stream-stub", model=getattr(agent, "model", "unknown"), choices=[SimpleNamespace( - index=0, message=_stub_msg, finish_reason="stop", + index=0, message=_stub_msg, finish_reason=_stub_finish_reason, )], usage=None, ) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index a0a0b32ad1..f435d92e7f 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -501,6 +501,7 @@ class ContextCompressor(ContextEngine): """Update tracked token usage from API response.""" self.last_prompt_tokens = usage.get("prompt_tokens", 0) self.last_completion_tokens = usage.get("completion_tokens", 0) + self.last_total_tokens = usage.get("total_tokens", self.last_prompt_tokens + self.last_completion_tokens) def should_compress(self, prompt_tokens: int = None) -> bool: """Check if context exceeds the compression threshold. @@ -789,7 +790,7 @@ class ContextCompressor(ContextEngine): into the warning log. """ self._summary_model_fallen_back = True - logging.warning( + logger.warning( "Summary model '%s' %s (%s). " "Falling back to main model '%s' for compression.", self.summary_model, reason, e, self.model, @@ -978,7 +979,7 @@ The user has requested that this compaction PRIORITISE preserving all informatio # No provider configured — long cooldown, unlikely to self-resolve self._summary_failure_cooldown_until = time.monotonic() + _SUMMARY_FAILURE_COOLDOWN_SECONDS self._last_summary_error = "no auxiliary LLM provider configured" - logging.warning("Context compression: no provider available for " + logger.warning("Context compression: no provider available for " "summary. Middle turns will be dropped without summary " "for %d seconds.", _SUMMARY_FAILURE_COOLDOWN_SECONDS) @@ -1074,7 +1075,7 @@ The user has requested that this compaction PRIORITISE preserving all informatio if len(err_text) > 220: err_text = err_text[:217].rstrip() + "..." self._last_summary_error = err_text - logging.warning( + logger.warning( "Failed to generate context summary: %s. " "Further summary attempts paused for %d seconds.", e, diff --git a/agent/context_engine.py b/agent/context_engine.py index 2947da54d8..c30a7a8475 100644 --- a/agent/context_engine.py +++ b/agent/context_engine.py @@ -200,6 +200,7 @@ class ContextEngine(ABC): base_url: str = "", api_key: str = "", provider: str = "", + api_mode: str = "", ) -> None: """Called when the user switches models or on fallback activation. diff --git a/agent/conversation_compression.py b/agent/conversation_compression.py index cd1b133fa4..a620f343e9 100644 --- a/agent/conversation_compression.py +++ b/agent/conversation_compression.py @@ -381,12 +381,12 @@ def compress_context( agent._session_db.end_session(agent.session_id, "compression") old_session_id = agent.session_id agent.session_id = f"{datetime.now().strftime('%Y%m%d_%H%M%S')}_{uuid.uuid4().hex[:6]}" - os.environ["HERMES_SESSION_ID"] = agent.session_id try: - from gateway.session_context import _SESSION_ID - _SESSION_ID.set(agent.session_id) + from gateway.session_context import set_current_session_id + + set_current_session_id(agent.session_id) except Exception: - pass + os.environ["HERMES_SESSION_ID"] = agent.session_id agent._session_db_created = False agent._session_db.create_session( session_id=agent.session_id, diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index caac0d3e8f..e34631377e 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -46,6 +46,7 @@ from agent.message_sanitization import ( _strip_non_ascii, ) from agent.model_metadata import ( + MINIMUM_CONTEXT_LENGTH, estimate_messages_tokens_rough, estimate_request_tokens_rough, get_next_probe_tier, @@ -73,6 +74,50 @@ from utils import base_url_host_matches, env_var_enabled logger = logging.getLogger(__name__) +def _ollama_context_limit_error(agent: Any, request_tokens: int) -> Optional[str]: + """Return a user-facing error when Ollama is loaded with too little context.""" + if not getattr(agent, "tools", None): + return None + + runtime_ctx = getattr(agent, "_ollama_num_ctx", None) + if not isinstance(runtime_ctx, int) or runtime_ctx <= 0: + return None + if runtime_ctx >= MINIMUM_CONTEXT_LENGTH: + return None + + model = getattr(agent, "model", "") or "the selected model" + base_url = getattr(agent, "base_url", "") or "unknown base URL" + provider = getattr(agent, "provider", "") or "unknown" + tool_count = len(getattr(agent, "tools", None) or []) + + logger.warning( + "Ollama runtime context too small for Hermes tool use: " + "model=%s provider=%s base_url=%s runtime_context=%d " + "minimum_context=%d estimated_request_tokens=%d tool_count=%d " + "session=%s", + model, + provider, + base_url, + runtime_ctx, + MINIMUM_CONTEXT_LENGTH, + request_tokens, + tool_count, + getattr(agent, "session_id", None) or "none", + ) + + return ( + f"Ollama loaded `{model}` with only {runtime_ctx:,} tokens of runtime " + f"context, but Hermes needs at least {MINIMUM_CONTEXT_LENGTH:,} tokens " + "for reliable tool use.\n\n" + "Increase the Ollama context for this model and restart/reload the " + "model before trying again. A known-good starting point is 65,536 " + "tokens. In Hermes config, set `model.ollama_num_ctx: 65536` " + "(and `model.context_length: 65536` if you also override the displayed " + "model context). If you manage the model through an Ollama Modelfile, " + "set `PARAMETER num_ctx 65536` there instead." + ) + + def _ra(): """Lazy reference to ``run_agent`` so callers can patch ``run_agent.handle_function_call`` / ``run_agent._set_interrupt`` / @@ -527,6 +572,7 @@ def run_conversation( api_call_count = 0 final_response = None interrupted = False + failed = False codex_ack_continuations = 0 length_continue_retries = 0 truncated_tool_call_retries = 0 @@ -883,6 +929,26 @@ def run_conversation( # Calculate approximate request size for logging total_chars = sum(len(str(msg)) for msg in api_messages) approx_tokens = estimate_messages_tokens_rough(api_messages) + approx_request_tokens = estimate_request_tokens_rough( + api_messages, tools=agent.tools or None + ) + + _runtime_context_error = _ollama_context_limit_error( + agent, approx_request_tokens + ) + if _runtime_context_error: + final_response = _runtime_context_error + failed = True + _turn_exit_reason = "ollama_runtime_context_too_small" + messages.append({"role": "assistant", "content": final_response}) + agent._emit_status("❌ Ollama runtime context is too small for Hermes tool use") + api_call_count -= 1 + agent._api_call_count = api_call_count + try: + agent.iteration_budget.refund() + except Exception: + pass + break # Thinking spinner for quiet mode (animated during API call) thinking_spinner = None @@ -923,6 +989,7 @@ def run_conversation( copilot_auth_retry_attempted=False thinking_sig_retry_attempted = False image_shrink_retry_attempted = False + multimodal_tool_content_retry_attempted = False oauth_1m_beta_retry_attempted = False llama_cpp_grammar_retry_attempted = False has_retried_429 = False @@ -1116,7 +1183,7 @@ def run_conversation( else str(_codex_error_obj) if _codex_error_obj else f"Responses API returned status '{_codex_resp_status}'" ) - logging.warning( + logger.warning( "Codex response status='%s' (error=%s). Routing to fallback. %s", _codex_resp_status, _codex_error_msg, agent._client_log_context(), @@ -1268,7 +1335,7 @@ def run_conversation( primary_recovery_attempted = False continue agent._emit_status(f"❌ Max retries ({max_retries}) exceeded for invalid responses. Giving up.") - logging.error(f"{agent.log_prefix}Invalid API response after {max_retries} retries.") + logger.error(f"{agent.log_prefix}Invalid API response after {max_retries} retries.") agent._persist_session(messages, conversation_history) return { "messages": messages, @@ -1281,7 +1348,7 @@ def run_conversation( # Backoff before retry — jittered exponential: 5s base, 120s cap wait_time = jittered_backoff(retry_count, base_delay=5.0, max_delay=120.0) agent._vprint(f"{agent.log_prefix}⏳ Retrying in {wait_time:.1f}s ({_failure_hint})...", force=True) - logging.warning(f"Invalid API response (retry {retry_count}/{max_retries}): {', '.join(error_details)} | Provider: {provider_name}") + logger.warning(f"Invalid API response (retry {retry_count}/{max_retries}): {', '.join(error_details)} | Provider: {provider_name}") # Sleep in small increments to stay responsive to interrupts sleep_end = time.time() + wait_time @@ -1347,7 +1414,18 @@ def run_conversation( finish_reason = "length" if finish_reason == "length": - agent._vprint(f"{agent.log_prefix}⚠️ Response truncated (finish_reason='length') - model hit max output tokens", force=True) + if getattr(response, "id", "") == "partial-stream-stub": + agent._vprint( + f"{agent.log_prefix}⚠️ Stream interrupted by network error " + f"(finish_reason='length' on partial-stream-stub)", + force=True, + ) + else: + agent._vprint( + f"{agent.log_prefix}⚠️ Response truncated " + f"(finish_reason='length') - model hit max output tokens", + force=True, + ) # Normalize the truncated response to a single OpenAI-style # message shape so text-continuation and tool-call retry @@ -1440,17 +1518,40 @@ def run_conversation( truncated_response_parts.append(assistant_message.content) if length_continue_retries < 3: - agent._vprint( - f"{agent.log_prefix}↻ Requesting continuation " - f"({length_continue_retries}/3)..." + # Distinguish a real output-token truncation + # from a partial-stream-stub network error + # (#30963). Same continuation machinery, + # but the prompt has to tell the truth or + # the model goes off rails ("I wasn't + # truncated, I'm done"). + _is_partial_stream_stub = ( + getattr(response, "id", "") == "partial-stream-stub" ) - continue_msg = { - "role": "user", - "content": ( + if _is_partial_stream_stub: + agent._vprint( + f"{agent.log_prefix}↻ Stream interrupted — " + f"requesting continuation " + f"({length_continue_retries}/3)..." + ) + _continue_content = ( + "[System: The previous response was cut off by a " + "network error mid-stream. Continue exactly where " + "you left off. Do not restart or repeat prior text. " + "Finish the answer directly.]" + ) + else: + agent._vprint( + f"{agent.log_prefix}↻ Requesting continuation " + f"({length_continue_retries}/3)..." + ) + _continue_content = ( "[System: Your previous response was truncated by the output " "length limit. Continue exactly where you left off. Do not " "restart or repeat prior text. Finish the answer directly.]" - ), + ) + continue_msg = { + "role": "user", + "content": _continue_content, } messages.append(continue_msg) agent._session_messages = messages @@ -1994,6 +2095,31 @@ def run_conversation( "or shrink didn't reduce size; surfacing original error." ) + # Multimodal-tool-content recovery: providers that follow + # the OpenAI spec strictly (tool message content must be a + # string) reject our list-type content with a 400. Strip + # image parts from any list-type tool messages, mark the + # (provider, model) as no-list-tool-content for the rest + # of this session so future tool results preemptively + # downgrade, and retry once. See issue #27344. + if ( + classified.reason == FailoverReason.multimodal_tool_content_unsupported + and not multimodal_tool_content_retry_attempted + ): + multimodal_tool_content_retry_attempted = True + if agent._try_strip_image_parts_from_tool_messages(api_messages): + agent._vprint( + f"{agent.log_prefix}📐 Provider rejected list-type tool content — " + f"downgraded screenshots to text and retrying...", + force=True, + ) + continue + else: + logger.info( + "multimodal-tool-content recovery: no list-type tool " + "messages with image parts found; surfacing original error." + ) + # Anthropic OAuth subscription rejected the 1M-context beta # header ("long context beta is not yet available for this # subscription"). Disable the beta for the rest of this @@ -2133,7 +2259,7 @@ def run_conversation( f"stripped all thinking blocks, retrying...", force=True, ) - logging.warning( + logger.warning( "%sThinking block signature recovery: stripped " "reasoning_details from %d messages", agent.log_prefix, len(messages), @@ -2158,7 +2284,7 @@ def run_conversation( from tools.schema_sanitizer import strip_pattern_and_format _, _stripped = strip_pattern_and_format(agent.tools) except Exception as _strip_exc: # pragma: no cover — defensive - logging.warning( + logger.warning( "%sllama.cpp grammar recovery: strip helper failed: %s", agent.log_prefix, _strip_exc, ) @@ -2169,7 +2295,7 @@ def run_conversation( f"stripped {_stripped} pattern/format keyword(s), retrying...", force=True, ) - logging.warning( + logger.warning( "%sllama.cpp grammar recovery: stripped %d " "pattern/format keyword(s) from tool schemas", agent.log_prefix, _stripped, @@ -2177,7 +2303,7 @@ def run_conversation( continue # No keywords found to strip — fall through to normal # retry path rather than loop forever on the same error. - logging.warning( + logger.warning( "%sllama.cpp grammar error but no pattern/format " "keywords to strip — falling through to normal retry", agent.log_prefix, @@ -2278,6 +2404,7 @@ def run_conversation( base_url=agent.base_url, api_key=getattr(agent, "api_key", ""), provider=agent.provider, + api_mode=agent.api_mode, ) # Context probing flags — only set on built-in # compressor (plugin engines manage their own). @@ -2391,7 +2518,7 @@ def run_conversation( error_context=error_context, ) else: - logging.info( + logger.info( "Nous 429 looks like upstream capacity " "(no exhausted bucket in headers or " "last-known state) -- not tripping " @@ -2451,7 +2578,7 @@ def run_conversation( if compression_attempts > max_compression_attempts: agent._vprint(f"{agent.log_prefix}❌ Max compression attempts ({max_compression_attempts}) reached for payload-too-large error.", force=True) agent._vprint(f"{agent.log_prefix} 💡 Try /new to start a fresh conversation, or /compress to retry compression.", force=True) - logging.error(f"{agent.log_prefix}413 compression failed after {max_compression_attempts} attempts.") + logger.error(f"{agent.log_prefix}413 compression failed after {max_compression_attempts} attempts.") agent._persist_session(messages, conversation_history) return { "messages": messages, @@ -2482,7 +2609,7 @@ def run_conversation( else: agent._vprint(f"{agent.log_prefix}❌ Payload too large and cannot compress further.", force=True) agent._vprint(f"{agent.log_prefix} 💡 Try /new to start a fresh conversation, or /compress to retry compression.", force=True) - logging.error(f"{agent.log_prefix}413 payload too large. Cannot compress further.") + logger.error(f"{agent.log_prefix}413 payload too large. Cannot compress further.") agent._persist_session(messages, conversation_history) return { "messages": messages, @@ -2535,7 +2662,7 @@ def run_conversation( if compression_attempts > max_compression_attempts: agent._vprint(f"{agent.log_prefix}❌ Max compression attempts ({max_compression_attempts}) reached.", force=True) agent._vprint(f"{agent.log_prefix} 💡 Try /new to start a fresh conversation, or /compress to retry compression.", force=True) - logging.error(f"{agent.log_prefix}Context compression failed after {max_compression_attempts} attempts.") + logger.error(f"{agent.log_prefix}Context compression failed after {max_compression_attempts} attempts.") agent._persist_session(messages, conversation_history) return { "messages": messages, @@ -2587,6 +2714,7 @@ def run_conversation( base_url=agent.base_url, api_key=getattr(agent, "api_key", ""), provider=agent.provider, + api_mode=agent.api_mode, ) # Context probing flags — only set on built-in # compressor (plugin engines manage their own). @@ -2608,7 +2736,7 @@ def run_conversation( if compression_attempts > max_compression_attempts: agent._vprint(f"{agent.log_prefix}❌ Max compression attempts ({max_compression_attempts}) reached.", force=True) agent._vprint(f"{agent.log_prefix} 💡 Try /new to start a fresh conversation, or /compress to retry compression.", force=True) - logging.error(f"{agent.log_prefix}Context compression failed after {max_compression_attempts} attempts.") + logger.error(f"{agent.log_prefix}Context compression failed after {max_compression_attempts} attempts.") agent._persist_session(messages, conversation_history) return { "messages": messages, @@ -2641,7 +2769,7 @@ def run_conversation( # Can't compress further and already at minimum tier agent._vprint(f"{agent.log_prefix}❌ Context length exceeded and cannot compress further.", force=True) agent._vprint(f"{agent.log_prefix} 💡 The conversation has accumulated too much content. Try /new to start fresh, or /compress to manually trigger compression.", force=True) - logging.error(f"{agent.log_prefix}Context length exceeded: {approx_tokens:,} tokens. Cannot compress further.") + logger.error(f"{agent.log_prefix}Context length exceeded: {approx_tokens:,} tokens. Cannot compress further.") agent._persist_session(messages, conversation_history) return { "messages": messages, @@ -2678,6 +2806,21 @@ def run_conversation( # retryable=True mapping takes effect instead. and not isinstance(api_error, ssl.SSLError) ) + # ``FailoverReason.billing`` (HTTP 402) is NOT in this + # exclusion set. By the time we reach this block: + # • credential-pool rotation (line ~2031) has already + # fired for billing and either ``continue``d or + # returned (False, ...) — pool is exhausted or absent. + # • the eager-fallback branch above (line ~2422) also + # fires on billing and ``continue``s if a fallback + # provider is configured. + # Falling through to here means BOTH recovery paths + # gave up. Treating 402 as retryable from this point + # just burns more paid requests against a depleted + # balance with no recovery mechanism left — see #31273 + # (real-world: ~$40 in 48h on a 24/7 gateway). Aborting + # mirrors how 401/403 (also ``should_fallback=True``) + # already behave once their recovery paths have failed. is_client_error = ( is_local_validation_error or ( @@ -2685,7 +2828,6 @@ def run_conversation( and not classified.should_compress and classified.reason not in { FailoverReason.rate_limit, - FailoverReason.billing, FailoverReason.overloaded, FailoverReason.context_overflow, FailoverReason.payload_too_large, @@ -2725,7 +2867,7 @@ def run_conversation( agent._vprint(f"{agent.log_prefix} 2. Then run `hermes auth` to re-authenticate.", force=True) else: agent._vprint(f"{agent.log_prefix} 💡 xAI OAuth token was rejected (HTTP 401). To fix:", force=True) - agent._vprint(f"{agent.log_prefix} re-authenticate with xAI Grok OAuth (SuperGrok Subscription) from `hermes model`.", force=True) + agent._vprint(f"{agent.log_prefix} re-authenticate with xAI Grok OAuth (SuperGrok / Premium+) from `hermes model`.", force=True) else: agent._vprint(f"{agent.log_prefix} 💡 Your API key was rejected by the provider. Check:", force=True) agent._vprint(f"{agent.log_prefix} • Is the key valid? Run: hermes setup", force=True) @@ -2734,7 +2876,7 @@ def run_conversation( agent._vprint(f"{agent.log_prefix} • Check credits: https://openrouter.ai/settings/credits", force=True) else: agent._vprint(f"{agent.log_prefix} 💡 This type of error won't be fixed by retrying.", force=True) - logging.error(f"{agent.log_prefix}Non-retryable client error: {api_error}") + logger.error(f"{agent.log_prefix}Non-retryable client error: {api_error}") # Skip session persistence when the error is likely # context-overflow related (status 400 + large session). # Persisting the failed user message would make the @@ -2811,7 +2953,7 @@ def run_conversation( force=True, ) - logging.error( + logger.error( "%sAPI call failed after %s retries. %s | provider=%s model=%s msgs=%s tokens=~%s", agent.log_prefix, max_retries, _final_summary, _provider, _model, len(api_messages), f"{approx_tokens:,}", @@ -3342,6 +3484,19 @@ def run_conversation( f"⚠️ Tool guardrail halted {decision.tool_name}: {decision.code}" ) messages.append({"role": "assistant", "content": final_response}) + # Emit the halt message to the client so it's not + # indistinguishable from a crash. The stream display + # was flushed (callback(None)) before tool execution, + # but the callback is still alive — fire the text + # through it so SSE/TUI clients see the explanation. + if final_response: + agent._safe_print(f"\n{final_response}\n") + if agent.stream_delta_callback: + try: + agent.stream_delta_callback(final_response) + agent.stream_delta_callback(None) + except Exception: + pass break # Reset per-turn retry counters after successful tool @@ -3848,7 +4003,11 @@ def run_conversation( ) # Determine if conversation completed successfully - completed = final_response is not None and api_call_count < agent.max_iterations + completed = ( + final_response is not None + and api_call_count < agent.max_iterations + and not failed + ) # Save trajectory if enabled. ``user_message`` may be a multimodal # list of parts; the trajectory format wants a plain string. @@ -3933,6 +4092,8 @@ def run_conversation( except Exception as _ver_err: logger.debug("file-mutation verifier footer failed: %s", _ver_err) + _response_transformed = False + # Plugin hook: transform_llm_output # Fired once per turn after the tool-calling loop completes. # Plugins can transform the LLM's output text before it's returned. @@ -3950,6 +4111,7 @@ def run_conversation( for _hook_result in _transform_results: if isinstance(_hook_result, str) and _hook_result: final_response = _hook_result + _response_transformed = True break # First non-empty string wins except Exception as exc: logger.warning("transform_llm_output hook failed: %s", exc) @@ -3998,8 +4160,10 @@ def run_conversation( "api_calls": api_call_count, "completed": completed, "turn_exit_reason": _turn_exit_reason, + "failed": failed, "partial": False, # True only when stopped due to invalid tool calls "interrupted": interrupted, + "response_transformed": _response_transformed, "response_previewed": getattr(agent, "_response_was_previewed", False), "model": agent.model, "provider": agent.provider, diff --git a/agent/credential_sources.py b/agent/credential_sources.py index ee03542602..6c0a081a47 100644 --- a/agent/credential_sources.py +++ b/agent/credential_sources.py @@ -285,7 +285,7 @@ def _remove_xai_oauth_loopback_pkce(provider: str, removed) -> RemovalResult: if _clear_auth_store_provider(provider): result.cleaned.append(f"Cleared {provider} OAuth tokens from auth store") result.hints.append( - "Run `hermes model` → xAI Grok OAuth (SuperGrok Subscription) to re-authenticate if needed." + "Run `hermes model` → xAI Grok OAuth (SuperGrok / Premium+) to re-authenticate if needed." ) return result diff --git a/agent/curator_backup.py b/agent/curator_backup.py index fe74920521..5e39443bae 100644 --- a/agent/curator_backup.py +++ b/agent/curator_backup.py @@ -50,6 +50,7 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Tuple from hermes_constants import get_hermes_home +from agent.skill_utils import is_excluded_skill_path logger = logging.getLogger(__name__) @@ -176,7 +177,9 @@ def get_keep() -> int: def _count_skill_files(base: Path) -> int: try: - return sum(1 for _ in base.rglob("SKILL.md")) + return sum( + 1 for p in base.rglob("SKILL.md") if not is_excluded_skill_path(p) + ) except OSError: return 0 diff --git a/agent/display.py b/agent/display.py index cdfc88f46a..02880a83e0 100644 --- a/agent/display.py +++ b/agent/display.py @@ -787,33 +787,65 @@ class KawaiiSpinner: # Cute tool message (completion line that replaces the spinner) # ========================================================================= +_ERROR_SUFFIX_MAX_LEN = 48 + + +def _trim_error(msg: str) -> str: + """Shrink an error message for inline display in a tool status line. + + Strips overly long absolute paths down to just the filename so the + suffix stays readable on narrow terminals. + """ + msg = msg.strip() + # Common case: "File not found: /very/long/absolute/path/foo.py" + if "File not found:" in msg: + _, _, tail = msg.partition("File not found:") + tail = tail.strip() + if "/" in tail: + msg = f"File not found: {tail.rsplit('/', 1)[-1]}" + if len(msg) > _ERROR_SUFFIX_MAX_LEN: + msg = msg[: _ERROR_SUFFIX_MAX_LEN - 3] + "..." + return msg + + def _detect_tool_failure(tool_name: str, result: str | None) -> tuple[bool, str]: """Inspect a tool result string for signs of failure. - Returns ``(is_failure, suffix)`` where *suffix* is an informational tag - like ``" [exit 1]"`` for terminal failures, or ``" [error]"`` for generic - failures. On success, returns ``(False, "")``. + Returns ``(is_failure, suffix)`` where *suffix* is a short informational + tag like ``" [exit 1]"`` for terminal failures, ``" [full]"`` for memory + overflow, or a trimmed error message (``" [File not found: foo.py]"``). + On success returns ``(False, "")``. """ if result is None: return False, "" if file_mutation_result_landed(tool_name, result): return False, "" + data = safe_json_loads(result) + + # Terminal: non-zero exit code is the canonical failure signal. if tool_name == "terminal": - data = safe_json_loads(result) if isinstance(data, dict): exit_code = data.get("exit_code") if exit_code is not None and exit_code != 0: + err_msg = data.get("error") + if err_msg: + return True, f" [{_trim_error(str(err_msg))}]" return True, f" [exit {exit_code}]" return False, "" - # Memory-specific: distinguish "full" from real errors + # Memory: distinguish "store full" from real errors. if tool_name == "memory": - data = safe_json_loads(result) if isinstance(data, dict): if data.get("success") is False and "exceed the limit" in data.get("error", ""): return True, " [full]" + # Structured error in JSON result (any tool that surfaces {"error": ...}). + if isinstance(data, dict): + err = data.get("error") or data.get("message") + if err and (data.get("success") is False or "error" in data): + return True, f" [{_trim_error(str(err))}]" + # Generic heuristic for non-terminal tools # Multimodal tool results (dicts with _multimodal=True) are not strings — # treat them as successes since failures would be JSON-encoded strings. @@ -921,11 +953,29 @@ def get_cute_tool_message( if tool_name == "todo": todos_arg = args.get("todos") merge = args.get("merge", False) + # Parse result for completion progress + total = 0 + done = 0 + if result: + try: + data = safe_json_loads(result) + if data: + s = data.get("summary", {}) + total = s.get("total", 0) + done = s.get("completed", 0) + except Exception: + pass if todos_arg is None: + if total > 0: + return _wrap(f"┊ 📋 plan {done}/{total} task(s) {dur}") return _wrap(f"┊ 📋 plan reading tasks {dur}") elif merge: + if total > 0 and done > 0: + return _wrap(f"┊ 📋 plan update {done}/{total} ✓ {dur}") return _wrap(f"┊ 📋 plan update {len(todos_arg)} task(s) {dur}") else: + if total > 0 and done > 0: + return _wrap(f"┊ 📋 plan {done}/{total} task(s) {dur}") return _wrap(f"┊ 📋 plan {len(todos_arg)} task(s) {dur}") if tool_name == "session_search": return _wrap(f"┊ 🔍 recall \"{_trunc(args.get('query', ''), 35)}\" {dur}") diff --git a/agent/error_classifier.py b/agent/error_classifier.py index 42eb42d680..0afcf66d44 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -50,6 +50,7 @@ class FailoverReason(enum.Enum): # Request format format_error = "format_error" # 400 bad request — abort or strip + retry + multimodal_tool_content_unsupported = "multimodal_tool_content_unsupported" # Provider rejected list-type content in tool messages (e.g. Xiaomi MiMo) — downgrade to text and retry # Provider-specific thinking_signature = "thinking_signature" # Anthropic thinking block sig invalid @@ -165,6 +166,32 @@ _IMAGE_TOO_LARGE_PATTERNS = [ # the likely culprit; we still try the shrink path before giving up. ] +# Providers that follow the OpenAI spec strictly require tool message +# ``content`` to be a string. Some (Anthropic native, Codex Responses, +# Gemini native, first-party OpenAI) extend this to accept a content-parts +# list (text + image_url) so screenshots from computer_use survive. Others +# (Xiaomi MiMo, some Alibaba endpoints, a long tail of OpenAI-compatible +# providers) reject the list with a 400 — the patterns below are the most +# common error shapes we see. Recovery: strip image parts from tool +# messages in-place, record the (provider, model) for the rest of the +# session so we don't waste another call learning the same lesson, retry. +# +# See: https://github.com/NousResearch/hermes-agent/issues/27344 +_MULTIMODAL_TOOL_CONTENT_PATTERNS = [ + # Xiaomi MiMo: {"error":{"code":"400","message":"Param Incorrect","param":"text is not set"}} + "text is not set", + # Generic "tool message must be string" shapes + "tool message content must be a string", + "tool content must be a string", + "tool message must be a string", + # OpenAI-compat servers that reject list-type tool content with a + # schema-validation message + "expected string, got list", + "expected string, got array", + # Alibaba/DashScope variant + "tool_call.content must be string", +] + # Context overflow patterns _CONTEXT_OVERFLOW_PATTERNS = [ "context length", @@ -213,6 +240,24 @@ _MODEL_NOT_FOUND_PATTERNS = [ "unsupported model", ] +# Request-validation patterns — the request is malformed and will fail +# identically on every retry. Some OpenAI-compatible gateways (notably +# codex.nekos.me) return these as 5xx instead of the standard 4xx, which +# makes the generic "5xx → retryable server_error" rule misfire: the retry +# loop hammers the same deterministic rejection 3+ times, then the +# transport-recovery path resets the counter and does it again, producing +# a request flood. When a 5xx body carries one of these unambiguous +# request-validation signals, classify as a non-retryable format_error so +# the loop fails fast and falls back instead of looping. +_REQUEST_VALIDATION_PATTERNS = [ + "unknown parameter", + "unsupported parameter", + "unrecognized request argument", + "invalid_request_error", + "unknown_parameter", + "unsupported_parameter", +] + # OpenRouter aggregator policy-block patterns. # # When a user's OpenRouter account privacy setting (or a per-request @@ -718,6 +763,23 @@ def _classify_by_status( ) if status_code in {500, 502}: + # Some OpenAI-compatible gateways return request-validation errors + # with a 5xx status (codex.nekos.me returns 502 for unknown/ + # unsupported parameters). These are deterministic — every retry + # gets the identical rejection — so the generic "5xx → retryable + # server_error" rule turns one bad request into a retry flood. + # Detect the unambiguous request-validation signals (in either the + # message text or the structured error code) and fail fast. + if ( + any(p in error_msg for p in _REQUEST_VALIDATION_PATTERNS) + or error_code.lower() in {"invalid_request_error", "unknown_parameter", + "unsupported_parameter"} + ): + return result_fn( + FailoverReason.format_error, + retryable=False, + should_fallback=True, + ) return result_fn(FailoverReason.server_error, retryable=True) if status_code in {503, 529}: @@ -781,6 +843,19 @@ def _classify_400( ) -> ClassifiedError: """Classify 400 Bad Request — context overflow, format error, or generic.""" + # Multimodal tool content rejected from 400. Must be checked BEFORE + # image_too_large because the recovery is different (strip image parts + # from tool messages, mark the model as no-list-tool-content for the + # rest of the session) and BEFORE context_overflow because some of the + # patterns ("text is not set") are ambiguous in isolation but become + # specific when combined with a 400 on a request known to contain + # multimodal tool content. + if any(p in error_msg for p in _MULTIMODAL_TOOL_CONTENT_PATTERNS): + return result_fn( + FailoverReason.multimodal_tool_content_unsupported, + retryable=True, + ) + # Image-too-large from 400 (Anthropic's 5 MB per-image check fires this way). # Must be checked BEFORE context_overflow because messages can trip both # patterns ("exceeds" + "image") and image-shrink is a cheaper recovery. @@ -922,6 +997,13 @@ def _classify_by_message( should_compress=True, ) + # Multimodal tool content patterns (from message text when no status_code) + if any(p in error_msg for p in _MULTIMODAL_TOOL_CONTENT_PATTERNS): + return result_fn( + FailoverReason.multimodal_tool_content_unsupported, + retryable=True, + ) + # Image-too-large patterns (from message text when no status_code) if any(p in error_msg for p in _IMAGE_TOO_LARGE_PATTERNS): return result_fn( diff --git a/agent/file_safety.py b/agent/file_safety.py index f8678b68c0..e8c80f2b6d 100644 --- a/agent/file_safety.py +++ b/agent/file_safety.py @@ -41,6 +41,11 @@ def build_write_denied_paths(home: str) -> set[str]: # Top-level .env, even when running under a profile — overwriting it # leaks credentials across every profile that inherits from root (#15981). str(hermes_root / ".env"), + # Active profile Anthropic PKCE credential store. + str(hermes_home / ".anthropic_oauth.json"), + # Top-level Anthropic PKCE credential store remains sensitive even + # when a profile is active; default/non-profile sessions still read it. + str(hermes_root / ".anthropic_oauth.json"), os.path.join(home, ".bashrc"), os.path.join(home, ".zshrc"), os.path.join(home, ".profile"), @@ -50,6 +55,7 @@ def build_write_denied_paths(home: str) -> set[str]: os.path.join(home, ".pgpass"), os.path.join(home, ".npmrc"), os.path.join(home, ".pypirc"), + os.path.join(home, ".git-credentials"), "/etc/sudoers", "/etc/passwd", "/etc/shadow", @@ -71,6 +77,7 @@ def build_write_denied_prefixes(home: str) -> list[str]: os.path.join(home, ".docker"), os.path.join(home, ".azure"), os.path.join(home, ".config", "gh"), + os.path.join(home, ".config", "gcloud"), ] ] @@ -97,6 +104,43 @@ def is_write_denied(path: str) -> bool: if resolved.startswith(prefix): return True + # Hermes control-plane files: block both the ACTIVE profile's view + # (hermes_home) AND the global root view. Without the root pass, a + # profile-mode session leaves /auth.json + /config.yaml + # writable — letting a prompt-injected write_file overwrite the global + # files that every profile inherits from (same shape as #15981). + control_file_names = ("auth.json", "config.yaml", "webhook_subscriptions.json") + mcp_tokens_dir_name = "mcp-tokens" + + hermes_dirs = [] + for base in (_hermes_home_path(), _hermes_root_path()): + try: + real = os.path.realpath(base) + if real not in hermes_dirs: + hermes_dirs.append(real) + except Exception: + continue + + for base_real in hermes_dirs: + for name in control_file_names: + try: + if resolved == os.path.realpath(os.path.join(base_real, name)): + return True + except Exception: + continue + try: + mcp_real = os.path.realpath(os.path.join(base_real, mcp_tokens_dir_name)) + if resolved == mcp_real or resolved.startswith(mcp_real + os.sep): + return True + except Exception: + pass + try: + pairing_real = os.path.realpath(os.path.join(base_real, "pairing")) + if resolved == pairing_real or resolved.startswith(pairing_real + os.sep): + return True + except Exception: + pass + safe_root = get_safe_write_root() if safe_root and not (resolved == safe_root or resolved.startswith(safe_root + os.sep)): return True @@ -105,21 +149,267 @@ def is_write_denied(path: str) -> bool: def get_read_block_error(path: str) -> Optional[str]: - """Return an error message when a read targets internal Hermes cache files.""" + """Return an error message when a read targets a denied Hermes path. + + Two categories are blocked: + + * Internal Hermes cache files under ``HERMES_HOME/skills/.hub`` — + readable metadata that an attacker could use as a prompt-injection + carrier. + * Credential / secret stores under HERMES_HOME and the global Hermes + root: ``auth.json``, ``auth.lock``, ``.anthropic_oauth.json``, + ``.env``, ``webhook_subscriptions.json``, ``auth/google_oauth.json``, + and anything under ``mcp-tokens/``. These hold plaintext provider keys, + OAuth tokens, and HMAC secrets that the agent never needs to read + directly — provider tools / gateway adapters consume them through + internal channels. + + **This is NOT a security boundary.** The terminal tool runs as the + same OS user with shell access; the agent can still ``cat auth.json`` + or ``cat ~/.hermes/.env`` and exfiltrate the file. The read-deny exists + as defense-in-depth that: + + * Returns a clear error to models that respect tool denials, which + empirically prompts most modern models to stop rather than reach + for the shell. + * Surfaces a visible audit trail when something tries to read + credentials — easier to spot in logs than a generic ``cat``. + + Treat any user-visible framing around this as "may help" rather than + "stops attackers." A determined model or malicious instruction can + always shell out. + + Callers that resolve relative paths against a non-process cwd + (e.g. ``TERMINAL_CWD`` in ``tools/file_tools.py``) MUST pre-resolve + and pass the absolute path string. This function's own ``resolve()`` + is anchored at the Python process cwd, so a relative input like + ``"auth.json"`` would otherwise miss the denylist when the task's + terminal cwd differs from the process cwd. + """ resolved = Path(path).expanduser().resolve() - hermes_home = _hermes_home_path().resolve() - blocked_dirs = [ - hermes_home / "skills" / ".hub" / "index-cache", - hermes_home / "skills" / ".hub", - ] - for blocked in blocked_dirs: + + # Resolve BOTH the active HERMES_HOME (profile-aware) AND the global + # Hermes root so credential stores at /auth.json etc. are also + # blocked when running under a profile (HERMES_HOME points at + # /profiles/ in profile mode). Same shape as the write + # deny widening (#15981, #14157). + hermes_dirs: list[Path] = [] + for base in (_hermes_home_path(), _hermes_root_path()): try: - resolved.relative_to(blocked) + real = base.resolve() + if real not in hermes_dirs: + hermes_dirs.append(real) + except Exception: + continue + + # Skills .hub: prompt-injection carriers. + for hd in hermes_dirs: + blocked_dirs = [ + hd / "skills" / ".hub" / "index-cache", + hd / "skills" / ".hub", + ] + for blocked in blocked_dirs: + try: + resolved.relative_to(blocked) + except ValueError: + continue + return ( + f"Access denied: {path} is an internal Hermes cache file " + "and cannot be read directly to prevent prompt injection. " + "Use the skills_list or skill_view tools instead." + ) + + # Credential / secret stores. Exact-file matches under either + # HERMES_HOME or . + credential_file_names = ( + "auth.json", + "auth.lock", + ".anthropic_oauth.json", + ".env", + "webhook_subscriptions.json", + os.path.join("auth", "google_oauth.json"), + ) + for hd in hermes_dirs: + for name in credential_file_names: + try: + blocked = (hd / name).resolve() + except Exception: + continue + if resolved == blocked: + return ( + f"Access denied: {path} is a Hermes credential store " + "and cannot be read directly. Provider tools consume " + "these credentials through internal channels. " + "(Defense-in-depth — not a security boundary; the " + "terminal tool can still bypass.)" + ) + + # mcp-tokens/: directory prefix match — anything inside is OAuth + # token material. + for hd in hermes_dirs: + try: + mcp_tokens = (hd / "mcp-tokens").resolve() + except Exception: + continue + if resolved == mcp_tokens: + return ( + f"Access denied: {path} is the Hermes MCP token directory " + "and cannot be read directly. (Defense-in-depth — not a " + "security boundary; the terminal tool can still bypass.)" + ) + try: + resolved.relative_to(mcp_tokens) except ValueError: continue return ( - f"Access denied: {path} is an internal Hermes cache file " - "and cannot be read directly to prevent prompt injection. " - "Use the skills_list or skill_view tools instead." + f"Access denied: {path} is a Hermes MCP token file " + "and cannot be read directly. (Defense-in-depth — not a " + "security boundary; the terminal tool can still bypass.)" ) + return None + + +# --------------------------------------------------------------------------- +# Cross-profile write guard (#TBD) +# +# Hermes profiles are separate HERMES_HOME dirs under +# ``/profiles//``. Each profile has its own skills/, plugins/, +# cron/, memories/. When an agent runs under one profile, writing into +# ANOTHER profile's directories is almost always wrong — those skills / +# plugins / cron jobs / memories affect a different session the user runs +# from a different shell. +# +# Soft guard, NOT a security boundary: the agent runs as the same OS user +# and has unrestricted terminal access, so this returns a warning the model +# can choose to honor or override with ``cross_profile=True``. Same shape +# as the dangerous-command approval flow — the agent is told the boundary +# exists, and explicit user direction is required to cross it. +# +# Reference: May 2026 incident where a hermes-security profile session +# edited skills under both ``~/.hermes/profiles/hermes-security/skills/`` +# AND ``~/.hermes/skills/`` (the default profile's skills) without realizing +# the second path belonged to a different profile. +# --------------------------------------------------------------------------- + +# Profile-scoped directories under HERMES_HOME / / /profiles// +# that should be guarded. Adding a new area here extends the guard with no +# other code change. +PROFILE_SCOPED_AREAS = ("skills", "plugins", "cron", "memories") + + +def _resolve_active_profile_name() -> str: + """Return the active profile name derived from HERMES_HOME. + + ``~/.hermes`` -> ``"default"`` + ``~/.hermes/profiles/X`` -> ``"X"`` + + Falls back to ``"default"`` on any resolution failure so the guard + never raises into the tool path. + """ + try: + home_real = _hermes_home_path().resolve() + root_real = _hermes_root_path().resolve() + except (OSError, RuntimeError): + return "default" + profiles_dir = root_real / "profiles" + try: + rel = home_real.relative_to(profiles_dir) + parts = rel.parts + if len(parts) >= 1: + return parts[0] + except ValueError: + pass + return "default" + + +def classify_cross_profile_target(path: str) -> Optional[dict]: + """Classify a write target as cross-profile if it lands in another + profile's scoped area (skills/plugins/cron/memories). + + Returns ``None`` when the target is outside Hermes scope, or is inside + the ACTIVE profile, or doesn't hit a profile-scoped area. Otherwise + returns a dict with: + + * ``active_profile``: name of the profile the agent is running as + * ``target_profile``: name of the profile the path belongs to + * ``area``: which scoped area (``"skills"``, ``"plugins"``, etc.) + * ``target_path``: the resolved path string + + The caller decides what to do with the result — surface a warning to + the model, prompt the user, or (with explicit consent / + ``cross_profile=True``) proceed anyway. + """ + try: + target = Path(os.path.expanduser(str(path))).resolve() + root_real = _hermes_root_path().resolve() + except (OSError, RuntimeError): + return None + + target_profile: Optional[str] = None + area: Optional[str] = None + + try: + rel = target.relative_to(root_real) + except ValueError: + return None + + parts = rel.parts + if not parts: + return None + + if parts[0] in PROFILE_SCOPED_AREAS: + # ``//...`` → default profile. + target_profile = "default" + area = parts[0] + elif ( + parts[0] == "profiles" + and len(parts) >= 3 + and parts[2] in PROFILE_SCOPED_AREAS + ): + # ``/profiles///...`` → named profile. + target_profile = parts[1] + area = parts[2] + else: + return None + + active_profile = _resolve_active_profile_name() + if target_profile == active_profile: + # In-profile write — not a cross-profile event. + return None + + return { + "active_profile": active_profile, + "target_profile": target_profile, + "area": area, + "target_path": str(target), + } + + +def get_cross_profile_warning(path: str) -> Optional[str]: + """Return a model-facing warning string when ``path`` is cross-profile. + + Returns ``None`` when the write is in-scope (same profile) or outside + Hermes entirely. Caller is expected to surface the warning to the + agent as a tool-result error, NOT to silently allow the write — the + agent must either get explicit user direction to proceed, or pass + ``cross_profile=True`` to its write tool. + + This is defense-in-depth: the terminal tool runs as the same OS user + and can write any of these paths without going through this guard. + Treat the guard as a confusion-reducer, not a security boundary. + """ + info = classify_cross_profile_target(path) + if info is None: + return None + return ( + f"Cross-profile write blocked by soft guard: {info['target_path']} " + f"belongs to Hermes profile {info['target_profile']!r}, but the " + f"agent is running under profile {info['active_profile']!r}. " + f"Editing another profile's {info['area']}/ will affect that " + f"profile's future sessions, not the one you are currently in. " + f"Confirm with the user before proceeding. To bypass this guard " + f"after explicit user direction, retry the call with " + f"``cross_profile=True``. (Defense-in-depth — not a security " + f"boundary; the terminal tool can still bypass.)" + ) diff --git a/agent/image_gen_provider.py b/agent/image_gen_provider.py index 47f65c1b34..a7f1b8c31f 100644 --- a/agent/image_gen_provider.py +++ b/agent/image_gen_provider.py @@ -191,6 +191,88 @@ def save_b64_image( return path +# Extension inference for save_url_image — keep small and explicit. We don't +# want to import mimetypes for a handful of formats every image_gen provider +# actually returns, and we never want to inherit a content-type that points +# at HTML or JSON when the API gives us a degenerate response. +_URL_IMAGE_CONTENT_TYPES = { + "image/png": "png", + "image/jpeg": "jpg", + "image/jpg": "jpg", + "image/webp": "webp", + "image/gif": "gif", +} + + +def save_url_image( + url: str, + *, + prefix: str = "image", + timeout: float = 60.0, + max_bytes: int = 25 * 1024 * 1024, +) -> Path: + """Download an image URL and write it under ``$HERMES_HOME/cache/images/``. + + Used by providers (xAI, fallback OpenAI) whose API returns an *ephemeral* + URL instead of inline base64 — those URLs frequently expire before a + downstream consumer (Telegram ``send_photo``, browser fetch) can resolve + them, so we materialise the bytes locally at tool-completion time. + Mirrors :func:`save_b64_image`'s shape so providers can swap in one line. + + Returns the absolute :class:`Path` to the saved file. Raises on any + network / HTTP / oversize / non-image-content-type error so callers can + fall back to returning the bare URL with a clear error message. + """ + import requests + + response = requests.get(url, timeout=timeout, stream=True) + response.raise_for_status() + + # Infer extension from the response content-type, falling back to the + # URL suffix when xAI / OpenAI omit a precise type (some CDNs return + # ``application/octet-stream``). Defaults to ``png``. + content_type = (response.headers.get("Content-Type") or "").split(";", 1)[0].strip().lower() + extension = _URL_IMAGE_CONTENT_TYPES.get(content_type) + if extension is None: + url_path = url.split("?", 1)[0].lower() + for ext in ("png", "jpg", "jpeg", "webp", "gif"): + if url_path.endswith(f".{ext}"): + extension = "jpg" if ext == "jpeg" else ext + break + if extension is None: + extension = "png" + + ts = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") + short = uuid.uuid4().hex[:8] + path = _images_cache_dir() / f"{prefix}_{ts}_{short}.{extension}" + + bytes_written = 0 + with path.open("wb") as fh: + for chunk in response.iter_content(chunk_size=64 * 1024): + if not chunk: + continue + bytes_written += len(chunk) + if bytes_written > max_bytes: + fh.close() + try: + path.unlink() + except OSError: + pass + raise ValueError( + f"Image at {url} exceeds {max_bytes // (1024 * 1024)}MB cap; refusing to cache." + ) + fh.write(chunk) + + if bytes_written == 0: + try: + path.unlink() + except OSError: + pass + raise ValueError(f"Image at {url} returned 0 bytes; refusing to cache.") + + return path + + def success_response( *, image: str, diff --git a/agent/model_metadata.py b/agent/model_metadata.py index b8ec0d6509..e9ec4bf03a 100644 --- a/agent/model_metadata.py +++ b/agent/model_metadata.py @@ -209,6 +209,7 @@ DEFAULT_CONTEXT_LENGTHS = { # via a custom provider. Values sourced from models.dev (2026-04). # Keys use substring matching (longest-first), so e.g. "grok-4.20" # matches "grok-4.20-0309-reasoning" / "-non-reasoning" / "-multi-agent-0309". + "grok-build": 256000, # grok-build-0.1 "grok-code-fast": 256000, # grok-code-fast-1 "grok-4-1-fast": 2000000, # grok-4-1-fast-(non-)reasoning "grok-2-vision": 8192, # grok-2-vision, -1212, -latest @@ -640,7 +641,7 @@ def fetch_model_metadata(force_refresh: bool = False) -> Dict[str, Dict[str, Any return cache except Exception as e: - logging.warning(f"Failed to fetch model metadata from OpenRouter: {e}") + logger.warning(f"Failed to fetch model metadata from OpenRouter: {e}") return _model_metadata_cache or {} diff --git a/agent/models_dev.py b/agent/models_dev.py index 8fabb27664..1249c6f197 100644 --- a/agent/models_dev.py +++ b/agent/models_dev.py @@ -167,6 +167,9 @@ PROVIDER_TO_MODELS_DEV: Dict[str, str] = { "gemini": "google", "google": "google", "xai": "xai", + # xAI OAuth is an authentication/transport path for the same xAI model + # catalog, so model metadata should resolve through the xAI provider. + "xai-oauth": "xai", "xiaomi": "xiaomi", "nvidia": "nvidia", "groq": "groq", diff --git a/agent/redact.py b/agent/redact.py index 1beb10450f..7ed241c5ef 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -176,6 +176,15 @@ _URL_USERINFO_RE = re.compile( r"(https?|wss?|ftp)://([^/\s:@]+):([^/\s@]+)@", ) +# HTTP access logs often use a relative request target rather than a full URL: +# `"POST /webhook?password=... HTTP/1.1"`. The full-URL redactor above only +# sees strings containing `://`, so handle request-target query strings too. +_HTTP_REQUEST_TARGET_QUERY_RE = re.compile( + r"\b((?:GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS|TRACE|CONNECT)\s+[^ \t\r\n\"']*?)" + r"\?([^ \t\r\n\"']+)", + re.IGNORECASE, +) + # Form-urlencoded body detection: conservative — only applies when the entire # text looks like a query string (k=v&k=v pattern with no newlines). _FORM_BODY_RE = re.compile( @@ -293,6 +302,15 @@ def _redact_url_userinfo(text: str) -> str: ) +def _redact_http_request_target_query_params(text: str) -> str: + """Redact sensitive query params in HTTP access-log request targets.""" + def _sub(m: re.Match) -> str: + prefix = m.group(1) + query = _redact_query_string(m.group(2)) + return f"{prefix}?{query}" + return _HTTP_REQUEST_TARGET_QUERY_RE.sub(_sub, text) + + def _redact_form_body(text: str) -> str: """Redact sensitive values in a form-urlencoded body. @@ -397,6 +415,11 @@ def redact_sensitive_text(text: str, *, force: bool = False, code_file: bool = F if "?" in text: text = _redact_url_query_params(text) + # HTTP access logs can contain relative request targets with query params + # and no URL scheme, e.g. `"POST /hook?password=... HTTP/1.1"`. + if "?" in text and "=" in text and _has_http_method_substring(text): + text = _redact_http_request_target_query_params(text) + # Form-urlencoded bodies (only triggers on clean k=v&k=v inputs). if "&" in text and "=" in text: text = _redact_form_body(text) @@ -456,6 +479,25 @@ def _has_known_prefix_substring(text: str) -> bool: return any(p in text for p in _PREFIX_SUBSTRINGS) +_HTTP_METHOD_SUBSTRINGS = ( + "GET ", + "POST ", + "PUT ", + "PATCH ", + "DELETE ", + "HEAD ", + "OPTIONS ", + "TRACE ", + "CONNECT ", +) + + +def _has_http_method_substring(text: str) -> bool: + """Cheap pre-check before scanning for access-log request targets.""" + upper = text.upper() + return any(method in upper for method in _HTTP_METHOD_SUBSTRINGS) + + class RedactingFormatter(logging.Formatter): """Log formatter that redacts secrets from all log messages.""" diff --git a/agent/secret_sources/__init__.py b/agent/secret_sources/__init__.py new file mode 100644 index 0000000000..e1564058ad --- /dev/null +++ b/agent/secret_sources/__init__.py @@ -0,0 +1,13 @@ +"""External secret source integrations. + +A secret source is anything that can supply environment-variable-shaped +credentials at process startup, _after_ ~/.hermes/.env has loaded. By +default sources are non-destructive: they only set values for env vars +that aren't already present, so .env and shell exports continue to win. + +Currently shipped: + + - ``bitwarden`` — Bitwarden Secrets Manager (`bws` CLI). See + ``agent.secret_sources.bitwarden`` for the integration and + ``hermes_cli.secrets_cli`` for the user-facing setup wizard. +""" diff --git a/agent/secret_sources/bitwarden.py b/agent/secret_sources/bitwarden.py new file mode 100644 index 0000000000..8c1e8dc567 --- /dev/null +++ b/agent/secret_sources/bitwarden.py @@ -0,0 +1,535 @@ +"""Bitwarden Secrets Manager (`bws` CLI) integration. + +Hermes pulls API keys from Bitwarden Secrets Manager at process startup +so they don't have to live in plaintext in ``~/.hermes/.env``. + +Design summary +-------------- + +* The ``bws`` binary is auto-installed into ``/bin/bws`` on + first use. Hermes pins one version (``_BWS_VERSION``) and downloads + the matching asset from the official GitHub Releases page, verifying + the SHA-256 against the release's published checksum file. +* The access token is stored in ``~/.hermes/.env`` as + ``BWS_ACCESS_TOKEN`` (or whatever name the user picked in + ``secrets.bitwarden.access_token_env``). This is the one + bootstrap secret — every other provider key can live in Bitwarden. +* Pulling secrets is a single ``bws secret list + --output json`` call. We cache the result in-process for + ``cache_ttl_seconds`` so back-to-back ``hermes`` invocations don't + hammer the API. +* Failures NEVER block Hermes startup. Missing binary, no network, + expired token, etc. all emit a one-line warning and continue with + whatever credentials ``.env`` already had. + +The module is intentionally subprocess-driven rather than going through +the ``bitwarden-sdk-secrets`` Python package: one cross-platform binary +is easier to lazy-install than a wheels-with-Rust-extension dependency. +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import os +import platform +import shutil +import stat +import subprocess +import sys +import tempfile +import time +import urllib.error +import urllib.request +import zipfile +from dataclasses import dataclass, field +from pathlib import Path +from typing import Dict, List, Optional, Tuple + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Configuration constants +# --------------------------------------------------------------------------- + +# Pinned upstream version. Bump in a follow-up PR — never auto-resolve +# "latest" because upstream release shape (asset names, CLI flags) is +# allowed to change between majors and we want updates to be deliberate. +_BWS_VERSION = "2.0.0" + +_BWS_RELEASE_BASE = ( + f"https://github.com/bitwarden/sdk-sm/releases/download/bws-v{_BWS_VERSION}" +) +_BWS_CHECKSUM_NAME = f"bws-sha256-checksums-{_BWS_VERSION}.txt" + +# How long to wait for bws subprocesses and HTTP downloads, in seconds. +_BWS_DOWNLOAD_TIMEOUT = 60 +_BWS_RUN_TIMEOUT = 30 + +# In-process cache so repeated load_hermes_dotenv() calls (CLI startup, +# gateway hot-reload, test suites) don't re-fetch from BSM. +_CacheKey = Tuple[str, str, str] # (access_token_fingerprint, project_id, server_url) +_CACHE: Dict[_CacheKey, "_CachedFetch"] = {} + + +@dataclass +class _CachedFetch: + secrets: Dict[str, str] + fetched_at: float + + def is_fresh(self, ttl_seconds: float) -> bool: + if ttl_seconds <= 0: + return False + return (time.time() - self.fetched_at) < ttl_seconds + + +# --------------------------------------------------------------------------- +# Public dataclasses +# --------------------------------------------------------------------------- + + +@dataclass +class FetchResult: + """Outcome of a single BSM pull.""" + + secrets: Dict[str, str] = field(default_factory=dict) + applied: List[str] = field(default_factory=list) # set into os.environ + skipped: List[str] = field(default_factory=list) # already set, not overridden + warnings: List[str] = field(default_factory=list) # non-fatal issues + error: Optional[str] = None # fatal: nothing was fetched + binary_path: Optional[Path] = None + + @property + def ok(self) -> bool: + return self.error is None + + +# --------------------------------------------------------------------------- +# Binary discovery + lazy install +# --------------------------------------------------------------------------- + + +def _hermes_bin_dir() -> Path: + """Where Hermes stores its managed binaries. Profile-aware.""" + from hermes_constants import get_hermes_home + + return get_hermes_home() / "bin" + + +def find_bws(*, install_if_missing: bool = False) -> Optional[Path]: + """Return a path to a usable ``bws`` binary, or None. + + Resolution order: + 1. ``/bin/bws`` (our managed copy — preferred) + 2. ``shutil.which("bws")`` (system PATH) + + When ``install_if_missing`` is True and neither resolves, this calls + :func:`install_bws` to download and verify the pinned version. + """ + managed = _hermes_bin_dir() / _platform_binary_name() + if managed.exists() and os.access(managed, os.X_OK): + return managed + + system = shutil.which("bws") + if system: + return Path(system) + + if install_if_missing: + try: + return install_bws() + except Exception as exc: # noqa: BLE001 — never block startup + logger.warning("bws auto-install failed: %s", exc) + return None + return None + + +def _platform_binary_name() -> str: + return "bws.exe" if platform.system() == "Windows" else "bws" + + +def _platform_asset_name() -> str: + """Map (uname, arch, libc) → the upstream asset filename. + + Asset names follow Rust's target triple convention. Linux defaults + to gnu (glibc); we switch to musl only if ldd --version says so. + """ + system = platform.system() + machine = platform.machine().lower() + + if system == "Darwin": + # Universal binary works on both Intel and Apple Silicon — no + # need to pick a per-arch asset. + return f"bws-macos-universal-{_BWS_VERSION}.zip" + + if system == "Windows": + arch = "aarch64" if machine in ("arm64", "aarch64") else "x86_64" + return f"bws-{arch}-pc-windows-msvc-{_BWS_VERSION}.zip" + + if system == "Linux": + arch = "aarch64" if machine in ("arm64", "aarch64") else "x86_64" + libc = "gnu" + # ldd --version writes to stderr on glibc, stdout on musl. We + # don't need bullet-proof detection — getting it wrong falls + # back to a clear error from the binary loader, which we catch. + try: + res = subprocess.run( + ["ldd", "--version"], + capture_output=True, + text=True, + timeout=2, + ) + if "musl" in (res.stdout + res.stderr).lower(): + libc = "musl" + except (OSError, subprocess.TimeoutExpired): + pass + return f"bws-{arch}-unknown-linux-{libc}-{_BWS_VERSION}.zip" + + raise RuntimeError( + f"Unsupported platform for bws auto-install: {system} {machine}" + ) + + +def install_bws(*, force: bool = False) -> Path: + """Download, verify, and install the pinned ``bws`` binary. + + Returns the path to the installed executable. Raises on any + failure (network, checksum, extraction) — callers in the auto-install + path catch these; the user-facing ``hermes secrets bitwarden setup`` + surface lets them propagate so the wizard can show a clear error. + """ + bin_dir = _hermes_bin_dir() + bin_dir.mkdir(parents=True, exist_ok=True) + target = bin_dir / _platform_binary_name() + + if target.exists() and not force: + return target + + asset_name = _platform_asset_name() + asset_url = f"{_BWS_RELEASE_BASE}/{asset_name}" + checksum_url = f"{_BWS_RELEASE_BASE}/{_BWS_CHECKSUM_NAME}" + + with tempfile.TemporaryDirectory(prefix="hermes-bws-") as tmpdir: + tmp = Path(tmpdir) + zip_path = tmp / asset_name + checksum_path = tmp / _BWS_CHECKSUM_NAME + + logger.info("Downloading %s", asset_url) + _http_download(asset_url, zip_path) + _http_download(checksum_url, checksum_path) + + expected = _expected_sha256(checksum_path, asset_name) + actual = _sha256_file(zip_path) + if expected.lower() != actual.lower(): + raise RuntimeError( + f"Checksum mismatch for {asset_name}: " + f"expected {expected}, got {actual}" + ) + + with zipfile.ZipFile(zip_path) as zf: + member = _pick_zip_member(zf, _platform_binary_name()) + zf.extract(member, tmp) + extracted = tmp / member + + # Move into place atomically. We write to a sibling tempfile in + # the final directory so the rename can't cross filesystems. + fd, staged = tempfile.mkstemp(dir=str(bin_dir), prefix=".bws_") + os.close(fd) + shutil.copy2(extracted, staged) + os.chmod( + staged, + stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR + | stat.S_IRGRP | stat.S_IXGRP + | stat.S_IROTH | stat.S_IXOTH, + ) + os.replace(staged, target) + + logger.info("Installed bws %s at %s", _BWS_VERSION, target) + return target + + +def _http_download(url: str, dest: Path) -> None: + req = urllib.request.Request(url, headers={"User-Agent": "hermes-agent"}) + try: + with urllib.request.urlopen(req, timeout=_BWS_DOWNLOAD_TIMEOUT) as resp: # noqa: S310 + with open(dest, "wb") as f: + shutil.copyfileobj(resp, f) + except urllib.error.URLError as exc: + raise RuntimeError(f"Failed to download {url}: {exc}") from exc + + +def _expected_sha256(checksum_file: Path, asset_name: str) -> str: + """Parse the upstream ``bws-sha256-checksums-X.Y.Z.txt`` file. + + Format is the standard ``sha256sum`` output: `` ``, + one per line. + """ + text = checksum_file.read_text(encoding="utf-8", errors="replace") + for line in text.splitlines(): + parts = line.strip().split() + if len(parts) >= 2 and parts[-1] == asset_name: + return parts[0] + raise RuntimeError( + f"No checksum entry for {asset_name} in {checksum_file.name}" + ) + + +def _sha256_file(path: Path) -> str: + h = hashlib.sha256() + with open(path, "rb") as f: + for chunk in iter(lambda: f.read(65536), b""): + h.update(chunk) + return h.hexdigest() + + +def _pick_zip_member(zf: zipfile.ZipFile, binary_name: str) -> str: + """Find the binary inside the upstream zip. + + Historically the archive has been flat (``bws`` at the root) but we + tolerate a top-level directory just in case upstream changes. + """ + candidates = [n for n in zf.namelist() if n.split("/")[-1] == binary_name] + if not candidates: + raise RuntimeError( + f"Could not find {binary_name} inside downloaded archive " + f"(members: {zf.namelist()[:5]}...)" + ) + # Prefer the shortest path (i.e. root over nested) for determinism. + candidates.sort(key=len) + return candidates[0] + + +# --------------------------------------------------------------------------- +# Secret fetch + apply +# --------------------------------------------------------------------------- + + +def _token_fingerprint(token: str) -> str: + """SHA-256 prefix used as a cache key — never logged, never displayed.""" + return hashlib.sha256(token.encode("utf-8")).hexdigest()[:16] + + +def fetch_bitwarden_secrets( + *, + access_token: str, + project_id: str, + binary: Optional[Path] = None, + cache_ttl_seconds: float = 300, + use_cache: bool = True, + server_url: str = "", +) -> Tuple[Dict[str, str], List[str]]: + """Pull the secrets for ``project_id`` from Bitwarden Secrets Manager. + + Returns ``(secrets_dict, warnings_list)``. + + Set ``server_url`` to point at a non-default Bitwarden region or a + self-hosted instance — e.g. ``https://vault.bitwarden.eu`` for EU + Cloud accounts. When empty, ``bws`` uses its built-in default + (``https://vault.bitwarden.com``, US Cloud). This is plumbed into + the subprocess as ``BWS_SERVER_URL``. + + Raises :class:`RuntimeError` for fatal conditions (missing binary, + auth failure, unparseable output). Callers in the env_loader path + catch this and emit a single warning; callers in the user-facing + setup wizard let it propagate. + """ + if not access_token: + raise RuntimeError("Bitwarden access token is empty") + if not project_id: + raise RuntimeError("Bitwarden project_id is empty") + + cache_key = (_token_fingerprint(access_token), project_id, server_url or "") + if use_cache: + cached = _CACHE.get(cache_key) + if cached and cached.is_fresh(cache_ttl_seconds): + return cached.secrets, [] + + bws = binary or find_bws(install_if_missing=True) + if bws is None: + raise RuntimeError( + "bws binary not available — auto-install failed and `bws` is " + "not on PATH. Install manually from " + "https://github.com/bitwarden/sdk-sm/releases or re-run " + "`hermes secrets bitwarden setup`." + ) + + secrets, warnings = _run_bws_list(bws, access_token, project_id, server_url) + _CACHE[cache_key] = _CachedFetch(secrets=secrets, fetched_at=time.time()) + return secrets, warnings + + +def _run_bws_list( + bws: Path, access_token: str, project_id: str, server_url: str = "" +) -> Tuple[Dict[str, str], List[str]]: + cmd = [str(bws), "secret", "list", project_id, "--output", "json"] + env = os.environ.copy() + env["BWS_ACCESS_TOKEN"] = access_token + # Make sure we're not echoing telemetry / colour codes into json. + env.setdefault("NO_COLOR", "1") + # Region / self-hosted support. bws defaults to https://vault.bitwarden.com + # (US Cloud); EU Cloud users need https://vault.bitwarden.eu, and + # self-hosted users need their own URL. When unset, fall back to whatever + # BWS_SERVER_URL the caller already had in their shell env (preserved by + # the copy above) so manual overrides keep working too. + if server_url: + env["BWS_SERVER_URL"] = server_url + + try: + proc = subprocess.run( # noqa: S603 — bws path is trusted + cmd, + env=env, + capture_output=True, + text=True, + timeout=_BWS_RUN_TIMEOUT, + ) + except subprocess.TimeoutExpired as exc: + raise RuntimeError( + f"bws timed out after {_BWS_RUN_TIMEOUT}s fetching secrets" + ) from exc + except OSError as exc: + raise RuntimeError(f"failed to invoke bws: {exc}") from exc + + if proc.returncode != 0: + # bws writes auth/network errors to stderr in plain English. + # Strip ANSI just in case and surface the first 200 chars. + err = (proc.stderr or proc.stdout or "").strip().replace("\x1b", "") + raise RuntimeError( + f"bws exited {proc.returncode}: {err[:200]}" + ) + + raw = proc.stdout.strip() + if not raw: + return {}, ["bws returned no output (empty project?)"] + + try: + payload = json.loads(raw) + except json.JSONDecodeError as exc: + raise RuntimeError(f"bws returned non-JSON output: {exc}") from exc + + if not isinstance(payload, list): + raise RuntimeError( + f"bws returned unexpected shape: {type(payload).__name__}" + ) + + secrets: Dict[str, str] = {} + warnings: List[str] = [] + for item in payload: + if not isinstance(item, dict): + continue + key = item.get("key") + value = item.get("value") + if not isinstance(key, str) or not isinstance(value, str): + continue + if not _is_valid_env_name(key): + warnings.append( + f"Skipping secret {key!r}: not a valid env-var name" + ) + continue + secrets[key] = value + return secrets, warnings + + +def _is_valid_env_name(name: str) -> bool: + if not name: + return False + if not (name[0].isalpha() or name[0] == "_"): + return False + return all(c.isalnum() or c == "_" for c in name) + + +# --------------------------------------------------------------------------- +# Public entry point — called from hermes_cli.env_loader +# --------------------------------------------------------------------------- + + +def apply_bitwarden_secrets( + *, + enabled: bool, + access_token_env: str = "BWS_ACCESS_TOKEN", + project_id: str = "", + override_existing: bool = False, + cache_ttl_seconds: float = 300, + auto_install: bool = True, + server_url: str = "", +) -> FetchResult: + """Pull secrets from BSM and set them on ``os.environ``. + + This is the function ``load_hermes_dotenv()`` calls after the .env + files have loaded. It is intentionally defensive — any failure + returns a :class:`FetchResult` with ``error`` set; it never raises. + + ``server_url`` selects the Bitwarden region or self-hosted endpoint + (e.g. ``https://vault.bitwarden.eu`` for EU Cloud). Empty string + means use ``bws``'s default (US Cloud). + + Parameters mirror the ``secrets.bitwarden.*`` config keys so the + caller can just splat the dict in. + """ + result = FetchResult() + + if not enabled: + return result + + access_token = os.environ.get(access_token_env, "").strip() + if not access_token: + result.error = ( + f"secrets.bitwarden.enabled is true but {access_token_env} is " + "not set. Run `hermes secrets bitwarden setup`." + ) + return result + + if not project_id: + result.error = ( + "secrets.bitwarden.project_id is empty. " + "Run `hermes secrets bitwarden setup`." + ) + return result + + binary = find_bws(install_if_missing=auto_install) + result.binary_path = binary + if binary is None: + result.error = ( + "bws binary not available and auto-install is disabled. " + "Run `hermes secrets bitwarden setup` to install." + ) + return result + + try: + secrets, warnings = fetch_bitwarden_secrets( + access_token=access_token, + project_id=project_id, + binary=binary, + cache_ttl_seconds=cache_ttl_seconds, + server_url=server_url, + ) + except RuntimeError as exc: + result.error = str(exc) + return result + + result.secrets = secrets + result.warnings.extend(warnings) + + for key, value in secrets.items(): + if key == access_token_env: + # Don't let BSM clobber the very token we used to fetch + # itself — that would be a footgun if someone stored the + # token as a BSM secret too. + result.skipped.append(key) + continue + if not override_existing and os.environ.get(key): + result.skipped.append(key) + continue + os.environ[key] = value + result.applied.append(key) + + return result + + +# --------------------------------------------------------------------------- +# Test hook — used by hermetic tests to flush the cache between cases. +# --------------------------------------------------------------------------- + + +def _reset_cache_for_tests() -> None: + _CACHE.clear() diff --git a/agent/skill_utils.py b/agent/skill_utils.py index 28424d7ed6..5b8e4c22a6 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -12,7 +12,7 @@ import sys from pathlib import Path from typing import Any, Dict, List, Optional, Set, Tuple -from hermes_constants import get_config_path, get_skills_dir +from hermes_constants import get_config_path, get_skills_dir, is_termux logger = logging.getLogger(__name__) @@ -24,7 +24,43 @@ PLATFORM_MAP = { "windows": "win32", } -EXCLUDED_SKILL_DIRS = frozenset((".git", ".github", ".hub", ".archive")) +EXCLUDED_SKILL_DIRS = frozenset( + ( + ".git", + ".github", + ".hub", + ".archive", + ".venv", + "venv", + "node_modules", + "site-packages", + "__pycache__", + ".tox", + ".nox", + ".pytest_cache", + ".mypy_cache", + ".ruff_cache", + ) +) + + +def is_excluded_skill_path(path) -> bool: + """True if any component of *path* is in EXCLUDED_SKILL_DIRS. + + Use this on every SKILL.md path produced by ``rglob`` to prune + dependency, virtualenv, VCS, and cache directories. Centralising the + check here keeps every skill-scanning site in sync with the shared + exclusion set. + + Accepts a Path or string. + """ + try: + parts = path.parts # Path + except AttributeError: + from pathlib import PurePath + parts = PurePath(str(path)).parts + return any(part in EXCLUDED_SKILL_DIRS for part in parts) + # ── Lazy YAML loader ───────────────────────────────────────────────────── @@ -100,6 +136,14 @@ def skill_matches_platform(frontmatter: Dict[str, Any]) -> bool: If the field is absent or empty the skill is compatible with **all** platforms (backward-compatible default). + + Termux note: on Termux/Android, ``sys.platform`` is ``"linux"`` on + older Pythons but became ``"android"`` on Python 3.13+. Termux is a + Linux userland riding on the Android kernel, so skills tagged + ``linux`` are treated as compatible in Termux regardless of which + ``sys.platform`` value Python reports. Individual Linux commands + inside a skill may still misbehave (no systemd, BusyBox utils, no + apt/dnf, etc.) but that is on the skill, not on platform gating. """ platforms = frontmatter.get("platforms") if not platforms: @@ -107,11 +151,21 @@ def skill_matches_platform(frontmatter: Dict[str, Any]) -> bool: if not isinstance(platforms, list): platforms = [platforms] current = sys.platform + running_in_termux = is_termux() for platform in platforms: normalized = str(platform).lower().strip() mapped = PLATFORM_MAP.get(normalized, normalized) if current.startswith(mapped): return True + # Termux runs a Linux userland on Android. Accept linux-tagged + # skills regardless of whether sys.platform is "linux" (pre-3.13 + # Termux) or "android" (Python 3.13+ Termux, and any other + # Android runtime). + if running_in_termux and mapped == "linux": + return True + # Explicit termux/android tags match a Termux session too. + if running_in_termux and mapped in ("termux", "android"): + return True return False @@ -478,7 +532,8 @@ def extract_skill_description(frontmatter: Dict[str, Any]) -> str: def iter_skill_index_files(skills_dir: Path, filename: str): """Walk skills_dir yielding sorted paths matching *filename*. - Excludes ``.git``, ``.github``, ``.hub``, ``.archive`` directories. + Excludes Hermes metadata, VCS, virtualenv/dependency, and cache + directories so dependencies cannot register nested skills. """ matches = [] for root, dirs, files in os.walk(skills_dir, followlinks=True): diff --git a/agent/system_prompt.py b/agent/system_prompt.py index bc29c9ef89..8fa4c19156 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -205,6 +205,40 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) if _env_hints: stable_parts.append(_env_hints) + # Active-profile hint — names the Hermes profile the agent is running + # under so it doesn't conflate ~/.hermes/skills/ (default profile) with + # ~/.hermes/profiles//skills/ (this profile's). Deterministic + # for the lifetime of the agent — profile name doesn't change + # mid-session, so this doesn't break the prompt cache. + # See file_safety._resolve_active_profile_name + classify_cross_profile_target + # for the matching tool-side guard. + try: + from agent.file_safety import _resolve_active_profile_name + active_profile = _resolve_active_profile_name() + except Exception: + active_profile = "default" + if active_profile == "default": + stable_parts.append( + "Active Hermes profile: default. Other profiles (if any) live " + "under ~/.hermes/profiles//. Each profile has its own " + "skills/, plugins/, cron/, and memories/ that affect a different " + "session than this one. Do not modify another profile's " + "skills/plugins/cron/memories unless the user explicitly directs " + "you to." + ) + else: + stable_parts.append( + f"Active Hermes profile: {active_profile}. This session reads " + f"and writes ~/.hermes/profiles/{active_profile}/. The default " + f"profile's data lives at ~/.hermes/skills/, ~/.hermes/plugins/, " + f"~/.hermes/cron/, ~/.hermes/memories/ — those belong to a " + f"different session run from a different shell. Do NOT modify " + f"another profile's skills/plugins/cron/memories unless the user " + f"explicitly directs you to. The cross-profile write guard will " + f"refuse such writes by default; pass cross_profile=True only " + f"after explicit direction." + ) + platform_key = (agent.platform or "").lower().strip() if platform_key in PLATFORM_HINTS: stable_parts.append(PLATFORM_HINTS[platform_key]) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index b161b507e8..438a633707 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -388,6 +388,7 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe agent.tool_progress_callback( "tool.completed", function_name, None, None, duration=tool_duration, is_error=is_error, + result=function_result, ) except Exception as cb_err: logging.debug(f"Tool progress callback error: {cb_err}") @@ -491,7 +492,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe try: function_args = json.loads(tool_call.function.arguments) except json.JSONDecodeError as e: - logging.warning(f"Unexpected JSON error after validation: {e}") + logger.warning(f"Unexpected JSON error after validation: {e}") function_args = {} if not isinstance(function_args, dict): function_args = {} @@ -822,6 +823,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe agent.tool_progress_callback( "tool.completed", function_name, None, None, duration=tool_duration, is_error=_is_error_result, + result=function_result, ) except Exception as cb_err: logging.debug(f"Tool progress callback error: {cb_err}") diff --git a/agent/transports/anthropic.py b/agent/transports/anthropic.py index 72024ac20f..d77ae63ef3 100644 --- a/agent/transports/anthropic.py +++ b/agent/transports/anthropic.py @@ -106,7 +106,17 @@ class AnthropicTransport(ProviderTransport): elif block.type == "tool_use": name = block.name if strip_tool_prefix and name.startswith(_MCP_PREFIX): - name = name[len(_MCP_PREFIX):] + stripped = name[len(_MCP_PREFIX):] + # Only strip the mcp_ prefix for OAuth-injected tools + # (where Hermes adds the prefix when sending to Anthropic + # and must remove it on the way back). Native MCP server + # tools (from mcp_servers: in config.yaml) are registered + # in the tool registry under their FULL mcp__ + # name and must NOT be stripped. GH-25255. + from tools.registry import registry as _tool_registry + if (_tool_registry.get_entry(stripped) + and not _tool_registry.get_entry(name)): + name = stripped tool_calls.append( ToolCall( id=block.id, diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index fa36301bd8..96997afca4 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -113,9 +113,8 @@ class ChatCompletionsTransport(ProviderTransport): self, messages: list[dict[str, Any]], **kwargs ) -> list[dict[str, Any]]: """Messages are already in OpenAI format — strip internal fields - that strict chat-completions providers reject with HTTP 400/422. - - Strips: + that strict chat-completions providers reject with HTTP 400/422 + (or, in the case of some OpenAI-compatible gateways, 5xx): - Codex Responses API fields: ``codex_reasoning_items`` / ``codex_message_items`` on the message, ``call_id`` / @@ -127,6 +126,16 @@ class ChatCompletionsTransport(ProviderTransport): ``Extra inputs are not permitted, field: 'messages[N].tool_name'``. Permissive providers (OpenRouter, MiniMax) silently ignore the field, which masked the bug for months. + - Hermes-internal scaffolding markers — any top-level message key + starting with ``_`` (e.g. ``_empty_recovery_synthetic``, + ``_empty_terminal_sentinel``, ``_thinking_prefill``). These are + bookkeeping flags the agent loop attaches to messages so the + persistence layer can later strip its own scaffolding; they must + never reach the wire. Permissive providers (real OpenAI, + Anthropic) silently drop unknown message keys, but strict + gateways (e.g. opencode-go, codex.nekos.me) reject with + ``Extra inputs are not permitted, field: 'messages[N]._empty_recovery_synthetic'``, + which then poisons every subsequent request in the session. """ needs_sanitize = False for msg in messages: @@ -139,6 +148,9 @@ class ChatCompletionsTransport(ProviderTransport): ): needs_sanitize = True break + if any(isinstance(k, str) and k.startswith("_") for k in msg): + needs_sanitize = True + break tool_calls = msg.get("tool_calls") if isinstance(tool_calls, list): for tc in tool_calls: @@ -160,6 +172,11 @@ class ChatCompletionsTransport(ProviderTransport): msg.pop("codex_reasoning_items", None) msg.pop("codex_message_items", None) msg.pop("tool_name", None) + # Drop all Hermes-internal scaffolding markers (``_``-prefixed). + # OpenAI's message schema has no ``_``-prefixed fields, so this + # is safe and future-proofs against new markers being added. + for key in [k for k in msg if isinstance(k, str) and k.startswith("_")]: + msg.pop(key, None) tool_calls = msg.get("tool_calls") if isinstance(tool_calls, list): for tc in tool_calls: diff --git a/agent/transports/codex_app_server_session.py b/agent/transports/codex_app_server_session.py index d9ee92dfbf..74e164d64d 100644 --- a/agent/transports/codex_app_server_session.py +++ b/agent/transports/codex_app_server_session.py @@ -87,6 +87,39 @@ class TurnResult: _TURN_ABORTED_MARKERS = ("", "") +def _coerce_turn_input_text(user_input: Any) -> str: + """Collapse Hermes/OpenAI rich content into app-server text input. + + The current `turn/start` path sends text items only. TUI image attachment + can hand us OpenAI-style content parts, so keep the text/path hints and + replace opaque image payloads with a small marker instead of putting a + Python list into the `text` field. + """ + if isinstance(user_input, str): + return user_input + if isinstance(user_input, list): + parts: list[str] = [] + for item in user_input: + if isinstance(item, str): + if item.strip(): + parts.append(item) + continue + if not isinstance(item, dict): + if item is not None: + parts.append(str(item)) + continue + item_type = item.get("type") + if item_type in {"text", "input_text"}: + text = item.get("text") or item.get("content") or "" + if text: + parts.append(str(text)) + elif item_type in {"image", "image_url", "input_image"}: + parts.append("[image attached]") + text = "\n\n".join(p for p in parts if p).strip() + return text or "What do you see in this image?" + return "" if user_input is None else str(user_input) + + # Substrings in codex stderr / JSON-RPC error messages that signal the # subprocess died because its OAuth credentials are no longer valid. # Kept conservative: we only redirect users to `codex login` when we're @@ -327,7 +360,7 @@ class CodexAppServerSession: def run_turn( self, - user_input: str, + user_input: Any, *, turn_timeout: float = 600.0, notification_poll_timeout: float = 0.25, @@ -365,6 +398,8 @@ class CodexAppServerSession: self._interrupt_event.clear() projector = CodexEventProjector() + user_input_text = _coerce_turn_input_text(user_input) + # Send turn/start with the user input. Text-only for now (codex # supports rich content but Hermes' text path is the common case). try: @@ -372,7 +407,7 @@ class CodexAppServerSession: "turn/start", { "threadId": self._thread_id, - "input": [{"type": "text", "text": user_input}], + "input": [{"type": "text", "text": user_input_text}], }, timeout=10, ) diff --git a/agent/tts_provider.py b/agent/tts_provider.py new file mode 100644 index 0000000000..c19166a702 --- /dev/null +++ b/agent/tts_provider.py @@ -0,0 +1,274 @@ +""" +Text-to-Speech Provider ABC +============================ + +Defines the pluggable-backend interface for text-to-speech synthesis. +Providers register instances via +``PluginContext.register_tts_provider()``; the active one (selected via +``tts.provider`` in ``config.yaml``) services every ``text_to_speech`` +tool call **only when the configured name is neither a built-in nor a +command-type provider declared under ``tts.providers.``**. + +Three coexisting TTS extension surfaces — in resolution order: + +1. **Built-in providers** (``BUILTIN_TTS_PROVIDERS`` in + :mod:`tools.tts_tool`) — native Python implementations (edge, openai, + elevenlabs, …). **Always win** — plugins cannot shadow them. +2. **Command-type providers** declared under ``tts.providers.: + type: command`` (PR #17843, commit ``2facea7f7``). Wire any local + CLI into Hermes with shell-template placeholders. **Wins over a + same-name plugin** — config is more local than plugin install. +3. **Plugin-registered providers** (this ABC). For backends that need a + Python SDK, streaming bytes, OAuth refresh, or voice-listing APIs + the shell-template grammar can't reasonably express. + +Built-ins-always-win is enforced at registration time +(:func:`agent.tts_registry.register_provider` rejects names in +``BUILTIN_TTS_PROVIDERS`` with a warning) AND at dispatch time +(:func:`tools.tts_tool._dispatch_to_plugin_provider` re-checks +defensively). The dispatcher also rejects plugin dispatch when a same- +name command provider is configured. + +Providers live in ``/plugins/tts//`` (built-in plugins, no +shipped today) or ``~/.hermes/plugins/tts//`` (user-installed). +None ship in-tree as of issue #30398 — the hook is additive +infrastructure waiting for a real consumer (Cartesia, Fish Audio, …). + +Response contract +----------------- +:meth:`TTSProvider.synthesize` writes the audio bytes to ``output_path`` +and returns the path as a string. Implementations should raise on +failure — the dispatcher converts exceptions into the standard +``{success: False, error: …}`` JSON envelope the rest of Hermes +expects. +""" + +from __future__ import annotations + +import abc +import logging +from typing import Any, Dict, Iterator, List, Optional + +logger = logging.getLogger(__name__) + + +DEFAULT_OUTPUT_FORMAT = "mp3" +VALID_OUTPUT_FORMATS = frozenset({"mp3", "wav", "ogg", "opus", "flac"}) + + +# --------------------------------------------------------------------------- +# ABC +# --------------------------------------------------------------------------- + + +class TTSProvider(abc.ABC): + """Abstract base class for a text-to-speech backend. + + Subclasses must implement :attr:`name` and :meth:`synthesize`. + Everything else has sane defaults — override only what your provider + needs. + """ + + @property + @abc.abstractmethod + def name(self) -> str: + """Stable short identifier used in ``tts.provider`` config. + + Lowercase, no spaces. Examples: ``cartesia``, ``fishaudio``, + ``deepgram``. Names that collide with a built-in TTS provider + (``edge``, ``openai``, ``elevenlabs``, ``minimax``, ``gemini``, + ``mistral``, ``xai``, ``piper``, ``kittentts``, ``neutts``) are + rejected at registration time. + """ + + @property + def display_name(self) -> str: + """Human-readable label shown in ``hermes tools``. + + Defaults to ``name.title()`` (e.g. ``Cartesia`` for ``cartesia``). + """ + return self.name.title() + + def is_available(self) -> bool: + """Return True when this provider can service calls. + + Typically checks for a required API key + that the SDK is + importable. Default: True (providers with no external + dependencies are always available). + + Must NOT raise — used by the picker and ``hermes setup`` for + availability displays and should fail gracefully. + """ + return True + + def list_voices(self) -> List[Dict[str, Any]]: + """Return voice catalog entries. + + Each entry:: + + { + "id": "voice-abc-123", # required + "display": "Aria — neutral female", # optional; defaults to id + "language": "en-US", # optional + "gender": "female", # optional + "preview_url": "https://...mp3", # optional + } + + Default: empty list (provider has no enumerable voices or + doesn't surface them via API). + """ + return [] + + def list_models(self) -> List[Dict[str, Any]]: + """Return model catalog entries. + + Each entry:: + + { + "id": "sonic-2", # required + "display": "Sonic 2", # optional + "languages": ["en", "es", "fr"], # optional + "max_text_length": 5000, # optional + } + + Default: empty list (provider has a single fixed model or + doesn't expose model selection). + """ + return [] + + def get_setup_schema(self) -> Dict[str, Any]: + """Return provider metadata for the ``hermes tools`` picker. + + Used by ``tools_config.py`` to inject this provider as a row in + the Text-to-Speech provider list. Shape:: + + { + "name": "Cartesia", # picker label + "badge": "paid", # optional short tag + "tag": "Ultra-low-latency streaming", # optional subtitle + "env_vars": [ # keys to prompt for + {"key": "CARTESIA_API_KEY", + "prompt": "Cartesia API key", + "url": "https://play.cartesia.ai/console"}, + ], + } + + Default: minimal entry derived from ``display_name`` with no + env vars. Override to expose API key prompts and custom badges. + """ + return { + "name": self.display_name, + "badge": "", + "tag": "", + "env_vars": [], + } + + def default_model(self) -> Optional[str]: + """Return the default model id, or None if not applicable.""" + models = self.list_models() + if models: + return models[0].get("id") + return None + + def default_voice(self) -> Optional[str]: + """Return the default voice id, or None if not applicable.""" + voices = self.list_voices() + if voices: + return voices[0].get("id") + return None + + @abc.abstractmethod + def synthesize( + self, + text: str, + output_path: str, + *, + voice: Optional[str] = None, + model: Optional[str] = None, + speed: Optional[float] = None, + format: str = DEFAULT_OUTPUT_FORMAT, + **extra: Any, + ) -> str: + """Synthesize ``text`` and write audio bytes to ``output_path``. + + Returns the absolute path to the written file as a string + (typically just echoes ``output_path``). Raises on failure — + the dispatcher converts exceptions to the standard + ``{success: False, error: ...}`` JSON envelope. + + Args: + text: The text to synthesize. Already truncated to the + provider's max length by the dispatcher. + output_path: Absolute path where the audio file should be + written. Parent directory is guaranteed to exist. + voice: Voice identifier from :meth:`list_voices`, or None + to use :meth:`default_voice`. + model: Model identifier from :meth:`list_models`, or None + to use :meth:`default_model`. + speed: Optional speech-rate multiplier (1.0 = normal). + Providers that don't support speed control should + ignore this argument. + format: Output audio format. Implementations should match + the requested format when possible; if unsupported, + pick the closest equivalent and ensure ``output_path`` + ends with the correct extension. + **extra: Forward-compat parameters future schema versions + may expose. Implementations should ignore unknown keys. + """ + + def stream( + self, + text: str, + *, + voice: Optional[str] = None, + model: Optional[str] = None, + format: str = "opus", + **extra: Any, + ) -> Iterator[bytes]: + """Stream synthesized audio bytes. + + Optional. Providers that don't support streaming raise + :class:`NotImplementedError` (the default) and the dispatcher + falls back to :meth:`synthesize` + read-whole-file. + + Args mirror :meth:`synthesize`. Default ``format`` is ``opus`` + because the primary streaming use case is voice-bubble + delivery (Telegram et al.) which requires Opus. + """ + raise NotImplementedError( + f"TTS provider {self.name!r} does not implement streaming " + "synthesis. Use synthesize() instead, or implement stream() " + "if your backend supports it." + ) + + @property + def voice_compatible(self) -> bool: + """Whether output is suitable for voice-bubble delivery. + + Mirrors the ``tts.providers..voice_compatible`` field + from PR #17843. When True, the gateway's voice-message + delivery pipeline runs ffmpeg conversion to Opus if needed. + When False, output is delivered as a regular audio attachment. + + Default: False (safe — providers opt in explicitly). + """ + return False + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def resolve_output_format(value: Optional[str]) -> str: + """Clamp an output_format value to the valid set. + + Invalid values are coerced to :data:`DEFAULT_OUTPUT_FORMAT` rather + than rejected so the tool surface is forgiving of agent mistakes. + """ + if not isinstance(value, str): + return DEFAULT_OUTPUT_FORMAT + v = value.strip().lower() + if v in VALID_OUTPUT_FORMATS: + return v + return DEFAULT_OUTPUT_FORMAT diff --git a/agent/tts_registry.py b/agent/tts_registry.py new file mode 100644 index 0000000000..7cf6e6cb00 --- /dev/null +++ b/agent/tts_registry.py @@ -0,0 +1,133 @@ +""" +TTS Provider Registry +===================== + +Central map of registered TTS providers. Populated by plugins at +import-time via :meth:`PluginContext.register_tts_provider`; consumed +by :mod:`tools.tts_tool` to dispatch ``text_to_speech`` tool calls to +the active plugin backend **when** the configured ``tts.provider`` +name is neither a built-in nor a command-type provider. + +Built-ins-always-win +-------------------- +Plugin names that collide with a built-in TTS provider (``edge``, +``openai``, ``elevenlabs``, ``minimax``, ``gemini``, ``mistral``, +``xai``, ``piper``, ``kittentts``, ``neutts``) are rejected at +registration with a warning. This invariant is also re-checked at +dispatch time in :func:`tools.tts_tool._dispatch_to_plugin_provider`. + +Command-providers-win-over-plugins +---------------------------------- +This registry doesn't enforce the command-vs-plugin precedence — that +lives in the dispatcher, which checks for a same-name +``tts.providers.: type: command`` entry before consulting the +registry. The rationale is locality: a name declared in the user's +``config.yaml`` is more specific to their setup than a plugin that +happens to be installed. +""" + +from __future__ import annotations + +import logging +import threading +from typing import Dict, List, Optional + +from agent.tts_provider import TTSProvider + +logger = logging.getLogger(__name__) + + +# Names reserved for native built-in TTS handlers. Plugins cannot +# register a name in this set — the registration call is rejected with +# a warning. **Kept in sync with ``BUILTIN_TTS_PROVIDERS`` in +# :mod:`tools.tts_tool`** — a regression test in +# ``tests/agent/test_tts_registry.py::TestBuiltinSync`` fails if the +# two lists drift. Importing from ``tools.tts_tool`` directly would +# create a circular dependency (``tools.tts_tool`` imports +# ``agent.tts_registry`` for dispatch). +_BUILTIN_NAMES = frozenset({ + "edge", + "elevenlabs", + "openai", + "minimax", + "xai", + "mistral", + "gemini", + "neutts", + "kittentts", + "piper", +}) + + +_providers: Dict[str, TTSProvider] = {} +_lock = threading.Lock() + + +def register_provider(provider: TTSProvider) -> None: + """Register a TTS provider. + + Rejects: + + - Non-:class:`TTSProvider` instances (raises :class:`TypeError`). + - Empty/whitespace ``.name`` (raises :class:`ValueError`). + - Names colliding with a built-in (logs a warning, silently + ignores — built-ins-always-win invariant). + + Re-registration (same ``name``) overwrites the previous entry and + logs a debug message — makes hot-reload scenarios (tests, dev + loops) behave predictably. + """ + if not isinstance(provider, TTSProvider): + raise TypeError( + f"register_provider() expects a TTSProvider instance, " + f"got {type(provider).__name__}" + ) + name = provider.name + if not isinstance(name, str) or not name.strip(): + raise ValueError("TTS provider .name must be a non-empty string") + key = name.strip().lower() + if key in _BUILTIN_NAMES: + logger.warning( + "TTS provider '%s' shadows a built-in name; registration ignored. " + "Built-in TTS providers (%s) always win — pick a different name.", + key, ", ".join(sorted(_BUILTIN_NAMES)), + ) + return + with _lock: + existing = _providers.get(key) + _providers[key] = provider + if existing is not None: + logger.debug( + "TTS provider '%s' re-registered (was %r)", + key, type(existing).__name__, + ) + else: + logger.debug( + "Registered TTS provider '%s' (%s)", + key, type(provider).__name__, + ) + + +def list_providers() -> List[TTSProvider]: + """Return all registered providers, sorted by name.""" + with _lock: + items = list(_providers.values()) + return sorted(items, key=lambda p: p.name) + + +def get_provider(name: str) -> Optional[TTSProvider]: + """Return the provider registered under *name*, or None. + + Name matching is case-insensitive and whitespace-tolerant — mirrors + how ``tools.tts_tool._get_provider`` normalizes the configured + ``tts.provider`` value. + """ + if not isinstance(name, str): + return None + return _providers.get(name.strip().lower()) + + +def _reset_for_tests() -> None: + """Clear the registry. **Test-only.**""" + with _lock: + _providers.clear() diff --git a/apps/dashboard/README.md b/apps/dashboard/README.md index 041427feed..bf06ea677d 100644 --- a/apps/dashboard/README.md +++ b/apps/dashboard/README.md @@ -63,3 +63,53 @@ src/ ├── main.tsx # React entry point └── index.css # Tailwind imports and theme variables ``` + +## Typography & contrast rules + +Read before adding or editing UI styles. These rules keep the dashboard legible across all built-in themes and stop drift back into the patterns the design system was just refactored out of. + +### Text size floor + +- **Minimum body size: `text-xs` (12px / 0.75rem).** Do not use arbitrary `text-[0.6rem]`, `text-[0.65rem]`, `text-[9px]`, `text-[10px]`, or `text-[11px]` on copy, hints, labels, counts, or badges. Use the standard scale: `text-xs`, `text-sm`, `text-base`. +- Smaller sizes are only acceptable on **decorative overlays** (chart stripes, empty-state icons) — never on text the user is meant to read. + +### Opacity floor on text + +- **Never apply opacity below 0.7 to text.** No `opacity-30`, `opacity-50`, `opacity-60` on ``s, `

`s, labels, etc. +- **Do not stack opacity tokens.** Patterns like `text-muted-foreground/60`, `text-midground/70`, `text-foreground/50` create unpredictable WCAG failures because the parent token already has alpha. +- Use the **semantic text tokens** from `@nous-research/ui`'s `globals.css`: + - `text-text-primary` — default body text. + - `text-text-secondary` — subtitles, meta, inactive nav. + - `text-text-tertiary` — small chrome labels, counts, footnotes. + - `text-text-disabled` — disabled states. + - `text-text-on-accent` — text on filled accent surfaces. + +### Brand uppercase via `text-display`, not raw `uppercase` + +- The dashboard preserves the Nous brand uppercase aesthetic, but it is **opt-in per element, not global**. +- Apply uppercase via the DS utility `text-display` on **brand chrome only** — page titles, nav section headings, badges, brand wordmark. DS components (`Button`, `Badge`, `Tabs`, `Segmented`, etc.) already self-apply `text-display`. +- **Do not introduce new `uppercase`** (the literal Tailwind class) in `hermes-agent/web/src`. Prefer `text-display` for new brand chrome. Legacy `uppercase` call sites (e.g. `components/ui/label.tsx`, `card.tsx`) remain until migrated. +- The app shell no longer forces uppercase globally, so blanket `normal-case` opt-outs are unnecessary. Use `normal-case` only where a DS component applies `text-display` but the label should stay sentence case — e.g. dynamic user content (model slugs, theme names) **or** fixed UI copy that is not brand chrome (EnvPage “not configured” toggle, sidebar “New chat”). + +### Fonts + +Typography is **opt-in per surface**, not global on layout shells — the app shell and page header keep their original theme/expanded fonts; Mondwest applies only where explicitly set. + +| Tier | Classes | Use for | +|------|---------|---------| +| Brand chrome | `font-mondwest text-display` (or `themedChrome`) | Sidebar nav, card section headers (`CardTitle`), Segmented filter buttons, filter panel headings | +| Themed body | `font-mondwest normal-case` (or `themedBody`) | Card content (`Card`, `CardDescription`), session/platform rows, analytics tables — **scoped to the component** | +| Page chrome | `font-expanded` | Page header h1 (`PageHeaderProvider`) — sentence case, not `text-display` | +| Wordmark | `Typography` + size/tracking only | Sidebar/mobile “Hermes Agent” — mixed case, no Mondwest, no `text-display` | +| Technical | `font-mono-ui` / `font-mono` / `font-courier` | Model slugs, env keys, schedules, YAML, repo URLs | + +- Do **not** put `themedBody` or `themedFont` on `

`, `App`, or other layout wrappers — it overrides component-scoped styles. +- **`Card`** applies `themedBody`; **`CardTitle`** uses `text-display` (uppercase chrome); **`CardDescription`** uses `themedBody`. +- **`NouiTypography`** defaults to `font-sans` unless a font prop is passed. +- Do **not** use raw `font-sans` or `font-display` (theme sans variable) on new dashboard UI — prefer Mondwest tiers above where brand-appropriate. + +### Color tokens + +- Prefer **semantic tokens** (`text-text-*`, `bg-card`, `border-border`, `text-foreground`, `text-destructive`, `text-success`, `text-warning`) over raw layer references (`text-midground`, `text-foreground`). +- `text-muted-foreground` is now wired to `--color-text-secondary`, so existing call sites stay correct, but new code should prefer the semantic name. +- When you genuinely need a non-token color (icon de-emphasis on a chart, terminal foreground via inline style), keep alpha at `≥ 0.7` for any text. diff --git a/apps/dashboard/package-lock.json b/apps/dashboard/package-lock.json index 6780172578..471df8bdbd 100644 --- a/apps/dashboard/package-lock.json +++ b/apps/dashboard/package-lock.json @@ -9,7 +9,7 @@ "version": "0.0.0", "dependencies": { "@hermes/shared": "file:../shared", - "@nous-research/ui": "^0.14.2", + "@nous-research/ui": "0.16.0", "@observablehq/plot": "^0.6.17", "@react-three/fiber": "^9.6.0", "@tailwindcss/vite": "^4.2.1", @@ -20,7 +20,6 @@ "@xterm/xterm": "^6.0.0", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", - "flag-icons": "^7.5.0", "gsap": "^3.15.0", "leva": "^0.10.1", "lucide-react": "^0.577.0", @@ -1092,9 +1091,9 @@ } }, "node_modules/@nous-research/ui": { - "version": "0.14.2", - "resolved": "https://registry.npmjs.org/@nous-research/ui/-/ui-0.14.2.tgz", - "integrity": "sha512-H3cMt2e0IpmcTNOmR6zVX+8ja48w4X4F/IFXhWCpaoVs8zKVRN12Ryb4RnX/ac8IrbUu6UsIds7ZtmXxPHcfdQ==", + "version": "0.16.0", + "resolved": "https://registry.npmjs.org/@nous-research/ui/-/ui-0.16.0.tgz", + "integrity": "sha512-JvSwf9vBOCEEGDSOYIRn/F/JJSBDh9DvGU3s3OFbX6K1otnSK7s47cZdgvfBoEPmeKFom2fWQDDqfzLV+eR7Qg==", "license": "MIT", "dependencies": { "@nanostores/react": "^1.1.0", @@ -4158,12 +4157,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/flag-icons": { - "version": "7.5.0", - "resolved": "https://registry.npmjs.org/flag-icons/-/flag-icons-7.5.0.tgz", - "integrity": "sha512-kd+MNXviFIg5hijH766tt+3x76ele1AXlo4zDdCxIvqWZhKt4T83bOtxUOOMlTx/EcFdUMH5yvQgYlFh1EqqFg==", - "license": "MIT" - }, "node_modules/flat-cache": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/flat-cache/-/flat-cache-4.0.1.tgz", diff --git a/apps/dashboard/package.json b/apps/dashboard/package.json index cb8271a9da..6db4c280c4 100644 --- a/apps/dashboard/package.json +++ b/apps/dashboard/package.json @@ -11,7 +11,7 @@ }, "dependencies": { "@hermes/shared": "file:../shared", - "@nous-research/ui": "^0.14.2", + "@nous-research/ui": "0.16.0", "@observablehq/plot": "^0.6.17", "@react-three/fiber": "^9.6.0", "@tailwindcss/vite": "^4.2.1", @@ -22,7 +22,6 @@ "@xterm/xterm": "^6.0.0", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", - "flag-icons": "^7.5.0", "gsap": "^3.15.0", "leva": "^0.10.1", "lucide-react": "^0.577.0", diff --git a/apps/dashboard/src/App.tsx b/apps/dashboard/src/App.tsx index 987252ce0b..aeac02ae78 100644 --- a/apps/dashboard/src/App.tsx +++ b/apps/dashboard/src/App.tsx @@ -326,7 +326,9 @@ export default function App() { api .getConfig() .then((cfg) => { - const dash = (cfg?.dashboard ?? {}) as { show_token_analytics?: unknown }; + const dash = (cfg?.dashboard ?? {}) as { + show_token_analytics?: unknown; + }; setShowTokenAnalytics(dash.show_token_analytics === true); }) .catch(() => setShowTokenAnalytics(false)); @@ -366,7 +368,9 @@ export default function App() { const base = embeddedChat ? [CHAT_NAV_ITEM, ...BUILTIN_NAV_REST] : BUILTIN_NAV_REST; - return showTokenAnalytics ? base : base.filter((n) => n.path !== "/analytics"); + return showTokenAnalytics + ? base + : base.filter((n) => n.path !== "/analytics"); }, [embeddedChat, showTokenAnalytics]); const sidebarNav = useMemo( @@ -416,7 +420,7 @@ export default function App() { return (
@@ -442,7 +446,7 @@ export default function App() { aria-label={t.app.openNavigation} aria-expanded={mobileOpen} aria-controls="app-sidebar" - className="text-midground/70 hover:text-midground" + className="text-text-secondary hover:text-midground" > @@ -498,7 +502,7 @@ export default function App() { Hermes @@ -512,7 +516,7 @@ export default function App() { size="icon" onClick={closeMobile} aria-label={t.app.closeNavigation} - className="lg:hidden text-midground/70 hover:text-midground" + className="lg:hidden text-text-secondary hover:text-midground" > @@ -542,7 +546,7 @@ export default function App() { @@ -671,10 +675,12 @@ function SidebarNavLink({ closeMobile, item, t }: SidebarNavLinkProps) { cn( "group relative flex items-center gap-3", "px-5 py-2.5", - "font-mondwest text-[0.8rem] tracking-[0.12em]", + "font-mondwest text-display uppercase text-sm tracking-[0.12em]", "whitespace-nowrap transition-colors cursor-pointer", "focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-midground", - isActive ? "text-midground" : "opacity-60 hover:opacity-100", + isActive + ? "text-midground" + : "text-text-secondary hover:text-midground", ) } style={{ @@ -746,7 +752,7 @@ function SidebarSystemActions({ onNavigate }: { onNavigate: () => void }) { {t.app.system} @@ -772,12 +778,12 @@ function SidebarSystemActions({ onNavigate }: { onNavigate: () => void }) { active={busy} className={cn( "gap-3 px-5 py-1.5 whitespace-nowrap", - "font-mondwest text-[0.75rem] tracking-[0.1em]", - "transition-opacity", + "font-mondwest text-display text-xs tracking-[0.1em]", + "transition-colors", busy - ? "text-midground opacity-100" - : "opacity-60 hover:opacity-100", - "disabled:opacity-30", + ? "text-midground" + : "text-text-secondary hover:text-midground", + "disabled:text-text-disabled", )} > {isPending ? ( diff --git a/apps/dashboard/src/components/AutoField.tsx b/apps/dashboard/src/components/AutoField.tsx index 0f96d42042..4e3451c10f 100644 --- a/apps/dashboard/src/components/AutoField.tsx +++ b/apps/dashboard/src/components/AutoField.tsx @@ -11,8 +11,8 @@ function FieldHint({ schema, schemaKey }: { schema: Record; sch return (
- {keyPath && {keyPath}} - {description && {description}} + {keyPath && {keyPath}} + {description && {description}}
); } diff --git a/apps/dashboard/src/components/BottomPickSheet.tsx b/apps/dashboard/src/components/BottomPickSheet.tsx index 1490f4090c..38cae8daa0 100644 --- a/apps/dashboard/src/components/BottomPickSheet.tsx +++ b/apps/dashboard/src/components/BottomPickSheet.tsx @@ -7,7 +7,7 @@ import { } from "react"; import { createPortal } from "react-dom"; import { Typography } from "@/components/NouiTypography"; -import { cn } from "@/lib/utils"; +import { cn, themedBody } from "@/lib/utils"; const CLOSE_DRAG_MIN_PX = 72; const CLOSE_DRAG_RATIO = 0.18; @@ -168,6 +168,7 @@ export function BottomPickSheet({ aria-modal="true" ref={sheetRef} className={cn( + themedBody, "relative flex max-h-[85dvh] min-h-0 flex-col rounded-t-xl border border-current/20", "bg-background-base/98 pb-[max(1rem,env(safe-area-inset-bottom))]", "shadow-[0_-12px_40px_-8px_rgba(0,0,0,0.55)] backdrop-blur-md", @@ -200,7 +201,7 @@ export function BottomPickSheet({ {title} diff --git a/apps/dashboard/src/components/ChatSidebar.tsx b/apps/dashboard/src/components/ChatSidebar.tsx index c78b2bf5fd..8f55893fed 100644 --- a/apps/dashboard/src/components/ChatSidebar.tsx +++ b/apps/dashboard/src/components/ChatSidebar.tsx @@ -304,13 +304,13 @@ export function ChatSidebar({ channel, className }: ChatSidebarProps) { return (