Repository navigation
Make the state publish meta data clearer to review and edit - #3868
Conversation
9b12c5f to
11e5bf1
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
MDrakos
left a comment
There was a problem hiding this comment.
Requesting changes for one untested line that every --edit revision now depends on.
1 inline comment below.
| return nil // saved to a meta file instead | ||
| } | ||
| if params.Edit && isRevision { | ||
| reqVars.Description = "" // kept as it was, and the platform rejects a description on a revision |
There was a problem hiding this comment.
This line is the only thing keeping the description out of an --edit revision request now, and nothing tests it. keepDescription always fills in the current description for the preview, so if this line goes away every --edit of an existing version gets rejected by the platform. I deleted it locally and go test ./internal/runners/publish/ still passed. Can we add a test that checks the request we actually send on an --edit revision has an empty description? Pulling this step into a small helper the confirm tests can call would work too.
… one
state publish --editor outside a project has no namespace until the editor
fills it in, so Run searched every namespace for the name derived from the file.
Logged in, that search took long enough to time out ("Could not search for
ingredient"), so the editor never opened. This is why the "New ingredient with
editor flag" integration test has failed nightly on master since at least
September. Its first match could also be any ingredient with that name.
Skip the lookup for a new ingredient without a namespace; confirmRequest
requires one and checkNamespaceEdit checks the version once it is filled in.
--edit still searches, since it needs to find the ingredient.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7c6bb1e to
0584f82
Compare
Without EDITOR set and without xdg-open, as on many headless Linux machines and the ubuntu-24.04-arm runner, OpenEditor fails and publish exited with "Could not publish ingredient. Error received: <no value>", even though the notice just before it says the file can be edited by hand. Say that no editor could be opened and wait for the confirmation as usual instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…name - TestPublishBuildEncrypted installs into a Python project that state init creates without a Linux/arm64 platform, so skip it there, like the other platform-specific skips. Integration tests only run on ARM for labelled PRs. - PerformanceIntegrationTestSuite's TearDownSuite stopped state-svc even when its tests were skipped by tag. On macOS a long branch channel makes the socket path too long to dial, so every labelled PR from such a branch failed there. Only stop the service when the Performance tests ran, using a new tagsuite.RunsForTags that OnlyRunForTags is now built on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0584f82 to
839266d
Compare
The test writes its edits to the file itself, but with no EDITOR set, macOS `open` hands the file to whatever app the runner has registered. On macos-latest, publish then read the file back without the test's edits and failed with "You have to supply the namespace". Point EDITOR at a command that doesn't exist, so publish takes the "edit the file by hand" path on every OS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
839266d to
7ee6a50
Compare
editorFileRx matched "file:\s*?(.*?)\.\s", whose lazy \s*? left the space after "file:" in the captured path whenever the temp file name had 9 digits instead of 10 (about one run in four). The test then wrote its edits to " /tmp/NNN.ingredient.yaml", a different file, so publish read the original back and failed with "You have to supply the namespace". The last commit's macOS explanation was wrong; this was the cause on every OS. Reproduced by driving a dev build through termtest 12 times: three runs captured the leading space before this change and none after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up to #3867 from testing the 0.48.2-RC2 build: - Without --namespace, a --build publish from a pyproject.toml now defaults to private/<org>/python (otherwise private/<org>, as before). - A new ingredient's namespace can be changed in the editor. It is checked like --namespace and put back, with the reason, if it can't be used. - The confirmation and editor say up front which fields can't be changed and why, and the error for changing one names it. - The preview and editor show plain yaml without a help comment on every field; the fields are described in the publish guide instead. The yaml-comment module is no longer used and is removed. - state export ingredient-meta now includes the ingredient's description. state publish --edit no longer refuses a different description on an existing version: it keeps the current one and says the change is ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7ee6a50 to
a519911
Compare
#3869 was squash-merged; this branch already had its commits.
The merge-base changed after approval.
* Update version.txt * Fix state publish meta data issues found testing 0.48.2-RC1 (#3867) * Fix state publish meta data issues found testing 0.48.2-RC1 - state export ingredient-meta writes plain yaml, unwrapped and without field help comments, so the file can be passed back with --meta. - Dependencies and conditions without a version range (a bare name in pyproject.toml, --depend without a version, or a condition inherited without requirements) get >=0. The platform rejected them with "Expecting an OBJECT with a field named 'versionRequirements'". - When --build can't determine the name or version, say which is missing, why pyproject.toml didn't provide it (no file, not in [project], or listed in [project].dynamic), and how to set it. - Answering 'n' at the publish confirmation offers to edit the meta data in the editor or save it to <name>-<version>.meta.yaml with the command to publish it, instead of only cancelling. With --build or --edit, a name, namespace or version change in the editor is saved to a file instead, as the package was already built and looked up with them. - Fields removed in the editor are removed from the request, and the publish API error is shown instead of <no value>. The platform requires the source with every publish, so --edit still needs --build or an archive; the readme now says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Update echo to v4.16.1 and drop an unused publish error argument echo v4.15.4 has GHSA-cxf3-9h37-7rr8 (critical), GHSA-9969-6676-49h5, GHSA-f48p-c9v4-h7xv and GHSA-h7wr-284f-5qvc, which failed the Scan job. err_uploadingredient_publish is defined in en-us.yaml without a {{.V0}}, so the joined message passed to it was never shown. The wrapped error is already printed beneath it, so drop the argument and the changelog claim. * Keep the API error in the publish failure message err_uploadingredient_publish in en-us.yaml ends in "Error received: {{.V0}}" and overrides the fallback text, so without the argument the message reads "<no value>". The wrapped API error isn't user-facing, so it isn't printed beneath it either; it only reaches the log. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix TestOpen failing on Windows; correct the publish error comment TestOpen removed its temp file while the hail watcher could still be reading it for a later write event: the remove was deferred after the cancel, so it ran first. On Windows an open file can't be removed. Stop the watcher and drain its channel before removing the file. The publish error comment said the wrapped API error is never printed; a transport failure becomes a user-facing BuildPlannerError and is. Only an error reported in the publish result (an errs.New) relies on {{.V0}}. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Validate each confirm pass and test the publish fixes through their call paths Address review on #3867: - Run the namespace and checkEditKeepsIdentity checks at the top of each pass of the confirm loop, now confirmRequest, so a namespace cleared or a description added in the editor is rejected with a clear message instead of failing in MarshalJSON or the API. - Add tests that fail if a fix is reverted: confirmRequest with a fake prompter (>=0 filling, editor removing fields, cleared namespace, description on a revision, private-builder restored, save, cancel), using the test binary as $EDITOR so it also runs on Windows; publishFailedError keeping the API error; metaFromIngredient filling conditions; and writeIngredientMeta writing plain yaml that round-trips. FetchMeta's pure part moves into metaFromIngredient and the export's writing into writeIngredientMeta so they can be tested without the API. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * Fix CI for 0.48.2-RC2: x/net CVEs, publish --editor, and runner-specific test failures (#3869) * Update golang.org/x/net to v0.62.0 for CVE-2026-78661 and CVE-2026-97045 The Scan job failed on both: x/net v0.60.0 is affected and v0.62.0 fixes them. go get also raised x/crypto, x/sys, x/term, x/text, x/sync and x/tools to the versions x/net v0.62.0 requires. * Don't search every namespace when publishing a new ingredient without one state publish --editor outside a project has no namespace until the editor fills it in, so Run searched every namespace for the name derived from the file. Logged in, that search took long enough to time out ("Could not search for ingredient"), so the editor never opened. This is why the "New ingredient with editor flag" integration test has failed nightly on master since at least September. Its first match could also be any ingredient with that name. Skip the lookup for a new ingredient without a namespace; confirmRequest requires one and checkNamespaceEdit checks the version once it is filled in. --edit still searches, since it needs to find the ingredient. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Let state publish --editor continue when no editor can be opened Without EDITOR set and without xdg-open, as on many headless Linux machines and the ubuntu-24.04-arm runner, OpenEditor fails and publish exited with "Could not publish ingredient. Error received: <no value>", even though the notice just before it says the file can be edited by hand. Say that no editor could be opened and wait for the confirmation as usual instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix integration tests that fail only because of the runner or branch name - TestPublishBuildEncrypted installs into a Python project that state init creates without a Linux/arm64 platform, so skip it there, like the other platform-specific skips. Integration tests only run on ARM for labelled PRs. - PerformanceIntegrationTestSuite's TearDownSuite stopped state-svc even when its tests were skipped by tag. On macOS a long branch channel makes the socket path too long to dial, so every labelled PR from such a branch failed there. Only stop the service when the Performance tests ran, using a new tagsuite.RunsForTags that OnlyRunForTags is now built on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Don't open a real editor in the publish --editor integration test The test writes its edits to the file itself, but with no EDITOR set, macOS `open` hands the file to whatever app the runner has registered. On macos-latest, publish then read the file back without the test's edits and failed with "You have to supply the namespace". Point EDITOR at a command that doesn't exist, so publish takes the "edit the file by hand" path on every OS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the publish --editor integration test's flaky file path match editorFileRx matched "file:\s*?(.*?)\.\s", whose lazy \s*? left the space after "file:" in the captured path whenever the temp file name had 9 digits instead of 10 (about one run in four). The test then wrote its edits to " /tmp/NNN.ingredient.yaml", a different file, so publish read the original back and failed with "You have to supply the namespace". The last commit's macOS explanation was wrong; this was the cause on every OS. Reproduced by driving a dev build through termtest 12 times: three runs captured the leading space before this change and none after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * Make the state publish meta data clearer to review and edit (#3868) * Update golang.org/x/net to v0.62.0 for CVE-2026-78661 and CVE-2026-97045 The Scan job failed on both: x/net v0.60.0 is affected and v0.62.0 fixes them. go get also raised x/crypto, x/sys, x/term, x/text, x/sync and x/tools to the versions x/net v0.62.0 requires. * Don't search every namespace when publishing a new ingredient without one state publish --editor outside a project has no namespace until the editor fills it in, so Run searched every namespace for the name derived from the file. Logged in, that search took long enough to time out ("Could not search for ingredient"), so the editor never opened. This is why the "New ingredient with editor flag" integration test has failed nightly on master since at least September. Its first match could also be any ingredient with that name. Skip the lookup for a new ingredient without a namespace; confirmRequest requires one and checkNamespaceEdit checks the version once it is filled in. --edit still searches, since it needs to find the ingredient. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Let state publish --editor continue when no editor can be opened Without EDITOR set and without xdg-open, as on many headless Linux machines and the ubuntu-24.04-arm runner, OpenEditor fails and publish exited with "Could not publish ingredient. Error received: <no value>", even though the notice just before it says the file can be edited by hand. Say that no editor could be opened and wait for the confirmation as usual instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix integration tests that fail only because of the runner or branch name - TestPublishBuildEncrypted installs into a Python project that state init creates without a Linux/arm64 platform, so skip it there, like the other platform-specific skips. Integration tests only run on ARM for labelled PRs. - PerformanceIntegrationTestSuite's TearDownSuite stopped state-svc even when its tests were skipped by tag. On macOS a long branch channel makes the socket path too long to dial, so every labelled PR from such a branch failed there. Only stop the service when the Performance tests ran, using a new tagsuite.RunsForTags that OnlyRunForTags is now built on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Don't open a real editor in the publish --editor integration test The test writes its edits to the file itself, but with no EDITOR set, macOS `open` hands the file to whatever app the runner has registered. On macos-latest, publish then read the file back without the test's edits and failed with "You have to supply the namespace". Point EDITOR at a command that doesn't exist, so publish takes the "edit the file by hand" path on every OS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fix the publish --editor integration test's flaky file path match editorFileRx matched "file:\s*?(.*?)\.\s", whose lazy \s*? left the space after "file:" in the captured path whenever the temp file name had 9 digits instead of 10 (about one run in four). The test then wrote its edits to " /tmp/NNN.ingredient.yaml", a different file, so publish read the original back and failed with "You have to supply the namespace". The last commit's macOS explanation was wrong; this was the cause on every OS. Reproduced by driving a dev build through termtest 12 times: three runs captured the leading space before this change and none after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Make the state publish meta data clearer to review and edit Follow-up to #3867 from testing the 0.48.2-RC2 build: - Without --namespace, a --build publish from a pyproject.toml now defaults to private/<org>/python (otherwise private/<org>, as before). - A new ingredient's namespace can be changed in the editor. It is checked like --namespace and put back, with the reason, if it can't be used. - The confirmation and editor say up front which fields can't be changed and why, and the error for changing one names it. - The preview and editor show plain yaml without a help comment on every field; the fields are described in the publish guide instead. The yaml-comment module is no longer used and is removed. - state export ingredient-meta now includes the ingredient's description. state publish --edit no longer refuses a different description on an existing version: it keeps the current one and says the change is ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up to #3867, from testing the 0.48.2-RC2 Windows build: changing the namespace in the
state publisheditor was refused, and nothing said in advance which fields were fixed.Changes
Default namespace: without
--namespace(or a namespace in the--metafile),state publish --buildnow usesprivate/<org>/pythonwhen the source directory has apyproject.toml. Otherwise it still usesprivate/<org>.Namespace edits: a new ingredient's namespace can now be changed in the editor.
--buildwrites the name and version into the package, but not the namespace. The new namespace gets the same checks as--namespace:--build, it must be under the project's organizationIf it fails a check, the previous namespace is put back, the user's other edits are kept, and the reason is shown. With
--editthe namespace still can't be changed, because it identifies the ingredient being revised.Fixed fields stated up front: with
--buildor--edit, the confirmation prompt and the editor notice say which fields can't be changed and why. If one is changed anyway, the error names that field (e.g. "The name can't be changed at this point, because it was already built into the package") instead of "name, namespace or version".No help comments in the meta data: the confirmation preview and the editor no longer show a comment on every field (
# The name of the ingredient, ...). They now show the same plain yaml as saved meta files andstate export ingredient-meta. The fields are described in a table in the publish guide instead. Theyaml-commentmodule, andmaruel/naturalwhich only it used, are removed fromgo.modandvendor/.Description in export and
--edit:state export ingredient-metaleft the description empty, although it is published. The description belongs to the ingredient, not the version, so export now fetches it from there.state publish --editno longer refuses a description that differs on an existing version. The confirmation shows the current description and says the change is ignored, and the other edits can still be published. The description is still left out of the request for a revision, because the platform rejects it.Testing
confirm_test.gocover the default namespace, a namespace edit that is kept, each check that puts the old namespace back, and the fixed-field errors for--buildand--edit. Tests also cover the preview having no comments, the exported description, and an ignored description change. Each test fails when its fix is reverted.🤖 Generated with Claude Code