diff --git a/.github/actions/detect-changes/action.yml b/.github/actions/detect-changes/action.yml index 0b990491ff48..18df8d78a4ea 100644 --- a/.github/actions/detect-changes/action.yml +++ b/.github/actions/detect-changes/action.yml @@ -5,6 +5,12 @@ description: >- the sub-workflows a PR can affect. Outputs are always "true" on push/dispatch events and fail open (everything "true") when the diff cannot be computed. +inputs: + github-token: + description: Token for the GitHub API (gh CLI). Pass secrets.AUTOFIX_BOT_PAT from the calling workflow. + required: false + default: ${{ github.token }} + outputs: python: description: Run Python tests / ruff / ty / windows-footguns. @@ -41,7 +47,7 @@ runs: id: classify shell: bash env: - GH_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ inputs.github-token }} REPO: ${{ github.repository }} EVENT_NAME: ${{ github.event_name }} BASE_SHA: ${{ github.event.pull_request.base.sha }} @@ -57,10 +63,26 @@ runs: # event payload instead of the "current PR files" endpoint. The SHAs # are frozen at trigger time, so the file list is deterministic even # if the PR receives a new push between trigger and detect. - CHANGED="$(gh api \ - --paginate \ - "repos/${REPO}/compare/${BASE_SHA}...${HEAD_SHA}" \ - --jq '.files[].filename' || true)" + # + # Retried: a rate-limit blip or eventual-consistency 404 on a + # freshly-pushed HEAD would otherwise silently fall open (all lanes + # run — safe, but wasteful and it masks the API failure). + CHANGED="" + for i in 1 2 3; do + if CHANGED="$(gh api \ + --paginate \ + "repos/${REPO}/compare/${BASE_SHA}...${HEAD_SHA}" \ + --jq '.files[].filename')"; then + break + fi + if [ "$i" = 3 ]; then + echo "::warning::compare API failed after 3 attempts — failing open (all lanes run)" + CHANGED="" + break + fi + echo "::warning::compare API failed (attempt $i); retrying in 10s" + sleep 10 + done fi echo "Changed files:" diff --git a/.github/actions/retry/action.yml b/.github/actions/retry/action.yml index 0eba2866ebec..dba9d6b3ec90 100644 --- a/.github/actions/retry/action.yml +++ b/.github/actions/retry/action.yml @@ -3,7 +3,8 @@ description: >- Run a shell command, retrying on non-zero exit. For dependency installs (npm ci, uv sync) whose only failures are transient network/toolchain flakes — a node-gyp header fetch, a registry blip — so CI self-heals - instead of needing a manual re-run. + instead of needing a manual re-run. Can also capture stdout as a step + output for commands whose result must be consumed by later steps. inputs: command: @@ -19,10 +20,16 @@ inputs: description: Directory to run in. default: "." +outputs: + stdout: + description: Captured stdout from the successful attempt (empty if not needed). + value: ${{ steps.retry.outputs.stdout }} + runs: using: composite steps: - - shell: bash + - id: retry + shell: bash working-directory: ${{ inputs.working-directory }} # command goes through env, never interpolated into the script body, so # a command with quotes/specials can't break or inject into the runner. @@ -32,12 +39,25 @@ runs: _DELAY: ${{ inputs.delay }} run: | set -uo pipefail + _OUTFILE="$(mktemp)" + trap 'rm -f "$_OUTFILE"' EXIT n=0 while :; do n=$((n + 1)) echo "::group::attempt $n/$_ATTEMPTS: $_CMD" - if bash -c "$_CMD"; then + # Run the command, capturing stdout to a temp file while still + # streaming to the log. We redirect first, then tee the file to + # stdout — this avoids pipefail + tee exit-code interactions that + # can cause the if-branch to be skipped under set -e. + if bash -c "$_CMD" > "$_OUTFILE"; then + cat "$_OUTFILE" echo "::endgroup::" + # Preserve newlines in the output via heredoc delimiter. + { + echo 'stdout<<__RETRY_STDOUT_EOF__' + cat "$_OUTFILE" + echo '__RETRY_STDOUT_EOF__' + } >> "$GITHUB_OUTPUT" exit 0 fi echo "::endgroup::" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f80b2230a62d..7528116d14ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,7 @@ jobs: detect: name: Detect affected areas runs-on: ubuntu-latest + timeout-minutes: 10 outputs: python: ${{ steps.classify.outputs.python }} frontend: ${{ steps.classify.outputs.frontend }} @@ -51,6 +52,8 @@ jobs: - name: Detect affected areas id: classify uses: ./.github/actions/detect-changes + with: + github-token: ${{ secrets.AUTOFIX_BOT_PAT }} # ───────────────────────────────────────────────────────────────────── # Lane-gated sub-workflows. Each runs in parallel after detect finishes. @@ -63,6 +66,7 @@ jobs: uses: ./.github/workflows/tests.yml with: slice_count: 8 + secrets: inherit lint: name: Python lints @@ -72,47 +76,55 @@ jobs: with: event_name: ${{ needs.detect.outputs.event_name }} ci_review: ${{ needs.detect.outputs.ci_review == 'true' }} + secrets: inherit js-tests: name: JS & TS checks needs: detect if: needs.detect.outputs.frontend == 'true' uses: ./.github/workflows/js-tests.yml + secrets: inherit docs-site: name: Docs Site needs: detect if: needs.detect.outputs.site == 'true' uses: ./.github/workflows/docs-site-checks.yml + secrets: inherit history-check: name: Deny unrelated histories needs: detect if: needs.detect.outputs.event_name == 'pull_request' uses: ./.github/workflows/history-check.yml + secrets: inherit contributor-check: name: Check contributors needs: detect if: needs.detect.outputs.python == 'true' uses: ./.github/workflows/contributor-check.yml + secrets: inherit uv-lockfile: name: Check uv.lock needs: detect uses: ./.github/workflows/uv-lockfile-check.yml + secrets: inherit lockfile-diff: name: package-lock.json diff needs: detect if: needs.detect.outputs.event_name == 'pull_request' && needs.detect.outputs.npm_lock == 'true' uses: ./.github/workflows/lockfile-diff.yml + secrets: inherit docker-lint: name: Lint Docker scripts needs: detect if: needs.detect.outputs.docker_meta == 'true' uses: ./.github/workflows/docker-lint.yml + secrets: inherit docker: name: Build&Test Docker image @@ -131,10 +143,12 @@ jobs: scan: ${{ needs.detect.outputs.scan == 'true' }} deps: ${{ needs.detect.outputs.deps == 'true' }} mcp_catalog: ${{ needs.detect.outputs.mcp_catalog == 'true' }} + secrets: inherit osv-scanner: name: OSV scan uses: ./.github/workflows/osv-scanner.yml + secrets: inherit # ───────────────────────────────────────────────────────────────────── # Gate: runs after everything. ``if: always()`` ensures it reports a @@ -161,6 +175,7 @@ jobs: # - docker if: always() runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Evaluate job results env: @@ -191,6 +206,7 @@ jobs: needs: [all-checks-pass, docker] if: always() runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -208,7 +224,7 @@ jobs: - name: Collect timings and generate report env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: | python3 scripts/ci/timings_report.py \ --baseline ci-timings-baseline.json \ @@ -217,6 +233,8 @@ jobs: --summary-out ci-timings-summary.md - name: Upload HTML report + # Advisory report — artifact-service blips must not fail the job. + continue-on-error: true uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 id: ci-timings-artifact with: diff --git a/.github/workflows/contributor-check.yml b/.github/workflows/contributor-check.yml index b7c3db7f8270..2c5db6f311de 100644 --- a/.github/workflows/contributor-check.yml +++ b/.github/workflows/contributor-check.yml @@ -9,6 +9,7 @@ permissions: jobs: check-attribution: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -27,7 +28,9 @@ jobs: exit 0 fi - # Check each email against AUTHOR_MAP in release.py + # An email is mapped if it has a file in contributors/emails/ + # (one file per email — conflict-free) or an entry in the frozen + # legacy AUTHOR_MAP in scripts/release.py. MISSING="" while IFS= read -r email; do # Skip teknium and bot emails @@ -36,9 +39,12 @@ jobs: continue ;; esac - # Check if email is in AUTHOR_MAP (either as a key or matches noreply pattern) if echo "$email" | grep -qP '\+.*@users\.noreply\.github\.com'; then - continue # GitHub noreply emails auto-resolve + continue # GitHub id+login noreply emails auto-resolve + fi + + if [ -f "contributors/emails/${email}" ]; then + continue # mapped via the contributors directory fi if ! grep -qF "\"${email}\"" scripts/release.py 2>/dev/null; then @@ -49,19 +55,19 @@ jobs: if [ -n "$MISSING" ]; then echo "" - echo "⚠️ New contributor email(s) not in AUTHOR_MAP:" + echo "⚠️ New contributor email(s) without a mapping:" echo -e "$MISSING" echo "" - echo "Please add mappings to scripts/release.py AUTHOR_MAP:" + echo "Add a mapping file (do NOT edit AUTHOR_MAP in release.py):" echo -e "$MISSING" | while read -r line; do email=$(echo "$line" | sed 's/^ *//' | cut -d' ' -f1) [ -z "$email" ] && continue - echo " \"${email}\": \"\"," + echo " python3 scripts/add_contributor.py ${email} " done echo "" echo "To find the GitHub username for an email:" echo " gh api 'search/users?q=EMAIL+in:email' --jq '.items[0].login'" exit 1 else - echo "✅ All contributor emails are mapped in AUTHOR_MAP." + echo "✅ All contributor emails are mapped." fi diff --git a/.github/workflows/deploy-site.yml b/.github/workflows/deploy-site.yml index 6e7dc84415d0..e06a0842a633 100644 --- a/.github/workflows/deploy-site.yml +++ b/.github/workflows/deploy-site.yml @@ -41,13 +41,15 @@ jobs: # doesn't auto-deploy via the deploy-docs path. if: github.event_name == 'release' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest + timeout-minutes: 30 steps: - name: Trigger Vercel Deploy - run: curl -X POST "${{ secrets.VERCEL_DEPLOY_HOOK }}" + run: curl -fsS --retry 3 --retry-delay 10 -X POST "${{ secrets.VERCEL_DEPLOY_HOOK }}" deploy-docs: if: github.repository == 'NousResearch/hermes-agent' runs-on: ubuntu-latest + timeout-minutes: 30 environment: name: github-pages url: ${{ steps.deploy.outputs.page_url }} @@ -65,12 +67,14 @@ jobs: python-version: '3.11' - name: Install PyYAML for skill extraction - run: pip install pyyaml==6.0.2 httpx==0.28.1 + uses: ./.github/actions/retry + with: + command: pip install pyyaml==6.0.2 httpx==0.28.1 - name: Prepare skills index (unified multi-source catalog) env: - GH_TOKEN: ${{ github.token }} - GITHUB_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} + GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} SKILLS_INDEX_RUN_ID: ${{ github.event.inputs.skills_index_run_id || '' }} REBUILD_SKILLS_INDEX: ${{ github.event.inputs.rebuild_skills_index || 'false' }} run: | @@ -150,8 +154,10 @@ jobs: run: python3 website/scripts/generate-skill-docs.py - name: Install dependencies - run: npm ci - working-directory: website + uses: ./.github/actions/retry + with: + command: npm ci + working-directory: website - name: Build Docusaurus run: npm run build diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index e19894c96fdc..f500aca99537 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -127,12 +127,13 @@ jobs: run: uv python install 3.11 - name: Install Python dependencies (for docker tests) - run: | - # ``dev`` extra pulls in pytest, pytest-asyncio — - # 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 sync --locked --python 3.11 --extra dev + # ``dev`` extra pulls in pytest, pytest-asyncio — + # 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. + uses: ./.github/actions/retry + with: + command: uv sync --locked --python 3.11 --extra dev - name: Run docker integration tests env: @@ -188,15 +189,23 @@ jobs: args+=("${IMAGE_NAME}@sha256:${digest_file}") done if [ "${{ github.event_name }}" = "release" ]; then - docker buildx imagetools create \ - -t "${IMAGE_NAME}:${RELEASE_TAG}" \ - "${args[@]}" + tags=(-t "${IMAGE_NAME}:${RELEASE_TAG}") else - docker buildx imagetools create \ - -t "${IMAGE_NAME}:main" \ - -t "${IMAGE_NAME}:latest" \ - "${args[@]}" + tags=(-t "${IMAGE_NAME}:main" -t "${IMAGE_NAME}:latest") fi + # Retry: Docker Hub API + just-pushed digest eventual consistency + # can transiently fail the create; the operation is idempotent. + for i in 1 2 3; do + if docker buildx imagetools create "${tags[@]}" "${args[@]}"; then + break + fi + if [ "$i" = 3 ]; then + echo "::error::imagetools create failed after 3 attempts" + exit 1 + fi + echo "::warning::imagetools create failed (attempt $i); retrying in 20s" + sleep 20 + done - name: Inspect image env: diff --git a/.github/workflows/docs-site-checks.yml b/.github/workflows/docs-site-checks.yml index 705f2171e5ce..41acf1790f48 100644 --- a/.github/workflows/docs-site-checks.yml +++ b/.github/workflows/docs-site-checks.yml @@ -9,6 +9,7 @@ permissions: jobs: docs-site-checks: runs-on: ubuntu-latest + timeout-minutes: 20 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/.github/workflows/history-check.yml b/.github/workflows/history-check.yml index 07e4fa348e43..a48dba8cb8af 100644 --- a/.github/workflows/history-check.yml +++ b/.github/workflows/history-check.yml @@ -22,6 +22,7 @@ permissions: jobs: check-common-ancestor: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index f0a384bafe60..4e4622f72d04 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -8,6 +8,7 @@ jobs: workspaces: name: List npm workspaces runs-on: ubuntu-latest + timeout-minutes: 20 outputs: packages: ${{ steps.set-matrix.outputs.packages }} steps: @@ -32,6 +33,7 @@ jobs: name: Typecheck & Test needs: workspaces runs-on: ubuntu-latest + timeout-minutes: 20 strategy: matrix: package: ${{ fromJson(needs.workspaces.outputs.packages) }} diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e88294dd94ed..3b83b6f7923d 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -177,15 +177,18 @@ jobs: steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Fetch PR labels + id: pr-labels + uses: ./.github/actions/retry + env: + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} + with: + command: gh pr view "${{ github.event.pull_request.number }}" --json labels --jq '.labels[].name' - name: Require ci-reviewed label id: label-check - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail - PR="${{ github.event.pull_request.number }}" - LABELS=$(gh pr view "$PR" --json labels --jq '.labels[].name' || true) - if echo "$LABELS" | grep -Fxq 'ci-reviewed'; then + if echo "${{ steps.pr-labels.outputs.stdout }}" | grep -Fxq 'ci-reviewed'; then echo "reviewed=true" >> "$GITHUB_OUTPUT" echo "ci-reviewed label present." exit 0 @@ -200,7 +203,7 @@ jobs: - name: Post or update review warning if: steps.label-check.outputs.reviewed != 'true' && github.event.pull_request.head.repo.fork != true env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: | set -euo pipefail PR="${{ github.event.pull_request.number }}" @@ -248,7 +251,7 @@ jobs: - name: Update previous warning to passed if: steps.label-check.outputs.reviewed == 'true' && github.event.pull_request.head.repo.fork != true env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: | set -euo pipefail PR="${{ github.event.pull_request.number }}" diff --git a/.github/workflows/lockfile-diff.yml b/.github/workflows/lockfile-diff.yml index 7b6f0ac1c688..2dcf66ea7c55 100644 --- a/.github/workflows/lockfile-diff.yml +++ b/.github/workflows/lockfile-diff.yml @@ -61,7 +61,7 @@ jobs: - name: Post or update PR comment env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} REPO: ${{ github.repository }} PR: ${{ github.event.pull_request.number }} CHANGED: ${{ steps.diff.outputs.changed }} diff --git a/.github/workflows/skills-index-freshness.yml b/.github/workflows/skills-index-freshness.yml index 856878def5f1..5a9bf98a0f46 100644 --- a/.github/workflows/skills-index-freshness.yml +++ b/.github/workflows/skills-index-freshness.yml @@ -20,6 +20,7 @@ jobs: check-freshness: if: github.repository == 'NousResearch/hermes-agent' runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Probe live index id: probe @@ -28,7 +29,7 @@ jobs: URL="https://hermes-agent.nousresearch.com/docs/api/skills-index.json" echo "Probing $URL" # -L follows redirects; -f fails on HTTP errors; -s suppresses progress - if ! curl -fsSL -o /tmp/skills-index.json "$URL"; then + if ! curl -fsSL --retry 3 --retry-delay 10 -o /tmp/skills-index.json "$URL"; then echo "status=fetch-failed" >> "$GITHUB_OUTPUT" echo "detail=Could not download $URL" >> "$GITHUB_OUTPUT" exit 0 @@ -110,7 +111,7 @@ jobs: - name: Open issue on degraded / failed probe if: steps.probe.outputs.status != 'ok' env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} STATUS: ${{ steps.probe.outputs.status }} DETAIL: ${{ steps.probe.outputs.detail }} run: | diff --git a/.github/workflows/skills-index.yml b/.github/workflows/skills-index.yml index 1997dedf5c75..8930a636fc08 100644 --- a/.github/workflows/skills-index.yml +++ b/.github/workflows/skills-index.yml @@ -20,6 +20,7 @@ jobs: # Only run on the upstream repository, not on forks if: github.repository == 'NousResearch/hermes-agent' runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -28,11 +29,13 @@ jobs: python-version: "3.11" - name: Install dependencies - run: pip install httpx==0.28.1 pyyaml==6.0.2 + uses: ./.github/actions/retry + with: + command: pip install httpx==0.28.1 pyyaml==6.0.2 - name: Build skills index env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: python scripts/build_skills_index.py - name: Upload index artifact @@ -49,8 +52,9 @@ jobs: needs: build-index if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Trigger Deploy Site workflow env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: gh workflow run deploy-site.yml --repo ${{ github.repository }} -f skills_index_run_id=${{ github.run_id }} diff --git a/.github/workflows/supply-chain-audit.yml b/.github/workflows/supply-chain-audit.yml index 201e92d174cc..4007668b7952 100644 --- a/.github/workflows/supply-chain-audit.yml +++ b/.github/workflows/supply-chain-audit.yml @@ -43,6 +43,7 @@ jobs: name: Scan PR for critical supply chain risks if: inputs.scan runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -52,7 +53,7 @@ jobs: - name: Scan diff for critical patterns id: scan env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: | set -euo pipefail @@ -141,7 +142,7 @@ jobs: - name: Post critical finding comment if: steps.scan.outputs.found == 'true' env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: | BODY="## 🚨 CRITICAL Supply Chain Risk Detected @@ -164,6 +165,7 @@ jobs: name: Check PyPI dependency upper bounds if: inputs.deps runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -201,7 +203,7 @@ jobs: - name: Post unbounded dep warning if: steps.bounds.outputs.found == 'true' env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: | BODY="## ⚠️ Unbounded PyPI Dependency Detected @@ -229,20 +231,27 @@ jobs: name: MCP catalog security review if: inputs.mcp_catalog runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 + - name: Fetch PR labels + id: pr-labels + uses: ./.github/actions/retry + env: + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} + with: + command: gh pr view "${{ github.event.pull_request.number }}" --json labels --jq '.labels[].name' - name: Require explicit MCP catalog review label env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} run: | set -euo pipefail PR="${{ github.event.pull_request.number }}" - LABELS=$(gh pr view "$PR" --json labels --jq '.labels[].name' || true) - if echo "$LABELS" | grep -Fxq 'mcp-catalog-reviewed'; then + if echo "${{ steps.pr-labels.outputs.stdout }}" | grep -Fxq 'mcp-catalog-reviewed'; then echo "MCP catalog review label present." exit 0 fi diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f74d574fa27d..1f29b25008e4 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -20,6 +20,7 @@ jobs: generate: name: "Generate slices" runs-on: ubuntu-latest + timeout-minutes: 10 outputs: matrix: ${{ steps.matrix.outputs.matrix }} steps: @@ -31,6 +32,12 @@ jobs: with: path: test_durations.json key: test-durations + # Saves use test-durations-${run_id}, so the exact key above never + # matches — without this prefix fallback the cache ALWAYS missed, + # LPT slicing ran on no data, and unbalanced slices pushed heavy + # files toward the per-file timeout under load. + restore-keys: | + test-durations- - name: Generate test slices id: matrix @@ -114,6 +121,9 @@ jobs: NOUS_API_KEY: "" - name: Upload per-slice durations + # Advisory artifact (feeds slice balancing) — a transient artifact- + # service blip must not fail an otherwise-green test slice. + continue-on-error: true uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: test-durations-slice-${{ matrix.slice.index }} @@ -126,6 +136,7 @@ jobs: needs: test if: needs.test.result == 'success' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Download all slice durations uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 diff --git a/.github/workflows/upload_to_pypi.yml b/.github/workflows/upload_to_pypi.yml index 03fad4eba0ce..de21ce473a4e 100644 --- a/.github/workflows/upload_to_pypi.yml +++ b/.github/workflows/upload_to_pypi.yml @@ -26,6 +26,7 @@ jobs: build: name: Build distribution 📦 runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -56,10 +57,24 @@ jobs: node-version: "22" - name: Build web dashboard - run: cd web && npm ci && npm run build + uses: ./.github/actions/retry + with: + command: npm ci + working-directory: web + + - name: Compile web dashboard + run: npm run build + working-directory: web - name: Build TUI bundle - run: cd ui-tui && npm ci && npm run build + uses: ./.github/actions/retry + with: + command: npm ci + working-directory: ui-tui + + - name: Compile TUI bundle + run: npm run build + working-directory: ui-tui - name: Bundle TUI into hermes_cli run: | @@ -90,6 +105,7 @@ jobs: name: Publish to PyPI needs: build runs-on: ubuntu-latest + timeout-minutes: 30 environment: name: pypi url: https://pypi.org/p/hermes-agent @@ -115,6 +131,7 @@ jobs: if: startsWith(github.ref, 'refs/tags/') needs: publish runs-on: ubuntu-latest + timeout-minutes: 30 permissions: contents: write # attach assets to the existing release id-token: write # sigstore signing @@ -128,7 +145,7 @@ jobs: - name: Wait for GitHub Release to exist env: - GITHUB_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} # release.py creates the GitHub Release after pushing the tag, # but this workflow starts from the tag push — wait for it. run: | @@ -154,7 +171,7 @@ jobs: - name: Attach signed artifacts to GitHub Release if: env.skip_sign != 'true' env: - GITHUB_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} # release.py already created the GitHub Release — just upload # the Sigstore signatures alongside the existing assets. run: >- diff --git a/.github/workflows/uv-lockfile-check.yml b/.github/workflows/uv-lockfile-check.yml index 8a7f52e899a4..27b072a99410 100644 --- a/.github/workflows/uv-lockfile-check.yml +++ b/.github/workflows/uv-lockfile-check.yml @@ -74,7 +74,20 @@ jobs: # rebase and regenerate uv.lock." - name: Verify uv.lock is up-to-date run: | - if ! uv lock --check; then + # uv lock --check re-resolves against PyPI (network). Retry so a + # registry blip doesn't read as "lockfile stale". A genuinely stale + # lockfile fails all attempts (deterministic), costing only seconds. + ok=false + for i in 1 2 3; do + if uv lock --check; then + ok=true + break + fi + [ "$i" = 3 ] && break + echo "::warning::uv lock --check failed (attempt $i); retrying in 10s" + sleep 10 + done + if [ "$ok" != true ]; then cat <<'EOF' >> "$GITHUB_STEP_SUMMARY" ## ❌ uv.lock is out of sync with pyproject.toml diff --git a/AGENTS.md b/AGENTS.md index 63247b2bf474..49596b9b41be 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1294,6 +1294,14 @@ scripts/run_tests.sh tests/agent/test_foo.py::test_x # one test scripts/run_tests.sh -v --tb=long # pass-through pytest flags ``` +**Flake policy:** the runner auto-retries a failing test FILE once in a fresh +subprocess (`--file-retries`, default 1; `HERMES_TEST_FILE_RETRIES=0` to +disable). Pass-on-retry counts as green but is printed in a `⚠ FLAKY` summary +section with both attempts' output. A FLAKY report is a bug to fix, not noise +to ignore — timing-sensitive tests must not assume a quiet runner (loose +wall-clock bounds ≥ 2s, event-based sync, no `assert not _wait_until(...)` +negative-timing races). + #### Subprocess-per-test-file isolation Every test file runs in a freshly-spawned Python subprocess via `run_tests_parallel.py`. This means module-level dicts/sets and diff --git a/Dockerfile b/Dockerfile index 6f957f779678..6803adc2e1db 100644 --- a/Dockerfile +++ b/Dockerfile @@ -26,8 +26,8 @@ ENV PLAYWRIGHT_BROWSERS_PATH=/opt/hermes/.playwright # 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 \ +RUN apt-get -o Acquire::Retries=3 update && \ + apt-get -o Acquire::Retries=3 install -y --no-install-recommends \ ca-certificates curl iputils-ping python3 python-is-python3 ripgrep ffmpeg gcc g++ make cmake python3-dev python3-venv libffi-dev libolm-dev procps git openssh-client docker-cli xz-utils && \ rm -rf /var/lib/apt/lists/* @@ -40,33 +40,30 @@ RUN apt-get update && \ # 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. +# We use `curl` instead of `ADD` for ALL three tarballs: `ADD` evaluates its +# URL at parse time (no ARG / TARGETARCH substitution) and — critically for +# CI reliability — cannot retry, so a single GitHub-release CDN blip fails +# the whole 15-45 min build. curl -fsSL --retry 3 self-heals those blips, +# and every tarball is still checksum-verified below before extraction. 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; \ + base="https://github.com/just-containers/s6-overlay/releases/download/v${S6_OVERLAY_VERSION}"; \ + curl -fsSL --retry 3 -o /tmp/s6-overlay-noarch.tar.xz \ + "${base}/s6-overlay-noarch.tar.xz"; \ + curl -fsSL --retry 3 -o /tmp/s6-overlay-symlinks-noarch.tar.xz \ + "${base}/s6-overlay-symlinks-noarch.tar.xz"; \ 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"; \ + "${base}/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; \ @@ -135,8 +132,11 @@ COPY apps/shared/ apps/shared/ # guards against a future regression if the source npm version changes. ENV npm_config_install_links=false -RUN npm install --prefer-offline --no-audit && \ - npx playwright install --with-deps chromium --only-shell && \ +RUN npm install --prefer-offline --no-audit --fetch-retries=5 && \ + for i in 1 2 3; do \ + npx playwright install --with-deps chromium --only-shell && break || \ + { [ "$i" = 3 ] && exit 1; echo "playwright install failed (attempt $i); retrying in 10s"; sleep 10; }; \ + done && \ npm cache clean --force # ---------- Layer-cached Python dependency install ---------- diff --git a/apps/desktop/src/components/assistant-ui/thread/streaming.test.tsx b/apps/desktop/src/components/assistant-ui/thread/streaming.test.tsx index fb844df0dc2e..1bd1fc04baeb 100644 --- a/apps/desktop/src/components/assistant-ui/thread/streaming.test.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/streaming.test.tsx @@ -243,7 +243,12 @@ function assistantImageMessage(running = false): ThreadMessage { } as ThreadMessage } -function StreamingHarness() { +interface StreamingControls { + emitSecond: () => void + complete: () => void +} + +function StreamingHarness({ onControls }: { onControls?: (controls: StreamingControls) => void } = {}) { const [messages, setMessages] = useState([userMessage()]) const [isRunning, setIsRunning] = useState(true) @@ -252,6 +257,19 @@ function StreamingHarness() { setMessages([userMessage(), assistantMessage('first chunk')]) }, 50) + if (onControls) { + onControls({ + emitSecond: () => { + setMessages([userMessage(), assistantMessage('first chunk second chunk')]) + }, + complete: () => { + setMessages([userMessage(), assistantMessage('first chunk second chunk', false)]) + setIsRunning(false) + } + }) + return () => window.clearTimeout(first) + } + const second = window.setTimeout(() => { setMessages([userMessage(), assistantMessage('first chunk second chunk')]) }, 500) @@ -266,7 +284,7 @@ function StreamingHarness() { window.clearTimeout(second) window.clearTimeout(complete) } - }, []) + }, [onControls]) const runtime = useExternalStoreRuntime({ messages, @@ -399,26 +417,30 @@ describe('assistant-ui streaming renderer', () => { }) it('renders assistant text incrementally before completion', async () => { - const { container } = render() + let controls: StreamingControls | undefined + const registerControls = (next: StreamingControls) => { + controls = next + } + const { container } = render() expect(screen.getByRole('status', { name: 'Hermes is loading a response' })).toBeTruthy() - await wait(80) - await waitFor(() => { expect(container.textContent).toContain('first chunk') }) expect(container.textContent).not.toContain('second chunk') expect(screen.queryByRole('status', { name: 'Hermes is loading a response' })).toBeNull() - await wait(500) - + // Producer-gated, not wall-clock-gated: the old test slept 80ms and + // assumed a 500ms timer could not fire before the assertion. On a loaded + // runner the test thread could be descheduled for >500ms, so both chunks + // arrived and this clean behavior test flaked. + act(() => controls?.emitSecond()) await waitFor(() => { expect(container.textContent).toContain('first chunk second chunk') }) - await wait(250) - + act(() => controls?.complete()) await waitFor(() => { expect(container.textContent).toContain('first chunk second chunk') }) diff --git a/contributors/README.md b/contributors/README.md new file mode 100644 index 000000000000..32ac5eaf4255 --- /dev/null +++ b/contributors/README.md @@ -0,0 +1,38 @@ +# Contributor email → GitHub login mappings + +This directory replaces appending entries to `AUTHOR_MAP` in +`scripts/release.py`. The old dict caused constant merge conflicts when +several salvage PRs landed at once — every PR edited the same lines of the +same file. Here, **each mapping is its own file**, and file additions never +conflict. + +## Adding a mapping + +One file per commit-author email, under `emails/`: + +```bash +python3 scripts/add_contributor.py +# or by hand: +echo "" > contributors/emails/ +``` + +- File **name** = the exact commit-author email (as shown by `git log --format='%ae'`). +- File **content** = the GitHub login on the first non-comment line. + Lines starting with `#` are comments (use them for the PR reference). + +Example — `contributors/emails/jane.doe@example.com`: + +``` +janedoe +# PR #12345 salvage (gateway: fix session key routing) +``` + +## Rules + +- Do NOT add new entries to `AUTHOR_MAP` in `scripts/release.py`. That dict + is frozen legacy data; the release tooling merges it with this directory + (directory entries win on duplicates). +- GitHub noreply emails (`+@users.noreply.github.com` and + `@users.noreply.github.com`) auto-resolve — no file needed. +- The `Contributor Attribution Check` CI job fails a PR whose commits carry + an unmapped email; the failure message prints the exact command to run. diff --git a/contributors/emails/.gitkeep b/contributors/emails/.gitkeep new file mode 100644 index 000000000000..74f551fbce84 --- /dev/null +++ b/contributors/emails/.gitkeep @@ -0,0 +1,2 @@ +_placeholder +# keeps the directory present in git; not a real mapping diff --git a/pyproject.toml b/pyproject.toml index 2b9242118e87..faf5b6efcf4e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -361,6 +361,7 @@ testpaths = ["tests"] markers = [ "integration: marks tests requiring external services (API keys, Modal, etc.)", "real_concurrent_gate: opt out of the autouse stub that disables _detect_concurrent_hermes_instances", + "real_agent_prewarm: opt out of the autouse stub that disables the tui_gateway deferred agent pre-warm timer", ] # integration tests take way too long to run in the normal CI environments addopts = "-m 'not integration'" diff --git a/scripts/add_contributor.py b/scripts/add_contributor.py new file mode 100644 index 000000000000..8192fe4f55c4 --- /dev/null +++ b/scripts/add_contributor.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Add a contributor email → GitHub login mapping. + +Writes one file per email under contributors/emails/ (filename = email, +content = login). File additions never merge-conflict, unlike the legacy +AUTHOR_MAP dict in scripts/release.py, which is frozen — do not append to it. + +Usage (from the repo root): + python3 scripts/add_contributor.py [comment...] + + # e.g. + python3 scripts/add_contributor.py jane@example.com janedoe "PR #12345 salvage" + +Idempotent: if the mapping already exists with the same login, prints +"present" and exits 0. If the email maps to a DIFFERENT login (here or in the +legacy AUTHOR_MAP), refuses with exit 1 so a typo can't silently reassign +someone's commits. +""" + +import re +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +EMAILS_DIR = REPO_ROOT / "contributors" / "emails" + +_EMAIL_RE = re.compile(r"^[^/\\\s]+@[^/\\\s]+$") +_LOGIN_RE = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9]|-(?=[A-Za-z0-9])){0,38}$") + + +def read_mapping_file(path: Path) -> str | None: + """Return the login from a mapping file (first non-comment line).""" + try: + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#"): + return line + except OSError: + pass + return None + + +def _legacy_login(email: str) -> str | None: + """Look the email up in the frozen legacy AUTHOR_MAP in release.py.""" + try: + sys.path.insert(0, str(REPO_ROOT / "scripts")) + from release import LEGACY_AUTHOR_MAP # noqa: PLC0415 + + return LEGACY_AUTHOR_MAP.get(email) + except Exception: + return None + + +def add_contributor(email: str, login: str, comment: str = "") -> int: + email = email.strip() + login = login.strip().lstrip("@") + + if not _EMAIL_RE.match(email): + print(f"error: {email!r} does not look like a commit-author email", file=sys.stderr) + return 2 + if not _LOGIN_RE.match(login): + print(f"error: {login!r} is not a valid GitHub login", file=sys.stderr) + return 2 + + path = EMAILS_DIR / email + existing = read_mapping_file(path) if path.is_file() else None + if existing is None: + existing = _legacy_login(email) + if existing is not None: + if existing == login: + print("present") + return 0 + print( + f"error: {email} already maps to {existing!r} (asked for {login!r}) — " + "resolve manually", + file=sys.stderr, + ) + return 1 + + EMAILS_DIR.mkdir(parents=True, exist_ok=True) + body = login + "\n" + if comment: + body += f"# {comment}\n" + path.write_text(body, encoding="utf-8") + print(f"added: contributors/emails/{email} -> {login}") + return 0 + + +def main() -> int: + if len(sys.argv) < 3: + print(__doc__, file=sys.stderr) + return 2 + email, login = sys.argv[1], sys.argv[2] + comment = " ".join(sys.argv[3:]) + return add_contributor(email, login, comment) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/contributor_audit.py b/scripts/contributor_audit.py index c4216cfa9093..9c371e4ae162 100644 --- a/scripts/contributor_audit.py +++ b/scripts/contributor_audit.py @@ -411,10 +411,10 @@ def main(): if all_unknowns: print() print(f"=== Unknown Emails ({len(all_unknowns)}) ===") - print("These emails are not in AUTHOR_MAP and should be added:") + print("These emails have no mapping and should be added via:") print() for email, name in sorted(all_unknowns.items()): - print(f' "{email}": "{name}",') + print(f" python3 scripts/add_contributor.py {email} # {name}") # ---- Strict mode: fail CI if new unmapped emails are introduced ---- if args.strict and all_unknowns: @@ -439,10 +439,10 @@ def main(): if new_unknowns: print() print(f"=== STRICT MODE FAILURE: {len(new_unknowns)} new unmapped email(s) ===") - print("Add these to AUTHOR_MAP in scripts/release.py before merging:") + print("Add mapping files before merging (do NOT edit AUTHOR_MAP):") print() for email, name in sorted(new_unknowns.items()): - print(f' "{email}": "",') + print(f" python3 scripts/add_contributor.py {email} # {name}") print() print("To find the GitHub username:") print(" gh api 'search/users?q=EMAIL+in:email' --jq '.items[0].login'") diff --git a/scripts/release.py b/scripts/release.py index 0b8cfddddff7..4b704bd2b7eb 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -43,8 +43,12 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json" # Git email → GitHub username mapping # ────────────────────────────────────────────────────────────────────── -# Auto-extracted from noreply emails + manual overrides -AUTHOR_MAP = { +# FROZEN legacy mappings — do NOT add new entries here. New contributor +# mappings live as one-file-per-email entries under contributors/emails/ +# (see contributors/README.md), which merge-conflict-free by construction. +# This dict is kept only so existing history keeps resolving; the effective +# AUTHOR_MAP below merges it with the directory (directory wins). +LEGACY_AUTHOR_MAP = { "122438640+ragingbulld@users.noreply.github.com": "ragingbulld", # PR #65606 salvage (non-finite API wait deadlines; #65746) "zzpigpinggai@users.noreply.github.com": "zzpigpinggai", # PR #66017 salvage of #63617 (OpenRouter explicit-provider picker visibility) "sam7894604@gmail.com": "sam7894604", # PR #55803 salvage (discord: /reasoning slash choices) @@ -2045,6 +2049,41 @@ AUTHOR_MAP = { } +# ────────────────────────────────────────────────────────────────────── +# Directory-based mappings: contributors/emails/ → login +# ────────────────────────────────────────────────────────────────────── +CONTRIBUTORS_EMAILS_DIR = REPO_ROOT / "contributors" / "emails" + + +def _load_contributor_dir(directory: "Path | None" = None) -> dict: + """Load one-file-per-email mappings from contributors/emails/. + + Filename = commit-author email, first non-comment line = GitHub login. + Additions never merge-conflict (each mapping is a distinct file), which + is why new entries go here instead of the frozen LEGACY_AUTHOR_MAP. + """ + directory = directory or CONTRIBUTORS_EMAILS_DIR + mapping = {} + if not directory.is_dir(): + return mapping + for path in sorted(directory.iterdir()): + if not path.is_file() or path.name.startswith("."): + continue + try: + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#"): + mapping[path.name] = line.lstrip("@") + break + except OSError: + continue + return mapping + + +# Effective map: frozen legacy dict + directory entries (directory wins). +AUTHOR_MAP = {**LEGACY_AUTHOR_MAP, **_load_contributor_dir()} + + def git(*args, cwd=None): """Run a git command and return stdout.""" result = subprocess.run( diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 602e033f3d6d..9a95d39d783e 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -85,6 +85,15 @@ _SKIP_PARTS = {"integration", "e2e", "docker"} # time while keeping a genuinely hung file bounded. _DEFAULT_FILE_TIMEOUT_SECONDS = 300.0 +# One-shot retry of failing test FILES. A file that exits non-zero is re-run +# once in a fresh subprocess; if the re-run passes, the file counts as passed +# but is loudly reported as FLAKY so it gets fixed rather than hidden. +# Deterministic failures fail both attempts — a real regression can never be +# laundered into green by this (it would have to flake in our favor twice in +# a row on the same runner, which is exactly the definition of a flake). +# Set to 0 to disable (env: HERMES_TEST_FILE_RETRIES). +_DEFAULT_FILE_RETRIES = 1 + # Duration cache: maps relative file paths to last-observed subprocess # wall-clock seconds. Used by ``--slice`` to distribute files across # CI jobs by estimated total time, so no one job gets all the slow files. @@ -224,11 +233,19 @@ def _run_one_file( pytest_args: List[str], repo_root: Path, file_timeout: float, + retries: int = 0, ) -> Tuple[Path, int, str, dict[str, int], float]: """Run ``python -m pytest `` in a fresh subprocess. Returns (file, returncode, captured_combined_output, summary_counts, subprocess_wall_seconds). + ``retries`` > 0 enables the one-shot flake retry: a non-zero exit is + re-run in a fresh subprocess; if the re-run passes, the file counts as + passed but the output is prefixed with a FLAKY banner and the file/output + are recorded in ``_FLAKY_RESULTS`` so the summary can call it out. A + deterministic failure fails every attempt, so real regressions cannot + be laundered green. + ``summary_counts`` is the result of ``_parse_pytest_summary(output)`` — pytest exit codes (https://docs.pytest.org/en/stable/reference/exit-codes.html): @@ -250,6 +267,44 @@ def _run_one_file( orphan onto PID 1. This outer timeout exists only to bound a pathologically slow or hung file as a whole. """ + file, rc, output, summary, subproc_wall = _run_one_file_once( + file, pytest_args, repo_root, file_timeout + ) + attempt = 0 + while rc != 0 and attempt < retries: + attempt += 1 + first_output = output + file, rc, output, summary, subproc_wall2 = _run_one_file_once( + file, pytest_args, repo_root, file_timeout + ) + subproc_wall += subproc_wall2 + if rc == 0: + output = ( + f"⚠ FLAKY: failed on attempt 1, passed on retry " + f"(attempt {attempt + 1}). Fix the flake — do not ignore this.\n" + f"--- first-attempt output ---\n{first_output}\n" + f"--- retry output ---\n{output}" + ) + with _flaky_lock: + _FLAKY_RESULTS.append((file, output)) + return file, rc, output, summary, subproc_wall + + +# Files that failed once and passed on retry, with both attempts' output. +# Keeping the traceback is load-bearing: a self-healed flake without its +# failing assertion is only a filename, which forces another expensive full +# run to rediscover the race. +_FLAKY_RESULTS: List[Tuple[Path, str]] = [] +_flaky_lock = threading.Lock() + + +def _run_one_file_once( + file: Path, + pytest_args: List[str], + repo_root: Path, + file_timeout: float, +) -> Tuple[Path, int, str, dict[str, int], float]: + """Single attempt of a per-file pytest subprocess (see _run_one_file).""" cmd = [sys.executable, "-m", "pytest", str(file), *pytest_args] subproc_start = time.monotonic() @@ -624,6 +679,19 @@ def main() -> int: f"Default: {_DEFAULT_FILE_TIMEOUT_SECONDS}s ({round(_DEFAULT_FILE_TIMEOUT_SECONDS/60)} min), env: HERMES_TEST_FILE_TIMEOUT." ), ) + parser.add_argument( + "--file-retries", + type=int, + default=int( + os.environ.get("HERMES_TEST_FILE_RETRIES", _DEFAULT_FILE_RETRIES) + ), + help=( + "Re-run a failing test FILE this many times in a fresh subprocess " + "before declaring it failed. A pass-on-retry counts as passed but " + "is reported as FLAKY in the summary. 0 disables. " + f"Default: {_DEFAULT_FILE_RETRIES}, env: HERMES_TEST_FILE_RETRIES." + ), + ) parser.add_argument( "--slice", metavar="I/N", @@ -684,7 +752,7 @@ def main() -> int: # (``-k=expr``, ``--tb=long``) are self-contained and need no lookahead. OUR_FLAGS = { "-j", "--jobs", "--paths", "--include-integration", - "--file-timeout", "--slice", "--generate-slices", "--files", + "--file-timeout", "--file-retries", "--slice", "--generate-slices", "--files", } # pytest short flags that consume the NEXT token as their value. PYTEST_VALUE_FLAGS = {"-k", "-m", "-p", "-o", "-c", "-r", "-W"} @@ -875,7 +943,8 @@ def main() -> int: for file in files: t0 = time.monotonic() fut = pool.submit( - _run_one_file, file, pytest_passthrough, repo_root, args.file_timeout + _run_one_file, file, pytest_passthrough, repo_root, + args.file_timeout, args.file_retries, ) fut.add_done_callback(lambda f, file=file, t0=t0: _on_done(file, t0, f)) futures.append(fut) @@ -890,6 +959,15 @@ def main() -> int: pct = min(100, (tests_done / approx_total_tests * 100)) if approx_total_tests else 0 print(f"=== Summary: {len(files)} files, {tests_passed} tests passed, {tests_failed} failed ({pct:.0f}% complete) in {elapsed:.1f}s ({args.jobs} workers) ===") + # Flaky files: failed once, passed on the automatic retry. Green, but + # loudly reported so they get fixed instead of silently re-flaking. + if _FLAKY_RESULTS: + print() + print(f"=== ⚠ {len(_FLAKY_RESULTS)} FLAKY file{'s' if len(_FLAKY_RESULTS) != 1 else ''} (failed once, passed on retry — fix these) ===") + for f, output in _FLAKY_RESULTS: + print(f" {_format_file(f, repo_root)}") + print(output.rstrip()) + # Save durations for future --slice runs. Each slice writes its own # partial test_durations.json; a CI merge step joins them later. # Locally, _save_durations merges with any existing cache so entries diff --git a/tests/agent/test_cascading_interrupt_6600.py b/tests/agent/test_cascading_interrupt_6600.py index 58fc28c4df0f..b335c6a6b17a 100644 --- a/tests/agent/test_cascading_interrupt_6600.py +++ b/tests/agent/test_cascading_interrupt_6600.py @@ -76,7 +76,7 @@ def test_non_streaming_cancel_does_not_surface_network_error(): # The forced RemoteProtocolError must NOT surface as the raised error. assert create_calls["n"] == 1 - assert elapsed < 3.0, f"interrupt took {elapsed:.1f}s — should be near-instant" + assert elapsed < 10.0, f"interrupt took {elapsed:.1f}s — should be near-instant (guarding the 30s+ hang)" def test_normal_transient_error_still_raises_when_not_cancelled(): diff --git a/tests/agent/test_compression_concurrent_fork.py b/tests/agent/test_compression_concurrent_fork.py index b23ad37da1e7..51870c5ffea5 100644 --- a/tests/agent/test_compression_concurrent_fork.py +++ b/tests/agent/test_compression_concurrent_fork.py @@ -252,7 +252,10 @@ def test_lock_refresh_keeps_owner_live_past_initial_ttl(tmp_path: Path, monkeypa db.create_session(parent_sid, source="discord") agent_a = _build_agent_with_db(db, parent_sid) - agent_a._compression_lock_ttl_seconds = 1.0 + # 3s TTL / 0.25s refresh: ~12 refresh opportunities per lease. A 1s TTL + # left one missed scheduling quantum between "refreshed" and "expired" + # on a loaded runner. + agent_a._compression_lock_ttl_seconds = 3.0 agent_a._compression_lock_refresh_interval = 0.25 compression_started = threading.Event() release_compression = threading.Event() @@ -276,9 +279,9 @@ def test_lock_refresh_keeps_owner_live_past_initial_ttl(tmp_path: Path, monkeypa try: assert compression_started.wait(timeout=10), "compression never acquired its lock" assert db.get_compression_lock_holder(parent_sid) is not None - time.sleep(1.2) + time.sleep(3.5) assert db.try_acquire_compression_lock( - parent_sid, "refresh_probe", ttl_seconds=1.0 + parent_sid, "refresh_probe", ttl_seconds=3.0 ) is False, "live owner lease expired and was reclaimable before compression finished" finally: release_compression.set() diff --git a/tests/agent/test_thread_scoped_output.py b/tests/agent/test_thread_scoped_output.py index 3899905463fb..d7543be6a6fc 100644 --- a/tests/agent/test_thread_scoped_output.py +++ b/tests/agent/test_thread_scoped_output.py @@ -60,8 +60,9 @@ def test_concurrent_thread_keeps_output_during_silence_window(): t2 = threading.Thread(target=loud_worker) t1.start() t2.start() - t1.join(timeout=3.0) - t2.join(timeout=3.0) + t1.join(timeout=15.0) + t2.join(timeout=15.0) + assert not t1.is_alive() and not t2.is_alive(), "worker threads didn't finish" captured = _run_with_real_stream(body) assert "SILENCED" not in captured @@ -139,7 +140,8 @@ def test_many_concurrent_silenced_and_loud_threads(): t.start() start.set() for t in threads: - t.join(timeout=3.0) + t.join(timeout=15.0) + assert not any(t.is_alive() for t in threads), "straggler thread would truncate captured output" captured = _run_with_real_stream(body) for i in range(5): diff --git a/tests/agent/test_title_generator.py b/tests/agent/test_title_generator.py index 321f9030ae08..bd281f728e02 100644 --- a/tests/agent/test_title_generator.py +++ b/tests/agent/test_title_generator.py @@ -375,10 +375,13 @@ class TestMaybeAutoTitle: ] with patch("agent.title_generator.auto_title_session") as mock_auto: + import threading + called = threading.Event() + mock_auto.side_effect = lambda *a, **k: called.set() maybe_auto_title(db, "sess-1", "hello", "hi there", history) - # Wait for the daemon thread to complete - import time - time.sleep(0.3) + # Event-based wait: sleep-sync flaked when the daemon thread + # wasn't scheduled within the fixed nap on a loaded runner. + assert called.wait(timeout=10), "auto_title thread never ran" mock_auto.assert_called_once_with( db, "sess-1", @@ -420,9 +423,11 @@ class TestMaybeAutoTitle: pass with patch("agent.title_generator.auto_title_session") as mock_auto: + import threading + called = threading.Event() + mock_auto.side_effect = lambda *a, **k: called.set() maybe_auto_title(db, "sess-1", "hello", "hi there", history, failure_callback=_cb) - import time - time.sleep(0.3) + assert called.wait(timeout=10), "auto_title thread never ran" mock_auto.assert_called_once_with( db, "sess-1", @@ -571,8 +576,10 @@ class TestRuntimeValidator: return True with patch("agent.title_generator.auto_title_session") as mock_auto: + import threading + called = threading.Event() + mock_auto.side_effect = lambda *a, **k: called.set() maybe_auto_title(db, "sess-1", "hello", "hi there", history, runtime_validator=_v) - import time - time.sleep(0.3) + assert called.wait(timeout=10), "auto_title thread never ran" kwargs = mock_auto.call_args.kwargs assert kwargs["runtime_validator"] is _v diff --git a/tests/cli/test_cli_interrupt_subagent.py b/tests/cli/test_cli_interrupt_subagent.py index 5b732425c8a2..7088bf30b767 100644 --- a/tests/cli/test_cli_interrupt_subagent.py +++ b/tests/cli/test_cli_interrupt_subagent.py @@ -151,10 +151,11 @@ class TestCLISubagentInterrupt(unittest.TestCase): print(f"Child {i}._interrupt_requested: {child._interrupt_requested}") # Wait for child to detect interrupt - detected = interrupt_detected.wait(timeout=3.0) + detected = interrupt_detected.wait(timeout=10.0) # Wait for delegate to finish - agent_thread.join(timeout=5) + agent_thread.join(timeout=15) + assert not agent_thread.is_alive(), "delegate thread did not finish" if delegate_error[0]: raise delegate_error[0] diff --git a/tests/conftest.py b/tests/conftest.py index bdef8bae944e..662324dce6dc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -214,6 +214,10 @@ _HERMES_BEHAVIORAL_VARS = frozenset({ "HERMES_KANBAN_CLAIM_LOCK", "HERMES_KANBAN_DISPATCH_IN_GATEWAY", "HERMES_TENANT", + # Honcho host selection changes which nested config block wins. A local + # shell override leaked "myhost" into the full suite and flipped 20 + # otherwise-unrelated config tests away from the default "hermes" host. + "HERMES_HONCHO_HOST", # Dashboard OAuth auth gate (PR #30156). When set, the bundled # dashboard-auth `nous` plugin auto-registers itself on plugin discovery, # which is triggered by any `/api/status` call. That leaks a provider @@ -342,6 +346,13 @@ def _hermetic_environment(tmp_path, monkeypatch): for name in _HERMES_BEHAVIORAL_VARS: monkeypatch.delenv(name, raising=False) + # Honcho's fallback host/config resolution legitimately reads the user's + # global ~/.honcho/config.json. Keep HOME stable (subprocess tests depend + # on it), but pin the host so ordinary tests cannot inherit a developer's + # defaultHost and silently select the wrong nested config block. Tests of + # custom host resolution override/delete this explicitly. + monkeypatch.setenv("HERMES_HONCHO_HOST", "hermes") + # 3. Redirect HERMES_HOME to a per-test tempdir. Code that reads # ``~/.hermes/*`` via ``get_hermes_home()`` now gets the tempdir. # diff --git a/tests/docker/test_profile_gateway.py b/tests/docker/test_profile_gateway.py index fdf017bbceb7..2adadf7e377c 100644 --- a/tests/docker/test_profile_gateway.py +++ b/tests/docker/test_profile_gateway.py @@ -66,6 +66,25 @@ def _svstat_wants_up(container: str) -> bool: return "want up" in state + +def _wait_for_want_state(container_name: str, want_up: bool, timeout: float = 15.0) -> None: + """Poll s6 want-state until it matches, instead of a fixed sleep. + + s6 state transitions are asynchronous; fixed two-second sleeps flaked + on loaded CI hosts. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if _svstat_wants_up(container_name) == want_up: + return + time.sleep(0.5) + state = "up" if want_up else "down" + raise AssertionError( + f"slot want-state never became {state} within {timeout}s: " + f"{_svstat(container_name)!r}" + ) + + def test_profile_create_then_gateway_start( built_image: str, container_name: str, ) -> None: @@ -88,20 +107,12 @@ def test_profile_create_then_gateway_start( # supervision-state contract holds. See ``_svstat_wants_up`` for # why we accept both ``up …`` (currently up) and ``down …, want # up`` (down but s6 wants up). - time.sleep(2) - assert _svstat_wants_up(container_name), ( - f"slot want-state is not up after gateway start: " - f"{_svstat(container_name)!r}" - ) + _wait_for_want_state(container_name, want_up=True) r = _sh(container_name, f"hermes -p {PROFILE} gateway stop", timeout=30) assert r.returncode == 0 - time.sleep(2) - assert not _svstat_wants_up(container_name), ( - f"slot want-state still up after gateway stop: " - f"{_svstat(container_name)!r}" - ) + _wait_for_want_state(container_name, want_up=False) def test_profile_delete_stops_gateway( @@ -113,7 +124,7 @@ def test_profile_delete_stops_gateway( _sh(container_name, f"hermes profile create {PROFILE}") _sh(container_name, f"hermes -p {PROFILE} gateway start", timeout=60) - time.sleep(3) + _wait_for_want_state(container_name, want_up=True) r = _sh( container_name, @@ -122,7 +133,11 @@ def test_profile_delete_stops_gateway( ) assert r.returncode == 0, f"profile delete failed: {r.stderr}" - time.sleep(2) - # Service slot should be gone. - r = _sh(container_name, f"test -d /run/service/gateway-{PROFILE}") + # Poll for slot removal instead of a fixed sleep. + deadline = time.monotonic() + 15 + while time.monotonic() < deadline: + r = _sh(container_name, f"test -d /run/service/gateway-{PROFILE}") + if r.returncode != 0: + break + time.sleep(0.5) assert r.returncode != 0, "s6 service slot still present after profile delete" \ No newline at end of file diff --git a/tests/docker/test_zombie_reaping.py b/tests/docker/test_zombie_reaping.py index dc14d3c93cae..e136a5108915 100644 --- a/tests/docker/test_zombie_reaping.py +++ b/tests/docker/test_zombie_reaping.py @@ -29,11 +29,18 @@ def test_orphan_zombies_reaped( docker_exec_sh( container_name, "( ( sleep 0.1 & ) & ); sleep 1", timeout=10, ) - time.sleep(1) - r = docker_exec(container_name, "ps", "axo", "stat,pid,comm") - zombies = [ - line for line in r.stdout.split("\n") - if line.strip().startswith("Z") - ] + # Poll for zombies-absent instead of a fixed sleep: reaping is + # asynchronous (SIGCHLD) and can lag on a loaded host. + deadline = time.monotonic() + 10 + zombies = ["(never checked)"] + while time.monotonic() < deadline: + r = docker_exec(container_name, "ps", "axo", "stat,pid,comm") + zombies = [ + line for line in r.stdout.split("\n") + if line.strip().startswith("Z") + ] + if not zombies: + break + time.sleep(0.5) assert not zombies, f"Zombies not reaped by PID 1: {zombies}" \ No newline at end of file diff --git a/tests/gateway/test_planned_stop_watcher.py b/tests/gateway/test_planned_stop_watcher.py index 451a3d8f8a75..47c801248f06 100644 --- a/tests/gateway/test_planned_stop_watcher.py +++ b/tests/gateway/test_planned_stop_watcher.py @@ -83,7 +83,7 @@ def test_watcher_fires_shutdown_when_marker_appears(tmp_path, monkeypatch): daemon=True, ) watcher.start() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) assert not watcher.is_alive(), "Watcher should exit after firing" assert len(loop._captured) == 1, ( @@ -117,7 +117,7 @@ def test_watcher_does_not_fire_when_marker_absent(tmp_path, monkeypatch): watcher.start() time.sleep(0.3) # let it poll a few times stop_event.set() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) assert not watcher.is_alive() assert loop._captured == [], ( @@ -153,7 +153,7 @@ def test_watcher_skips_when_runner_already_draining(tmp_path, monkeypatch): watcher.start() time.sleep(0.2) stop_event.set() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) assert loop._captured == [], "Watcher fired while runner was already draining" @@ -182,7 +182,7 @@ def test_watcher_skips_when_runner_not_started(tmp_path, monkeypatch): watcher.start() time.sleep(0.2) stop_event.set() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) assert loop._captured == [], "Watcher fired before runner was running" @@ -207,11 +207,11 @@ def test_watcher_responds_to_stop_event_promptly(tmp_path, monkeypatch): time.sleep(0.05) started_stop = time.monotonic() stop_event.set() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) elapsed = time.monotonic() - started_stop assert not watcher.is_alive() - assert elapsed < 0.5, f"Watcher took {elapsed:.2f}s to honour stop_event" + assert elapsed < 2.0 # 0.05s-poll thread; loose bound for scheduler stalls, f"Watcher took {elapsed:.2f}s to honour stop_event" def test_watcher_fires_only_once_when_marker_persists(tmp_path, monkeypatch): @@ -239,7 +239,7 @@ def test_watcher_fires_only_once_when_marker_persists(tmp_path, monkeypatch): ) watcher.start() # Let the watcher tick several times — but it should exit after the first fire. - watcher.join(timeout=1.0) + watcher.join(timeout=10.0) assert not watcher.is_alive() assert len(loop._captured) == 1, ( @@ -276,7 +276,7 @@ def test_watcher_tolerates_marker_path_resolution_errors(tmp_path, monkeypatch, watcher.start() time.sleep(0.2) stop_event.set() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) assert not watcher.is_alive(), "Watcher should still honour stop_event after errors" # No shutdown fired because the marker never reported existence. @@ -326,7 +326,7 @@ def test_watcher_does_not_fire_for_foreign_pid_marker(tmp_path, monkeypatch): watcher.start() time.sleep(0.3) # several poll cycles stop_event.set() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) assert not watcher.is_alive() assert loop._captured == [], ( @@ -360,7 +360,7 @@ def test_watcher_cleans_up_stale_marker_and_keeps_running(tmp_path, monkeypatch) watcher.start() time.sleep(0.3) stop_event.set() - watcher.join(timeout=2.0) + watcher.join(timeout=10.0) assert not watcher.is_alive() assert loop._captured == [], "Stale marker must not fire shutdown" diff --git a/tests/gateway/test_session_store_lock_io.py b/tests/gateway/test_session_store_lock_io.py index a1d1fd1e8856..8c82a036a7a9 100644 --- a/tests/gateway/test_session_store_lock_io.py +++ b/tests/gateway/test_session_store_lock_io.py @@ -209,16 +209,16 @@ def test_concurrent_same_key_returns_one_published_session(tmp_path): def synchronized_query(**kwargs): owner_started.set() - assert release_owner.wait(timeout=2) + assert release_owner.wait(timeout=10) return original_query(**kwargs) store._query_recoverable_session = synchronized_query # type: ignore[method-assign] with ThreadPoolExecutor(max_workers=2) as pool: owner = pool.submit(store.get_or_create_session, source) - assert owner_started.wait(timeout=2) + assert owner_started.wait(timeout=10) follower = pool.submit(store.get_or_create_session, source) release_owner.set() - entries = [owner.result(timeout=2), follower.result(timeout=2)] + entries = [owner.result(timeout=10), follower.result(timeout=10)] key = store._generate_session_key(source) assert entries[0] is entries[1] @@ -238,16 +238,16 @@ def test_concurrent_force_new_returns_one_published_session(tmp_path): def synchronized_impl(*args, **kwargs): owner_started.set() - assert release_owner.wait(timeout=2) + assert release_owner.wait(timeout=10) return original_impl(*args, **kwargs) store._get_or_create_session_impl = synchronized_impl # type: ignore[method-assign] with ThreadPoolExecutor(max_workers=2) as pool: owner = pool.submit(store.get_or_create_session, source, True) - assert owner_started.wait(timeout=2) + assert owner_started.wait(timeout=10) follower = pool.submit(store.get_or_create_session, source, True) release_owner.set() - entries = [owner.result(timeout=2), follower.result(timeout=2)] + entries = [owner.result(timeout=10), follower.result(timeout=10)] assert entries[0] is entries[1] created_ids = {call.kwargs["session_id"] for call in db.create_session.call_args_list} @@ -296,7 +296,7 @@ def test_legacy_and_off_lock_saves_share_one_serialization_lock(tmp_path): call_number = write_count if call_number == 1: first_write_started.set() - assert release_first_write.wait(timeout=2) + assert release_first_write.wait(timeout=10) persisted = dict(entries) db.replace_gateway_routing_entries.side_effect = replace @@ -314,12 +314,12 @@ def test_legacy_and_off_lock_saves_share_one_serialization_lock(tmp_path): with ThreadPoolExecutor(max_workers=2) as pool: future_a = pool.submit(store._save_entries) - assert first_write_started.wait(timeout=2) + assert first_write_started.wait(timeout=10) _seed_entry(store, key_b, "sid-b") future_b = pool.submit(store._save) release_first_write.set() - future_a.result(timeout=2) - future_b.result(timeout=2) + future_a.result(timeout=10) + future_b.result(timeout=10) assert set(persisted) == {key_a, key_b} @@ -340,7 +340,7 @@ def test_save_serialization_snapshots_latest_routing_index(tmp_path): call_number = write_count if call_number == 1: first_write_started.set() - assert release_first_write.wait(timeout=2) + assert release_first_write.wait(timeout=10) persisted = dict(entries) db.replace_gateway_routing_entries.side_effect = replace @@ -358,12 +358,12 @@ def test_save_serialization_snapshots_latest_routing_index(tmp_path): with ThreadPoolExecutor(max_workers=2) as pool: future_a = pool.submit(store._save_entries) - assert first_write_started.wait(timeout=2) + assert first_write_started.wait(timeout=10) entry_b = _seed_entry(store, key_b, "sid-b") future_b = pool.submit(store._save_entries) release_first_write.set() - future_a.result(timeout=2) - future_b.result(timeout=2) + future_a.result(timeout=10) + future_b.result(timeout=10) assert set(store._entries) == {key_a, key_b} assert set(persisted) == {key_a, key_b} diff --git a/tests/gateway/test_telegram_network.py b/tests/gateway/test_telegram_network.py index 8406973b9483..1615200dff34 100644 --- a/tests/gateway/test_telegram_network.py +++ b/tests/gateway/test_telegram_network.py @@ -730,7 +730,7 @@ class TestDiscoverFallbackIps: elapsed = _time.monotonic() - start assert ips == ["149.154.167.220"] - assert elapsed < 1.0, f"discovery gated on hung system DNS ({elapsed:.2f}s)" + assert elapsed < 1.4, f"discovery gated on hung system DNS ({elapsed:.2f}s)" @pytest.mark.asyncio async def test_hung_system_dns_with_no_doh_answers_bounded_seed_fallback(self, monkeypatch): @@ -755,4 +755,4 @@ class TestDiscoverFallbackIps: elapsed = _time.monotonic() - start assert ips == tnet._SEED_FALLBACK_IPS - assert elapsed < 1.0, f"seed fallback gated on hung system DNS ({elapsed:.2f}s)" + assert elapsed < 1.4, f"seed fallback gated on hung system DNS ({elapsed:.2f}s)" diff --git a/tests/hermes_cli/test_api_key_providers.py b/tests/hermes_cli/test_api_key_providers.py index 66b76ed38a90..30193e294167 100644 --- a/tests/hermes_cli/test_api_key_providers.py +++ b/tests/hermes_cli/test_api_key_providers.py @@ -147,21 +147,29 @@ class TestProviderRegistry: # Provider Resolution tests # ============================================================================= -PROVIDER_ENV_VARS = ( - "OPENROUTER_API_KEY", "OPENAI_API_KEY", "ANTHROPIC_API_KEY", "ANTHROPIC_TOKEN", - "CLAUDE_CODE_OAUTH_TOKEN", - "LM_API_KEY", "LM_BASE_URL", - "GLM_API_KEY", "ZAI_API_KEY", "Z_AI_API_KEY", - "KIMI_API_KEY", "KIMI_BASE_URL", "STEPFUN_API_KEY", "STEPFUN_BASE_URL", - "MINIMAX_API_KEY", "MINIMAX_CN_API_KEY", - "KILOCODE_API_KEY", "KILOCODE_BASE_URL", - "GMI_API_KEY", "GMI_BASE_URL", - "DASHSCOPE_API_KEY", "OPENCODE_ZEN_API_KEY", "OPENCODE_GO_API_KEY", - "NOUS_API_KEY", "GITHUB_TOKEN", "GH_TOKEN", - "OPENAI_BASE_URL", "HERMES_COPILOT_ACP_COMMAND", "COPILOT_CLI_PATH", +# Derived from the live PROVIDER_REGISTRY so the list can never drift when a +# new provider (and its env var) is added — a hand-maintained tuple here was +# missing HF_TOKEN/DEEPINFRA_API_KEY, which made the auto-detection tests +# env-dependent (they failed on any machine with HF_TOKEN exported). +from hermes_cli.auth import PROVIDER_REGISTRY as _REGISTRY + +_EXTRA_ENV_VARS = ( + # Checked directly in resolve_provider("auto"), not via the registry. + "OPENROUTER_API_KEY", "NOUS_API_KEY", + # Base URLs / paths that influence detection but aren't api_key_env_vars. + "LM_BASE_URL", "KIMI_BASE_URL", "STEPFUN_BASE_URL", "KILOCODE_BASE_URL", + "GMI_BASE_URL", "OPENAI_BASE_URL", + "HERMES_COPILOT_ACP_COMMAND", "COPILOT_CLI_PATH", "HERMES_COPILOT_ACP_ARGS", "COPILOT_ACP_BASE_URL", ) +PROVIDER_ENV_VARS = tuple( + dict.fromkeys( + [var for cfg in _REGISTRY.values() for var in cfg.api_key_env_vars] + + list(_EXTRA_ENV_VARS) + ) +) + @pytest.fixture(autouse=True) def _clear_provider_env(monkeypatch): diff --git a/tests/honcho_plugin/test_async_memory.py b/tests/honcho_plugin/test_async_memory.py index 6e28e8aecb40..607bef997a02 100644 --- a/tests/honcho_plugin/test_async_memory.py +++ b/tests/honcho_plugin/test_async_memory.py @@ -10,7 +10,7 @@ Covers: """ import json -import time +import threading from unittest.mock import MagicMock, patch @@ -312,17 +312,16 @@ class TestAsyncWriterThread: sess.add_message("user", "async msg") flushed = [] + flushed_event = threading.Event() - def capture(s): - flushed.append(s) + def capture(session): + flushed.append(session) + flushed_event.set() return True mgr._flush_session = capture mgr._async_queue.put(sess) - # Give the daemon thread time to process - deadline = time.time() + 2.0 - while not flushed and time.time() < deadline: - time.sleep(0.05) + assert flushed_event.wait(timeout=10), "async writer never flushed" mgr.shutdown() assert len(flushed) == 1 @@ -332,7 +331,7 @@ class TestAsyncWriterThread: mgr = _make_manager(write_frequency="async") thread = mgr._async_thread mgr.shutdown() - thread.join(timeout=3) + thread.join(timeout=10) assert not thread.is_alive() @@ -347,20 +346,20 @@ class TestAsyncWriterRetry: sess.add_message("user", "msg") call_count = [0] + retry_done = threading.Event() - def flaky_flush(s): + def flaky_flush(session): call_count[0] += 1 if call_count[0] == 1: raise ConnectionError("network blip") - # second call succeeds silently + retry_done.set() + return True mgr._flush_session = flaky_flush with patch("time.sleep"): # skip the 2s sleep in retry mgr._async_queue.put(sess) - deadline = time.time() + 3.0 - while call_count[0] < 2 and time.time() < deadline: - time.sleep(0.05) + assert retry_done.wait(timeout=10), "async writer never retried" mgr.shutdown() assert call_count[0] == 2 @@ -371,18 +370,19 @@ class TestAsyncWriterRetry: sess.add_message("user", "msg") call_count = [0] + retry_done = threading.Event() - def always_fail(s): + def always_fail(session): call_count[0] += 1 + if call_count[0] >= 2: + retry_done.set() raise RuntimeError("always broken") mgr._flush_session = always_fail with patch("time.sleep"): mgr._async_queue.put(sess) - deadline = time.time() + 3.0 - while call_count[0] < 2 and time.time() < deadline: - time.sleep(0.05) + assert retry_done.wait(timeout=10), "async writer never retried" mgr.shutdown() # Should have tried exactly twice (initial + one retry) and not crashed @@ -395,18 +395,19 @@ class TestAsyncWriterRetry: sess.add_message("user", "msg") call_count = [0] + retry_done = threading.Event() - def fail_then_succeed(_session): + def fail_then_succeed(session): call_count[0] += 1 + if call_count[0] >= 2: + retry_done.set() return call_count[0] > 1 mgr._flush_session = fail_then_succeed with patch("time.sleep"): mgr._async_queue.put(sess) - deadline = time.time() + 3.0 - while call_count[0] < 2 and time.time() < deadline: - time.sleep(0.05) + assert retry_done.wait(timeout=10), "async writer never retried" mgr.shutdown() assert call_count[0] == 2 diff --git a/tests/honcho_plugin/test_client.py b/tests/honcho_plugin/test_client.py index f7371abd12ac..2f51be5af505 100644 --- a/tests/honcho_plugin/test_client.py +++ b/tests/honcho_plugin/test_client.py @@ -507,7 +507,10 @@ class TestResolveActiveHost: def test_profiles_import_failure_falls_back(self): import sys - with patch.dict(os.environ, {}, clear=False): + with patch.dict(os.environ, {}, clear=False), patch( + "plugins.memory.honcho.client.resolve_config_path", + return_value=Path("/nonexistent/test-honcho-config.json"), + ): os.environ.pop("HERMES_HONCHO_HOST", None) # Temporarily remove hermes_cli.profiles to simulate import failure saved = sys.modules.get("hermes_cli.profiles") diff --git a/tests/scripts/test_contributor_map.py b/tests/scripts/test_contributor_map.py new file mode 100644 index 000000000000..3109a5e31434 --- /dev/null +++ b/tests/scripts/test_contributor_map.py @@ -0,0 +1,137 @@ +"""Tests for the conflict-free contributor mapping system. + +New contributor email → GitHub login mappings live as one file per email +under contributors/emails/ (additions never merge-conflict). The legacy +AUTHOR_MAP dict in scripts/release.py is frozen; release.py merges both at +import time with the directory winning on duplicates. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPTS_DIR = REPO_ROOT / "scripts" + +sys.path.insert(0, str(SCRIPTS_DIR)) + +import release # noqa: E402 +from add_contributor import add_contributor, read_mapping_file # noqa: E402 + + +# ── directory loader behavior ───────────────────────────────────────── + + +def test_loader_reads_login_from_first_noncomment_line(tmp_path): + d = tmp_path / "emails" + d.mkdir() + (d / "jane@example.com").write_text("# salvage PR #1\njanedoe\n# trailing note\n") + mapping = release._load_contributor_dir(d) + assert mapping == {"jane@example.com": "janedoe"} + + +def test_loader_strips_at_prefix_and_skips_dotfiles(tmp_path): + d = tmp_path / "emails" + d.mkdir() + (d / "a@b.com").write_text("@somelogin\n") + (d / ".gitkeep").write_text("_placeholder\n") + mapping = release._load_contributor_dir(d) + assert mapping == {"a@b.com": "somelogin"} + + +def test_loader_missing_directory_returns_empty(tmp_path): + assert release._load_contributor_dir(tmp_path / "nope") == {} + + +def test_effective_map_merges_legacy_and_directory(): + # Invariant: every legacy entry survives into the effective map unless + # shadowed by a directory entry, and the directory contributes on top. + assert set(release.LEGACY_AUTHOR_MAP) <= ( + set(release.AUTHOR_MAP) | set(release._load_contributor_dir()) + ) + for email, login in release._load_contributor_dir().items(): + assert release.AUTHOR_MAP[email] == login + + +def test_resolve_author_uses_directory_entry(tmp_path, monkeypatch): + d = tmp_path / "emails" + d.mkdir() + (d / "dirwin@example.com").write_text("dirwinner\n") + merged = {**release.LEGACY_AUTHOR_MAP, **release._load_contributor_dir(d)} + monkeypatch.setattr(release, "AUTHOR_MAP", merged) + assert release.resolve_author("Dir Winner", "dirwin@example.com") == "@dirwinner" + + +# ── add_contributor.py CLI behavior ─────────────────────────────────── + + +@pytest.fixture() +def emails_dir(tmp_path, monkeypatch): + import add_contributor + + d = tmp_path / "contributors" / "emails" + monkeypatch.setattr(add_contributor, "EMAILS_DIR", d) + return d + + +def test_add_creates_mapping_file(emails_dir): + rc = add_contributor("new@example.com", "newperson", "PR #999 salvage") + assert rc == 0 + path = emails_dir / "new@example.com" + assert path.is_file() + assert read_mapping_file(path) == "newperson" + assert "# PR #999 salvage" in path.read_text() + + +def test_add_is_idempotent(emails_dir): + assert add_contributor("x@y.com", "xperson") == 0 + assert add_contributor("x@y.com", "xperson") == 0 + assert read_mapping_file(emails_dir / "x@y.com") == "xperson" + + +def test_add_refuses_conflicting_login(emails_dir): + assert add_contributor("x@y.com", "xperson") == 0 + assert add_contributor("x@y.com", "someoneelse") == 1 + # original mapping untouched + assert read_mapping_file(emails_dir / "x@y.com") == "xperson" + + +def test_add_refuses_login_conflicting_with_legacy_map(emails_dir): + email, login = next(iter(release.LEGACY_AUTHOR_MAP.items())) + assert add_contributor(email, login + "x") == 1 + assert not (emails_dir / email).exists() + + +def test_add_rejects_invalid_email_and_login(emails_dir): + assert add_contributor("not-an-email", "ok") == 2 + assert add_contributor("has space@x.com", "ok") == 2 + assert add_contributor("a/b@x.com", "ok") == 2 # path separator + assert add_contributor("a@b.com", "-bad-") == 2 + assert not emails_dir.exists() or not any( + p for p in emails_dir.iterdir() if not p.name.startswith(".") + ) + + +def test_add_strips_at_prefix(emails_dir): + assert add_contributor("z@z.com", "@zeta") == 0 + assert read_mapping_file(emails_dir / "z@z.com") == "zeta" + + +def test_cli_entrypoint_end_to_end(tmp_path): + # Run the real script in a subprocess against a temp repo layout. + scripts = tmp_path / "scripts" + scripts.mkdir() + for name in ("add_contributor.py",): + (scripts / name).write_text((SCRIPTS_DIR / name).read_text()) + # Minimal stub release.py so the legacy lookup import works + (scripts / "release.py").write_text("LEGACY_AUTHOR_MAP = {}\n") + proc = subprocess.run( + [sys.executable, str(scripts / "add_contributor.py"), + "cli@example.com", "cliperson", "via subprocess"], + cwd=tmp_path, capture_output=True, text=True, + ) + assert proc.returncode == 0, proc.stderr + out = (tmp_path / "contributors" / "emails" / "cli@example.com").read_text() + assert out.splitlines()[0] == "cliperson" diff --git a/tests/test_hermes_state_compression_locks.py b/tests/test_hermes_state_compression_locks.py index 53e3bc0dec43..4e44d92a4457 100644 --- a/tests/test_hermes_state_compression_locks.py +++ b/tests/test_hermes_state_compression_locks.py @@ -84,8 +84,8 @@ def test_locks_are_per_session(db: SessionDB) -> None: def test_expired_lock_is_reclaimable(db: SessionDB) -> None: """A crashed compressor must not permanently block the session.""" # Acquire with a very short TTL - db.try_acquire_compression_lock("sess1", "crashed_holder", ttl_seconds=0.05) - time.sleep(0.1) + db.try_acquire_compression_lock("sess1", "crashed_holder", ttl_seconds=0.5) + time.sleep(1.0) # Holder check honours expiry assert db.get_compression_lock_holder("sess1") is None # New holder can claim it diff --git a/tests/test_honcho_startup_fail_open.py b/tests/test_honcho_startup_fail_open.py index 2570a7a6c739..0ed3be796eee 100644 --- a/tests/test_honcho_startup_fail_open.py +++ b/tests/test_honcho_startup_fail_open.py @@ -211,10 +211,15 @@ def test_first_turn_base_wait_is_shared_by_init_and_context_fetch(): started = time.perf_counter() assert provider.prefetch("what do you know about me?") == "" elapsed = time.perf_counter() - started - assert 0.4 <= elapsed < 0.65 + # Property: prefetch waits for init (0.3s sleep) but is bounded by + # first_turn_base_wait rather than blocking forever on the slow + # context fetch. The old 0.4..0.65 window was 0.25s wide — pure + # scheduler noise on a loaded runner. Lower bound proves the wait + # happened; loose upper bound proves it didn't hang. + assert 0.25 <= elapsed < 2.0 finally: release_context.set() - provider._init_thread.join(timeout=1) + provider._init_thread.join(timeout=10) diff --git a/tests/test_run_tests_parallel.py b/tests/test_run_tests_parallel.py index 3cba46fab00d..db5436f6f1ef 100644 --- a/tests/test_run_tests_parallel.py +++ b/tests/test_run_tests_parallel.py @@ -277,3 +277,85 @@ def test_positional_path_not_treated_as_flag(tmp_path: Path) -> None: # Discovery found the probe file (2 tests), proving the positional path # was consumed as a root, not forwarded to pytest as a bad flag. assert "test_flagprobe.py" in proc.stdout, proc.stdout + + +def test_file_retry_self_heals_and_prints_both_attempts(tmp_path: Path) -> None: + """A pass-on-retry is green, loud, and retains the failing traceback.""" + repo_root = Path(__file__).resolve().parent.parent + runner = repo_root / "scripts" / "run_tests_parallel.py" + marker = tmp_path / "ran-once" + probe = tmp_path / "test_flaky_probe.py" + probe.write_text( + textwrap.dedent( + f""" + from pathlib import Path + + def test_flaky_once(): + marker = Path({str(marker)!r}) + if not marker.exists(): + marker.write_text("failed once") + assert False, "simulated first-attempt flake" + assert True + """ + ), + encoding="utf-8", + ) + + proc = subprocess.run( + [ + sys.executable, + str(runner), + "--files", + str(probe), + "--file-retries", + "1", + "-j", + "1", + "-q", + ], + cwd=repo_root, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + timeout=60, + ) + + assert proc.returncode == 0, proc.stdout + assert "FLAKY file" in proc.stdout + assert "simulated first-attempt flake" in proc.stdout + assert "first-attempt output" in proc.stdout + assert "retry output" in proc.stdout + + +def test_file_retry_does_not_launder_deterministic_failure(tmp_path: Path) -> None: + """A real regression fails both attempts and the runner remains red.""" + repo_root = Path(__file__).resolve().parent.parent + runner = repo_root / "scripts" / "run_tests_parallel.py" + probe = tmp_path / "test_red_probe.py" + probe.write_text( + "def test_always_red():\n assert False, 'deterministic regression'\n", + encoding="utf-8", + ) + + proc = subprocess.run( + [ + sys.executable, + str(runner), + "--files", + str(probe), + "--file-retries", + "1", + "-j", + "1", + "-q", + ], + cwd=repo_root, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + timeout=60, + ) + + assert proc.returncode == 1, proc.stdout + assert "deterministic regression" in proc.stdout + assert "FLAKY file" not in proc.stdout diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index cdd69c371159..810e2cf226ec 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -17,6 +17,26 @@ from hermes_cli.browser_connect import ChromeDebugLaunch from tui_gateway import server +@pytest.fixture(autouse=True) +def _neuter_agent_prewarm_timer(request, monkeypatch): + """Stub the deferred agent pre-warm timer for every test in this module. + + ``session.create`` and non-eager ``session.resume`` fire a 50 ms + background ``threading.Timer`` (``_schedule_agent_build``) that calls + whatever ``server._make_agent`` is patched in AT FIRE TIME. Left live, + a timer armed by one test outlives it and lands in the NEXT test's + ``_make_agent`` mock, racily corrupting its captured state (the + ``'tip' == 'cont_tip'`` flakes in the session_resume tests). Tests that + exercise the deferred build itself opt back in with + ``@pytest.mark.real_agent_prewarm``. + """ + if request.node.get_closest_marker("real_agent_prewarm"): + yield + return + monkeypatch.setattr(server, "_schedule_agent_build", lambda *a, **k: None) + yield + + def test_session_create_rejects_at_active_session_limit(monkeypatch, tmp_path): home = tmp_path / ".hermes" home.mkdir() @@ -1420,14 +1440,9 @@ def test_session_resume_uses_parent_lineage_for_display(monkeypatch): monkeypatch.setattr( server, "_init_session", lambda sid, key, agent, history, cols=80, **_kwargs: None ) - # This resume takes the deferred (non-eager) path, which fires a 50ms - # background Timer (`_schedule_agent_build`) that later calls whatever - # `server._make_agent` is patched in AT THAT MOMENT. Left un-stubbed, that - # timer outlives this test and lands in the *next* test's `_make_agent` - # mock, racily corrupting its captured state (the `assert 'tip' == - # 'cont_tip'` flake in test_session_resume_follows_compression_tip). Neuter - # the pre-warm here — this test only asserts the returned display history. - monkeypatch.setattr(server, "_schedule_agent_build", lambda *a, **k: None) + # The deferred pre-warm timer is neutered module-wide by the autouse + # _neuter_agent_prewarm_timer fixture; this test only asserts the + # returned display history. resp = server.handle_request( {"id": "1", "method": "session.resume", "params": {"session_id": "tip"}} @@ -6841,6 +6856,7 @@ def test_mirror_slash_compress_does_not_prelock_history(monkeypatch): # --------------------------------------------------------------------------- +@pytest.mark.real_agent_prewarm def test_session_create_close_race_does_not_orphan_worker(monkeypatch): """Regression guard: if session.close runs while session.create's _build thread is still constructing the agent, the build thread @@ -6967,6 +6983,7 @@ def test_session_create_close_race_does_not_orphan_worker(monkeypatch): ) +@pytest.mark.real_agent_prewarm def test_session_create_no_race_keeps_worker_alive(monkeypatch): """Regression guard: when session.close does NOT race, the build thread must install the worker + notify normally and leave them @@ -7077,6 +7094,7 @@ def test_get_db_degrades_cleanly_when_sessiondb_init_fails(monkeypatch): assert server._db_error == "locking protocol" +@pytest.mark.real_agent_prewarm def test_session_create_continues_when_state_db_is_unavailable(monkeypatch): class _FakeWorker: def __init__(self, key, model, profile_home=None): diff --git a/tests/tools/test_clarify_gateway.py b/tests/tools/test_clarify_gateway.py index bd44ecdb961c..c617a4be6946 100644 --- a/tests/tools/test_clarify_gateway.py +++ b/tests/tools/test_clarify_gateway.py @@ -40,7 +40,7 @@ class TestClarifyPrimitive: cm.resolve_gateway_clarify("id1", "B") threading.Thread(target=resolver).start() - result = cm.wait_for_response("id1", timeout=2.0) + result = cm.wait_for_response("id1", timeout=10.0) assert result == "B" def test_open_ended_auto_awaits_text(self): @@ -149,7 +149,7 @@ class TestClarifyPrimitive: time.sleep(0.05) cancelled = cm.clear_session("sk7") assert cancelled == 1 - result = fut.result(timeout=2.0) + result = fut.result(timeout=10.0) # clear_session sets response="" then the wait returns it assert result == "" @@ -177,7 +177,7 @@ class TestClarifyPrimitive: cm.unregister_notify("sk9") # unregister_notify calls clear_session; thread unwinds - result = fut.result(timeout=2.0) + result = fut.result(timeout=10.0) assert result == "" def test_session_index_isolation(self): diff --git a/tests/tools/test_docker_environment.py b/tests/tools/test_docker_environment.py index c85402a3c2df..1f89c1aad9fb 100644 --- a/tests/tools/test_docker_environment.py +++ b/tests/tools/test_docker_environment.py @@ -1199,7 +1199,7 @@ def test_wait_for_cleanup_returns_true_when_no_thread_started(): shutdowns.""" env = docker_env.DockerEnvironment.__new__(docker_env.DockerEnvironment) # No _cleanup_thread set — simulates an env that was never cleanup()'d. - assert env.wait_for_cleanup(timeout=1.0) is True + assert env.wait_for_cleanup(timeout=10.0) is True def test_wait_for_cleanup_after_cleanup_returns_true(monkeypatch): diff --git a/tests/tools/test_execution_flag_detection.py b/tests/tools/test_execution_flag_detection.py index da07b54bb487..985af3bde3c9 100644 --- a/tests/tools/test_execution_flag_detection.py +++ b/tests/tools/test_execution_flag_detection.py @@ -557,7 +557,11 @@ def test_long_separator_free_token_hits_early_cap_before_regexes(size): "command parser limit exceeded", "command parser limit exceeded", ) - assert elapsed < 0.15, f"{size} byte token took {elapsed:.3f}s" + # Guards against catastrophic regex backtracking (seconds-to-minutes). + # The bound is deliberately loose: on a loaded shared CI runner even a + # trivially-fast call can see 100s of ms of scheduler stall, so a tight + # bound flakes without catching anything extra. + assert elapsed < 2.0, f"{size} byte token took {elapsed:.3f}s" def test_max_accepted_separator_free_input_is_fast(): @@ -569,4 +573,6 @@ def test_max_accepted_separator_free_input_is_fast(): elapsed = time.perf_counter() - started assert result == (False, None, None) - assert elapsed < 0.15, f"max accepted token took {elapsed:.3f}s" + # Loose bound: catches the O(n^2)/backtracking regression class without + # flaking on CI scheduler stalls (see comment above). + assert elapsed < 2.0, f"max accepted token took {elapsed:.3f}s" diff --git a/tests/tools/test_interrupt.py b/tests/tools/test_interrupt.py index 8c71f10ffd97..aca47df9e19b 100644 --- a/tests/tools/test_interrupt.py +++ b/tests/tools/test_interrupt.py @@ -50,7 +50,7 @@ class TestInterruptModule: # Target the checker thread's ident so it sees the interrupt set_interrupt(True, thread_id=t.ident) - t.join(timeout=1) + t.join(timeout=5) assert seen["value"] set_interrupt(False, thread_id=t.ident) diff --git a/tests/tools/test_local_background_child_hang.py b/tests/tools/test_local_background_child_hang.py index dd9f112a5678..805b96585af3 100644 --- a/tests/tools/test_local_background_child_hang.py +++ b/tests/tools/test_local_background_child_hang.py @@ -43,7 +43,7 @@ class TestBackgroundChildDoesNotHang: result = local_env.execute(cmd, timeout=15) elapsed = time.monotonic() - t0 - assert elapsed < 4.0, ( + assert elapsed < 10.0, ( # hang under guard is 15s+; loose bound rides out runner stalls f"terminal_tool hung for {elapsed:.1f}s — drain thread " f"is still blocking on backgrounded child's inherited pipe fd" ) @@ -63,7 +63,7 @@ class TestBackgroundChildDoesNotHang: result = local_env.execute(cmd, timeout=15) elapsed = time.monotonic() - t0 - assert elapsed < 4.0, f"setsid+disown path hung for {elapsed:.1f}s" + assert elapsed < 10.0, f"setsid+disown path hung for {elapsed:.1f}s" assert result["returncode"] == 0 assert "started" in result["output"] finally: @@ -77,7 +77,7 @@ class TestBackgroundChildDoesNotHang: elapsed = time.monotonic() - t0 # Loop body sleeps ~0.6s total — elapsed should be close to that. - assert 0.5 < elapsed < 3.0 + assert 0.5 < elapsed < 10.0 assert result["returncode"] == 0 for expected in ("tick 1", "tick 2", "tick 3", "done"): assert expected in result["output"], f"missing {expected!r}" @@ -148,7 +148,7 @@ class TestBackgroundChildDoesNotHang: result = local_env.execute(command, timeout=1, bounded_capture=True) elapsed = time.monotonic() - started - assert elapsed < 4.0 + assert elapsed < 10.0 assert result["returncode"] == 124 assert len(result["output"]) <= 5_000 assert "[OUTPUT TRUNCATED" in result["output"] @@ -160,13 +160,13 @@ class TestBackgroundChildDoesNotHang: result = local_env.execute("sleep 30", timeout=2) elapsed = time.monotonic() - t0 - assert elapsed < 4.0 + assert elapsed < 10.0 assert result["returncode"] == 124 assert "timed out" in result["output"].lower() def test_utf8_output_decoded_correctly(self, local_env): """Multibyte UTF-8 chunks must decode cleanly under select-based reads.""" - result = local_env.execute("echo 日本語 café résumé", timeout=5) + result = local_env.execute("echo 日本語 café résumé", timeout=30) assert result["returncode"] == 0 assert "日本語" in result["output"] assert "café" in result["output"] @@ -209,7 +209,7 @@ class TestBackgroundChildDoesNotHang: 'sys.stdout.buffer.write(b"\\xff\\xfe"); ' 'sys.stdout.buffer.write(b" after\\n")\'' ) - result = local_env.execute(cmd, timeout=5) + result = local_env.execute(cmd, timeout=15) assert result["returncode"] == 0 assert "before" in result["output"] assert "after" in result["output"] diff --git a/tests/tools/test_local_interrupt_cleanup.py b/tests/tools/test_local_interrupt_cleanup.py index 29f3d4667841..364223b9698a 100644 --- a/tests/tools/test_local_interrupt_cleanup.py +++ b/tests/tools/test_local_interrupt_cleanup.py @@ -49,7 +49,7 @@ def _process_group_snapshot(pgid: int) -> str: ).stdout.strip() -def _wait_for_pgid_exit(pgid: int, timeout: float = 30.0) -> bool: +def _wait_for_pgid_exit(pgid: int, timeout: float = 60.0) -> bool: """Wait for a process group to disappear under loaded xdist hosts. The cleanup chain is: SIGTERM → 3s TimeoutStopSec → SIGKILL → reap. @@ -150,7 +150,7 @@ def test_wait_for_process_kills_subprocess_on_keyboardinterrupt(): # does init_session() (one spawn) before the real command, so we need # to wait until a sleep 30 is visible. Use pgrep-style lookup via # /proc to find the bash process running our sleep. - deadline = time.monotonic() + 5.0 + deadline = time.monotonic() + 20.0 # generous: init_session + spawn dilate under CI load target_pid = None while time.monotonic() < deadline: # Walk our children and grand-children to find one running 'sleep 30' @@ -201,8 +201,8 @@ def test_wait_for_process_kills_subprocess_on_keyboardinterrupt(): # run the except-block cleanup (_kill_process), and exit. Under # xdist load the SIGTERM → 3s wait → SIGKILL chain can take longer # than 5s before the worker's join() returns; bumped to 15s. - t.join(timeout=15.0) - assert not t.is_alive(), "worker didn't exit within 15 s of the interrupt" + t.join(timeout=30.0) + assert not t.is_alive(), "worker didn't exit within 30 s of the interrupt" # The critical assertion: the subprocess GROUP must be dead. Not # just the bash wrapper — the 'sleep 30' child too. Under xdist load, diff --git a/tests/tools/test_mcp_cancelled_error_propagation.py b/tests/tools/test_mcp_cancelled_error_propagation.py index 13636c3caac6..88f232fc254e 100644 --- a/tests/tools/test_mcp_cancelled_error_propagation.py +++ b/tests/tools/test_mcp_cancelled_error_propagation.py @@ -46,7 +46,7 @@ class TestCancelledErrorPropagation: # CancelledError propagation or clean exit) rather than # hanging forever. try: - await asyncio.wait_for(task, timeout=2.0) + await asyncio.wait_for(task, timeout=15.0) except asyncio.CancelledError: return "cancelled_cleanly" except asyncio.TimeoutError: @@ -82,7 +82,7 @@ class TestCancelledErrorPropagation: server._shutdown_event.set() server._task.cancel() try: - await asyncio.wait_for(server._task, timeout=2.0) + await asyncio.wait_for(server._task, timeout=15.0) except (asyncio.CancelledError, asyncio.TimeoutError): pass return server._task.done() diff --git a/tests/tools/test_mcp_circuit_breaker.py b/tests/tools/test_mcp_circuit_breaker.py index f356c94e1280..357589d06621 100644 --- a/tests/tools/test_mcp_circuit_breaker.py +++ b/tests/tools/test_mcp_circuit_breaker.py @@ -483,7 +483,7 @@ def test_run_loop_parks_instead_of_exiting_then_revives(monkeypatch, tmp_path): task._shutdown_event.set() task._reconnect_event.set() try: - await asyncio.wait_for(run_task, timeout=2) + await asyncio.wait_for(run_task, timeout=15) except (asyncio.TimeoutError, asyncio.CancelledError, Exception): run_task.cancel() @@ -564,7 +564,7 @@ def test_initial_connect_budget_parks_instead_of_exiting_then_revives(monkeypatc task._shutdown_event.set() task._reconnect_event.set() try: - await asyncio.wait_for(run_task, timeout=2) + await asyncio.wait_for(run_task, timeout=15) except (asyncio.TimeoutError, asyncio.CancelledError, Exception): run_task.cancel() diff --git a/tests/tools/test_mcp_parked_self_probe.py b/tests/tools/test_mcp_parked_self_probe.py index 95decf096e02..af53fdea2d4d 100644 --- a/tests/tools/test_mcp_parked_self_probe.py +++ b/tests/tools/test_mcp_parked_self_probe.py @@ -101,7 +101,7 @@ def test_parked_server_self_probes_and_revives(monkeypatch, tmp_path): task._shutdown_event.set() task._reconnect_event.set() try: - await asyncio.wait_for(run_task, timeout=2) + await asyncio.wait_for(run_task, timeout=15) except (asyncio.TimeoutError, asyncio.CancelledError, Exception): run_task.cancel() diff --git a/tests/tools/test_mcp_reconnect_retry_reset.py b/tests/tools/test_mcp_reconnect_retry_reset.py index 50442aa602a5..0bef5ca7f5c6 100644 --- a/tests/tools/test_mcp_reconnect_retry_reset.py +++ b/tests/tools/test_mcp_reconnect_retry_reset.py @@ -120,7 +120,7 @@ def test_reconnect_counter_resets_after_successful_session(monkeypatch, tmp_path task._shutdown_event.set() task._reconnect_event.set() try: - await asyncio.wait_for(run_task, timeout=2) + await asyncio.wait_for(run_task, timeout=15) except (asyncio.TimeoutError, asyncio.CancelledError, Exception): run_task.cancel() @@ -189,7 +189,7 @@ def test_reconnect_counter_still_parks_on_consecutive_failures(monkeypatch, tmp_ task._shutdown_event.set() task._reconnect_event.set() try: - await asyncio.wait_for(run_task, timeout=2) + await asyncio.wait_for(run_task, timeout=15) except (asyncio.TimeoutError, asyncio.CancelledError, Exception): run_task.cancel() diff --git a/tests/tools/test_mcp_stability.py b/tests/tools/test_mcp_stability.py index b9a1b92c9cde..f204615aca0d 100644 --- a/tests/tools/test_mcp_stability.py +++ b/tests/tools/test_mcp_stability.py @@ -539,8 +539,8 @@ class TestStdioPgroupReaping: ) parent_pgid = os.getpgid(parent.pid) # Wait for parent to exit and grandchild to spin up. - parent.wait(timeout=5) - deadline = _time.time() + 5 + parent.wait(timeout=15) + deadline = _time.time() + 15 # fresh CPython spinup dilates under CI load while _time.time() < deadline and not grandchild_pid_file.exists(): _time.sleep(0.05) assert grandchild_pid_file.exists(), "grandchild did not start" @@ -577,7 +577,7 @@ class TestStdioPgroupReaping: pass # Grandchild should be gone — SIGTERM via killpg in phase 1 reached it. - deadline = _time.time() + 3 + deadline = _time.time() + 10 while _time.time() < deadline and psutil.pid_exists(grandchild_pid): _time.sleep(0.05) assert not psutil.pid_exists(grandchild_pid), ( diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index be41088e4eb7..c2208165fa4b 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -871,7 +871,7 @@ class TestRunOnMCPLoopInterrupts: try: with pytest.raises(InterruptedError, match="User sent a new message"): - mcp_mod._run_on_mcp_loop(_slow_call(), timeout=2) + mcp_mod._run_on_mcp_loop(_slow_call(), timeout=10) deadline = time.time() + 2 while time.time() < deadline and not cancelled.is_set(): @@ -880,7 +880,7 @@ class TestRunOnMCPLoopInterrupts: finally: set_interrupt(False, waiter_tid) loop.call_soon_threadsafe(loop.stop) - thread.join(timeout=2) + thread.join(timeout=10) loop.close() mcp_mod._mcp_loop = old_loop mcp_mod._mcp_thread = old_thread @@ -917,7 +917,7 @@ class TestRunOnMCPLoopInterrupts: assert cancelled.is_set() finally: loop.call_soon_threadsafe(loop.stop) - thread.join(timeout=2) + thread.join(timeout=10) loop.close() mcp_mod._mcp_loop = old_loop mcp_mod._mcp_thread = old_thread diff --git a/tests/tools/test_process_registry.py b/tests/tools/test_process_registry.py index 817eaabcc8e2..beeb41fe1ea1 100644 --- a/tests/tools/test_process_registry.py +++ b/tests/tools/test_process_registry.py @@ -403,7 +403,7 @@ class TestOrphanedPipeReconciliation: assert result["status"] == "exited", result assert result["exit_code"] == 0 - assert elapsed < 0.3, f"wait() should wake on completion; took {elapsed:.3f}s" + assert elapsed < 0.9 # must stay under the old 1s poll tick being regression-tested, f"wait() should wake on completion; took {elapsed:.3f}s" # ========================================================================= @@ -2110,8 +2110,11 @@ class TestSigkillEscalation: sometimes a child). The escalation now re-probes every target directly. """ import psutil + # 2.0s grace (not 1.0): with three interpreters mid-startup on a + # loaded runner, a 1s SIGTERM->partition window races child spawn and + # is how a child PID escaped the live-system guard in CI. monkeypatch.setattr(ProcessRegistry, "_daemon_term_grace_seconds", - staticmethod(lambda: 1.0)) + staticmethod(lambda: 2.0)) # Parent spawns 2 children; all trap SIGTERM. Parent prints child pids # after the handler is installed. parent_src = ( @@ -2125,6 +2128,12 @@ class TestSigkillEscalation: ) parent = subprocess.Popen([sys.executable, "-c", parent_src], stdout=subprocess.PIPE, text=True) + # Bound the readline: if the parent wedges before printing, fail THIS + # test with a clear message instead of letting the per-file timeout + # SIGKILL the whole pytest process (opaque rc=124 in CI). + import select as _select + ready, _, _ = _select.select([parent.stdout], [], [], 20.0) + assert ready, "parent process failed to print child pids within 20s" child_pids = [int(x) for x in parent.stdout.readline().split()] all_pids = [parent.pid] + child_pids try: diff --git a/tests/tools/test_registry.py b/tests/tools/test_registry.py index 49d1193cf676..97415c991e29 100644 --- a/tests/tools/test_registry.py +++ b/tests/tools/test_registry.py @@ -505,7 +505,7 @@ class TestThreadSafety: def blocking_check(): check_started.set() - writer_completed_during_check["value"] = writer_done.wait(timeout=1) + writer_completed_during_check["value"] = writer_done.wait(timeout=10) return True reg.register( @@ -529,7 +529,7 @@ class TestThreadSafety: errors.append(exc) def writer(): - assert check_started.wait(timeout=1) + assert check_started.wait(timeout=10) reg.register( name="gamma", toolset="new", @@ -542,8 +542,8 @@ class TestThreadSafety: writer_thread = threading.Thread(target=writer) reader_thread.start() writer_thread.start() - reader_thread.join(timeout=2) - writer_thread.join(timeout=2) + reader_thread.join(timeout=15) + writer_thread.join(timeout=15) assert not reader_thread.is_alive() assert not writer_thread.is_alive() @@ -565,7 +565,7 @@ class TestThreadSafety: def blocking_check(): check_started.set() - writer_completed_during_check["value"] = writer_done.wait(timeout=1) + writer_completed_during_check["value"] = writer_done.wait(timeout=10) return True reg.register( @@ -589,7 +589,7 @@ class TestThreadSafety: errors.append(exc) def writer(): - assert check_started.wait(timeout=1) + assert check_started.wait(timeout=10) reg.deregister("beta") writer_done.set() @@ -597,8 +597,8 @@ class TestThreadSafety: writer_thread = threading.Thread(target=writer) reader_thread.start() writer_thread.start() - reader_thread.join(timeout=2) - writer_thread.join(timeout=2) + reader_thread.join(timeout=15) + writer_thread.join(timeout=15) assert not reader_thread.is_alive() assert not writer_thread.is_alive() diff --git a/tests/tools/test_voice_cli_integration.py b/tests/tools/test_voice_cli_integration.py index dc6f2061f2c7..bf96ff501281 100644 --- a/tests/tools/test_voice_cli_integration.py +++ b/tests/tools/test_voice_cli_integration.py @@ -1322,6 +1322,7 @@ class TestRefreshLevelLock: with lock: recording = False - t.join(timeout=1) + t.join(timeout=10) + assert not t.is_alive() assert not t.is_alive(), "Refresh thread did not stop" assert iterations > 0, "Refresh thread never ran" diff --git a/tests/tools/test_voice_mode.py b/tests/tools/test_voice_mode.py index 8f6a8e67784e..b092250e2b44 100644 --- a/tests/tools/test_voice_mode.py +++ b/tests/tools/test_voice_mode.py @@ -738,7 +738,7 @@ class TestAudioRecorderProperties: # Force start time to 1 second ago recorder._start_time = time.monotonic() - 1.0 elapsed = recorder.elapsed_seconds - assert 0.9 < elapsed < 2.0 + assert 0.9 < elapsed < 10.0 # loose upper bound; only the lower bound is the property recorder.cancel() diff --git a/tests/tui_gateway/test_inline_rpc_gil_starvation.py b/tests/tui_gateway/test_inline_rpc_gil_starvation.py index 80244b71a731..99c63c746280 100644 --- a/tests/tui_gateway/test_inline_rpc_gil_starvation.py +++ b/tests/tui_gateway/test_inline_rpc_gil_starvation.py @@ -109,7 +109,7 @@ def test_dispatch_inline_rpc_does_not_block_under_gil_pressure(server): fast_elapsed = time.monotonic() - t0 assert fast_resp["result"] == {"ok": True} - assert fast_elapsed < 0.5, ( + assert fast_elapsed < 2.0, ( f"fast handler blocked for {fast_elapsed:.2f}s behind slow session.list — " f"the WS read loop would stall, causing false 'needs setup' (#50005)." ) @@ -140,7 +140,7 @@ def test_dispatch_pet_info_does_not_block_prompt_submit(server): elapsed = time.monotonic() - t0 assert resp["result"] == {"status": "streaming"} - assert elapsed < 0.5, ( + assert elapsed < 2.0, ( f"prompt.submit blocked for {elapsed:.2f}s behind slow pet.info — " f"the user's message would appear stuck under GIL pressure (#50005)." ) diff --git a/tests/tui_gateway/test_iso_certify_seam.py b/tests/tui_gateway/test_iso_certify_seam.py index 3a9a41342a42..14618d38c273 100644 --- a/tests/tui_gateway/test_iso_certify_seam.py +++ b/tests/tui_gateway/test_iso_certify_seam.py @@ -56,7 +56,7 @@ def test_synth_turn_holds_duration_and_streams(): result = agent.run_conversation(spec, stream_callback=deltas.append) elapsed = time.monotonic() - t0 # Held for ~the requested wall time (allow generous upper bound under load). - assert 0.35 <= elapsed <= 1.5, elapsed + assert 0.35 <= elapsed <= 5.0, elapsed assert result["interrupted"] is False assert len(deltas) >= 3 # Token accounting advanced (the 100K-token heavy-turn proxy). @@ -79,7 +79,7 @@ def test_synth_turn_interrupt_aborts_promptly(): result = agent.run_conversation('{"duration_s": 10.0}') elapsed = time.monotonic() - t0 assert result["interrupted"] is True - assert elapsed < 1.5, elapsed + assert elapsed < 5.0, elapsed def test_synth_turn_non_json_prompt_uses_defaults(monkeypatch): diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 5aa7c7d8425a..49177c7694e0 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -1852,7 +1852,7 @@ def test_dispatch_long_handler_does_not_block_fast_handler(server): fast_elapsed = time.monotonic() - t0 assert fast_resp["result"] == {"pong": True} - assert fast_elapsed < 0.5, f"fast handler blocked for {fast_elapsed:.2f}s behind slow handler" + assert fast_elapsed < 2.0, f"fast handler blocked for {fast_elapsed:.2f}s behind slow handler" released.set() @@ -1875,7 +1875,7 @@ def test_dispatch_session_compress_does_not_block_fast_handler(server): fast_elapsed = time.monotonic() - t0 assert fast_resp["result"] == {"pong": True} - assert fast_elapsed < 0.5, f"fast handler blocked for {fast_elapsed:.2f}s behind session.compress" + assert fast_elapsed < 2.0, f"fast handler blocked for {fast_elapsed:.2f}s behind session.compress" released.set() @@ -1940,6 +1940,6 @@ def test_slow_completion_does_not_block_fast_handler(completion_method, server): fast_elapsed = time.monotonic() - t0 assert fast_resp["result"] == {"pong": True} - assert fast_elapsed < 0.5, f"fast handler blocked for {fast_elapsed:.2f}s behind {completion_method}" + assert fast_elapsed < 2.0, f"fast handler blocked for {fast_elapsed:.2f}s behind {completion_method}" released.set() diff --git a/tests/tui_gateway/test_wait_for_mcp_discovery.py b/tests/tui_gateway/test_wait_for_mcp_discovery.py index ab5bb5f6ddc9..d4b3eacf5ac6 100644 --- a/tests/tui_gateway/test_wait_for_mcp_discovery.py +++ b/tests/tui_gateway/test_wait_for_mcp_discovery.py @@ -25,7 +25,7 @@ def test_no_thread_is_noop(): entry._mcp_discovery_thread = None start = time.monotonic() entry.wait_for_mcp_discovery(timeout=5.0) - assert time.monotonic() - start < 0.1 + assert time.monotonic() - start < 1.0 # fast path; loose bound for loaded runners finally: _restore_thread_slot(saved) @@ -40,7 +40,7 @@ def test_already_finished_thread_is_noop(): entry._mcp_discovery_thread = t start = time.monotonic() entry.wait_for_mcp_discovery(timeout=5.0) - assert time.monotonic() - start < 0.1 + assert time.monotonic() - start < 1.0 # fast path; loose bound for loaded runners finally: _restore_thread_slot(saved) @@ -71,7 +71,7 @@ def test_hung_thread_is_bounded_by_timeout(): start = time.monotonic() entry.wait_for_mcp_discovery(timeout=0.3) elapsed = time.monotonic() - start - assert 0.25 <= elapsed < 1.0 # bounded near the timeout, not forever + assert 0.25 <= elapsed < 3.0 # bounded near the timeout, not forever assert t.is_alive() # thread still running; we did not block on it finally: stop.set() diff --git a/ui-tui/src/__tests__/terminalSetup.test.ts b/ui-tui/src/__tests__/terminalSetup.test.ts index 59e725e98877..a9664a6dd45b 100644 --- a/ui-tui/src/__tests__/terminalSetup.test.ts +++ b/ui-tui/src/__tests__/terminalSetup.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { configureDetectedTerminalKeybindings, @@ -9,6 +9,19 @@ import { stripJsonComments } from '../lib/terminalSetup.js' +// Tests run from developer shells as well as CI. An inherited SSH_* variable +// must not silently force every configure call down the remote-session reject +// path; remote behavior is tested explicitly with per-call env objects below. +beforeEach(() => { + vi.stubEnv('SSH_CONNECTION', '') + vi.stubEnv('SSH_TTY', '') + vi.stubEnv('SSH_CLIENT', '') +}) + +afterEach(() => { + vi.unstubAllEnvs() +}) + describe('terminalSetup helpers', () => { it('detects VS Code family terminals from environment', () => { expect(detectVSCodeLikeTerminal({ CURSOR_TRACE_ID: 'x' } as NodeJS.ProcessEnv)).toBe('cursor')