Ventis fixes extracted from the CLI packaging work - #70
Merged
Conversation
generate_docker()/generate_workflow_docker() now recursively sweep every .py file under the project directory into the build context, preserving directory structure, so helper files that aren't declared as an agent entrypoint still make it into the image. Generated dirs (docker_container/, stubs/, grpc_stubs/) are excluded at the project root only, not at every depth. Stub files are placed at their agent's declared entrypoint path (mapped from global_controller.yaml) instead of a hardcoded guess, so a stub overwrites the exact real file it replaces. Guards against absolute and '..'-containing entrypoints, symlinked sources, and symlinked-destination escapes, with warnings on unsafe or unmapped stubs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…heckpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> # Conflicts: # examples/portfolio/agents/advisor_agent.py
Users had no way to get API keys (OpenAI, Anthropic, embedding models) into an agent container. Add a top-level `env_file` key to global_controller.yaml pointing at a local .env file, which reaches every container as `docker run --env-file`. - resolve_env_file validates the path before anything launches, so a missing .env fails at deploy time instead of deep inside a container. Relative paths resolve against the project root, matching entrypoint. - env_file_args is a context manager owning the local-vs-remote decision and the cleanup, so both runtimes share one code path. Local containers read the original file; remote containers get a copy that is deleted as soon as `docker run` returns, whether or not it succeeded. - GlobalController._push_file streams the file over ssh under `umask 077` rather than scp, so the copy is never briefly world-readable and the secret never lands in a command line. _run_cmd's ssh options moved to a shared _ssh_args. --env-file is appended after the explicit -e VENTIS_* flags; Docker gives those precedence regardless of order, so a stray VENTIS_* line in someone's .env cannot break agent wiring. Closes #50
Two holes in the remote staging path, both found reviewing the feature commit. `umask 077` only governs files the shell creates, and `>` follows symlinks -- so it did not actually guarantee a 0600 copy. The destination path is fully predictable (`/tmp/ventis-env-ventis-ec2-<agent>-<n>`), so a local user on the remote host could pre-create it world-readable, or point it at a file of their own, and collect the API keys. Remove whatever sits at the path before writing; `rm -f` unlinks a symlink rather than following it, so `cat >` then creates a fresh file under the umask. `_run_cmd` joins its argv with spaces and hands the result to a remote shell unquoted. `_push_file` quoted its path but the cleanup `rm` did not, so a container name containing a space split the `rm` into two arguments that matched nothing -- it exited 0 while the secrets file stayed on the host, and the returncode check logged nothing. Scrub the name down to [A-Za-z0-9_.-] in remote_env_path, which also closes the same gap in the `--env-file` argument and in any future use of that path. Still open, tracked separately: a push that dies mid-transfer can leave a copy behind, since the cleanup only covers the `docker run` that follows. On EC2 the instance is terminated on that path, which disposes of it.
…nup-race fix (pre-pull checkpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This project's agents/workflow import each other's stubs by flat module name, not by the exporting agent's own entrypoint path. Applying _stub_destination's entrypoint-mirroring broke both the Workflow (ModuleNotFoundError: intent_agent) and agent-to-agent calls (MetricsAgent -> price_agent) on live redeploy. Keeps PR #51's actual fix (project_dir sweep for unstubbed helper files) intact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Metrics/telemetry latency scaled with instance count x per-instance round-trip time since every instance was polled sequentially, one blocking the next, with the following tick only starting after the whole pass finished. Extracted the per-instance body into _poll_one_instance (whole body wrapped in one top-level try/except, since ThreadPoolExecutor.map() re-raises on first exception when results are consumed) and run all instances concurrently via the same ThreadPoolExecutor pattern _trigger_cleanup already used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Metrics/telemetry latency scaled with instance count x per-instance round-trip time since every instance was polled sequentially, one blocking the next, with the following tick only starting after the whole pass finished. Extracted the per-instance body into _poll_one_instance (whole body wrapped in one top-level try/except, since ThreadPoolExecutor.map() re-raises on first exception when results are consumed) and run all instances concurrently via the same ThreadPoolExecutor pattern _trigger_cleanup already used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolved conflicts: - metrics_agent.py: removed duplicate imports - portfolio_workflow.py: used main's simpler intent handling - cli.py: kept both comment explanations - stub_generator.py: used main's version (no entrypoint injection)
Brings in all OTel cleanup work: - Removed legacy single-destination fallback - Removed langfuse Basic-Auth auto-injection - Updated DESIGN.md to remove legacy references - Simplified config env-var expansion - Fixed langfuse config examples - Merged latest from main (joke_writer, porting skill, etc.) All conflicts resolved by taking feature/otel-exporter's cleaner versions.
… and remove _write_entrypoint_file - Fixed langfuse example to use generic env-var headers pattern - Removed _write_entrypoint_file (directory structure preservation via _sweep_py_files is cleaner)
Re-applied the parallel instance polling that was lost during conflict resolution. The _poll_controllers method now uses ThreadPoolExecutor to poll all instances concurrently via _poll_one_instance, preventing one slow instance's Redis/Postgres round-trip from blocking the entire poll tick.
…ull redeploy Moves otel.destinations from a one-shot VENTIS_OTEL_DESTINATIONS env var (frozen at exporter subprocess spawn) to a Redis key (otel:destinations), mirroring the existing routing-table live-reload pattern. GlobalController writes it at startup and again in reload_config() (SIGHUP); otel_exporter.py's existing 5s poll tick re-reads it each cycle and rebuilds its BatchSpanProcessors only when it changed. No signal-forwarding, no subprocess restart, no ProcessSupervisor.restart -- just a small ProcessSupervisor.is_registered() so reload_config knows whether the exporter is even running. Kept in scope: exporter start-gating at boot is unchanged (still skipped entirely if otel.destinations is absent at startup); destinations added after boot only take effect if the exporter was already running.
Drops the redis-connection-info env plumbing (VENTIS_REDIS_HOST/PORT/DB, GlobalController._otel_exporter_env) added in the previous commit -- otel_exporter and GlobalController always run on the same host, and RedisClient's own defaults already are localhost:6379/db0, so passing them through was dead flexibility for a case that doesn't exist yet.
Kept comments only where behavior is genuinely non-obvious (why the exporter polls Redis instead of restarting, why reload_config gates on is_registered, the invalid-update-keeps-old-processors fallback). Dropped comments/docstrings that just narrated 'this was added' on trivial pass-through code.
Everything under ventis/ and tests/ that the canyonos CLI branch depends on, lifted off feature/config-reloading with no cli/ or examples/ changes. - Package layout: move deploy/future/ventis_context/bedrock/utils under ventis/controller/, add ventis/Dockerfile and ventis/README.md. - ventis/server.py: Flask control surface the CLI's container talks to (/deploy, /clean, /status), replacing the ad-hoc entrypoint. - ventis/cli.py: fold `build` into `deploy`, support the .car artifact layout (.car/app sources, .car/config declarations, .car/stubs), and resolve env_file against the project dir so it matches how the GlobalController resolves it at runtime. - GlobalController: persist a dashed-uuid project_id and publish the controller identity to Redis. - OTLP exporter: generate Future.id at 64 bits (secrets.token_hex(8)) so it is a valid OTel span_id without truncation, and cost lookups that fail (no pricing table on a local deploy) now cost at 0 instead of dropping the whole telemetry row. - stub_generator: a stub is written to exactly one location, the path of the entrypoint it replaces, rather than being duplicated at the flat basename as well. Flat is only the fallback for a stub with no entrypoint mapping or one whose mapping escapes the build context. Carries over the placement half of 692d17c from feature/all-the-files, which never reached this line; the entrypoint-adjacent YAML discovery from that commit is deliberately left out, since the .car layout already resolves declarations from .car/config. - stub_generator: fail loudly when an agent has no declaration or entrypoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| config_path = data.get("config_path", "config/global_controller.yaml") | ||
| full_path = os.path.join(WORKSPACE_DIR, config_path) | ||
|
|
||
| if not os.path.isfile(full_path): |
Stub placement is now mirrored-only, so a workflow importing the flat basename no longer resolves -- the stub is written to agents/<name>.py and nothing is left at the context root. Switch the four example workflows to the nested form. Cherry-picked from 7f925ef on fix/remove-duplicate-stub. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ull redeploy Moves otel.destinations from a one-shot VENTIS_OTEL_DESTINATIONS env var (frozen at exporter subprocess spawn) to a Redis key (otel:destinations), mirroring the existing routing-table live-reload pattern. GlobalController writes it at startup and again in reload_config() (SIGHUP); otel_exporter.py's existing 5s poll tick re-reads it each cycle and rebuilds its BatchSpanProcessors only when it changed. No signal-forwarding, no subprocess restart, no ProcessSupervisor.restart -- just a small ProcessSupervisor.is_registered() so reload_config knows whether the exporter is even running. Kept in scope: exporter start-gating at boot is unchanged (still skipped entirely if otel.destinations is absent at startup); destinations added after boot only take effect if the exporter was already running.
Drops the redis-connection-info env plumbing (VENTIS_REDIS_HOST/PORT/DB, GlobalController._otel_exporter_env) added in the previous commit -- otel_exporter and GlobalController always run on the same host, and RedisClient's own defaults already are localhost:6379/db0, so passing them through was dead flexibility for a case that doesn't exist yet.
Kept comments only where behavior is genuinely non-obvious (why the exporter polls Redis instead of restarting, why reload_config gates on is_registered, the invalid-update-keeps-old-processors fallback). Dropped comments/docstrings that just narrated 'this was added' on trivial pass-through code.
fixes/ventis-cli-fixes and feature/config-reloading are parallel lines off a common ancestor far back (75d7705), not a linear descendant relationship -- each independently re-implemented overlapping OTel/otel-exporter work after diverging. Resolved per-file based on which side is the actual superset, verified with full-file diffs against both parents, not a blanket "one side wins" rule: - env_file.py (add/add): took feature/config-reloading's side -- it already has the managed-secrets platform_secrets_file()/DEFAULT_SECRETS_FILE support merged in from main (via this session's PR #60/#61 work), which fixes/ventis-cli-fixes predates and lacks entirely. - db.py, convert.py, test_otel_exporter_fields.py (add/add): took fixes/ventis-cli-fixes -- cost-lookup try/except fallback and 64-bit Future.id/span_id, both strict additions config-reloading never touched. - _runtime.py: took fixes/ventis-cli-fixes -- adds a port-conflict retry loop around docker run; verified the whole-file diff is confined to this one function, and that registering the Redis endpoint AFTER the retry loop (instead of before, like config-reloading) is required once host_port can change mid-retry, not just a style choice. - global_controller.py: took fixes/ventis-cli-fixes -- verified via full diff (not just the 5 conflict markers) that every other difference in the file, conflicted or already auto-merged, also favors this side (.car-layout project root resolution, persisted dashed-uuid project_id, VENTIS_REDIS_HOST env override for a containerized GC's own Redis connection). - cli.py, test_cli.py: took fixes/ventis-cli-fixes -- folds build into deploy per this PR's own description, fixes entrypoint-only stub placement, and incidentally fixes the recurring duplicate-keyword SyntaxError (project_dir=/stub_entrypoints= repeated) that config-reloading's copy still carries since it branched before that got fixed elsewhere this session. - otel_exporter.py (add/add): user decision -- kept fixes/ventis-cli-fixes's RedisClient(host="host.docker.internal") as-is, despite it not matching the VENTIS_REDIS_HOST env-var pattern this same PR introduces for GlobalController's own Redis connection a few lines away in global_controller.py. Flagged as a known inconsistency, not fixed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
feature/config-reloading (PR #70's original base) was squash-merged into main during this session (commit bdb8ecd), so main's tree is now byte-identical to feature/config-reloading's tip -- but a different commit graph (squash vs. real history), so re-merging main against this branch's already-resolved tip produces its own merge-base and one fresh conflict, even though the content driving it is unchanged. Only ventis/OTLP_Exporter/otel_exporter.py conflicted again (same spot as the previous resolution): the ventis.controller.utils.redis_client import path (kept, matches this branch's package move) and the Redis host for the exporter subprocess (kept RedisClient(host="host.docker.internal"), per the same user decision as the previous merge commit -- see that commit's message for the full reasoning and the known inconsistency with this branch's own VENTIS_REDIS_HOST pattern elsewhere). Everything else carried over cleanly from the previous resolution. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Saaketh0
added a commit
that referenced
this pull request
Sep 9, 2026
This branch's own last conflict-resolution notes were against an earlier tip of fixes/ventis-cli-fixes; that branch just moved forward (this session's PR #70 resolution), reopening one conflict. _runtime.py: took this branch's side throughout. Every hunk is fixes/ventis-cli-fixes's pre-network-refactor scheme (host.docker.internal + dynamic host_port + host-based redis_host) that this PR replaces with container-network routing (--network ventis-local, fixed CONTAINER_PORT, runtime_id-based addressing). Confirmed via full-file diff that the unconflicted parts of the file (provision_instance's redis_host, routing_endpoint_for) already consistently use this branch's container-name scheme, and _container_routing_host -- which fixes/ventis-cli-fixes's losing hunk calls -- no longer exists in this file at all; taking that side would have been a NameError at runtime. global_controller.py and env_file.py, which also conflicted against the pre-PR-#70-push tip of fixes/ventis-cli-fixes, now auto-merge cleanly as a side effect of that push already landing the needed content. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Saaketh0
added a commit
that referenced
this pull request
Sep 9, 2026
Not just the README the user initially spotted -- 8 files conflicted. Resolved per-file, verifying full diffs before picking a side (caught and reverted two premature whole-file takes on metrics_agent.py and global_controller.yaml before pushing, see below): - README.md: kept "canyonos build" over main's "canyonos integrate" -- checked cli/cli.py, the actually-implemented subcommand is "build". Kept main's added "Passing secrets to agents" section (pure addition, documents the env_file feature this branch predates). - pyproject.toml: kept this branch's "canyonos" workspace dependency + [tool.uv.sources] entry -- the whole point of this PR is adding cli/. - uv.lock: regenerated with `uv lock` against the resolved pyproject.toml rather than hand-merging a lockfile. - ventis/controller/utils/env_file.py (add/add): took main's side, same managed-secrets superset as every other PR resolved this session. - ventis/server.py (add/add): user decision -- took this branch's version. It has a workspace-escape security check (rejects a config_path that resolves outside /workspace) and _primary_redis()/_workflow_endpoints() routing helpers that main's simpler version (landed minutes earlier via PR #70) lacks entirely. - examples/portfolio/agents/metrics_agent.py: kept this branch's try/except PriceAgent import fallback (matches the current entrypoint-mirrored stub placement from PR #70's stub_generator changes) over main's fixed sys.path.insert(..., "stubs") approach, which predates that layout change. Initially misjudged this as a trivial additive-import diff and took main's side by mistake -- caught it by re-checking the full diff before pushing and reverted. - examples/portfolio/config/global_controller.yaml: kept this branch's version wholesale -- it's a coherent local-provider example (provider: local throughout, a local OTel destination pointing at host.docker.internal, dashboard_port) whereas main's is a coherent EC2-provider example (provider: EC2 throughout, railway/grafana OTel destinations, a database url). These describe two different deployment scenarios for the same example; splicing hunks from both would produce an inconsistent config (e.g. EC2 provider pointing at host.docker.internal). Local provider matches this whole branch's purpose. Also initially misjudged as additive and reverted before pushing. - examples/portfolio/workflow/portfolio_workflow.py: took main's side as a correctness fix, not a preference. This branch's `json.loads(intent_agent.parse(query=query).value())` calls .value() on a plain dict (IntentAgent.parse() is typed -> dict and identical on both branches) -- would raise AttributeError. main's direct `intent_agent.parse(query=query)` matches the actual return type. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Saaketh0
added a commit
that referenced
this pull request
Sep 9, 2026
11 conflicts, almost all explained by one architectural swap this branch makes: agents importing a custom call_bedrock() wrapper from ventis.controller.bedrock, replaced by agents calling boto3 directly, transparently redirected through the new ventis/llm_proxy/ service via AWS_ENDPOINT_URL_BEDROCK_RUNTIME. bedrock.py is deleted outright since nothing needs the wrapper once boto3 talks to the proxy directly. Resolved per-file: - ventis/controller/bedrock.py (rename/delete): kept the deletion -- main only relocated it (ventis/llm/ -> ventis/controller/, PR #70's package move), still on the old wrapper pattern this branch replaces. - examples/portfolio/agents/advisor_agent.py, intent_agent.py, examples/text2sql/agents/vllm_agent.py: took this branch's side -- boto3-direct-via-proxy pattern in all three, verified via full diff each is the same single swap. - ventis/controller/cloud_provider_logic/Local/_runtime.py: took this branch's side -- adds the AWS_ENDPOINT_URL_BEDROCK_RUNTIME docker env var, purely additive. - requirements.txt: took this branch's side -- adds `requests`, purely additive (verified via full diff). - ventis/stub_generator.py (2 hunks, real merge not pick-one-side): BASE_AGENT_REQUIREMENTS combined main's cleanup (dropped unused ipdb/ipython) with this branch's additions (flask, requests) needed by the proxy. Second hunk dropped copying bedrock.py into agent containers (main's addition) since it no longer exists. - ventis/controller/utils/env_file.py (add/add): took main's side, same managed-secrets superset as every other PR resolved this session. - examples/portfolio/workflow/portfolio_workflow.py: took main's side as a correctness fix, not related to the llm-proxy work -- json.loads(intent_agent.parse(query=query).value()) calls .value() on a plain dict (IntentAgent.parse() is typed -> dict on both branches); same bug already found and fixed the same way in PR #72. - cli/README.md (add/add): trailing-newline-only difference, took this branch's side. - uv.lock: regenerated with `uv lock` against the resolved requirements.txt/pyproject.toml rather than hand-merged. Verified: full py_compile across ventis/examples/cli/tests, `uv run python3 -c "import ventis"` succeeds (both ventis and canyonos packages build), no stray ventis.controller.bedrock references left anywhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
3 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.
Everything under ventis/ and tests/ that the canyonos CLI branch depends on, lifted off feature/config-reloading with no cli/ or examples/ changes.
buildintodeploy, support the .car artifact layout (.car/app sources, .car/config declarations, .car/stubs), and resolve env_file against the project dir so it matches how the GlobalController resolves it at runtime.