mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(ci): review fixes — PR-read-only buildx cache, per-arch profile labels
Address review findings on the ARC migration: - docker.yml: WIF auth (and therefore Artifact Registry cache WRITES) now only runs on non-PR events. The build job runs PR-controlled code and the publish job reads the same buildcache ref, so a PR-writable cache was a layer-poisoning vector. PRs of any origin keep cache READS via the runner pod's GKE Workload Identity — that's where the 15min -> 2-3min win comes from; main pushes repopulate writes. - docker.yml: profile label is now docker-tests-<arch>. Both matrix legs uploaded resource-profile-docker-tests; upload-artifact v4+ rejects the duplicate and continue-on-error swallowed it, silently dropping one arch's profile. - actions/profile: run the wrapped command with bash -eo pipefail to match normal `run:` step semantics (a failing `source .venv/...` must fail the step, not fall through). - js/e2e/site workflows: bake node22 into the node_modules cache keys so a future node-version bump can't restore stale native builds (node-pty, electron postinstall) against an unchanged lockfile. - test_container_restart_stale_pid: forward deadline_s/interval_s to wait_for_log instead of silently dropping them. - doctor.py: refresh a stale comment on the in-container docker branch.
This commit is contained in:
parent
304aae33cf
commit
6c21f63c96
9 changed files with 44 additions and 20 deletions
5
.github/actions/profile/action.yml
vendored
5
.github/actions/profile/action.yml
vendored
|
|
@ -37,7 +37,10 @@ runs:
|
|||
env:
|
||||
_CMD: ${{ inputs.command }}
|
||||
run: |
|
||||
bash -c "$_CMD"
|
||||
# -e / pipefail: match the semantics of a normal `run:` step
|
||||
# (bash -e {0}) so a failing early line (e.g. `source .venv/...`)
|
||||
# fails the step instead of silently running the rest.
|
||||
bash -eo pipefail -c "$_CMD"
|
||||
|
||||
- name: Stop profiler and collect results
|
||||
id: stop-profiler
|
||||
|
|
|
|||
4
.github/workflows/deploy-site.yml
vendored
4
.github/workflows/deploy-site.yml
vendored
|
|
@ -71,7 +71,9 @@ jobs:
|
|||
id: npm-cache
|
||||
with:
|
||||
path: website/node_modules
|
||||
key: node-modules-cache-${{ hashFiles('website/package-lock.json') }}
|
||||
# node22: native builds + engine checks are node-major-specific;
|
||||
# keep the key in sync with setup-node's node-version above.
|
||||
key: node-modules-cache-node22-${{ hashFiles('website/package-lock.json') }}
|
||||
|
||||
- name: Install website dependencies
|
||||
uses: ./.github/actions/retry
|
||||
|
|
|
|||
24
.github/workflows/docker.yml
vendored
24
.github/workflows/docker.yml
vendored
|
|
@ -34,15 +34,15 @@ jobs:
|
|||
# Buildx layer cache lives in Artifact Registry (us-central1, same region
|
||||
# as the ARC runners) instead of GitHub's cache CDN. Reads are keyless via
|
||||
# GKE Workload Identity on the runner pods; writes are keyless via GitHub
|
||||
# OIDC -> GCP WIF (google-github-actions/auth). Fork PRs get neither an
|
||||
# id-token nor pod WI outside our cluster, so they build cache-cold —
|
||||
# same behavior type=gha gave them.
|
||||
# OIDC -> GCP WIF (google-github-actions/auth) and happen ONLY on trusted
|
||||
# main-push/release contexts — PR builds of any origin are read-only so
|
||||
# PR-controlled code can never write cache layers the publish job reads.
|
||||
build:
|
||||
if: github.repository == 'NousResearch/hermes-agent'
|
||||
permissions:
|
||||
contents: read
|
||||
# OIDC token for WIF cache writes. Fork-PR runs receive no id-token;
|
||||
# the auth step below is skipped for them.
|
||||
# OIDC token for WIF cache writes — only minted on non-PR events
|
||||
# (see the gcp-auth step); PR runs stay secret-free.
|
||||
id-token: write
|
||||
strategy:
|
||||
fail-fast: false
|
||||
|
|
@ -76,13 +76,15 @@ jobs:
|
|||
if: steps.buildx.outcome == 'failure'
|
||||
uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4.2.0
|
||||
|
||||
# Keyless GCP auth via GitHub OIDC -> WIF. Fork PRs have no id-token
|
||||
# (permissions are read-only in their context), so this step is
|
||||
# skipped and the build runs cache-read-only via the pod's GKE
|
||||
# Workload Identity.
|
||||
# Keyless GCP auth via GitHub OIDC -> WIF. PR builds (fork or
|
||||
# same-repo) get NO write token: this job runs PR-controlled code
|
||||
# and the publish job reads the same cache ref, so a PR-writable
|
||||
# cache would be a layer-poisoning vector. PRs read the cache via
|
||||
# the pod's GKE Workload Identity; writes happen only on trusted
|
||||
# main-push/release contexts.
|
||||
- name: Authenticate to GCP (WIF, cache writes)
|
||||
id: gcp-auth
|
||||
if: github.event.pull_request.head.repo.fork != true
|
||||
if: github.event_name != 'pull_request'
|
||||
uses: google-github-actions/auth@7c6bc770dae815cd3e89ee6cdf493a5fab2cc093 # v3.0.0
|
||||
with:
|
||||
project_id: hermes-agent-github-actions
|
||||
|
|
@ -158,7 +160,7 @@ jobs:
|
|||
# env -i into the parallel runner.
|
||||
uses: ./.github/actions/profile
|
||||
with:
|
||||
label: docker-tests
|
||||
label: docker-tests-${{ matrix.arch }}
|
||||
command: HERMES_TEST_WORKERS=8 HERMES_TEST_IMAGE="${{ env.IMAGE_NAME }}:test" scripts/run_tests.sh tests/docker/ --file-timeout 600
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
4
.github/workflows/docs-site-checks.yml
vendored
4
.github/workflows/docs-site-checks.yml
vendored
|
|
@ -21,7 +21,9 @@ jobs:
|
|||
id: npm-cache
|
||||
with:
|
||||
path: website/node_modules
|
||||
key: node-modules-cache-${{ hashFiles('website/package-lock.json') }}
|
||||
# node22: native builds + engine checks are node-major-specific;
|
||||
# keep the key in sync with setup-node's node-version above.
|
||||
key: node-modules-cache-node22-${{ hashFiles('website/package-lock.json') }}
|
||||
|
||||
- name: Install website dependencies
|
||||
uses: ./.github/actions/retry
|
||||
|
|
|
|||
4
.github/workflows/e2e-desktop.yml
vendored
4
.github/workflows/e2e-desktop.yml
vendored
|
|
@ -46,7 +46,9 @@ jobs:
|
|||
ui-tui/packages/*/node_modules
|
||||
web/node_modules
|
||||
tests-js/node_modules
|
||||
key: node-modules-full-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
# node22 in the key: node-pty/electron native artifacts are
|
||||
# node-major-specific; sync with setup-node's node-version.
|
||||
key: node-modules-full-node22-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
|
||||
# Full npm ci (not --ignore-scripts): electron's postinstall
|
||||
# downloads the binary we launch, and node-pty's native build is
|
||||
|
|
|
|||
3
.github/workflows/js-autofix.yml
vendored
3
.github/workflows/js-autofix.yml
vendored
|
|
@ -81,7 +81,8 @@ jobs:
|
|||
ui-tui/packages/*/node_modules
|
||||
web/node_modules
|
||||
tests-js/node_modules
|
||||
key: node-modules-noscripts-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
# node22 in the key: sync with setup-node's node-version.
|
||||
key: node-modules-noscripts-node22-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
|
||||
# --ignore-scripts: eslint only needs TS sources + eslint packages.
|
||||
- uses: ./.github/actions/retry
|
||||
|
|
|
|||
7
.github/workflows/js-tests.yml
vendored
7
.github/workflows/js-tests.yml
vendored
|
|
@ -32,7 +32,8 @@ jobs:
|
|||
tests-js/node_modules
|
||||
# noscripts flavor: MUST NOT share a key with full `npm ci`
|
||||
# installs (electron postinstall binary, node-pty build).
|
||||
key: node-modules-noscripts-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
# node22 in the key: sync with setup-node's node-version.
|
||||
key: node-modules-noscripts-node22-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
|
||||
- uses: ./.github/actions/retry
|
||||
with:
|
||||
|
|
@ -92,7 +93,9 @@ jobs:
|
|||
ui-tui/packages/*/node_modules
|
||||
web/node_modules
|
||||
tests-js/node_modules
|
||||
key: node-modules-full-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
# node22 in the key: node-pty/electron native artifacts are
|
||||
# node-major-specific; sync with setup-node's node-version.
|
||||
key: node-modules-full-node22-${{ runner.arch }}-${{ hashFiles('package-lock.json') }}
|
||||
|
||||
- uses: ./.github/actions/retry
|
||||
with:
|
||||
|
|
|
|||
|
|
@ -1758,7 +1758,10 @@ def run_doctor(args):
|
|||
elif _is_termux():
|
||||
check_info("Docker backend is not available inside Termux (expected on Android)")
|
||||
elif running_in_container:
|
||||
pass # already explained above
|
||||
# In-container with a non-docker backend: the info line above only
|
||||
# prints for the implicit `local` case, but either way a missing
|
||||
# docker binary inside the container is expected — stay quiet.
|
||||
pass
|
||||
else:
|
||||
check_warn("docker not found", "(optional)")
|
||||
|
||||
|
|
|
|||
|
|
@ -49,7 +49,13 @@ def _wait_for_reconcile_log_mention(
|
|||
) -> str:
|
||||
"""Poll until /opt/data/logs/container-boot.log mentions `profile`.
|
||||
"""
|
||||
return wait_for_log(container, "/opt/data/logs/container-boot.log", f"profile={profile}")
|
||||
return wait_for_log(
|
||||
container,
|
||||
"/opt/data/logs/container-boot.log",
|
||||
f"profile={profile}",
|
||||
deadline_s=deadline_s,
|
||||
interval_s=interval_s,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue