Add the Golang CLI (vip-next) alongside the Node CLI - #3013
Open
rinatkhaziev wants to merge 32 commits into
Open
Add the Golang CLI (vip-next) alongside the Node CLI#3013rinatkhaziev wants to merge 32 commits into
rinatkhaziev wants to merge 32 commits into
Conversation
Adds the Go module rooted at github.com/Automattic/vip, and the .gitignore entries for Go build artifacts and the vendored go-search-replace binaries. The schema.gql negation is required: the blanket schema.gql rule would otherwise exclude internal/gql/schema.gql, which is checked in. go.mod and go.sum are intentionally not `go mod tidy`-clean: tidy strips the go.sum entries for genqlient's own dependencies, which breaks `make verify-gql-stale`. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
third_party/go-search-replace/MANIFEST pins the go-search-replace release; the per-platform binaries are fetched and checksum-verified by `make vendor-search-replace` and stay out of git. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
The schema and the 19 .graphql operation documents are the codegen inputs; the generated bindings land in the next commit so they can be reviewed separately, or skipped. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Generated by genqlient from the schema and operations in the previous commit. Regenerate with `make tidy-gql`; `make verify-gql-stale` fails CI if this file drifts from its inputs. Reviewers can skip this file. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
The hand-written half of internal/gql: request execution, HTTP transport, retry and backoff, error shaping, and the rechallenge hook that drives step-up auth when the API demands it. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
searchreplace and sqlreport live here rather than with the root command: both exist to serve `vip import sql`. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
All 36 files sit behind `//go:build parity` so they do not affect a normal build. The harness diffs vip-next against the Node CLI built from src/; `make test-parity-unit` runs it. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Recordings are scrubbed: app and environment IDs, hostnames, emails and tokens are synthetic and internally consistent across sequenced fixtures. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
The Node CLI is a build input: the job runs `npm ci` and diffs vip-next against it, so changes under src/ retrigger this workflow. devenv-e2e.yml and windows-tests.yml are deliberately not touched — they are existing Node workflows and trunk's copies are newer than the rewrite branch's. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Builds, signs and notarizes vip-next for macOS, Linux and Windows, and produces a universal stapled .pkg. Coexists with the existing sea-build-sign.yml, which signs the Node SEA build. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Contributor
Dependency ReviewThe following issues were found:
License Issues.github/workflows/ci-go.yml
Gemfile
OpenSSF ScorecardScorecard details
Scanned Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+46
to
+113
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.27' | ||
| check-latest: true | ||
| cache: true | ||
|
|
||
| # The Node CLI is the reference implementation the differential parity | ||
| # scenarios diff vip-next against. Without it, every Node-vs-Go scenario | ||
| # skips and this job goes green having compared nothing — which is | ||
| # exactly how ~90 parity divergences reached a review unnoticed. | ||
| # | ||
| # Version comes from .nvmrc (lts/*), the same source ci.yml pins by hand. | ||
| # It must satisfy package.json#engines (>=22.19.0 on trunk 4.1.0) or the | ||
| # `postinstall` guard (helpers/check-version.js) aborts the install with | ||
| # exit 1. | ||
| - name: Set up Node.js environment | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version-file: '.nvmrc' | ||
| cache: npm | ||
| cache-dependency-path: package-lock.json | ||
|
|
||
| # `npm ci` runs the `prepare` lifecycle script (clean + babel build), | ||
| # which is what produces dist/bin/vip.js. No separate build step needed. | ||
| - name: Install Node dependencies and build the Node CLI | ||
| run: npm ci | ||
|
|
||
| # Fail loudly if the above did not actually yield a runnable Node CLI. | ||
| # `make test-parity-unit` only WARNS in that case, on purpose, so that a | ||
| # contributor without node_modules is not hard-failed. CI has no such | ||
| # excuse: here a skipped differential is a broken build. | ||
| - name: Assert the Node-vs-Go differential can actually run | ||
| run: make require-node-vip-bin | ||
|
|
||
| - name: go mod download | ||
| run: go mod download | ||
|
|
||
| - name: Verify generated GraphQL code is fresh | ||
| run: make verify-gql-stale | ||
|
|
||
| # Via make, not bare `go vet ./...` / `go test ./...`: now that `npm ci` | ||
| # has run, node_modules is inside the module and a bare `./...` would | ||
| # compile and vet an npm dependency's vendored Go package | ||
| # (node_modules/flatted/golang/pkg/flatted). The make targets discover | ||
| # the package list and drop node_modules from it. | ||
| - name: go vet | ||
| run: make lint | ||
|
|
||
| - name: go test | ||
| run: make test | ||
|
|
||
| # This is the step the Node CLI was installed for. On a Linux runner the | ||
| # credential the Node CLI reads comes from configstore rather than a | ||
| # system keyring (see internal/parity/keychain.go); the harness seeds | ||
| # through Node's own getKeychain(), so it lands in whichever store Node | ||
| # itself would read, and a store that cannot be driven at all produces a | ||
| # loud skip rather than a hang or a false pass. | ||
| - name: Offline compatibility and parity-harness tests | ||
| run: make test-parity-unit | ||
|
|
||
| - name: make build | ||
| run: make build | ||
|
|
||
| - name: smoke | ||
| run: ./bin/vip-next --version |
rinatkhaziev
changed the title
The Node repo runs prettier and eslint over the whole checkout, so landing the Go tree made both fail: prettier flagged 214 files and eslint flagged internal/wpstream/testdata/fixture-server.js, a WebSocket server fixture for Go tests. The parity recordings under testdata/ must stay byte-exact. Reformatting them would change the payloads the harness replays and invalidate the expected_drift signatures, so they are ignored rather than reformatted. The four new docs and .buildkite/pipeline.yml are prettier-formattable and are simply formatted to the repo's style; pipeline.yml was re-validated as YAML after formatting. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
ci-go had never actually executed before this PR — every run of it in vip-cli-golang is a startup_failure at 0s — so the harness had only ever run on macOS, where an OS keyring is always available. On a headless Linux runner there is no D-Bus secret service, so vip-next falls back to a 0600 credentials file and says so once on stderr. The Node CLI uses configstore and has no equivalent concept, so it says nothing. That one line failed 32 differential scenarios on Linux that all pass on macOS. The divergence is real and user-visible, so the warning stays and is announced in docs/CUTOVER-BREAKING-CHANGES.md (new section 4d). What changes is the harness: ambientStderrRules strips environment-dependent noise before any comparison, including before the expected_drift signature is computed, so an accepted divergence fingerprints identically on both platforms. Recorded once as an environment property rather than as 32 per-scenario expected_drift entries, because it appears in every scenario on one platform and none on the other. The rule is deliberately one anchored whole-line pattern; a test asserts that a real stderr message still diverges, since a pattern one character too broad would blind the harness to what it exists to catch. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
Three things, all aimed at the four scenarios still failing on Linux: A signature mismatch previously reported two hashes and nothing else, which says a blessed drift moved but not how, and turns every mismatch into a bisect. It now prints the normalized output the signature was taken over. TestPhpmyadminSilentParity asserts on stderr directly rather than through Diff, so the ambient keychain notice reached it untouched. It now strips the same ambient rules, testing the flag rather than the runner. Whether --silent ought to suppress that notice too is a real question about the flag's contract, and a separate one. The three envvar prompt scenarios gained an ANSI strip on stderr, since Node renders those prompts through enquirer. That strip is a no-op on macOS, so it is not yet confirmed to be what differs on Linux — the improved diagnostics above are there to find out. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
The three non-TTY prompt scenarios compared Node's raw enquirer output byte-for-byte, which made their drift signatures macOS-specific. They could never have passed on Linux, which went unnoticed because ci-go had never actually run. Two platform-dependent things, both on stdout rather than stderr: - colour and cursor-control sequences around the prompt - the pointer glyph, U+203A on macOS and U+2023 on Linux The second is what actually broke these in CI; the ANSI rule alone was not enough. Both are now normalized, the glyph to a literal <POINTER> so the substitution is visible in any future diff rather than silently erased. Signatures are recomputed over the normalized text and are now identical on both platforms. The prompt's wording survives normalization, so the drift these scenarios exist to record - Node prompting where vip-next refuses to in a non-interactive shell - is still compared in full. Ported from vip-cli-golang@72ef2f89 (feature/go-rewrite).
|
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
2 participants
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 this is
The Go rewrite of the VIP CLI (
vip-next), ported from the internalvip-cli-golangrepo at72ef2f89(feature/go-rewrite).This is additive. The Node CLI is untouched and still ships as
vip. TheGo binary ships as
vip-next. Cutover is a separate, later PR.How to read it
The 7,107-commit rewrite history is deliberately not replayed. The 28 commits
here are hand-authored to be read in order, bottom-up — primitives, then
transport, then domain, then the command surface, then verification and
release:
Only the PR head is green. Individual commits are a reading device, not
bisect points.
Reviewers can skip commit 5 —
internal/gql/generated.go, 8,935 lines ofgenqlient output.
make verify-gql-stalefails CI if it drifts from itsinputs.
Verification
Run locally against the branch head:
make build— cleanmake test— 50 packages, no failuresmake test-parity-unit— passesmake test-parity-unit-hostile— passes (proves fixtures are ambient-independent)make lint— cleanmake verify-gql-stale— generated bindings up to dateThe Windows build is unverified locally.
make.ps1needs a Windows host.It must be confirmed in CI before merge.
The
Run Windows Testsjob is the pre-existing Node workflow and is flakyindependently of this PR — it fails with
Failed to connect to Dockeron theWindows runner, has failed the same way on unrelated branches, and passed four
times on this branch. This PR changes no Node or Windows-workflow files.
Parity
ci-go.ymlbuilds the Node CLI (npm ci) and diffsvip-nextagainst it, sochanges under
src/retrigger the workflow. 16 scenarios carry an accepted,documented Node-vs-Go divergence recorded as
expected_driftwith anormalized-output signature.
Fixtures under
testdata/parity/were scrubbed before this PR: theapp-listrecordings carried real app IDs and a real repo name from a live capture, now
synthetic (
42/43,wpcomvip/example-multisite). Twoexpected_driftsignatures were recomputed as a direct consequence; the drift reasons
(parity register 1.2 and 1.3, KEEP) are unchanged. SQL, archive and image
fixtures were inspected individually and were already synthetic.
ci-go had never actually run before this PR
Every
ci-gorun invip-cli-golangis astartup_failureat 0s. This PR isthe first time the workflow has ever executed, which means the parity harness
had only ever been exercised locally on macOS. Three Linux-only defects fell
out, all fixed here, all in the harness rather than in product code:
The keychain fallback notice. On a host with no D-Bus secret service,
vip-nextstores credentials in a 0600 file and says so once on stderr;the Node CLI has no equivalent concept and says nothing. That one line
failed 32 of the differential scenarios. It is now normalized away globally
(
ambientStderrRules), and announced as a real user-visible difference indocs/CUTOVER-BREAKING-CHANGES.mdsection 4d. The warning itself stays —storing a credential in plaintext is worth saying out loud.
The enquirer prompt scenarios. Three non-TTY prompt scenarios compared
Node's raw prompt byte-for-byte, so their drift signatures were
macOS-specific and could never have passed on Linux. Two things differ:
the ANSI colour/cursor sequences, and — the one that actually broke them —
enquirer's pointer glyph,
U+203Aon macOS andU+2023on Linux. Both arenormalized; signatures recomputed and now identical on both platforms.
Diagnostics. A drift-signature mismatch used to report two hashes and
nothing else, which says a blessed drift moved but not how. It now prints
the normalized output the signature was taken over.
In every case the scenario's actual subject — the Node-vs-Go behavioural
divergence — is still compared in full. A test asserts the ambient rule stays
narrow enough that a real stderr message still diverges.
Notes for reviewers
go.mod/go.sumare intentionally notgo mod tidy-clean. genqlient isrun as a tool (
go run github.com/Khan/genqlient) with notools.goand notooldirective, so tidy cannot see its imports and strips 18go.sumentries — which makes
make verify-gql-stalefail. Please don't tidy.ci-go.yml's threenpm-shrinkwrap.jsonreferences were corrected topackage-lock.jsonduring the port. Neither repo has annpm-shrinkwrap.json; as written the npm cache missed and the workflow didnot retrigger on lockfile changes.
devenv-e2e.ymlandwindows-tests.ymlare deliberately not included.They already exist on trunk as Node workflows, and trunk's copies are newer
than the rewrite branch's (
checkout@v7/setup-node@v7vsv6). Portingthem would have silently reverted trunk's Node CI.
sea-build-sign.yml, whichsigns the Node SEA build. Worth confirming they don't contend for the same
Apple credentials, notarization profile, or agent queue.