[VPEX][13] Document the error-code contract as source of truth#5964
Open
rugpanov wants to merge 1 commit into
Open
[VPEX][13] Document the error-code contract as source of truth#5964rugpanov wants to merge 1 commit into
rugpanov wants to merge 1 commit into
Conversation
rugpanov
changed the title
Collaborator
Integration test reportCommit: 6ff8a75
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 10 slowest tests (at least 2 minutes):
|
rugpanov
force-pushed
the
dbconnect/13-usage-error
branch
from
July 17, 2026 13:27
f20628a to
6b4a2a4
Compare
rugpanov
force-pushed
the
dbconnect/14-errcode-reconcile
branch
from
July 17, 2026 13:27
47c9f11 to
3f6b218
Compare
Reconcile the error codes with the spec table (spec item #10). The code already emits the correct set; the spec's table was stale (it omitted E_NOT_WRITABLE and E_PYTHON_INSTALL and wrongly listed E_UV_MISSING as reserved). Rather than change behavior, annotate each ErrorCode constant with the phase that emits it and document the two codes the CLI deliberately never emits (E_PYTHON_POLICY, E_AUTH), making this block the authoritative reference the spec mirrors. Also fix a TargetInfo doc comment that still said 'four precedence sources' — with --cluster-name there are now five flag sources, though the JSON 'source' value is still one of cluster/serverless/job/bundle. Co-authored-by: Isaac
rugpanov
force-pushed
the
dbconnect/14-errcode-reconcile
branch
from
July 17, 2026 13:44
3f6b218 to
6ff8a75
Compare
anton-107
approved these changes
Jul 17, 2026
anton-107
left a comment
anton-107
left a comment
Contributor
There was a problem hiding this comment.
Reviewed as part of the full stack (#5960–#5965). The annotated error-code block reads well and makes the constant set a genuine source of truth; documenting why E_PYTHON_POLICY/E_AUTH are deliberately never emitted is a nice touch. Approving.
Since this is the comment-accuracy PR, two adjacent comment fixes it could sweep in while it's here:
-
[low] Stale
v4fallback comments. After #5962 bumped the default to v5,cmd/environments/compute.go:78and:116still describe the serverless empty-version fallback as v4. This PR touched onlyresult.go, so these were missed. -
[nit]
ResolveTargetprecedence comment still lists four sources. This PR updated theTargetInfodoc to acknowledge--cluster-name("now five flag sources"), but the sibling precedence comment atlibs/localenv/target.go:64still reads--cluster-id → --serverless-version → --job-id → bundle, omitting--cluster-name. Same class of inaccuracy this PR set out to fix.
Reviewed with AI assistance (build + unit tests + adversarial verification against the checked-out top of stack).
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
3 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.
Stacked on #5963 (E_USAGE) → #5962 → #5961 → #5960 → #5959.
What
Reconciles the error codes with the
[P0] CLI Changesspec table (spec item #10). No behavior change — the code already emits the correct set. The spec's table was stale:E_NOT_WRITABLE(preflight) andE_PYTHON_INSTALL(provision), andE_UV_MISSINGas "reserved / not emitted" when the CLI does emit it at preflight.Changes
ErrorCodeconstant with the phase that emits it, and document the two spec codes the CLI deliberately never emits:E_PYTHON_POLICY(no signal source yet) andE_AUTH(handled earlier byMustWorkspaceClient). This makes the constant block the authoritative reference the spec mirrors.TargetInfodoc comment that still said "four precedence sources" — with--cluster-namethere are now five flag sources (the JSONsourcevalue is still one of cluster/serverless/job/bundle).Note
The spec doc's error-code table itself (in the VPEX ERD, a Google Doc) has been updated separately to match — that's outside this repo.
Testing
go build ./..., lint (0 issues), deadcode clean, unit + acceptance green (comment-only code change).This pull request and its description were written by Isaac.