[REHEARSAL] 761 head on main @ 6306f8e — do not merge, do not review - #11
[REHEARSAL] 761 head on main @ 6306f8e — do not merge, do not review#11michael-moffett wants to merge 4 commits into
Conversation
Runs the command from solana-foundation#567 on the run that scaffolds a txtx.yml. The child is spawned with all three stdio handles nulled and is never waited on, so a slow, failing or absent install cannot affect startup. It is reaped on a detached thread so it does not sit defunct. Two tests cover the invocation and all three failure modes: a missing binary, a non-zero exit, and a hang.
Three follow-ups on the first-scaffold install, from review of the previous commit. The invocation carried no version, so a first scaffold ran whatever the registry called latest at that moment. It is pinned to skills@1.5.22. An exact version is the only form that resolves the same way twice; a range still floats to the newest release inside it. The child inherited the process working directory rather than the project being scaffolded. Those differ whenever -m points at a manifest outside the current directory, so the skills could land somewhere other than the project. It now runs in the manifest's directory. The spawn sat at the top of scaffold_iac_layout, so a cancelled prompt or a failure part way through left an install running behind a scaffold that never finished, and the next start began a second one. It now runs only once the scaffold has finished, from either exit that reaches that point. One added test pins the working directory. The existing test that pins the invocation now pins the version with it, and fails on a range or a bare package name.
Declining the deployment prompt printed "Deployment canceled" and fell through to the install, which spawned against the project anyway. Route the install through the confirmation so a decline builds no command, and replace the comment above it, which claimed every exit on that path was an Err.
Greptile SummaryThe PR adds a background post-scaffold installation of Solana development skills after deployment confirmation.
Confidence Score: 3/5The PR does not appear safe to merge until the skill repository content is pinned to an immutable revision. The installer package is version-pinned, but every confirmed scaffold still installs all skills from an unpinned GitHub repository revision, leaving generated projects dependent on mutable or compromised upstream content. Files Needing Attention: crates/cli/src/scaffold/mod.rs
|
| Filename | Overview |
|---|---|
| crates/cli/src/scaffold/mod.rs | Adds asynchronous skill installation and tests, but the previously reported mutable repository reference remains in the generated command. |
Reviews (2): Last reviewed commit: "fix(cli): drop the install from the exis..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| const DEV_SKILL_REPO: &str = "https://github.com/solana-foundation/solana-dev-skill"; |
There was a problem hiding this comment.
Pinning skills@1.5.22 does not make scaffolding reproducible because the command installs every skill from the repository's mutable default branch. Later upstream changes or a repository compromise can alter files installed by the same Surfpool version; pin the repository content to an immutable revision as well.
How this was verified: The generated command passes the bare GitHub repository URL to skills add --skill * without a tag, commit, or digest.
That call site sat in the arm taken when runbooks/deployment/main.tx is already present, and returned before the confirmation is bound further down, so on that path the install started with nothing to decline. It is removed rather than routed through a prompt because the arm exists to return early. A project with a runbooks tree and no txtx.yml now scaffolds the manifest and installs nothing; the only install left is the one behind the confirmation.
|
Superseded. This rehearsal PR carried head Closing rather than letting it re-run: its check results at Branch deliberately NOT deleted. Rehearsal only — was never for merge or review. |
1 participant
Internal CI rehearsal. Do not merge, do not review.
Head is
ad6bfaa6a67774c395d6763acfcade7cb09f94ba, unchanged and byte-identical to the head ofsolana-foundation/surfpool#761. Base is forkmainat6306f8ee56de85687d42593b803622fbdc69e146,which is upstream's current tip.
This PR exists only to make
pull_requestworkflows fire against that base. It supersedes fork #10,whose CI ran against the older base
e3b7df0. Nothing here is pushed to the head branch.