[REHEARSAL] cli: stop a declined confirmation from starting the skill install - #10
Closed
michael-moffett wants to merge 3 commits into
Closed
[REHEARSAL] cli: stop a declined confirmation from starting the skill install#10michael-moffett wants to merge 3 commits into
michael-moffett wants to merge 3 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 SummaryThis PR adds a pinned, asynchronous development-skill installation after successful on-disk scaffolding while ensuring a declined deployment confirmation does not start that installation.
Confidence Score: 5/5The implementation appears safe to merge, although the PR description explicitly marks this rehearsal change as not intended to land. The declined-confirmation path constructs and spawns no installer command, while successful scaffold paths launch the pinned installer only after their required filesystem work completes.
|
| Filename | Overview |
|---|---|
| crates/cli/src/scaffold/mod.rs | Adds best-effort skill installation and confirmation gating; no actionable changed-code defect was identified. |
Reviews (1): Last reviewed commit: "fix(cli): stop a declined confirmation f..." | Re-trigger Greptile
Member
Author
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
1 participant
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.
DO NOT MERGE. Fork-internal CI rehearsal for the fix carried into upstream PR solana-foundation#761. Opened against
mainsorust.yml(pull_request: branches: ["main"]) can actually trigger; the previous attempt (#8) was based onrehearsal-base-761and no workflow in this repo can trigger on a PR into a branch by that name.Scope:
crates/cli/src/scaffold/mod.rs. This PR exists to produce check runs, not to land.