REHEARSAL ONLY — mechanical green at 15b9930 (do not merge) - #15
Closed
michael-moffett wants to merge 15 commits into
Closed
REHEARSAL ONLY — mechanical green at 15b9930 (do not merge)#15michael-moffett wants to merge 15 commits into
michael-moffett wants to merge 15 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.
…t once --agent universal sat in the asserted arg array eight lines below a comment saying the pin was the only departure. Name both, and say what the second buys, since the reviewer reads the comment and the array together. Also drop the version re-parse below it: the arg-vector assert already fails if the pin changes shape. The "*" guard stays; that typo is one character wide. And drop three test doc-comments whose names already carry the claim.
The arg-vector assert_eq! spells DEV_SKILL_INSTALLER on both sides, so it moves with the const: set the pin to skills@latest and that assert still passes. The re-parse dropped in bcac703 was the only thing in the tree asserting the pin's value, so dropping it made the comment above it -- "dropping it has to fail this" -- and the const's own "Exact, not a range" both untrue. Back, compact: three dotted numeric parts, and a why so the next reader does not delete it for the same reason again.
--yes with nothing on record installed the solana-dev skill without ever asking and recorded nothing, so every later scaffold silently reinstalled. The flag pre-answers surfpool's own prompts; it was never consent to pull mutable, unpinned content from a repo nobody was asked about. An absent answer is now a skip, announced on stdout with the run that does ask. A recorded yes still installs and a recorded no still skips, so a user who consented once is not re-asked and CI keeps working for them; --yes still never prompts. Nothing installs unless a human said yes.
Greptile SummaryThe PR adds an optional, consent-gated installation of the pinned Solana development skill after successfully scaffolding a new project.
Confidence Score: 5/5The changes appear safe to merge, with no concrete changed-code failure identified. The installer is limited to new-project scaffolding, requires recorded or interactive consent, uses the scaffold root as its working directory, and cannot propagate spawn or process failures into startup.
|
| Filename | Overview |
|---|---|
| crates/cli/src/scaffold/mod.rs | Adds the optional dev-skill installation workflow and comprehensive gate/command tests; no actionable changed-code defect was established. |
Reviews (1): Last reviewed commit: "fix(cli): --yes answers our prompts, not..." | Re-trigger Greptile
michael-moffett
changed the title
Member
Author
|
Rehearsal complete. Evidence retained in the run history; closing so it stops showing as open. |
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.
Rehearsal PR on our own fork. Runs surfpool's rust.yml against
15b9930merged intomain@8773ade(= upstream/main).Not for merge. Not connected to solana-foundation#761. Zero writes to upstream.
Ticket: T-DIR-P030-MECHGREEN-761-REHEARSAL-2026-08-31.