Skip to content

Guyen/develop guard code overwrite 2 - #171

Open
ggankhuy wants to merge 2 commits into
developfrom
guyen/develop-guard-code-overwrite-2
Open

Guyen/develop guard code overwrite 2#171
ggankhuy wants to merge 2 commits into
developfrom
guyen/develop-guard-code-overwrite-2

Conversation

@ggankhuy

@ggankhuy ggankhuy commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Motivation

Add a pre-flight check before copying the script directory contents (models.json, the
run script, and friends) into model_dir. If any filename collides with an existing
file in the destination, the copy aborts with an error instead of silently overwriting.

Technical Details

When a url is specified in models.json, model_dir is a checked-out repository.
The script directory is then copied on top of it. Because most models ship a run.sh
as their default run script, a run.sh already present in the checked-out repo gets
clobbered — the model_dir repo's own version is destroyed with no warning or trace.
The guard compares each source file against the destination directory and exits with an
error on the first name match, so the overwrite can never happen unnoticed.

Resolving a collision

Rename the colliding file on one side and update the reference in models.json. PR
#46
is a one-time
demonstration of this: run.sh was renamed to run_inference.sh, with the matching
update in models.json.

Test Plan

Run demo PR with the madengine branch containing this PR.

  1. Negative test case: deliberately cause name collision (run.sh in this case was not renamed) and ensure workflow will exit with said error.
  2. Positive test case: ensure no false error as well, in case of no name collision, do not raise false error: run the same negative case now with the renamed run.sh => run_inference.sh and ensure workflow completed successfully (both build and exec).

Test Result

See comment section below for negative (#171 (comment)) and positive (#171 (comment)) test case results.

Submission Checklist

Test Steps:

Negative test (case of name collision)

  • use madengine branch: "guyen/develop-guard-code-overwrite-2"
  • run inference workflow: example model/tag can use: "pyt_deepspeed_megatron_gpt3_125m"
  • During either build or exec, madengine should force error and refuse to continue with following error:
  • [MAD Benchmark Workflow/mad-benchmark] | > docker exec 56f9b77e5821 bash -c 'for f in scripts/pyt_deepspeed_megatron_gpt3_125m/*; do if [ -e "Megatron-DeepSpeed/$(basename $f)" ]; then echo "ERROR: $f already exists in Megatron-DeepSpeed"; exit 1; fi; done'

**[MAD Benchmark Workflow/mad-benchmark]   | ERROR: scripts/pyt_deepspeed_megatron_gpt3_125m/run.sh already exists in Megatron-DeepSpeed**
[MAD Benchmark Workflow/mad-benchmark]   | ===== EXCEPTION =====
[MAD Benchmark Workflow/mad-benchmark]   | Exception: Subprocess 'docker exec 56f9b77e5821 bash -c 'for f in 
[MAD Benchmark Workflow/mad-benchmark]   | scripts/pyt_deepspeed_megatron_gpt3_125m/*; do if [ -e 
[MAD Benchmark Workflow/mad-benchmark]   | "Megatron-DeepSpeed/$(basename $f)" ]; then echo "ERROR: $f already exists in 
[MAD Benchmark Workflow/mad-benchmark]   | Megatron-DeepSpeed"; exit 1; fi; done'' failed with exit code 1
[MAD Benchmark Workflow/mad-benchmark]   | Traceback (most recent call last):
[MAD Benchmark Workflow/mad-benchmark]   |   File "/home/***/extdir/gg/git/amdeng/MAD-internal/madengine-act/src/madengine/execution/container_runner.py", line 1549, in run_container
[MAD Benchmark Workflow/mad-benchmark]   |     model_docker.sh(
[MAD Benchmark Workflow/mad-benchmark]   |   File "/home/***/extdir/gg/git/amdeng/MAD-internal/madengine-act/src/madengine/core/docker.py", line 136, in sh
[MAD Benchmark Workflow/mad-benchmark]   |     return self.console.sh(

Positive test:

  • Simply rename run.sh to i.e. run_inference.sh
  • Since models.json treats run.sh as default, change script name/path in models.json appropriately i.e. and re-run the same way :

From:
scripts/dummy/models.json:5: "scripts": "run.sh",
To:
scripts/dummy/models.json:60: "scripts": "run_inference.sh",

27991 – personal token.

@ggankhuy

Copy link
Copy Markdown
Contributor Author

@ggankhuy

Copy link
Copy Markdown
Contributor Author

@ggankhuy
ggankhuy marked this pull request as ready for review August 13, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant