Repository navigation
fix!: close divergences from the reference CLI and gate on go-tools - #37
Merged
Merged
Conversation
Run `go fix ./...` (Go 1.27.3) over the tree: `any` for `interface{}`,
maps.Copy, slices.Contains, strings.CutPrefix and SplitSeq, range over
int, errors.AsType, and omitempty dropped where it cannot apply to a
struct field. The change is mechanical and keeps behavior: build, vet
and the unit tests pass unchanged, so the commits after it show only
deliberate changes.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
A differential run of read-configuration against the TypeScript oracle over every upstream fixture, a whole-repository mutation run and a dead code review against the reference found behavior the parity matrix never compared. This change ports the reference behavior, each with a unit test or a new parity case that fails on the previous code: - read-configuration: forwardPorts merge as raw values; mergedConfiguration keeps every config key; image build info for Dockerfile, compose and image configs (local inspect, then registry, then pull); Feature errors propagate instead of being dropped. - Features: FEATURE ADVISORIES warning; direct tarball digest checked against the lockfile; disallowed Features rejected in document order, after --remove-existing-container, stopping a running container; the install layout and Dockerfile of the reference (cache folder per build, consecutiveId folders, builtin env, install wrapper, BuildKit bind mounts or a content image); a Feature's updateContentCommand kept in the image metadata, so up runs it; sourceInformation of local and tarball Features in the reference shape; control manifest errors logged as the reference logs them. - features package/publish and templates publish: packaging, collection refs, fail-fast publish, legacy ids, template metadata; <target> is required. - up, run-user-commands, set-up: hook errors name the command and its origin; lifecycle markers and dotfiles order shared by the three commands; the reference entrypoint script; --id-label alone no longer attaches without a config; no workspace mount without a workspace; compose runtime override applied without Features, additional_contexts with Compose >= 2.17; updateRemoteUserUID decided on the merged config, built for the image platform, and its error propagated. The cleanup the review asked for lands in the same files and functions: unreachable code removed (each checked against the reference), the httpx client, pfs wrappers and Dockerfile builder replaced by the standard library, duplicated env, enum, option and probe helpers shared. It is not split into its own commit because it changes signatures the fixes above call from the same functions; an intermediate commit would not build. TestDifferentialReadConfiguration (task parity:differential) keeps the comparison running. docs/DIVERGENCES.md records the remaining choices and docs/migration.md the upgrade steps. BREAKING CHANGE: `features package`, `features publish` and `templates publish` require the <target> folder, as the reference does; pass it explicitly (for example `devcontainer features package .`). read-configuration fails on a legacy GitHub Releases Feature instead of omitting it, and `up --id-label` without a config or workspace folder fails instead of attaching. Co-Authored-By: Claude Opus 5.5 <[email protected]>
`task lint` now runs golangci-lint at the version versions.yaml pins (the existing .golangci.yml was never run), plus go-tools' ctxlife, testquality, refs, mutate -stale-exempts, deadgate over deadcode, taskgate and sfw. `task vuln` runs vulngate instead of an unpinned govulncheck@latest. Pull requests get a mutate job over their diff. versions.yaml is the single pin for the golangci-lint image and the distroless base, which the Dockerfile and GoReleaser read by digest. The shell in release:resolve and metrics moves to cmd/devtool, with tests. The toolchain is go1.27.3 and go-tools is pinned at 0ee2224, which carries the taskgate submodule fix (spin-stack/go-tools#64). Co-Authored-By: Claude Opus 5.5 <[email protected]>
A runtime parity shard took 10.7 min at p50 and 14.4 at p90 on GitHub's 4-vCPU runners (20 shards, 2026-10-24..25), against 5.0 on Blacksmith's 4 vCPU: nearly all of it is container builds and pulls. The jobs that build and run containers (runtime parity, e2e, mutate, publish parity and the release's runtime gate) now land on the docker8 fleet, an ephemeral 8-vCPU EC2 instance per job. A pull request from a fork stays on GitHub's runners, so its code never reaches spin-stack's AWS account. The runtime matrix is two shards at -parallel 4 (2 on GitHub's runners), stops at its first failing shard on push and pull request, and every job has a timeout. A newer push cancels the run before it, except on main. ci:prepare-runner deletes the preinstalled SDKs only on a runner with less than 60 GB free. Co-Authored-By: Claude Opus 5.5 <[email protected]>
go tool mutate found edits no test refused: the scanner buffer's sizes and the ':' cut in ParseProfile change nothing a profile can show (a position holds no '/', and no profile line nears the default limit), so both go; MeanMillis's divide by 1000 becomes time.Millisecond, and the runs=1 path and the unit of the mean get tests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Mutation testing showed edits in compose_up.go that no caller could observe, because the code under them did nothing a result depends on: - composeFeatureSets returned early on no Features, which fetchFeatureSetsWithOrder already answers with nil; - filepath.Abs on the compose Dockerfile path: os.ReadFile and the .dockerignore copy resolve a relative path against the same working directory; - the runServices filter was written twice, for compose build and up; it is now composeServicesFilter; - the container-labels condition was evaluated twice, once for the log line and once for the restore decision; - newRandomUUID allocated a byte nothing printed and set the version and variant bits in one expression whose two indexes read the same random byte. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
No unit test drove startComposeContainer, buildAndExtendCompose, writeComposeFeaturesOverride or composeFeatureSets, and the pure helpers next to them (splitShellWords, dockerfileSupportsBuildContexts, checkPersistedComposeFile, the override renderers' single-entry cases) were checked only partly: go tool mutate left 167 edits standing. The flow is now run against the seams the docker packages already have: a fake docker.API for the engine and a fake exec.Runner for the docker and docker compose processes. The tests assert what leaves the flow: the compose build and up command lines, the build and container override files and the Dockerfile-with-features they point to, the lockfile, and the errors. Local and tarball Features keep it offline. Co-Authored-By: Claude Opus 5.5 <[email protected]>
No test ran stop or down, so mutate found every decision in runStopOrDown unheld. The tests drive the command against a fake Docker Engine API (DOCKER_HOST at an httptest server) and a recording docker CLI for Compose, and assert the JSON result and what reached the engine or Compose: the current directory's container, a no-op when there is none, down's removal, the Compose project path with and without volumes, and the errors of an engine, Compose or stop that fails. The workspace now defaults to the current directory whenever it is empty: --container-id and --id-label already take precedence when the container is resolved, so the extra conditions changed nothing. Co-Authored-By: Claude Opus 5.5 <[email protected]>
set-up's flag validation, log, engine and container lookup errors now live in runSetUp, which unit tests drive without a daemon. The config load, the metadata merge and the result building are pure functions (setUpConfig, setUpMerged, setUpResult) with tests; setUpContainer keeps only the wiring that needs a real container and names the parity cases that run it as its mutate lane. The remoteUser override by the config was redundant (the config is the last metadata entry, and remoteUser is last-non-empty-wins), and the probe/remote-env merge now shares up's helpers. Output is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
mutate found 133 edits to up.go that no test refused. The decisions buried in the engine-driving flow are now small functions with unit tests: resolveUpPaths (cwd default), upHasNothingToRead, upRemoteUser, addUpConfiguration, containerRunUser, upWorkspaceMounts, parseFlagMounts, secretsRemoteEnv, hookRemoteEnv, appendConfigHooks and dropRanHooks. resolveComposeProjectName, findUserArg, parseComposeServiceBuild and dotfilesAfterPostCreate get tests as they are (the first through ComposeClient's Runner). The functions that only drive the engine name their Docker lane with mutate-lane and the cases that run them. --cache-image and the restart of a stopped container had no lane, so TestE2E_UpCacheImage and TestE2E_UpReusesStoppedContainer cover them. Redundant code went: a build target or context that is empty changes nothing, an empty dotfiles repository is already a no-op in InstallDotfiles, and the configFilePath copy into mergedConfiguration is done by the spread of the configuration. Output is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
go tool mutate found edits to newImageInspector, getImageBuildInfo,
imageBuildInfoFromDockerfile, replace, trimOneQuote and findValue that
no test refused. The new tests check the inspector against a fake
Docker engine through the existing docker.API seam: local inspect,
registry failure then pull, and no engine. They also cover a compose
build without a context, a USER resolved from the base image's env,
${var:-word}/${var:+word} when the variable is set and when it is
unset, quote trimming, and parent-stage ARG scoping, as in TS
dockerfileUtils.ts.
replace now reads the option as its single character, which removes an
index edit nobody could observe. Two edits stay exempt because they
are equivalent: any negative n means every match, and m[4]/m[5] are -1
together.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
runReadConfiguration mixed every output decision with config loading, Docker lookups and Feature fetches, so mutate's survivors there could only be reached through a container. The decisions are now small functions with their own tests: readConfigurationPaths, loadReadConfiguration, configurationOutput, workspaceOutput, needsFeaturesConfiguration, mergedConfigurationOutput, substituteContainerEnv, plus readConfigurationContainerID and inspectContainerConfig, which are tested with a fake engine. Command tests cover the exit codes and the wiring that is left. Output is unchanged. Dead code is gone: the cid fallback that could never fire, the merged image and configFilePath, which the config spread always overwrote, and the fallback for an unmarshal that cannot fail, which is now returned as an error. The merged configuration reuses the Features already resolved for featuresConfiguration instead of fetching them again. TS readConfiguration also passes that one featuresConfig to getDevcontainerMetadata. The container is now inspected once instead of three times. Co-Authored-By: Claude Opus 5.5 <[email protected]>
go tool mutate found 79 edits to build.go that no test refused: the flag checks, image-name defaults and image/compose/Dockerfile decisions sat inside functions that also build images, so only the Docker-backed lanes ever ran them and those do not see mutants. The decisions are now small functions with unit tests: buildFlagConflict, foldFrozenLockfile, newBuildDockerClient, imageBuildFor, checkLegacyFeatures, imageNamesOr, dockerfileImageMetadata, dockerfileBaseExport, buildxSecrets, composeBaseImageName and composeRetag. A runBuild test with a fake --docker-path holds that --buildkit never skips BuildKit detection. What remains in buildDockerfile, buildImage and buildCompose is the wiring that builds images; they are marked mutate-lane: TestParityMatrix, naming the build parity cases and e2e tests that run them. Behaviour is unchanged. checkLegacyFeatures strips a tag at any colon index instead of only past the first character; ":x" and "x" both name no known legacy Feature, so the outcome is the same. Co-Authored-By: Claude Opus 5.5 <[email protected]>
go tool mutate found 45 edits to runUserCommands that no test refused: the function validated flags, loaded the config and drove the container in one body, so no unit test reached past the first early return. Flag validation and the --workspace-folder default move to prepareRunUserCommandsOpts; runUserCommands keeps the pre-Docker config load, tested without a daemon; the container half is runUserCommandsInContainer, marked mutate-lane: TestParityMatrix with the run-user-commands runtime cases. The pure pieces it used inline are configFromFlag, lifecycleRemoteUser, lifecycleUserEnvProbe and lifecycleRemoteEnv, each tested. Two hand-written copies go: the container lookup is resolveContainerID with no config file (the same label order), and the secrets are buildSecretsFromFile. The fallback config load dropped a workspace-folder branch that could not run, since a loaded workspace never reaches it. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
go tool mutate found 50 edits in runExec, newExecCmd and Execute that no test refused. The decisions (workspace default, --config path, tolerated config errors, remote user, probe strategy, remote env merge and substitution, docker exec arguments, output routing, exit code) are now small functions with unit tests; runExec and Execute keep only the wiring, which the parity lane runs with the built binary and real containers, so they carry mutate-lane markers naming TestParityMatrix and its exec cases. The environment is still probed as the image metadata's user while the config's remoteUser applies afterwards; TS probes as the merged remoteUser, a divergence left for a change with a runtime parity case. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The runner's own up and exec are reached through two package variables, a test-only seam: both re-run this binary, which in a test would be the test binary itself. With them replaced, runAutoTests and runScenarioTests are tested end to end (staging, generated config, filtering, local Feature rewriting, results). The generated config and the parsing of up's result are pure functions with tests; the process spawning left in upTestContainer is marked as held by TestParityMatrix (features.test-single-scenario-success). File modes that makeWorkspaceAccessible or the reader make irrelevant are exempt. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Mutations survived in publishCollectionWith, packaging and generate-docs. publish now logs to its Output's stderr (os.Stderr in production), so its error lines can be asserted; new tests cover what counts as published, legacy ids, annotation key order, required properties, optionalPaths narrowing, target arguments and the README parts. A redundant SplitN limit becomes strings.Cut, and FormatFloat's negative precision is exempt: any negative value is the shortest form. Co-Authored-By: Claude Opus 5.5 <[email protected]>
runFeaturesInfo and runOutdated had no test reaching a registry. A read-only OCI registry on 127.0.0.1 (plain HTTP, as the client speaks to loopback) serves tags and manifests, and the tests assert each info mode's text and JSON output, the failure outputs, and outdated's current/wanted/latest columns, lockfile pins, --config and failures. Co-Authored-By: Claude Opus 5.5 <[email protected]>
setup's recheck decision and result rendering, and templates apply's flag parsing, move into pure functions with tests. Two guards that changed nothing go: appPortPublishArgs' empty check (decoding nothing fails anyway) and bridgeBuildAuth's no-registry return (ResolveBuildAuth resolves nothing then), as do the nil options map and the omit-paths and '[]' features guards. mergeAdditionalFeatures, secretValuesFromFile, string metadata mounts and build auth bridging get tests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Mutation testing showed edits to these lines change nothing a caller
can see, so the code says less than it seems to:
- a lockfile entry's missing integrity reads as "" from the map, so
the separate presence check before pinning was redundant;
- the owner of a namespace and the engine's major version are the text
before the first "/" or ".", which strings.Cut says directly;
- the extended image's reported names are exactly its tags;
- resolvePath("") already is the current directory, so
resolve-dependencies needs no separate default;
- the staging counter's start only names temporary folders.
The legacy-shorthand version split keeps its bound with a mutate-exempt:
a colon at index 1 leaves a one-character name, and no legacy Feature
has one.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
No test refused many edits to the Feature install path. The new tests exercise it at its real boundaries without Docker: an in-memory oci.Registry, httptest tarballs, temp dirs, the docker.API interface and the exec.Runner seam (plus a shell-script docker for the build auth environment, which only the OS runner passes on). They pin what callers see: annotation-over-layer metadata and the lockfile pin for OCI Features, tarball ids and digests, every fetch and staging error, resolve-dependencies' output, exit code and failures, the extended image's tags, users, syntax directive, builder switch, content image and lockfile handling, the engine version check, and when a disallowed Feature's container is stopped. Co-Authored-By: Claude Opus 5.5 <[email protected]>
go tool mutate over the pull request's diff left two edits in mounts.go no test refused: an object mount's explicit type versus the bind default, and an empty string mount being dropped from the metadata entry. Both are now asserted. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The pull request's mutate job, on the docker8 fleet, left three edits
no unit test refused: isTerminalWriter's file-mode check (only a buffer
was tried), singleContainerRemoteUserUID with an image inspection that
has no Config, and win32's ${localWorkspaceFolderBasename}. A local run
refused them only through Docker-backed tests the job does not reach.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The parity cases run through `/bin/sh -lc`, and a login shell rebuilds PATH from the system profile. On the docker8 fleet that puts the image's node 22 (/usr/local/bin) before setup-node's node 24, so the oracle loaded the reference's node-pty, built for node 24, failed, and printed node-pty's innerError on stderr: read-configuration.compose-path-terminal and read-configuration.missing-required differed on every run there. The oracle's command now names the node on the test process's PATH by its absolute path. Co-Authored-By: Claude Opus 5.5 <[email protected]>
aledbf
force-pushed
the
refactor/parity-cleanup
branch
from
October 25, 2026 20:49
6b4904f to
51bf37e
Compare
UpdateFromOldProperties moved extensions, settings and devPort into customizations.vscode on the typed config, and the substituted re-read right after overwrote customizations with the unmoved one whenever the file already had customizations: `read-configuration` then lost the extensions and settings that TS reports under customizations.vscode. The move now happens on the parsed map before anything reads it, as TS updateFromOldProperties does (devPort moves only when customizations.vscode has none). LoadResult.RawConfig keeps that map before substitution, the config's raw that TS writes into the metadata label; LoadResult.Raw was documented as pre-substitution but held the substituted map. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The measured command was re-run once on a transient build error, but a case's setup_cmd was not. Cases that build Features without BuildKit race on the fixed dev_container_feature_content_temp image both CLIs build, and with four cases at a time on a docker8 runner a setup lost that race (exec.workspace-dockerfile-with-features-hello-success: "/tmp/build-features/hello_0: not found"). Setups now get the same one retry; a failure that reproduces still fails the case. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The CI mutation sample left DetectActiveBuilder's and FindDockerDriverBuilder's failure checks unrefused: no test ran a buildx command that exits non-zero without an exec error. Locally an e2e test refused them, which the CI job does not reach. Co-Authored-By: Claude Opus 5.5 <[email protected]>
A direct Feature tarball was read through io.LimitReader(512 MiB) and a larger one was cut short in silence: the truncated bytes were extracted and their digest compared with the lockfile as if they were the whole tarball. readCapped now fails the download one byte past the cap. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The PR's mutate job runs without Docker, so only unit tests count, and 35 edits to MergeConfiguration's scalars, mergeHostRequirements, mergeGPURequirements, jsString, ParseFeatureFileInfo, generateContainerEnvs and the rename warning of FeatureInstallWrapperScript were held only by Docker-backed e2e tests. Unit tests now pin those results against the TS mergeConfiguration, mergeHostRequirements, mergeGpuRequirements, getFeatureEnvVariables, getFeatureInstallWrapperScript and generateContainerEnvs. Guards whose edit changed no outcome are dropped without changing behaviour: the options null/empty check the decoder already covers, the empty-input early returns of the mount and forwarded-port de-dup, and the (int, bool) port helper whose int was unused on false, now a key helper. FormatFloat's -1 precision carries a mutate-exempt. Co-Authored-By: Claude Opus 5.5 <[email protected]>
TS optionsCompareTo orders an undefined option value after a present one, but options come from JSON, where a value is never undefined: a null is an object to TS, so a null and a string value compare equal and the two references to the Feature dedup into one node. The Go port read nil as undefined and kept both nodes. Drop that branch. The new unit tests also hold the install order optionsCompareTo gives same-resource Features, which only Docker-backed tests refused before. Co-Authored-By: Claude Opus 5.5 <[email protected]>
TS updateControlManifest writes the empty manifest to the cache when the fetch fails and nothing is cached, so the containers.dev request is not retried for five minutes; the Go port returned it without caching and fetched again on every call. FeatureAdvisories' guard on an empty feature list or a nil manifest is dropped: GetControlManifest never returns nil and an empty list yields no advisories either way. The temp file's write and close errors are joined, which states the same condition with one check. Unit tests now hold the cache folder mode, the 2xx bound and unparsable advisory ranges without Docker. Co-Authored-By: Claude Opus 5.5 <[email protected]>
TS getCredential takes the first entry whose first `|` field is the
registry and sends `${split[1]}:${split[2]}`: fields after the third
are dropped and a missing one is the string undefined. The Go port
kept a `|` inside the token and skipped entries with fewer fields. Its
trace line now quotes the registry with single quotes, as TS does.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Unit tests against httptest registries now refuse the edits that only Docker-backed tests refused: the pushed manifest's schemaVersion and layer title, the major.minor tag range, the index entry chosen for a variant, and Docker Hub images read from registry-1.docker.io with the credentials stored for docker.io. PushArtifact no longer defaults an empty collection type to feature: its one caller always passes feature or template. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Unit tests now refuse edits that only Docker-backed tests refused: the lockfile's dependsOn list, the mermaid node hash length, its aliases label and its options-sensitive hash, and log masking that replaces each non-empty secret whole. Co-Authored-By: Claude Opus 5.5 <[email protected]>
TS getDefaultImageName separates project and service with "_" when isEarlierVersion(version, [2, 8, 0]), which counts a missing component as 0: a Compose version of just "2" is earlier than 2.8.0. The Go check needed a minor component to call a 2.x version earlier, so it named the image "<project>-<service>" there. Compare as isEarlierVersion does and hold every boundary with a unit test that runs without Docker. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Only Docker-backed e2e tests refused these edits, and CI's mutation run has no Docker. Unit tests through the recording runner now hold that buildx inspect keeps the builder's own Name/Driver over its nodes', that a --mount readonly value is true only for "true" and "1", and that the UID image builds from its Dockerfile's directory unless a context and platform are given. FindDockerDriverBuilder drops a blank-line check strings.Fields already makes. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Only Docker-backed e2e tests refused these edits. Unit tests through the fake executors and shell now hold that skipPostAttach skips that hook alone, that dotfiles install once right after postCreateCommand, that a parallel command's header appears only on success and TS's "failed with exit code" line only for a non-zero code, that empty and null commands are dropped, and that the env probe merges sbin entries for root alone, leaves PATH when the container has none and caches a single variable. Co-Authored-By: Claude Opus 5.5 <[email protected]>
TS updateFromOldProperties moves devPort whenever it is not undefined, so "devPort": null ends up as customizations.vscode.devPort: null. Go treated null as absent and left it at the top level. Treat a present devPort as TS does, and hold the JavaScript truthiness the other deprecated properties move by with a unit test. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Only Docker-backed e2e tests refused these edits. Unit tests now hold
the missing --config error, the config file path and what substitution
errors name, the substituted workspaceFolder and workspaceMount, the
Git root subfolder, .env's COMPOSE_FILE and host-phase
${containerWorkspaceFolder}. The loader computes the config file path
once, computeWorkspaceConfig drops conditions that could not change its
result, and ComputeDevContainerID pads without a no-op length check.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
read-configuration dropped settings, extensions and devPort from its configuration even when the loader had left them in place, as TS updateFromOldProperties does for a devPort whose customizations.vscode already has one or for a falsy settings. The TS CLI prints them there, so the output now keeps whatever the loader kept; a null devPort, which TS moves into customizations.vscode and the loader does not, stays out of the top level as before. The test also holds configMetadataEntry's single forwarded port, an edit no unit test refused without Docker. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Without a reachable Docker daemon, as the pull request's mutate job runs, edits to runBuild's error branch, addTemplateProps' dockerfile type, splitShellWords' trailing escape, extendImageWithFeatures' host substitution and buildx context, downloadFeatureTarball's status range and context, the image inspector's five pulls, mountsFromMetadata and singleContainerRemoteUserUID's image metadata survived: only e2e tests refused them. Unit tests over the existing fakes now do. readCapped's limit is exempt: any limit past max fails a body over the cap exactly where max+1 does. Co-Authored-By: Claude Opus 5.5 <[email protected]>
A configuration file holding null, an array or a scalar loaded as an empty configuration (null) or failed with a Go unmarshal error (array). TS readDevContainerConfigFile rejects both with "Dev container config (<file>) must contain a JSON object literal.", which up prints as its message and description and on which read-configuration exits 1. The loader now returns that error, and returns a base file's read error at once instead of reading the file a second time to report it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
read-configuration dropped a null devPort from the configuration it prints, written for a loader that did not move a null devPort into customizations.vscode. The loader now does, as TS does, so a null devPort stays at the top level only when customizations.vscode has its own, and TS prints it there. The host phase's containerWorkspaceFolder resolution becomes two conditions, so each one is held by a test. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.
Summary
A KISS/YAGNI review of the repository asked why the parity matrix never caught divergences from the reference CLI. A differential run of
read-configurationagainst the TypeScript oracle over every upstream fixture, a whole-repositorygo tool mutaterun and a dead-code review against the reference answered it. The matrix only compared what someone had thought to write down.This PR closes those divergences, removes what the reference does not have, and wires the go-tools gates so the gap stays closed. The shared plan with every item, its TS evidence and its test is here.
Commits
refactor: apply go fix modernizersis mechanical (go fix ./...on Go 1.27.3). Read it last, or skip it.fix!: close divergences from the reference CLIports reference behavior in read-configuration, Features, publish/package, and up/run-user-commands/set-up. Every fix has a unit test or a parity case that fails on the previous code. It also carries the cleanup in the same functions: unreachable code checked against the reference, the httpx client,pfswrappers and the Dockerfile builder replaced by the standard library, and duplicated helpers shared. The cleanup is not a separate commit because it changes signatures the fixes call from the same functions, so an intermediate commit would not build.build: gate lint, vulnerabilities and releases on go-toolsmakestask lintrun golangci-lint (pinned inversions.yaml;.golangci.ymlwas never run before), ctxlife, testquality, refs, mutate, deadgate, taskgate and sfw.task vulnuses vulngate, pull requests get a mutate job, and the release shell moves tocmd/devtool. go-tools is pinned at 0ee2224, which includes taskgate: a submodule a task names is a directory, not a script go-tools#64.Each commit builds, vets and passes the unit tests on its own.
Breaking changes
features package,features publishandtemplates publishrequire<target>, as the reference does. Earlier releases defaulted it to the current folder.up --id-labelwithout a config or workspace folder fails instead of attaching.docs/migration.mdhas a new section for upgrading from an earlier release of this CLI.Validation
task lint(golangci, ctxlife, testquality, refs, mutate -stale-exempts, deadgate, taskgate, sfw)task test:racetask spec:compliancetask parity:contracttask parity:runtime(CI builder env)build.cache-to-local*cases passed alone after the oracle failed to download a Feature from ghcr.io under the parallel run, while Go succeededtask parity:publish,task parity:networktask test:e2etask parity:differentialtask vulngoreleaser check(v2.13.1)Runtime parity ran with
BUILDX_BAKE_ENTITLEMENTS_FS=0 BUILDX_BUILDER=default, as CI's runner does.🤖 Generated with Claude Code