[CAN-238] Add CanyonOS Core porting skill - #56
Open
nickhuo wants to merge 47 commits into
Open
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>
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.
The skill answers, before anyone writes an adapter, the four questions a port turns on: what class Ventis actually loads, what the adapter has to fix, what has to be declared (import root, requirements, env_file), and what to report rather than fix. `ventis-contract.md` pins each claim to the code that makes it true; `traps.md` maps symptoms back to causes. Its one rule: rewrite orchestration, import everything else. A port that restates a prompt, a tool body or a model call has copied the source instead of reusing it.
The skill's worked example. `joke_writer.py` is upstream's map-reduce joke
graph, unedited: three prompts, two schemas, three nodes. What could not come
across is the graph itself -- `StateGraph`, the `Send` fan-out and the
`operator.add` reducer are control flow owned by the LangGraph runtime, and
Ventis has no runtime to execute them.
So the edges are re-expressed as ordinary Python in the workflow, where the
fan-out becomes N calls dispatched across JokeAgent's three replicas, and the
nodes those edges connected are imported unchanged. The adapter restates
nothing.
The workflow entry point is `main(query)` because the deployment platform's
test endpoint posts to a hardcoded /main with a strictly validated
{query: string} body. Ventis itself would serve any name and any kwargs.
…unh/can-228-porting-to-ventis-skill # Conflicts: # ventis/cli.py # ventis/stub_generator.py
PR #51 moved each generated stub from the context root to its agent's entrypoint path, so the stub overwrites the real implementation the new sweep places there. That part is right, but it was a move rather than an addition, and the flat copy is what every caller actually imports: ModuleNotFoundError: No module named 'joke_agent' File "/app/workflow_launcher.py", line 21, in <module> exec(open("joke_workflow.py").read()) /app is sys.path[0], so `from joke_agent import JokeAgent` needs the stub at /app/joke_agent.py. Every example's workflow does this -- helloworld's `from example_agent import ExampleAgent` breaks the same way. _stub_destination becomes _stub_destinations and returns both paths, flat first. An agent's own entrypoint is still copied afterwards and wins its flat name back, so in an agent image /app/joke_agent.py is the real adapter while /app/agents/joke_agent.py is the stub; in a workflow image both are the stub. Verified end to end on examples/joke_writer against live Bedrock: the three generate_joke calls landed on replicas 0, 1 and 2, one each.
…unh/can-228-porting-to-ventis-skill
A skill enforces nothing. A prose MUST is a hope that the model reads and obeys it, and this one had obligation spread through paragraphs -- "has to fix", "needs a default" -- while `must` also sat on things that were merely true. So the word now marks exactly one thing: a rule whose violation breaks the port. Set in capitals, indexed in the MUST list, and nowhere else in the file, so `grep -nE '\b(MUST|NEVER)\b' SKILL.md` returns the rules and only the rules. `validate.py` decides the twenty-two of them a machine can. It parses YAML and Python to an AST and never imports the port, so it runs on a tree whose dependencies are not installed. Errors are provable contract violations and exit 1; the rewrite smells -- a prompt copied out of the source, a hardcoded key, a dirty source tree -- are warnings that exit 0, because a heuristic cannot be allowed to block a correct port. What the review turned up is that a block of this skill's hardest claims is false against the branch it merges into. `env_file:` needs PR #53. `-e .` and the all-file sweep have no PR at all -- they exist only on an abandoned branch. So the script probes the importable ventis for each feature and prints what it found, and a rule whose feature is missing is reported UNAVAILABLE rather than silently skipped. Both docs now name the PR behind every claim that does not hold on main. Two claims were simply wrong. `policy.yaml` is optional, as SKILL.md said -- but past the isfile() guard the read is unguarded, so a present-and-empty file kills `ventis deploy` before any container starts. And the contract's "the sweep takes every file" belongs to the same unproposed branch as `-e .`; `_sweep_py_files` takes `.py` only. Calibrated against all five examples until every remaining finding was true.
…s PR policy.yaml claimed the file was mandatory and that a missing one crashes deploy. It is the reverse: `_load_policy_rules` returns [] when the file is absent and `_check_policy` then allows everything. The real hazard is a half-written file -- an empty one, or a null `rules:`, raises inside GlobalController.__init__ before a single container starts. `env_file: .env` needs PR #53, still open. On main nothing reads the key, so the config line is inert and every request answers a Bedrock credential error. Said so where a reader meets it: the config, and the README's setup steps.
…an-238 test harness branch
CAN-238. Eight-stage pipeline where only the port stage runs an agent, so failures at validate/build/deploy/serve are attributable to the port itself. Two constraints from the skill's own rules shape the design: the source tree stays read-only (M19/M20), so Bedrock is reached by env var against its native OpenAI- and Anthropic-compatible surfaces plus a model-id rewriting shim; and the skill is pinned per run rather than auto-edited, so results within a run are comparable and results across runs are diffable.
… build The schema was carrying tables for things derivable after the fact from the run's artifacts. Cut back to the ticket's two tables plus the three SHAs that cannot be reconstructed once a run is over, and an artifacts directory that every later analysis reads from. Stage 5 no longer halts the pipeline. A validation that was wrong to block is only observable if the build runs anyway, and that observation cannot be recovered later.
Runs porting-to-ventis against a repo list and records how far each got. Only stage 4 runs an agent; the rest are deterministic subprocesses, which is what makes a build or deploy failure a fact about the port. The shim rewrites the model id on the way to Bedrock and forwards everything else unchanged, so no repo's source is edited to reach a provider. Stage 5 records validate.py's verdict without gating the build, so the runs where it was wrong to block are observable; report prints the confusion matrix. Stages 6-8 hold a global lock: they collide on image tags, the workflow port, and the Redis container ventis deploy starts.
Measured against a real Bedrock key rather than assumed. The OpenAI Chat Completions surface works; the Anthropic Messages surface is closed on this account -- every Messages-capable Claude answers permission_error, and Claude 3 Haiku names the cause as an unsubmitted use case form. Claude is reachable via Converse, but that is a third wire format and routing to it would mean the protocol translation this design avoids. Two ids in the model map were wrong: openai.gpt-oss-120b lacks the version suffix Bedrock requires, and claude-sonnet-5 does not exist on this account. A surface with no configured target is now closed, and the screen rejects repos whose SDK needs it instead of spending an agent budget on a port that cannot reach a model. Reopening it is one line of repos.yaml.
… skill working as the skill failing Two faults the first real run exposed. The harness never put its own venv on PATH. The ventis CLI was simply not found, so stage 6 would have exited 127 and recorded a harness setup fault as a defect in the port; and the agent in stage 4, having no interpreter that could import ventis, went three directories up and out of the tree under test to find one. Every subprocess now inherits the harness interpreter's bin, and validate.py runs on sys.executable -- on any other interpreter its capability probe reports every capability absent. Stage 4 also scored report-and-stop as a port failure. The skill's report-rather- than-fix paths are all triggered by something Ventis cannot do, so an agent that takes one has followed the skill exactly. Those runs are now status=blocked with the report filed as a core issue, which is where a finding with a Ventis owner belongs; a run that writes neither a port nor a report is still a failure.
…creen-only command The screen called a repo flat when it merely had no src/ directory, so langchain-academy -- whose 17 modules all sit under module-N/studio/ -- was passed to an agent that spent four minutes rediscovering statically that no port of it can be loaded. Flat now means what the port needs it to mean: a module the adapter can import from the project root. The threshold follows the Ventis under test rather than being fixed. Without an editable install M24 holds strictly and only root-flat modules import, whatever the packaging says; with one, packaging metadata decides. The harness asks the code, the way validate.py does. The new screen subcommand clones and screens candidates without porting any of them, which is how the repo list gets assembled -- a stage 2 verdict costs a shallow clone and no agent budget.
…etadata Sweeping the tree into the image does not make it importable. The process starts at the context root, so sys.path[0] is /app and only modules sitting there resolve -- a src/ layout resolves to nothing, and an adapter importing one raises ModuleNotFoundError inside _load_agent, which surfaces only as "No agent loaded" on the first request. `-e .` hands the import root to the project's own packaging metadata so Ventis never guesses a directory name. A project declaring none gets the previous install, unchanged. Taken from jiajunh/can-228-create-a-skill-to-convert-a-langchain-project-to-ventis. Merging that branch whole was rejected: it is an older parallel line carrying its own pre-validate.py copy of the skill and an earlier env_file than the one PR #53 put on this branch, so the merge conflicted on twelve files and would have regressed the artifact under test. Measured motivation: of six LangChain sample repositories screened, five are src/ layouts with pyproject.toml that no port could load without this.
The capability probe asked for _sweep_project_files; the function is named _sweep_py_files. sweeps_all_files therefore reported absent on every tree that has it, including this branch, which merged feature/all-the-files. A false negative here is worse than no probe: the skill tells an agent to trust the probe over its own assumptions, so the agent reasons from a capability it has been told it lacks. The other three probes were checked against the installed package and are correct.
…s imports Every LangGraph template reaches its model through init_chat_model with a "<provider>/<model>" string, so a repo can depend entirely on Anthropic while importing nothing named anthropic. The screen read only imports and classified three such repos as having no redirectable provider, then passed them to agents that spent budget porting projects whose provider surface is closed. It now reads the provider strings and classifies on what will actually be resolved. A closed surface also raised KeyError inside the shim's request handler, which killed the connection with no reply. It now answers 503 saying which surface is closed, and logs a warning -- a repo reaching a closed surface is evidence the screen let something through, and that is worth seeing rather than swallowing. .claude is now skipped when reading a tree: stage 4 copies the skill under test into the repo, so re-screening a tree that has been through a run would read validate.py's own imports as the repo's.
Killing the harness orphaned its agents. Two of them survived a run being aborted, kept spending their budget for another eight minutes, and were still calling the shim of the run that started afterwards -- their calls showed up as a repo reaching a surface it had been screened out of, which is a confusing lie to leave in a log. Children now start in their own process group and are tracked, a timeout kills the group rather than the one process the harness holds, and SIGINT/SIGTERM/ SIGHUP plus atexit take every live child down first.
This reverts commit 80eb546.
The editable install added in f9dd2d9 could never have worked. It emits `uv pip install -e .`, but the build context is docker_container/<Agent>/ and the sweep put only .py files there, so /app held no pyproject.toml and every build died with "does not appear to be a Python project". _install_step and _sweep_project_files are one change on can-228 and only the first half was taken. The sweep now carries every project file, because packaging metadata routinely points at a README or a license and a .py-only sweep leaves nothing installable. Hidden files stay out: .env holds credentials and has no business in an image. This widens what PR #51's narrower _sweep_py_files copied. It also restores validate.py's original probe -- `_sweep_project_files` was that branch's name for the broader capability, not a typo, and d78a5b9 reverted the rename that made the probe report a capability this branch did not have. Verified on retrieval-agent-template: build exits 0, the context carries pyproject.toml and no .env, and both of SKILL.md Step 4's probes pass -- the runtime imports and the agent constructs.
…thing Bedrock is gone from the path. Repos now keep their own provider and their own model ids and are given real keys, which satisfies M20 by construction rather than by a mapping table that had to be maintained and audited -- and it removes the model-id rewriting, the Bedrock id formats, and the surface entitlement that had closed Anthropic entirely. The shim stays, as a pass-through. It earns its place on two things a direct connection cannot give: per-repo token accounting for the results table, and one place that sees which models a repo actually calls. It swaps the key in so no real credential is written into a repo or baked into an image, and rewrites nothing else -- the rewrite rules are configurable and empty. A provider with no key is simply absent, so the screen rejects repos needing it at a shallow clone rather than after an agent has been paid to port them, and wire writes base URLs only for surfaces that can actually answer. Verified against the live OpenAI API: gpt-4o-mini passes through unchanged and is served as gpt-4o-mini, usage is attributed to the calling repo, and the unconfigured anthropic surface answers 503 rather than failing obscurely.
…vices
The run reached a real request and failed on ELASTICSEARCH_API_KEY. Two things
the screen should have caught first, each costing an agent budget to rediscover.
An import hit was short-circuiting the runtime provider strings. A repo can
import langchain_openai for its embeddings while its chat model comes from
init_chat_model("anthropic/..."), and letting the import decide reported that
repo as openai-only. Both signals now count and neither wins.
A repo that reads ELASTICSEARCH_URL or PINECONE_API_KEY gets all the way to a
served request before failing on a credential nobody supplied. That is a fact
about the repo's dependencies, not a defect in the port, so it is now a stage 2
rejection -- and when one slips through anyway, a bare missing env var at stage
8 is recorded as blocked rather than failed.
Verified on retrieval-agent-template: what took ten minutes and one agent budget
to discover is now visible from a shallow clone.
…nfusion matrix cost_usd sat empty while the shim counted tokens nobody stored. The counts now land in the row as tokens_in/tokens_out/llm_calls, which are measured; cost_usd stays for when there is a price table to multiply by. A repo stopped by its own missing backing service never put the port to the test, so counting it as a validation miss blamed validate.py for an Elasticsearch instance nobody configured. Blocked rows are excluded, and the report says so where the number is printed. Columns added after a database exists are now added in place, so a schema change costs an ALTER rather than another agent budget.
The first repo to reach stage 8 answered with status done at the Ventis layer and status failed inside its own result: it is an SSH operations agent, and the harness had asked it about animals. The port did exactly what the skill promises -- carried a request to the source and returned the source's own result -- but recording that as an unqualified pass would let a hundred-repo pass rate mean much less than it appears to. served still means the port worked, because that is what is under test. An application-level error inside the payload is now recorded alongside it, so the two can be told apart when the corpus is large enough to summarise.
The pipeline now runs inside Claude Code as testing-porting-to-ventis, rather than as an orchestrator that shells out to claude -p. Three files: the procedure, the schema, and a helper that writes a row without hand-quoting JSON into SQL. The database loses the columns nothing needed. Token counts and cost came from a proxy that existed mostly to produce them; stars, framework, is_multiagent, description, core_issue, skill_issue and analysis are written by the agent that ran the port, which is the only thing in the loop that can judge them. Static screening is gone with it. It was wrong twice in ways that each cost a full agent budget -- it read imports and missed that every LangGraph template picks its provider from an init_chat_model string, and it did not look for the backing services a repo needs -- so the skill asks the agent to read the repo and tells it, from those failures, exactly what to look for. What the harness enforced mechanically the skill now has to state, so the rules that made results attributable are written down: the source tree is never edited, the skill under test is pinned and never edited mid-corpus, validate.py does not gate the build, both probes run, and a run that proved nothing is blocked rather than failed. The agent judges scope and writes the analysis; every other stage is a command whose exit code it records rather than interprets. The harness is recoverable at f112eca if any of that turns out to be worth having back as code.
… a list
Running the skill found the hole in its own step 2. The backing-service check
named prefixes -- ELASTICSEARCH_*, PINECONE_*, MONGODB_* -- and a list reads as a
checklist: it passed a repo whose every node runs commands over SSH, because
SSH_HOST was not on it. That repo had already been through the whole pipeline in
an earlier run and served a response; the response was the NoneType error from
int(os.getenv("SSH_PORT")), and the run proved nothing.
The check is now the question the list was standing in for -- read the env vars
the source reads, and for each ask what would have to be running for this to
work -- with the grep to find them and the SSH case as the worked example.
Also states that a repo whose work is reaching such a service stays out of scope
even when a port of it builds and serves, since that is exactly the case that
looked like a pass.
…oying The skill had no cleanup step at all, and the omission cost a run. ventis deploy holds a container per replica, one for the workflow and a Redis it starts itself; none of it stops when a request finishes. A previous repo's containers were still holding :8080 when the next deploy ran, and it failed with 'Failed to launch ventis-local-workflow-0' and nothing more, because _runtime.py drops docker's stderr. Diagnosing that needed docker inspect. So: a preflight in step 7 that costs a second, a step 9 that runs on the failure paths too, and a line in the mistakes table saying what a leak actually breaks -- not the repo that leaked, the one after it. Both blocks were run as written. xargs -r rather than a command substitution: the substitution form calls docker rm with no arguments when nothing is left, and the 2>/dev/null needed to hide that would hide a real failure too.
…t is a claim The last run was a compromised measurement. I had read stub_generator.py before writing the port, so I stepped over the ImportError the build log walks you into; I knew probe 2 wanted --env-file and that -e . installs the project's own dependencies. None of that came from the skill. Every trap I already knew was a trap the skill got credit for warning about, and I graded a port whose every decision I had made. Step 4 now dispatches to a general-purpose subagent and says why never a fork: a fork inherits this conversation, which reproduces exactly the contamination the split exists to remove. The prompt is deliberately thin -- a trap you spare the porter is a trap the skill is credited for. The subagent still runs the skill's own validate and probe steps, because those are instructions in the artifact under test and stopping it would measure something else. But the observer re-runs them from scratch: there is no transcript to read, so what gets recorded is what this thread's own commands said, and a subagent claiming a green build where ours fails is a finding rather than a discrepancy to reconcile. Deploy and serve stay with the observer so step 9 has an owner. Recorded as a known gap: a subagent has no --max-budget-usd, which the Python harness had.
…rry its own metadata stub_generator.py was a hybrid -- PR #51's base with can-228's _sweep_project_files and _stub_destinations swapped in. It is now #51's file, verbatim, plus the editable install that #51 does not have. _sweep_py_files and _stub_destination are #51's, untouched, which means the flat stub copy that 01a70f2 added is gone again: #51 places a stub at one path, not two. That regression is #51's to decide on, and this branch no longer hides it. _install_step stays, because without it every src/ layout with a pyproject.toml -- the shape every langchain-ai template ships -- is unportable, and the corpus returns to empty. It no longer leans on the sweep to deliver pyproject.toml: _packaging_files copies the metadata and the README/LICENSE that metadata names, so the divergence from #51 is purely additive and confined to a feature #51 has no opinion about. Verified by rebuilding memory-agent: build exits 0, the context carries pyproject.toml, README.md and LICENSE, the install line keeps -e ., and both probes pass with the src/ package importable inside the image. test_stub_generator.py passes; the two test_cli failures predate this and come from PR #53's global os.path.isfile patch.
The skill said where the stub lands only obliquely and never gave the import line, so a porter reads the worked example instead -- and the example's `from joke_agent import JokeAgent` raises ModuleNotFoundError in the workflow image. The build copies the stub to exactly one path, and for the workflow that path is agents/<basename>.py. Both imports are now written down, with the two traps that sit on them: the flat form the examples use, and the class name the build log announces. The log says 'Generated stub class <AgentName>Stub' while the code writes <AgentName> -- the message is computed separately from the class -- and importing what the message names raises ImportError. Both cost this session real time. Verified in the image: the flat form raises ModuleNotFoundError, the agents. form imports, no __init__.py needed since agents/ is a namespace package. A memory-agent port rewritten to the documented form deployed and served.
Two rules the skill states and the validator did not check. V023 is new: the workflow must import a stub as `from agents.<basename> import <AgentName>`. It catches both ways of getting this wrong, because the project walks a reader into both -- the flat form, which is what examples/joke_writer uses and which raises ModuleNotFoundError in the workflow image, and `<AgentName>Stub`, which is the name ventis build prints while writing the class without the suffix. Run against joke_writer it flags the example, which is the point: the example is wrong. W006 now knows what the editable install resolves. It flagged langgraph and langchain_core on a project whose pyproject.toml requires both, while printing editable_install: yes in its own header three lines above. A false warning about a dependency is worse than none -- it teaches the reader to dismiss the check. Metadata it cannot read (setup.py, or no tomllib) still warns, but says so, because silence there would hide the real case. Also corrected a provenance line that has been false since the branch aligned to #51: stub_two_destinations is not on PR #51. #51 places a stub at one path; the fix that also places it flat is 01a70f2 on the skill branch and nobody has proposed merging it.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus 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}
| header = f"{finding['check']} {finding['level']:<5}" | ||
| print(f"{header} {where}" if where else header) | ||
| for line in _wrap(finding["summary"], 78, " "): | ||
| print(line) |
Comment on lines
+1237
to
+1246
| json.dumps( | ||
| { | ||
| "project_dir": project_dir, | ||
| "capabilities": capabilities, | ||
| "errors": errors, | ||
| "warnings": warnings, | ||
| "findings": report.findings, | ||
| }, | ||
| indent=2, | ||
| ) |
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.
Summary
porting-to-canyonos-coreskill, validator, and focused runtime referencesTesting
git diff --check