feat(tools): add Tenki cloud sandbox terminal backend - #64190
feat(tools): add Tenki cloud sandbox terminal backend#64190hashbender wants to merge 19 commits into
Conversation
Adds Tenki (tenki.cloud) as a seventh terminal execution backend alongside local, docker, ssh, singularity, modal, and daytona. Hermes creates Tenki sandboxes on demand for terminal, file tools, and execute_code, and terminates them on cleanup by default (opt-in pause/resume persistence via container_persistent: true). Core: - tools/environments/tenki.py: TenkiEnvironment — sandbox lifecycle, exec, pause/resume persistence, remote file sync-back - tools/tenki_config.py: profile-scope-aware auth/workspace/project/endpoint resolution from the Tenki CLI config or environment - Shared _container_config_from_env_config() helper replaces the three duplicated container-config dicts (terminal, file tools, execute_code) - Setup wizard, doctor, status, gateway, and CLI wiring; website docs, env-var reference, and cli-config.yaml.example - Optional tenki extra (tenki-sandbox==0.1.1), lazy-installed like modal/daytona Security & correctness hardening: - Do not inject the supervisor's control-plane Tenki token into the model-controlled guest env; host-side SDK auth is unchanged. Nested-sandbox creation is an explicit opt-in via terminal.tenki_forward_env (which also forwards the resolved token so `tenki login` credentials work), and logs a warning when the control-plane token is forwarded. - Resolve Tenki credentials and forwarded env through agent.secret_scope so an active profile scope wins over process-global os.environ and the shared machine CLI login is skipped when a profile scope is authoritative. - Strip TENKI_AUTH_TOKEN / TENKI_API_KEY from spawned subprocess environments (provider blocklist + always-strip tier), matching modal/daytona. - Namespace persistent sandbox identity by a per-profile token (name + metadata + reuse match) and resolve the snapshot-store path per profile, bound at construction so background-thread cleanup writes to the right home. - Durability gate: a non-durable snapshot is not recorded (cleanup pauses and preserves prior state); a failed pause leaves the sandbox live rather than terminating it. Restore falls back to a base image only for an unrecoverable snapshot (gone / non-durable / snapshot-specific invalid state), preserving the pointer on transient errors. - Config: blank tenki_api_endpoint default across both config loaders so the documented env/CLI fallback is reachable; allow the guest-home subtree (/home/tenki/*) as a valid cwd at all container-cwd guards. Known follow-up (pre-existing, backend-agnostic): the process-global terminal environment cache (_active_environments, keyed "default") is not profile-scoped, so under the multiplexing gateway forwarded credentials are not isolated across profiles. Tracked separately; documented in the credential-forwarding notes. Tests: tests/tools/test_tenki_environment.py plus terminal/file/config/scrub coverage, including profile-scope, durability, restore-classification, and cwd-subtree regression pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Required by contributor-check for nick@luxor.tech commits in NousResearch#64190. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alt-glitch
added
type/feature
…l race cancel() can null self._sandbox between _ensure_sandbox() and the dereference in _start_process/_exec_raw/_transfer_sandbox, turning a user interrupt into an AttributeError. _require_sandbox() captures the reference under the lock and raises a typed RuntimeError if the sandbox was torn down. Found by Tenki Code Reviewer on the mirrored PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two findings from Tenki Code Reviewer on the true-base verification PR: - _close_client: swallow close() exceptions. cleanup() resets _cleanup_in_progress only after closing the client, so an escaping network error during teardown left the flag stuck and every later _ensure_sandbox() failed with 'Tenki cleanup is in progress'. - prompt_builder probe: replace the fourth inline container-config copy with the shared _container_config_from_env_config() builder; the inline dict omitted tenki_sync_hermes_home, tenki_forward_env, and docker_network, so probe environments diverged from real ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keep the PR current with main (248 commits of drift, 22 overlapping files). Verified locally: clean merge, uv lock --check passes, all Tenki/terminal tests pass, and the full tests/tools suite shows no failures beyond those already present on upstream main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the thorough backend integration. This is a legitimate opt-in feature: current main still has no Tenki backend in tools/terminal_tool.py:1514-1539.
Problems
- Blocking:
tools/environments/tenki.py:769-770and:787dereferenceself._sandboxafter_ensure_sandbox(). But the PR's cancel path clears that attribute at:941-946. Capture one reference with_require_sandbox()and use it through the full single/bulk upload operation; otherwise cancellation can raiseAttributeErroror split a bulk transfer across sandboxes. - Blocking:
pyproject.toml:159addstenki-sandbox==0.1.1.AGENTS.md:561-576requires bounded PyPI ranges, including a two-minor upper bound for pre-1.0 packages.
Suggested changes
- Add cancellation-race coverage for
_tenki_uploadand_tenki_bulk_uploadafter converting them to use one captured sandbox reference. - Use a bounded dependency range and regenerate
uv.lock.
Automated hermes-sweeper review.
teknium1
added
sweeper:risk-security-boundary
_tenki_upload and _tenki_bulk_upload re-read self._sandbox for each step, but cancel() nulls that field out concurrently. A cancel landing mid-flow made the mkdir and the upload target different sandboxes, or dereferenced None outright (AttributeError: 'NoneType' has no attribute 'fs'). The bulk flow was worse: its mkdir/upload/untar/rm could span two sandboxes, extracting the tar somewhere other than where it landed. Capture the sandbox once via _require_sandbox() and thread that one reference through every filesystem and exec call, matching what _tenki_bulk_download already does with _transfer_sandbox(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tenki removed projects from its API: the SDK has no project_id on sandbox creation, no list_project, and no IdentityProject. The terminal.tenki_project_id key and its TERMINAL_TENKI_PROJECT_ID / TENKI_PROJECT_ID env overrides therefore configure nothing. Remove the config key from both loaders, the gateway env bridge, and the container-config plumbing, along with the documentation rows that advertised it. The workspace remains the unit that decides where sandboxes are created. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Per AGENTS.md's dependency policy, use a bounded range rather than an exact runtime pin: pre-1.0 packages get a two-minor ceiling, so tenki>=0.5.1,<0.7. uv.lock refreshed. The package was renamed on PyPI: 0.5.1 ships as `tenki` (tenki-sandbox stops at 0.4.0). The compat `tenki_sandbox` module still ships inside `tenki`, but new code should import `tenki`, so the imports, the find_spec probes, and every install hint move over. 0.5 is a real API migration, not just a version string: - Client.create no longer accepts project_id, and list_project / list_workspace folded into list(workspace_id=...). _create_kwargs filtered against Sandbox.create, which is a bare **kwargs passthrough that names nothing it accepts, so filtering was a no-op and any dropped kwarg reached the client as an unexpected keyword. Introspect Client.create — the real validator — and pass the client-construction kwargs (base_url, auth_token) explicitly, since Sandbox.create pops those before forwarding. - RegistryArtifactNotFoundError was renamed RegistryImageNotFoundError. The single combined `from tenki_sandbox import (...)` failed outright on the rename, silently dropping the isinstance check for every class in it and leaving only the name-based fallback. Resolve each class independently and accept both names. Tests: the fake SDK now mirrors 0.5 — Client.create declares its real parameter list with no **kwargs catch-all, so an unsupported name raises TypeError exactly as the real client would. Adds regressions for the kwarg filtering and for snapshot-error classification across the rename. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # hermes_cli/config.py # uv.lock # website/docs/guides/tips.md # website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md
_terminal_env_type_for_task now reads the terminal env config before the active-environment lookup, because the environment registry's cache key is derived from env_type. That hoist put the config read inside the function's outer try, so any config failure aborted the whole lookup and returned "local" even when a container backend was registered -- silently resolving container paths against the host, which is the misrouting this module exists to prevent. Give the config read its own handler so it degrades to an empty mapping instead of pre-empting the live-environment lookup. Restores the precedence asserted by test_container_path_detection_uses_live_docker_environment, which was red on this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_rewrite_real_sudo_invocations and _count_real_sudo_invocations were the same shell tokenizer walk -- tracking command_start, skipping comments, handling the &&/||/;;/;|&() operators and leading env assignments -- with the counter differing only in dropping the output list. Two copies is two chances to drift on quote and comment handling, and the rewriter already returns the count the counter recomputes. Collapse the walk into _rewrite_sudo_command_words(command, replacement). Both public names survive as thin callers, so no call site changes. No behaviour change: the old and new implementations were differentially compared over roughly 67k generated command strings, covering quotes, escapes, comments, operators, env-assignment prefixes and subshells, with identical rewritten output and identical counts throughout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deduplication pass over the Tenki snapshot and binding registry. No change to durability, lock ordering, or which errors propagate. - Adopt the shared _rewrite_sudo_command_words walker and drop Tenki's private third copy of it. - _mutate_store() replaces the resolve-path/lock/load/save preamble that was repeated across 11 registry mutators. - _RemoteBinding replaces an unnamed six-tuple return and the six parallel instance attributes that mirrored it. - One parameterised file-lock helper serves both the blocking snapshot-store lock and the non-blocking task-ownership lock, which had each hand-rolled the same fcntl/msvcrt branch. - _record_field() and _fail_ambiguous_lineage() collapse the dict-or-scalar unwrap and the set-flag-then-raise pattern. - Rename _load_json_store to _load_recovery_registry. It shadowed base._load_json_store with the opposite error semantics: base returns an empty mapping on an unreadable file, this one deliberately raises so a corrupt registry cannot erase the only recovery pointer. - Move the durable atomic-write primitive to base._atomic_save_json_durable so other backends can reuse it; Tenki keeps the uncertain-commit policy wrapper on top. Four sites were deliberately left on their existing code because routing them through a shared helper would have changed when a write becomes durable relative to a remote RPC: _queue_snapshot_retirement (its early return must not write), _retire_pending_snapshot_if_unreferenced (holds one lock across tombstone, remote delete and clear, which is the non-resurrection guarantee), _confirm_snapshot_store_durable (fsync-only, not load-mutate-save), and the ownership lock's file-close ownership. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why the create-attempt journaling is hereThe largest chunk of The obvious review reaction is "why not just retry the create, or look it up by name?" I checked both against the Tenki backend and the answer is that neither works today. Recording it here so the next reader doesn't have to re-derive it. 1. Sandbox create has no server-side idempotency.
This is not an oversight in my reading of the code: the same codebase does implement idempotency keys where it wants them — OpenCode runs ( Aggravating factor: 2. Sandbox names are not unique per workspace, and there is no lookup by name.
Same contrast as above: templates, registry images, and volumes all do carry 3.
That is a real hazard rather than a theoretical one — the sandbox domain already works around replica lag in three other places, with comments saying so explicitly and a Prometheus counter tracking the fallback. Those guards just aren't on list or get. Net: "did my create land, and did it fork?" genuinely cannot be answered by a single API call right now. Deterministic naming plus a list scan is not sufficient, because the name isn't an identity key and the list may not show the sandbox yet. Hence the local journal. If we want to shrink it laterThe one server-side lever that exists is tags: GIN-indexed with a real containment filter ( Pushing candidate-set narrowing into a tag would make reconciliation cheaper, but it would not remove the journal: you would still need local state to tell your own create apart from a collision, and you would still have to tolerate a just-created sandbox being briefly invisible. Tag constraints if anyone picks this up: max 20 tags, each ≤32 chars, The real fix is server-side: an idempotency key on create would let most of this layer be deleted. Worth raising with the Tenki side. Caveat on freshness: the backend checkout I read was a couple of weeks behind |
Follow-up cleanups from a four-angle review (reuse, simplification, efficiency, altitude) of this branch. No behaviour change. - Delete dead code: TenkiEnvironment._max_duration (written, never read), _save_snapshots (no callers, and it bypassed _mutate_store's read-modify-write discipline while looking like the sanctioned write path), a _stdin_mode override restating BaseEnvironment's default, and the unused explicit/key parameters in tenki_config. - Hoist _normalize_forward_env_names into file_sync as normalize_forward_env_names, taking a setting_name for the warning text. docker and tenki carried verbatim copies of it. Rendered warnings are byte-identical to before. - Add TenkiEnvironment._dispose_remote for the repeated "walk the sandbox disposal methods, retrying each" block. Two of the five sites adopt it. The other three either clear the remote-binding marker inside the walk (so a failed clear deliberately falls through to the next method) or log once per failing method, and sharing them would change behaviour. Each exclusion is commented at the site. - Make base._atomic_save_json_durable's five keyword parameters required. Its single caller passed all five explicitly, so the defaults and their coalescing were unreachable. - Precompute the expected sandbox-name map in __init__. _sandbox_matches_task ran a regex substitution per profile/task candidate per listed sandbox -- measured around 205 per environment init against a 50-sandbox workspace listing, which is itself scanned up to three times per create. Now a dict lookup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolves five conflicts accumulated while main moved ~1000 commits ahead. - hermes_cli/config.py: main extracted DEFAULT_CONFIG and OPTIONAL_ENV_VARS into the new hermes_cli/config_defaults.py leaf module while this branch edited DEFAULT_CONFIG in place. Took main's import and ported the eleven terminal.tenki_* defaults into config_defaults.py, updating the container resource-limit comment to include tenki. The branch's _normalize_terminal_backend_defaults and apply_terminal_backend_transition live outside the moved block and merged cleanly, as did the TERMINAL_TENKI_* entries in the env-var map. - model_tools.py: union of the two import lists. Both _CHECK_FN_TTL_SECONDS (this branch) and tool_error (main) are used in the file. - gateway/run.py: main removed a duplicate _expand_env_vars import that this branch had extended. _expand_env_vars is already imported a few lines above, so kept only _normalize_terminal_backend_defaults, which is used below at the terminal-defaults bridge. - uv.lock: regenerated from the merged pyproject.toml (tenki 0.5.4, within the pinned >=0.5.1,<0.7). - website/docs/user-guide/configuration.md: both sides rewrote the same sync section. Kept main's more detailed text (retry count, 2 GiB archive limit, bind-mount note) and preserved both Tenki caveats: participation gated on terminal.tenki_sync_hermes_home, and that cleanup terminates the sandbox under the default non-persistent mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Upstream added a vercel_sandbox terminal backend touching the same membership sets, dispatch branches, wizard indices and docs prose that this branch touches for tenki, so most of the 70 conflicting hunks across 39 files resolve as a union of both backends. Notable non-union resolutions: - hermes_cli/web_server.py: upstream extracted the tools routes into hermes_cli/web_routers/tools.py. Took upstream's extraction (keeping HEAD's block would have double-registered the routes) and ported this branch's two changes into the extracted router: the profile secret-scope plumbing in get_terminal_backends, and apply_terminal_backend_transition in select_terminal_backend. Verified no duplicate routes. - hermes_cli/web_server.py: added a vercel_sandbox row and probe to the backend picker introduced by this branch. Without it the dashboard would report a vercel_sandbox config as local and reject selecting it. - tools/terminal_tool.py: kept _container_config_from_env_config at all four call sites and added upstream's vercel_runtime key to it, rather than reverting to the inline dicts upstream edited. Kept the runtime- scoped getenv accessor over os.getenv, and reconstructed both the vercel_sandbox and tenki branches of _create_environment. - Tests: upstream ran a suite-wide prune. Every conflicted test was classified against the merge base — this branch's new tests are kept, upstream's deletions of pre-existing tests are honoured. - hermes_cli/setup.py: wizard indices resolved as vercel_sandbox 5, tenki 6. - uv.lock regenerated from the merged pyproject; both extras retained. - Backend counts in prose corrected 7 -> 8 in README.md, configuration.md and architecture.md (including its ASCII diagram). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # agent/prompt_builder.py # hermes_cli/config.py # model_tools.py # tools/environments/file_sync.py # tools/registry.py # tools/terminal_tool.py # website/docs/user-guide/configuration.md
# Conflicts: # hermes_cli/web_routers/tools.py # tools/code_execution_tool.py # tools/environments/docker.py # tools/file_tools.py # tools/terminal_tool.py # uv.lock
|
Merged current main (ed5e17f, ~1,000 commits) — conflicts in 6 files, all resolved at head (5a48519). The semantic reconciliations, since a few went beyond textual merging:
Verification: |
Resolves two conflicts against upstream's interrupted-command cwd fix (16a173a): - tools/environments/base.py: keep upstream's expanded _extract_cwd_from_output docstring (cwd_observed semantics) with Tenki added to the backend list. - tools/file_tools.py: keep this branch's environment-identity cache check (_select_active_environment already touches _last_activity) and adopt upstream's fill-only session-cwd rescue — only record the cached snapshot when the session has no record of its own. TenkiEnvironment inherits the base marker parsing, so it picks up result["cwd_observed"] without changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Merged current main (1f1b4d9) — conflicts in 2 files, both against the interrupted-command cwd fix (16a173a), resolved at head (1d7b5ab):
Verification: |
3 participants
What does this PR do?
Adds Tenki as a seventh terminal execution backend alongside
local,docker,ssh,singularity,modal, anddaytona. Withterminal.backend: "tenki", Hermes creates Tenki cloud sandboxes on demand for the terminal tool, file tools, andexecute_code, and terminates them on cleanup by default. Pause/resume persistence across sessions is opt-in viacontainer_persistent: true.The integration deliberately follows the existing modal/daytona pattern: an optional extra (
tenki>=0.5.1,<0.7) that is lazy-installed on first use, aBaseEnvironmentsubclass intools/environments/, and the same setup-wizard / doctor / status / gateway wiring. Along the way it folds the Tenki keys into the shared_container_config_from_config()helper (upstream's dedup of the previously copy-pasted container-config dicts) and repoints the remaining inline copies (file tools, execute_code, prompt builder) at it, so future backends only need to touch one place.Security hardening is built in rather than bolted on:
terminal.tenki_forward_env, and forwarding the control-plane token logs a warning.agent.secret_scope, so an active profile scope wins over process-globalos.environand the shared machine CLI login is skipped when a profile scope is authoritative.TENKI_AUTH_TOKEN/TENKI_API_KEYare stripped from spawned subprocess environments (provider blocklist + always-strip tier), matching modal/daytona.Known follow-up (pre-existing and backend-agnostic, not introduced here): the process-global terminal environment cache (
_active_environments, keyed"default") is not profile-scoped, so under the multiplexing gateway forwarded credentials are not isolated across profiles. Documented in the credential-forwarding notes and tracked separately.Related Issue
No existing issue — this is a new backend integration in the same vein as the open E2B (#18348) and Sprites (#30112) backend PRs. Happy to open a tracking issue if maintainers prefer.
Type of Change
Changes Made
Core backend
tools/environments/tenki.py(new) —TenkiEnvironment: sandbox lifecycle, exec, pause/resume persistence, remote file sync-backtools/tenki_config.py(new) — profile-scope-aware resolution of auth token, workspace, and API endpoint from the Tenki CLI config or environmenttools/terminal_tool.py,tools/file_tools.py,tools/code_execution_tool.py— backend wiring; tenki keys folded into the shared_container_config_from_config()helper, with file tools / execute_code / prompt builder repointed to ittools/environments/__init__.py,tools/environments/base.py,tools/environments/local.py— registration and base-class support/home/tenki/*) as a valid cwdSecurity
tools/approval.py,tools/env_probe.py,tools/file_operations.py—TENKI_AUTH_TOKEN/TENKI_API_KEYadded to the provider blocklist and always-strip tierterminal.tenki_forward_env(see above)CLI / UX
hermes_cli/setup.py— setup-wizard option for the Tenki backendhermes_cli/doctor.py,hermes_cli/status.py— Tenki auth/SDK checks and backend statuscli.py,gateway/run.py,hermes_cli/config.py— config plumbing; blanktenki_api_endpointdefault in both config loaders so the documented env/CLI fallback is reachablePackaging
pyproject.toml,uv.lock,tools/lazy_deps.py,nix/packages.nix— optionaltenkiextra (tenki>=0.5.1,<0.7), lazy-installed like modal/daytonaDocs & config
cli-config.yaml.example— "OPTION 7: Tenki cloud execution" block with alltenki_*keyswebsite/docs/— configuration guide, environment-variable reference, security notes, architecture pageAGENTS.md,CONTRIBUTING.md— backend lists updated to include tenkiTests
tests/tools/test_tenki_environment.py(new, ~1,300 lines) — lifecycle, exec, persistence, durability gate, restore classification, profile scopingtest_terminal_config_env_sync.py,test_terminal_requirements.py,test_terminal_tool_requirements.py,test_file_tools_container_config.py,test_parse_env_var.py,test_container_cwd_sanitize.py,test_local_env_blocklist.py,test_hardline_blocklist.py,test_command_guards.py,tests/hermes_cli/test_setup.py, and othersHow to Test
pip install 'hermes-agent[tenki]'(or let lazy install handle it on first use) and authenticate viatenki loginorTENKI_AUTH_TOKEN/TENKI_API_KEY.cli-config.yaml, setterminal.backend: "tenki"(see the new OPTION 7 block incli-config.yaml.examplefor all keys).hermes -q "run uname -a in the terminal"— a Tenki sandbox is created on demand and terminated on cleanup. Exercise file tools andexecute_codethe same way.hermes doctorandhermes statusreport Tenki SDK/auth state and backend status.container_persistent: true, run a session, exit, run again — the sandbox pauses on cleanup and resumes on the next session.pytest tests/ -q— full suite passes.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (one environment-specific failure on macOS,test_approval.py::TestDetectDangerousRm::test_nonrecursive_verification_artifact_cleanup_is_not_dangerous, fails identically at the merge-base without this change — pre-existing, unrelated)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Targeted run of every test file touched by this PR:
After the tenki 0.5.1 bump, re-run via
scripts/run_tests.sh(per-file isolation, same as CI) over every file this PR touches:Full
tests/tools/viascripts/run_tests.shon macOS 26.5.1: the only failures are pre-existing ones that reproduce identically with this branch's changes stashed (test_approval.py::TestDetectDangerousRm::test_nonrecursive_verification_artifact_cleanup_is_not_dangerous, plustest_base_environment.pyandtest_file_tools.py).tests/hermes_cli/andtests/gateway/likewise: the 10 systemd/WSL/service-manager failures are byte-identical at the merge base.tyon the touched modules drops from 5 diagnostics to 2 (both pre-existing) — the three it loses are exactly theAttribute 'fs' is not defined on Noneerrors behind the upload race fixed here.