[Improvement] Keep Canyon artifacts under .car - #57
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team 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 |
6ae546c to
2423ff3
Compare
d8df835 to
e4cdeff
Compare
|
Small effort changes USER line in the Dockerfile, non root. This is a Must fix. Costs one line, closes a real gap. Skip symlinks in the .car/app rsync, not just in the later per-agent sweep. Again a Must to avoid security issues. Closes a path traversal gap. Sweep all the files an agent needs, not just .py. Must fix. Measure the compilation time though for each workload across a few models with this skill file for now, thats short term. Medium effort Stage into a temp dir and swap instead of writing straight to final paths. This is a Must. Partial builds leaving a mixed state is a real reliability problem. Tag images with a build id or digest instead of the static ventis-. this is good to have. Useful for debugging and rollback, but doesn't block anything working today. Staleness check between .car/app and real source. Again, good to have. Dev experience improvement, the port model already assumes you explicitly re-run it. Long term Check for StrictHostKeyChecking=no, on SSH connection - right now its ok but in long term as disabling host key verification will trust whoever answers. for integrity, hash stubs/grpc_stubs/images and verify against each other so the protobuf mismatch kind of bug gets caught at build time. This is good to have, not must to fix. Be mindful of full build copies, you want to copy once and not N times its needed. This is what we discussed in standup as performance and user experience issue, not correctness. |
e4cdeff to
106b9ac
Compare
|
|
||
| # Add generated grpc_stubs from the local project to the path | ||
| sys.path.insert(0, os.path.abspath("grpc_stubs")) | ||
| # Add generated grpc_stubs to the path. Commands run from the application |
There was a problem hiding this comment.
Will this break existing functionality.
There was a problem hiding this comment.
The context is that the current solution of this PR always assumes the project has a .car folder. If it doesn’t, it will break until the developer runs canyonos integrate to instrument the code and generate the .car.
After discussing with @Saaketh0 , we're planning to set up a fallback like:
prefix(root):
if isdir(root / ".car"): return ".car"
else: return "" # fall back to legacy logic
| project_root = os.path.join(script_dir, "..", "..") | ||
| default_config = os.path.join(project_root, "config", "global_controller.yaml") | ||
| default_config = os.path.join( | ||
| os.getcwd(), ".car", "config", "global_controller.yaml" |
There was a problem hiding this comment.
Same question, what happens if a person just wrote in Ventis.
| "--config", | ||
| default=default_config, | ||
| help="Path to the YAML config file (default: config/global_controller.yaml)", | ||
| help="Path to the YAML config file (default: .car/config/global_controller.yaml)", |
There was a problem hiding this comment.
How will we handle default settings ?
| DEFAULT_CONFIG_PATH = "config/global_controller.yaml" | ||
| ARTIFACT_DIR_NAME = ".car" | ||
| SOURCE_DIR_NAME = "app" | ||
| DEFAULT_CONFIG_PATH = f"{ARTIFACT_DIR_NAME}/config/global_controller.yaml" |
| # policy.yaml declare no `agent.name`, so they fall out here. | ||
| yaml_by_name = {} | ||
| for yaml_path in yaml_files: | ||
| for yaml_path in sorted(glob.glob(os.path.join(config_dir, "*.yaml"))): |
106b9ac to
872d72b
Compare
|
The latest fallback changes have already been committed, but there’s a new convention we should follow to keep things consistent: for .car, all agent.yaml files should live under the config folder., i.e.
If we want to standardize on placing the agent.yaml under /config, we should align on that convention first, or the portfolio will break. I'll recommend standardize it, i.e. project with .car or without .car should have their agent.yaml under /config 
|
3 participants
Put all skill porting outputs under
.car, mainly adjusting the file paths in build and deploy.The corresponding skill file change is #67, stacked on this branch. Merge this PR first; #67 retargets to
mainon its own once this lands.Scope
ventisonly —cli.py,stub_generator.py,global_controller.py, the Local runtime,README.md,.gitignoreand the test suite. No.claude/skills/changes here anymore; those moved to #67.