elide() already returns short text unchanged, so the eval runner's length
guard was a second copy of the same check. The timeout-diagnostic test
read its log as utf-8-sig although the writer never emits a BOM, and it
interrupted a stub child that never ran.
Keeps #122392's test_timeout_diagnostic_marks_long_goal_as_non_original,
moved into the existing timeout-diagnostic test file and using its
fixture/stub instead of a new file. Red on base (bare marker).
Co-authored-by: Halldrix <12357213+Halldrix@users.noreply.github.com>
Fresh fix for #119975 (PR #119993 was deleted; nothing to salvage).
Three paths collapsed into app_not_running with the sentence '<slug> is
not running. Start <slug>': (1) a server_json probe with the app running
AND its endpoint PRESENT — the exact case from the report, where the only
thing missing is Hermes' own MCP connection; (2) every non-server_json,
non-interactive_session liveness kind; (3) static and unregistered
liveness, which cannot observe the app at all yet still claimed it was
stopped. Telling the user to start an app that IS running is the wrong
instruction.
Add a hermes_not_connected LivenessState ('<app>'s MCP connection is
missing. Reconnect <app> in Hermes, then try again.'), map the
running+endpoint-present branch and the static/unknown fallback to it, and
wire it through the TUI gateway contract (PluginServerState) and the
Desktop Plugins tab (AgentPluginServerState, SERVER_TONE, serverStates
i18n in en/de/es/fr).
Also stop composing the sentence from the declaration's slug: describe()
takes an optional display_name, and _plugin_server_rows passes the
curated catalog title (fallback: the manifest name) so the Plugins tab
reads 'NVIDIA App' instead of a raw server slug.
Fixes#119975
An OpenAI-SDK-shaped transcription response can be a structured object whose
``text`` is None and whose ``error`` carries the provider's failure. Every
caller fell back to ``str(transcription)``, so the object repr —
``Transcription(text=None, logprobs=None, usage=None, error='Transcription failed')``
— was logged as a successful transcript and returned in the
``{"success": true, "transcript": ...}`` envelope. Desktop conversation mode
then injected that repr as the user's message instead of the audio.
``_extract_transcript_text`` now raises ``STTResponseError`` (a ``ValueError``)
for any structured response — SDK object or JSON dict — with a missing or
non-string ``text``: the provider's ``error`` when there is one, else
"Transcription response contained no text". ``_with_openai_client`` and
``_cloud_failure`` surface that message verbatim, so the openai, groq,
deepinfra, mistral, xAI and ElevenLabs paths all return their existing failure
envelope instead. ``_transcribe_groq`` uses the shared normalizer rather than
its own ``str(transcription)``. Plain strings, objects/dicts with a string
``text`` (including ``""``, so silence stays non-fatal) and unknown scalars are
unchanged; only the repr fallback for structured responses is gone. No desktop
change is required.
Fixes#78098
An MCP tool result reaches the model as the handler envelope
`{"result": <text>, ...}` (tools/mcp_tool_handlers.py::_render_call_tool_result) so
structured metadata survives inline delivery. When that string crossed the persistence
threshold it was written to $HERMES_HOME/cache/spillover verbatim, so a ~200 KB document
landed on ONE line with every newline escaped (`\n`), making the read_file offset/limit
pagination the <persisted-output> block recommends unusable.
maybe_persist_tool_result now unwraps that envelope before persisting: the spill file and
the preview carry the model-facing text with real newlines. The envelope is recognized by
SHAPE (a JSON object whose keys are a subset of {"result", "structuredContent", "_meta"}
with a non-empty string "result") rather than by tool name, so opaque JSON from any other
tool is still persisted verbatim -- and the aggregate path is covered too, since
enforce_turn_budget persists under __budget_enforcement__ where a `mcp__` prefix test would
miss exactly the results it has to fix. Sibling members (structuredContent/_meta) are
appended after the text in a delimited metadata block instead of being dropped: they are
the payloads _render_call_tool_result keeps for the model on purpose (#115430), and the
spill file is the only copy left once the envelope is replaced by the preview.
Fixes#90426
`_prepend_path` inserted the resolved command's directory only when it was
absent from the child's PATH. The Hermes installer appends its managed Node
dir to the user PATH, so for anyone with a system Node (<22.12) earlier on
PATH the check no-oped and the managed dir stayed behind it. npm lifecycle
children (`node install.js`) then resolved the older system Node and failed
with ERR_REQUIRE_ESM even though Hermes had provisioned a compatible runtime.
Strip every existing case/trailing-separator variant of the directory first,
then prepend it, so the canonical entry is the one that wins and PATH does
not grow duplicates.
Fixes#82309
A `terminal(background=true, heartbeat=N)` tick queued a notification every N seconds
whether or not the process had printed anything, and every queued event costs the owning
session a full model turn. On Desktop and the TUI that turn painted the wake as a user
bubble ("[Background process ... heartbeat #9 ... (no new output since the last
heartbeat)]") followed by the model's "Still running normally." — over and over, for a
process whose row on the status stack already said it was running — and while the wake
held the session's turn, the user's own prompt sat queued behind it.
- `ProcessRegistry._emit_heartbeat` skips a tick with no new output. The sequence counts
delivered beats only; the "(no new output)" placeholder in the formatter is gone.
- TUI/Desktop type heartbeat rows `display_kind: hidden` (the kind both clients and the
transcript preview already honour); the CLI paints a one-line receipt and persists the
row hidden, so reopening the session in Desktop shows only the agent's reply.
- Desktop hydration drops heartbeat rows persisted by older backends the same way.
- `display.background_process_notifications: off` is honored by the TUI/Desktop poller and
the CLI drain, not just the messaging gateway. `off` mutes process-driven wakes only:
a finished `delegate_task(background=true)` still lands.
Supersedes #123123 (cherry-picked; scoped so `off` keeps subagent results) and #119202
(cherry-picked; `heartbeat: 0` is schema-valid so models that materialize every field
stop tripping the foreground guard).
Windows has no POSIX parent-death supervisor/killpg safety net, so an
ungraceful exit of the hermes process left every stdio MCP child tree
(npx.cmd -> node.exe) running as orphans with ParentId=null, piling up
across session restarts.
- _run_stdio now attaches the process to a KILL_ON_JOB_CLOSE job object
before spawning stdio children (self-guarded no-op off Windows), so the
whole child tree dies with the parent at the kernel level.
- Windows reaps kill the process tree (direct child + descendants) in the
lifecycle orphan sweep and the spawn-ledger startup sweep, where there
is no pgid to group-kill.
Fixes#61059
A volume that already owns /workspace skipped the configured working
directory, so tools treated that host path as unmounted. Bind it at a
second mount, or point tools at the volume that already has it, for any
drive path.
Desktop paste/file attachments land in Hermes-managed staging dirs on the
GATEWAY (composer-pastes/ for large text pastes, attachments/ for dropped
files), but on the Remote SSH topology the workspace root (TERMINAL_CWD) is a
path on the SSH HOST - the two filesystems are fully disjoint, as the issue
thread confirms. Two gaps combined to reject every staged attachment with
"path is outside the allowed workspace":
- _resolve_path admitted only allowed_root + composer-paste roots, so a
gateway-staged attachments/ path was refused outright. Admit the
_CACHE_DIRS staging roots (attachments/, images/, cache/*, composer-pastes/)
via a helper that asks get_cache_directory_mounts - the gateway's OWN
payload is never a workspace escape, and the path-traversal and
credential-deny guards in _ensure_reference_path_allowed still run after.
Anything else outside the workspace stays blocked.
- composer-pastes/ was missing from _CACHE_DIRS, so its bytes never reached
the remote: ssh/daytona/vercel_sandbox sync via iter_sync_files ->
iter_cache_files, and to_agent_visible_cache_path only translates mounted
dirs - a paste attached on a fresh session dangled on the remote host.
Tests cover the disjoint-filesystem SSH topology end-to-end (text inlines,
binary renders the synced ~/.hermes path), the still-refused stranger path,
local-backend unchanged, and the composer-pastes mount+sync enumeration.
Consolidates PR #110387 by Finn763 (the _agent_staged_path guard widening and
the SSH-topology tests, adapted to the current _ensure_reference_path_allowed
ordering) with PR #103412 by ericmaddox (whose mapping insight is subsumed by
the _CACHE_DIRS entry, which fixes both the sync and the translation).
Co-authored-by: ericmaddox <ericmaddox@users.noreply.github.com>
A Desktop Docker session that registers an absolute host directory
such as /mnt/... or /srv/... as its cwd skipped the /Users|/home/
drive-letter heuristic, so the mount check never ran and commands
were wrapped with cd to that host path. Classify the mounted host
directory as unusable before that heuristic, and remap it to
/workspace in the live-env write and the per-command resolver.
Checkpoint refresh treated a failed start-time check as a collected exit
and queued a completion. A live PID whose start time matches, or whose
start time cannot be read, stays running. A reused PID is closed without
being signalled. A gone PID is pruned. A completion is emitted only when
an exit status was collected.
Existing directories were reported as success:true while the preview
pane opened nothing. Fail closed with an explicit error and do not
emit preview.open. HTTP(S) URLs and regular files are unchanged.
A shell with `set -x` (user rc, BASH_ENV) traces `+ echo <sentinel>` into
the merged output. That line is an extra separator for _split_segments, so
the segment count mismatched and read_file_raw (the V4A/replace write-back
source) failed with "Failed to read file".
_fenced_read now turns xtrace off before the fence; `set +x`'s own trace
goes to the group's discarded stderr.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
'.../.ssh/link/' split to an empty basename, so the entry check degenerated
to checking the link's TARGET: get_write_denied_error(entry=True) and
is_protected_path(follow=False) let the delete remove a link inside ~/.ssh,
and _resolve_entry_for_task fell back to full resolution, so a V4A
'*** Delete File: dir/link/' deleted the file the link points to (the
original bug, trailing-slash form).
split_entry() drops trailing separators (keeping a bare '/' or drive root)
before the parent/leaf split and is used by all three entry-mode sites.
is_protected_path(follow=False) now normcases the joined entry, not only
its parent, so a case-variant spelling of the exe/venv entry still matches
on Windows.
The previous fold guarded each Delete/Move entry by running
get_write_denied_error on dirname(path). That coordinate is wrong both ways:
- runtime self-protection treats ANCESTORS of the running venv/interpreter
as protected, so a plain file directly in ~, ~/.hermes, the checkout root
or the uv python dir could no longer be deleted or moved ("'/Users/x' is a
protected system/credential file");
- credential-dir prefixes end in os.sep and match via startswith, so the
bare dir ~/.ssh never matched and a link directly inside ~/.ssh, ~/.aws,
~/.gnupg, ... was unlinked/renamed.
The existing classifier gains an entry=True mode (get_write_denied_error /
_classify_write_denial, and is_protected_path(follow=False)) that vets
realpath(parent)/basename — the entry, leaf not dereferenced — in addition
to the resolved target. A file inside a protected dir or prefix is denied;
a file merely beside the venv is allowed. delete_file and move_file make
one entry-mode call per entry instead of the duplicated path+dirname loop.
The new parametrized test covers both directions (plain Delete/Move next to
a monkeypatched runtime venv succeeds; a link in <home>/.ssh is refused with
link and target intact); all four cases fail on the previous fold head.
Delete and Move remove or rename the directory entry itself (a symlink,
not its target), but get_write_denied_error realpaths its argument, so it
only ever vetted the link's target. A link outside HERMES_WRITE_SAFE_ROOT
(or inside ~/.hermes/sessions) pointing at a file inside the root passed
the guard and the link was deleted/renamed outside the allowed area.
delete_file and move_file now also run the same guard on each entry's
parent directory, which realpaths to where the entry really lives.
The symlink test gains a safe-root case (red without this change), and
its Move cases always assert success, the rename, the link target and
files_modified instead of tolerating a refusing move primitive; expected
values are parameters rather than header introspection, and json is a
module-level import.
patch_tool rewrites every V4A header to the path _resolve_path_for_task
returns, and on a host backend that is Path.resolve(), which follows a
symlink in the last component. For Update/Add that is harmless: the shell
layer reads and writes the target through the link either way. Delete and
Move act on the directory entry, so "*** Delete File: config/local.yaml"
(a link to base.yaml) deleted base.yaml and left the link dangling, and
"*** Move File: current.txt -> previous.txt" renamed the link's target,
both reported as success.
Delete headers and both Move endpoints now resolve their parent directory
only (_resolve_entry_for_task), keeping the final component, so the link
is removed or renamed. The same paths are locked and reported in
files_modified. Update/Add headers are unchanged.
(cherry picked from commit 7b1fe43d30db6015155349b67ca814b340df0e18)
The kernel eviction on a failed cell ship and the raise in _execute_checked
had no test teeth: removing either left the suite green. Extend the existing
shared-host lockdown test (no new test functions) so a failed cell ship must
raise and empty the registry, and a failed dir setup must spawn nothing and
ship nothing.
Also reuse the loop's parsed sandbox.env ship/token in the per-call lockdown
test instead of re-parsing it, and fix a comment that still described the
removed pipe-vs-heredoc stdin branch.
Same fake-env gap as the file-RPC Shell: remote writes now always pass
stdin_data, and the shared RunToCompletionEnv fixture rejected the kwarg.
That made test_rpc_carries_authority_to_real_dispatch[remote-file] fail
with TypeError at the stack head. Accept stdin_data and feed it as the
subprocess input, matching BaseEnvironment.execute's contract.
The lockdown tests pinned literal substrings ("( set -a", "exec python3",
"umask 077", "chmod 700"), which is change-detection. Replay the recorded
commands through a real bash under a temp root instead: dirs come out
0700 and the archive 0600, the sourced env reaches the child, the
child's exit code is the command's, and the token does not leak into the
outer shell (the session-snapshot class). The token-never-in-argv
assertions stay. No new test functions; the replay is skipped on
Windows, where the stdin/argv invariants are still checked.
_remote_write branched on getattr(env, "_stdin_mode", "pipe") and only
passed stdin_data on pipe backends, echoing base64 into argv elsewhere.
BaseEnvironment.execute already embeds stdin_data as a heredoc for
heredoc-mode backends (modal/daytona/vercel), and managed_modal forwards
it as stdinData; _write_to_sandbox already relies on that for every
backend. The branch duplicated base-class logic, and its defensive
getattr default meant a fake env with neither _stdin_mode nor a
stdin_data parameter raised TypeError on every RPC response write. The
poll loop swallowed the error, so no res_* file appeared and
test_code_execution_file_rpc hung forever (it passes on base).
Collapse to one path that always passes stdin_data, and teach the
file-RPC Shell fake to accept it and feed it as input. ScriptedEnv no
longer needs its _stdin_mode stub.
Keep one invariant per spawn path (persistent remote kernel and per-call
sandbox): token never on argv, dirs owner-only. Drop the command-string
duplicates, the malformed-seq replay test (the guard itself stays in
tools/code_execution_rpc.py) and the real-fs E2E class. The dropped
st_mode == 0o600 assertions were the Windows-failing ones flagged in
review, so no POSIX guard is needed on what remains.
On shared remote backends the execute_code channel created kernel and
sandbox dirs under shared temp at the process umask (775 group-writable
under umask 002), wrote request/result files group-readable, and carried
HERMES_RPC_TOKEN on remote command lines where co-tenant users read argv
via ps for the whole run. A co-tenant could read tool arguments and
results, and on group-writable dirs forge RPC requests dispatched under
the user's approval context.
- All remote dirs are created owner-only (umask 077 + chmod 700, checked
fail-closed) and every Hermes file write is mode 600.
- The token travels in a sourced env file inside a subshell so the vars
never enter the backend's session-snapshot dump, and ships via stdin on
pipe-capable backends so it never enters argv at all.
- The RPC poll loop rejects non-int seq requests before dispatch instead
of replaying them every cycle.
- tool_result_storage gets the same owner-only treatment for archived
tool output.
(cherry picked from commit aef21731d7fb8a4e0a6ada4ff9889264df4a8893)
The test monkeypatched reset_skill_view_dedup and asserted it was called,
so it only guarded the since-removed skills= kwarg rather than behaviour.
Keeps the stack within its +2 test budget; the micro-compaction splice and
native-checkpoint re-arm tests remain as the behavioural guards.
skill_view returns a stub on a repeat view of an unchanged file, pointing
the model at the earlier full result in its own context. That contract only
holds while the earlier result is still there, so a compaction boundary has
to advance the dedup generation -- which is what _reset_read_dedup_caches()
is for, and why its stub message promises "Re-issued after context
compression, this returns the full content again."
The Codex app-server path passed skills=False and reset only the file-read
half. A skill re-viewed after that compaction got a stub naming content the
compaction had just summarised away, so the model had nothing to copy from
and reconstructed it from memory instead. Observed with a delegating skill
whose child contract is a JSON schema: successive dispatches shipped
progressively degraded schemas -- a dropped allOf, then a bare
{"type": "array"} -- until the generation-time check no longer constrained
anything and the work was abandoned.
The opt-out carried the pre-refactor asymmetry forward mechanically
(0e9d46511a lifted these call sites without changing behaviour); nothing
depends on it. Drop it so both paths reset both caches. Tests cover the
helper and drive the codex path end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: aurorabotticus-svg <aurorabotticus-svg@users.noreply.github.com>
(cherry picked from commit 22f36fe10ec27bf68e003058b3f1dda292e535e8)
A dispatcher SIGKILLed between _call_spawn_fn and _set_worker_pid leaves a
live worker on a run with worker_pid NULL. release_stale_claims only extends
an expired claim for a recorded live pid, so on TTL expiry it reclaimed the
card and spawned a second worker beside the first: double billing, double
side effects, and a board showing one clean completed run (the first
worker's kanban_complete is refused as stale). Main CI hit it in
test_dispatcher_sigkill_mid_tick_never_destroys_or_duplicates_cards.
The worker now records its own pid on its run before the first model call
(adopt_worker_pid, worker_registered event, host-local claims only) and
exits without working the card when its run was already reclaimed. The
reclaim UPDATE also compares worker_pid so a registration landing between
the stale-claim SELECT and the UPDATE keeps the claim.
Repro: temporary sleep between spawn and pid record + kill 0.2 s after the
spawned event + slow first model reply -> 4/4 red on main with the CI
signature, 8/8 green here.
Fixes#121556
The LocalEnvironment payload-mode test passed on base: it exercises
base.py/local.py, which this stack does not change, so it guarded nothing
here. The Daytona/Vercel staged-stdin transport had no coverage at all.
Swap it for one Vercel test on the existing fake SDK. It checks that the
160 KiB + NUL/0xFF payload is uploaded byte-exact with mode 0o600, never
appears in run_command argv, and that the script starts with the
exec-redirect prefix. It fails on base, where the payload is dropped. Also
drop the change-detector _stdin_mode assert from the Modal test; the
assertions after it already prove the behaviour. The stack still adds two
tests (Modal + Vercel).
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
Keep the Modal SDK-stdin test and the payload-mode byte-exact write test;
drop the Daytona/Vercel staging/cancel tests and the compound-read test
carried by #122218 (duplicative, and the cancel tests pinned the lifecycle
that the follow-up commits replace). The pre-existing Vercel cancel test
covers the kill path.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
discover_mcp_tools binds the owner secret scope only around the config load (#113746), so a
routed profile's reconciliation ran with no ambient scope. The adopter's stdio identity then
resolved unscoped, and with a source-tagged secret name get_secret raised UnscopedSecretError:
the stack's None sentinel kept that safe (the share was refused) but two profiles holding the
same value never shared the owner's child.
The omitted-name config load and the per-name identity resolution now run under this profile's
own secret scope (_owner_secret_scope), outside the registry lock.
Salvage resolution: the out-of-lock, once-per-name resolution, the None refuse sentinel and the
per-server refusal were already on the stack (_adopter_identity_digest / resolved_ids), so this
keeps that one implementation and takes the contributor's scope binding. The contributor's
unscoped-routed test is folded as an assertion into the kept multi-credential test (test budget);
their 'one unresolvable identity refuses only that share' test duplicates the kept 'boom' case and
is dropped, as are the test tweaks written against their _resolved_identity signature.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
(cherry picked from commit a8627071e7367cd544af77f921b4403a0b6c3e36)
The adopter resolves each foreign-held name's connection identity before
taking the registry lock, and a resolver failure is caught per server so
it refuses adoption of that server alone. Nothing pinned that: dropping
the per-server except would let one broken secret backend abort discovery
for the whole scope and stop every healthy shared connection from being
adopted. Extend the existing credentials test so a raising resolver for
one server leaves a healthy same-identity connection adopted.
The module function `_resolved_identity(name, config)` (adopter
recomputation) shared its name with the attribute
`server._resolved_identity` (owner's published digest) and the
`resolved_identity` kwarg; a gateway test monkeypatched the function
while its fake set the attribute, which read as one thing. Renaming the
function to `_adopter_identity_digest` makes the owner/adopter split
visible at every call site.
The stack's shape gate allows two invariant tests. Keep the real
two-profile gateway tests (secret-source env on stdio, identity_header
value_from: profile on HTTP) and drop the unit-level npx/runtime-file/
published-digest and ssl_verify/strict_redirect_headers parametrized
tests; the fixture updates that make existing multiplex tests publish an
owner identity stay.
Both are consumed by the HTTP transport but were in neither config_fingerprint nor
_connection_identity, so a profile whose config differs only in TLS verification or
redirect-header policy adopted another profile's live connection under that profile's policy.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
(cherry picked from commit 8b7175a2cf0e1935ea77805db9b99e4cba9fdcea)
The connecting loop hashed the resolved inputs, then the transport resolved them again; a runtime-file rotation between the two reads published endpoint A's digest alongside a session opened with B. The transport now resolves once per attempt (_http_endpoint / _stdio_launch), connects with that value and publishes its digest; the adopter recomputes through the same resolvers. The loop clears the digest per attempt.
(cherry picked from commit f543a4d3aadcdf7ec10093ebb99e14a740d43494)
The cross-profile identity digest covered config headers, identity_header,
secret-source env and cwd, but not two more per-profile resolutions the
transport performs: _run_http() swaps in a server_json live endpoint (URL and
bearer token) read from the active profile, and _run_stdio() resolves a bare
npx/npm/node through _resolve_stdio_command(), which can land under the
profile's own HERMES_HOME. Two profiles with those differences hashed equal
and could share one live connection.
_resolved_identity now resolves both exactly as the transport does. The new
test module imports the repository YAML module and passes explicit encodings
so it collects under scripts/run_tests.sh and clears the Windows-footgun gate.
(cherry picked from commit 637872d55bc81609657ed82139546709f6b99ccd)
_same_server_route judged "same credentials" from the static config alone, but a
connection is also opened with per-profile values the config never shows: a stdio
child's env carries every external secret-source value (Bitwarden, 1Password,
secrets.command) from the owner's secret scope, identity_header value_from: profile
resolves to the owner's profile name, and a stdio child's default cwd is the owner
session's runtime cwd. A multiplexed profile with a byte-identical config therefore
adopted the owner's live connection and its MCP calls ran as the owner.
The connecting task now records a SHA-256 of those resolved inputs on every connect
attempt, in its own scope; cross-profile adoption and the stale-overlay check
recompute it under the adopter's scope and refuse to share on a mismatch. Only the
hash is kept. Same-profile checks are unchanged.
(cherry picked from commit e59d0dfb9918db03482ccd4bf5a7599efc3cd72e)
The remote guard contract only pinned refusal of an existing target-side
binary. Nothing pinned the other half: a path the backend proves absent must
stay writable, or a guard that over-reports "exists" (e.g. treating a
"missing" probe as present) would block every new .pdf/.sqlite on Docker/SSH
without a red test. Extend the kept T1 test instead of adding a function
(test budget); verified it fails 6/6 under a missing->exists mutation.
The substitution pairs were built by looping over a one-element tuple (left
over from a multi-pair version), and _bash_safe() only existed to do a
function-local import from a module already imported at the top. Import
_bash_safe_path directly and build the three pairs as a literal list.
Keep the two contracts that pin #122662: (1) write_file / patch replace /
V4A update against a binary that exists only on an unhinted non-local
backend (VercelSandboxEnvironment, scoped config claiming 'local') is
refused with bytes untouched, for .sqlite and .pdf; (2) a probe transport
failure (raise or error return) fails closed. Creation-allowed, locality
and tilde-fallback cases are covered by the first invariant's setup or by
tests/tools/test_binary_document_write_guard.py, and the salvage shape
caps a stack at two invariant tests.