Sweep the whole project into images, not only .py - #65
Open
nickhuo wants to merge 8 commits into
Open
Conversation
The build's fallback sweep collected only `.py`, so anything else the source opens at runtime -- PDFs, notes.txt, pyproject.toml, langgraph.json -- never reached the image, and the port failed only once the agent was serving. This accounts for 13 findings across 10 corpus repos. `_sweep_py_files` becomes `_sweep_project_files` and takes every file at its relative path. The rename also fixes the porting skill's capability probe, which checked `hasattr(stub_generator, "_sweep_project_files")` and so reported `sweeps_all_files: no` unconditionally. Broadening the sweep needs guardrails, since "copy everything" would otherwise bake secrets and host junk into images: - skipped: hidden files and directories (where .env and .git live), the generated stubs/, grpc_stubs/, docker_container/ at the root, __pycache__, node_modules, venv, site-packages, *.egg-info, host bytecode, and symlinks; - skipped with a warning: private key material (.pem, .key, .p12, .pfx), and the root Dockerfile, requirements.txt, and workflow_launcher.py. The build context writes its requirements.txt before the copy runs, so a project's own file at that name would have overwritten the generated one and stripped the base runtime dependencies out of the image.
|
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 |
Review of the previous commit found three ways it was wrong, plus one hole the end-to-end run exposed. Silent drops. Hidden paths were dropped without a word, which is the same bug CAN-282 exists to fix -- a project keeping runtime assets in .streamlit/ or .prompts/ still failed only once the agent was serving, now with no trace of why. Every exclusion is reported: hidden paths as one aggregated note, private keys and reserved names individually. Name-based key matching was theater. It caught .pem and waved through id_rsa, which has no extension at all. PEM material is now found by its armor whatever the file is called, and a certificate -- public, and sometimes needed -- ships instead of being lumped in with private keys. The docstring says plainly that this is not a secret scanner: credentials.json still ships, because nothing can recognize it. No size signal. Broadening the sweep means a dataset or a virtualenv under a name _SKIPPED_DIRS misses now lands in every image, where before only .py did. Past 100 MB the sweep says how big it got and what the largest file was, which is also the backstop for whatever the hardcoded lists fail to catch. The hole: the build context is assembled inside the project root, so the sweep copied it into itself. `docker_container/` happens to be in _GENERATED_DIRS, so the CLI path was covered by coincidence rather than by construction; any other output directory nested a copy of the context inside itself. The context is now excluded by resolved path. Those lists are hardcoded with no way for a project to override them, which is a consequence of assembling the context by hand instead of letting Docker's own ignore mechanism run. The comment says so.
Restating a constant's contents in prose above it is noise. What stays is the part the code cannot state: why the lists are hardcoded with no override, why requirements.txt is reserved, why keys are matched by armor and not by name, and why exclude_dir is compared by resolved path.
largest = max(largest, (size, rel_dst)) compares tuples, so two files of equal size fall through to comparing their paths -- and the initial (0, None) meets the first zero-byte file the sweep finds. An empty __init__.py is in nearly every Python project, so this took the build down with a TypeError. Every test passed because the test helper writes one byte by default. Compare the size and nothing else. The hidden-path note fired on .git, .gitignore, .venv and every tool cache, which is to say on every real build. A note that always fires is wallpaper, and it takes the one that matters -- a project keeping prompts in .prompts/ -- down with it. Ordinary repo furniture is still held back, just no longer announced. A wrong guess in that list costs a line of output rather than a missing file, which is why it sits apart from the lists that decide what ships.
The old sweep pruned only hidden directories, so a .py file under venv/, node_modules/, site-packages/ or an .egg-info/ did reach the image. _SKIPPED_DIRS drops those directories entirely, which is right, but doing it silently is a behavior change nobody can see -- the same silent-drop bug this ticket exists to fix. They are now reported. __pycache__ stays silent because it never held anything shippable, which leaves the invariant clean: the only quiet drops are bytecode, symlinks, __pycache__, and the build's own output, and not one of those could ever have shipped.
Symlinked files were already skipped. Symlinked directories were skipped too, but only because os.walk defaults to followlinks=False -- a security property resting on a default someone can flip, and one that cannot report what it skipped. Both cases are now pruned by the same rule in the same function: a link to /etc or to a home directory would copy files from outside the project into the image. They are reported rather than dropped quietly, because the target may be a shared config a monorepo expects in the image. The answer there is to copy the target in, not to follow the link. No new constant and no new knob: one predicate, two loops, one note.
The directory loop built os.path.join(root, name) three times and the first one ran to 89 characters, the one formatting deviation this branch added to an already-unformatted file. One local name fixes both.
nickhuo
changed the title
Saaketh0
reviewed
Sep 3, 2026
| return b"-----BEGIN" in head and b"PRIVATE KEY-----" in head | ||
|
|
||
|
|
||
| def _sample(paths, limit=5): |
Collaborator
There was a problem hiding this comment.
this is a nitpick, but could the _worth_reporting and _sample functions be named better? the name was not intuitive at all and the docstring below didn't help too much in understanding the purpose of the two functions
Saaketh0
reviewed
Sep 3, 2026
Saaketh0
left a comment
Saaketh0
left a comment
Collaborator
There was a problem hiding this comment.
lgtm, made a nitpicky comment but everything else good
nickhuo
marked this pull request as ready for review
September 3, 2026 03:09
Collaborator
Author
|
@saurabh.a this pr is good to be reviewed |
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
2 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.
Closes CAN-282. Second half of CAN-228; follows CAN-230 (#51).
The bug
The build's fallback sweep collected only
.py. Anything else the source opens at runtime — PDFs,notes.txt,pyproject.toml,langgraph.json— never reached the image, and the port failed only once the agent was serving. 13 findings across 10 corpus repos trace to this.The fix
_sweep_py_filesbecomes_sweep_project_filesand takes every file at its relative path.The rename matters beyond readability: the porting skill's
validate.pyprobeshasattr(stub_generator, "_sweep_project_files")for itssweeps_all_filescapability, so that probe reportednounconditionally. It now reports the truth.