fix(cli): keep egc subcommands interactive and make the prompt-library gate honest - #1228
Merged
Conversation
…y gate honest Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
… Unreleased Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
❌ The last analysis has failed. |
Owner
Author
|
Note for the record: the analysis-failed comment above is a stale-report race, not a quality failure. Two pushes landed minutes apart, and the older report (1eae27e) reached SonarCloud after the newer one (8eafe90) had already been processed; the compute engine rejects older reports by design. The PR quality gate reports OK via the API, and the post-merge main-branch analysis is the binding one. |
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
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.
What
Three defects around one symptom from the v1.1.18 Windows testing report (#1217): after
npm install -g @egchq/egc+egc install, the ecosystem/prompt-library step silently never ran, leavingegc doctorwith a stale install-state (previous repo version recorded, 61 drifted managed files, 5 missing source files).egc.jsran every subcommand over a captured pipe.runCommandspawned children with the default piped stdio, printed their output only after exit, and left stdin a dead pipe. Any interactive step downstream was unreachable:install.ps1'sRead-Hostreturned$nullinstantly, and the user saw the prompt text only in the final buffered dump, which is exactly what the report describes ("it was not possible to respond to this prompt, as the installation continued automatically"). Subcommands now run on the inherited stdio: prompts reach the terminal, progress streams live, exit codes unchanged.install.ps1's interactivity gate could not see a redirected stdin.[Environment]::UserInteractivestaystruefor a child with piped stdio, and$null -eq ''isfalsein PowerShell, so the whole ecosystem block vanished with no message. The gate now also checks[Console]::IsInputRedirected, the default-Y branch accepts a$nullanswer via[string]::IsNullOrEmpty, and both installers announce the skip instead of vanishing (install.shalready gated correctly with[ -t 0 ]; it announces now too).Both installers advertised fossilized catalog counts (62 agents, 228 skills, 74 commands) while the CI-tested README ships 61/230/77. Corrected, and locked by a new parity test that reads the counts out of
install.ps1,install.shandREADME.mdso the three can never drift apart silently again.Why the doctor report looked the way it did
The skipped step is the one that reapplies managed files and rewrites install-state. Skipping it on an upgrade leaves the previous version's files behind: recorded repo version 1.1.17, 61 drifted managed files (1.1.17 content vs the 1.1.18 package), and 5 missing source files (files the 1.1.18 package no longer ships).
Out of scope, by design
The two crashes from the same report (the PowerShell 5.1 quoting SyntaxError at
install.ps1:221and the crusher-shimpathResultTypeError) are being fixed by their reporter in a follow-up PR.Tests
tests/scripts/egc.test.js: new guard thatrunCommandhands the terminal to the child; the 19 existing delegation tests all pass over the inherited stdio (output reaches the caller's descriptors directly).tests/scripts/install-ps1.test.js: new three-way parity test for the prompt-library counts, and a gate test coveringIsInputRedirected, the null-safe default-Y branch, and the announced skip in both installers.Summary by cubic
Makes
egcsubcommands interactive by inheriting stdio and fixes the installer gate so the prompt-library step runs or clearly skips instead of disappearing. Updates catalog counts to 61/230/77, adds tests to keep installers and README in sync, and records the changes in the ROADMAP.stdio: 'inherit'so prompts and progress stream live; exit codes unchanged.[Console]::IsInputRedirected; default-Y accepts null; both installers announce skip in non-interactive sessions.install.ps1,install.sh, and README.Written for commit 8eafe90. Summary will update on new commits.