Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
preinlein
changed the title
preinlein
force-pushed
the
paul.reinlein/add-smp-cli-as-dependency
branch
from
ae17f70 to
9fd4e7c
Compare
preinlein
changed the title
|
Container build errors are fixed in #1827 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fd4e7c0f9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
preinlein
force-pushed
the
paul.reinlein/add-smp-cli-as-dependency
branch
from
9fd4e7c to
c81ae74
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c81ae74a17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
preinlein
force-pushed
the
paul.reinlein/add-smp-cli-as-dependency
branch
from
c81ae74 to
cc4fe22
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4fe22365
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| warn "smp found on PATH but failed to run" | ||
| fi | ||
| else | ||
| note "smp not found" |
There was a problem hiding this comment.
With the new SKILL.md wording, only [X] checks are supposed to stop the workflow and trigger a suggested fix. Emitting note "smp not found" here leaves the script with exit code 0 and a PASS status, so an agent following /lading-preflight will keep going without ever offering the new .claude/skills/lading-preflight/scripts/install_smp remediation this patch added.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Claude is smart enough to still suggest it. I tried this exact use case myself
| mkdir -p "$SMP_INSTALL_DIR" | ||
| tar -xzf "${TMP_DIR}/${ASSET_NAME}" -C "$SMP_INSTALL_DIR" | ||
| chmod +x "${SMP_INSTALL_DIR}/smp" |
There was a problem hiding this comment.
smp on PATH instead of shadow-installing it
This always installs into ~/.local/bin, even when the script got here because an existing smp earlier on PATH was detected as broken. In that case the later smp invocations still resolve to the old binary while this script prints a successful install message, so the advertised "reinstalling" path never actually repairs the user's environment.
Useful? React with 👍 / 👎.
2 participants
What does this PR do?
Adds SMP CLI, GitHub CLI, and AWS CLI to the lading-preflight environment validation checks, along with an automated install script for the SMP CLI binary.
Motivation
The SMP CLI is used for submitting and analyzing benchmark jobs but wasn't part of preflight validation, so engineers could start optimization work only to discover the tool was missing mid-workflow. Adding it (and its dependencies
ghandaws) to preflight catches this earlier.Related issues
N/A
Additional Notes
install_smpscript resolves the latest stable version viagh release list, detects the platform, and downloads the binary from S3 to~/.cargo/bin.gh/aws/smpare reported as notes (not hard failures) since they're only needed for SMP workflows.