fix(update): a contended Windows venv is never mutated — failed shim quarantine refuses instead of warning (#87331) - #92617
Merged
Merged
Conversation
…ne now refuses instead of warning (#87331) The #87331 remaining half: when hermes.exe (or a sibling shim) could not be renamed aside, the updater printed a warning and ran the installer anyway — which died partway on the same locks and stranded the venv between versions. - _run_quarantined_install gains strict_quarantine: any shim whose rename failed every retry aborts BEFORE the install command runs (successful renames rolled back), raising ShimQuarantineError. - The update dependency sync passes strict_quarantine=True. The update boundary turns the error into a refusal: defer via the update-incomplete marker, exit 2 (recorded as refused by the receipt net), never ZIP-fallback. Post-sync repair installs keep warn-and-try (their venv is already mutated; refusing buys nothing). - The recovery installer (_install_repair._run_install_cmd) is strict unconditionally: marker survives, next launch retries after the holder exits. - Live Windows E2E for the wine2e lane: a real child holds hermes.exe without FILE_SHARE_DELETE (the exact field lock shape), strict path refuses with zero installer invocations, releases roll back, and the same path proceeds once the holder exits. Sabotage-verified: reverting the strict wiring makes both fail-closed tests fail.
teknium1
force-pushed
the
hermes/hermes-83bfdb1e
branch
from
August 23, 2026 01:44
6f531d5 to
2096868
Compare
૮ >ﻌ< ა ci reviewran on a5d46a2 — test(bedrock): make the botocore stub windows airtight — kil
|
alt-glitch
added
type/bug
…env open (' as bare open()
…ndored-import flake CI flake mechanism (PR #92617 red, reproduced standalone): tests plant fake botocore modules via patch.dict; when the REAL botocore.exceptions is first imported in an interpreter state where a fake parent is (or was) installed, its 'from botocore.vendored import requests' resolves against a module with no __path__ and every exception test in the worker dies with "No module named 'botocore.vendored'" — ordering-dependent, so green locally, red in CI workers. Defenses (both, in depth): - test_bedrock_adapter.py pre-imports the real botocore.exceptions at module scope, before any test can stub sys.modules — later imports are cache hits that can never re-execute the vendored import under a poisoned parent. Proven standalone: fake-parent repro fails without the pre-import, succeeds with it. - autouse _boto_sys_modules_hygiene fixtures in all three files that plant fake boto* modules (adapter, integration, model-picker): snapshot every boto* sys.modules entry before each test, evict+restore after — no stub window can leak state into a later test regardless of worker ordering. - importorskip targets botocore.exceptions (the module the tests actually need) instead of bare botocore, so a torn install skips instead of erroring. 148/148 across the four affected suites.
Collaborator
Author
|
Also on this branch (it redded this PR's first CI run): the bedrock test flake is now fixed at the class level — commit a5d46a2. Mechanism reproduced standalone: tests plant fake botocore modules; the real |
This was referenced Aug 23, 2026
teknium1
added a commit
that referenced
this pull request
Aug 23, 2026
Sibling-test blast radius from #92617: the salvaged fixture's fake _run_quarantined_install predates the strict_quarantine kwarg the update sync now passes.
teknium1
added a commit
that referenced
this pull request
Aug 23, 2026
Sibling-test blast radius from #92617: the salvaged fixture's fake _run_quarantined_install predates the strict_quarantine kwarg the update sync now passes.
melon-xf
added a commit
to melon-xf/hermes-agent
that referenced
this pull request
Sep 3, 2026
…ndored-import flake CI flake mechanism (PR NousResearch#92617 red, reproduced standalone): tests plant fake botocore modules via patch.dict; when the REAL botocore.exceptions is first imported in an interpreter state where a fake parent is (or was) installed, its 'from botocore.vendored import requests' resolves against a module with no __path__ and every exception test in the worker dies with "No module named 'botocore.vendored'" — ordering-dependent, so green locally, red in CI workers. Defenses (both, in depth): - test_bedrock_adapter.py pre-imports the real botocore.exceptions at module scope, before any test can stub sys.modules — later imports are cache hits that can never re-execute the vendored import under a poisoned parent. Proven standalone: fake-parent repro fails without the pre-import, succeeds with it. - autouse _boto_sys_modules_hygiene fixtures in all three files that plant fake boto* modules (adapter, integration, model-picker): snapshot every boto* sys.modules entry before each test, evict+restore after — no stub window can leak state into a later test regardless of worker ordering. - importorskip targets botocore.exceptions (the module the tests actually need) instead of bare botocore, so a torn install skips instead of erroring. 148/148 across the four affected suites.
melon-xf
added a commit
to melon-xf/hermes-agent
that referenced
this pull request
Sep 3, 2026
Sibling-test blast radius from NousResearch#92617: the salvaged fixture's fake _run_quarantined_install predates the strict_quarantine kwarg the update sync now passes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A Windows update can no longer mutate a venv it failed to quarantine — the "warn and install anyway" path that stranded installs half-updated is now a clean refusal that defers to the next launch (#87331's remaining half; the ZIP-fallback destruction legs died in #92013).
Root cause: when
hermes.exe/sibling shims could not be renamed aside (a process holds them withoutFILE_SHARE_DELETE),_quarantine_running_hermes_exeprinted a warning and the installer ran anyway — then died partway on the same locks, leaving the venv between versions (3 field occurrences on one machine).Changes
hermes_cli/main.py:_run_quarantined_install(strict_quarantine=True)— a shim whose rename failed every retry raisesShimQuarantineErrorBEFORE the install command runs, with successful renames rolled back. The update dependency sync (_install_python_dependencies_with_optional_fallback) is strict; post-sync repair installs keep warn-and-try (their venv is already mutated — refusing buys nothing).hermes_cli/update_cmd.py: boundary handling — the error becomes a refusal: update-incomplete marker written, exit 2 (receipt net recordsrefused), explicit no-ZIP-fallback. Covers both the git path and the sync inside the ZIP handler.hermes_cli/_install_repair.py: the marker-recovery installer (_run_install_cmd) is strict unconditionally — marker survives, next launch retries after the holder exits.Validation
hermes.exewith noFILE_SHARE_DELETE— the exact field lock shapeInfographic