REHEARSAL ONLY — ship tree on upstream main e4e3c54 (do not merge) - #14
Closed
michael-moffett wants to merge 12 commits into
Closed
REHEARSAL ONLY — ship tree on upstream main e4e3c54 (do not merge)#14michael-moffett wants to merge 12 commits into
michael-moffett wants to merge 12 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.
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.
The install rode the deployment confirmation, so confirming a deploy also consented to a skill nobody had been asked about. It gets its own gate: silent when the skill is already present in the project or in the home directory, silent when a previous decline was recorded, otherwise one prompt naming what lands and where. The recorded decline is checked before --yes rather than after, so a machine that declined once is not talked round by a flag. Declining the deployment now records nothing, because that is "not now", not "never". The invocation also names a single agent. Detecting none of its own agents installed, the installer fanned the skill out to all 77 in its registry, dropping .claude/, a non-hidden agent/ and skills-lock.json into a project that has no .gitignore yet. Naming universal reduces that to the canonical .agents/skills copy and the lock file, which is what the prompt is now able to promise.
The 1.5.22 to 1.5.23 diff is 578 lines and touches none of install, lock-file writing, the .agents constants, the #ref clone parsing or symlinking. It is the interactive picker, which stdio-nulled invocation never reaches, plus a git tree-hash helper confined to the global update command. Its one change on our path adds an agent to the registry, which the named agent makes moot.
57 comment lines to 37 across the production half, ratio 0.107 to 0.070, against scaffold/mod.rs at 0.058 and its siblings at 0.00 to 0.11. Zero code lines changed. Their file carried 25 comment lines and this PR had added 32 more. Comments that narrated the line below them are gone; the ones carrying a decision the code cannot show are kept, shortened where prose had crept in. Kept in full: the DEV_SKILL_AGENT block, because the 77-agent fanout it prevents appears nowhere in the code. Kept, shortened: the gate ordering, since 'a recorded no outranks --yes' is a decision and not a mechanism. Verified: cargo +nightly fmt --check PASS, clippy clean on this file (the 11 crate warnings are pre-existing and none are here). The test bar did not run: postgres is not up on this host after a restart, and their CI supplies it as a service container. A comments-only diff cannot fail tests without failing the compile, which it does not.
Greptile SummaryThe PR adds optional installation of the pinned Solana development skill after on-disk scaffolding.
Confidence Score: 3/5The PR is not safe to merge because the previously reported installer-consent bypass remains outstanding. The runbook-only Files Needing Attention: crates/cli/src/scaffold/mod.rs
|
| Filename | Overview |
|---|---|
| crates/cli/src/scaffold/mod.rs | Adds the development-skill installation command, consent-state helpers, asynchronous execution, scaffold integration, and focused unit tests. |
Reviews (2): Last reviewed commit: "feat(cli): record the yes too, so neithe..." | Re-trigger Greptile
michael-moffett
changed the base branch from
rehearse-base/761-2026-08-30
to
main
August 31, 2026 02:19
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| base_location, | ||
| &base, | ||
| Path::new(&home), | ||
| auto_generate_runbooks, |
There was a problem hiding this comment.
When a fresh project is started with --yes, the runbook-only flag is passed as auto_accept and silently authorizes npx to download and execute an external installer that writes .agents/skills/solana-dev and skills-lock.json. The flag is documented only as “Skip runbook generation prompts,” so this installation occurs without informed consent. How this was verified: The flag was traced from its CLI definition through the scaffold caller to the branch that bypasses the dedicated installation prompt and spawns npx.
michael-moffett
changed the title
Member
Author
|
Superseded by the 15b9930 rehearsal. Closing. |
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.
Mechanical rehearsal vehicle. Not for merge. Fork-internal. Zero contact with solana-foundation.
Head
03570b0= locald0ef99b(the solana-foundation#761 ship tree, incl. the 2026-08-30 consent fix) rebased onto upstreammaine4e3c54. 12 commits replayed, zero conflicts.Answers the question the rehearsal exists for: what does the rebase break?
Expected job set (derived 2026-08-24,
REHEARSAL_761_POST_DROP_2026-08-24.md§2):license_check,run_cargo_checks,build, plus theGreptile Reviewapp check.Local pre-run on this exact tree:
cargo deny check licensesPASS,cargo +nightly fmt --all -- --checkPASS,cargo clippy --workspace --all-targets0 errors.