Repository navigation
feat(api): name invalid fields from the single-field body validators - #443
Conversation
Refs #352 Change-Id: I800425256699e5b867adff6a622f3bb338fb052d Signed-off-by: Thomas Kosiewski <tk@coder.com>
…types Change-Id: Iaeb0a35a1cd444c3b334ffac03ec57d04b423ac3 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The single-field body validators on upstream's
ValidationProblemDetailsroutes now name their fields ininvalid_params. This is PR 5, the last PR of the plan in #352 (issuecomment-6143529650). PR 4 (#436) addedinvalid_paramsfor decode failures. The PR stays under 500 lines (315), so it needs no split by route family.Which checks name a field
namereasondefault_branch,base_repo.default_branchis required,is not a valid branch namebase_repo.name,base_repo.auth.tokenis required/is required for a copybase_repoform)base_repo.owner,base_repo.name,base_repo.provider,base_repo.operation,base_repo.upstream_hostis required,is not supported,must be sync or fork,is not a valid host,is denied by the sync egress policyupstreamform)upstream.provider,upstream.owner,upstream.name,upstream.access,upstream.hostmust be app or public,must be publictarget_branch,name(tag),refmust be a valid branch name,must be a valid tag name,is requiredpassword,idis required,is a reserved valuequery.pattern,context.before/context.after,limits.max_lines,limits.max_matches_per_file,pagination.limit,pagination.cursor,paths.Nis required,must be at least 0,must be between 1 and 2000,must be at least 1,is not a valid cursor,must be a non-empty pathmax_blob_size,include_globs.N/exclude_globs.N,archive.prefixmust be at least 0,must be a non-empty glob,must be a relative path inside the archiveA check that finds several empty fields names each one. For example,
{"upstream":{"provider":"github"}}gives two items,upstream.ownerandupstream.name.These name no field: rules across fields (
base_repo and upstream cannot both be set,force requires target_branch, "one of base_ref, base_branch or preferred_base"), query parameters (including adefault_branchquery override), and result-envelope bodies (tier 3 in #352). Delete-branch's name is also left out, because it can come from the query or the body.Review follow-ups from PR 4 included here
must be an object, like type errors. The message text is unchanged.jsonKindreportsmust be a stringfor types that decode from a JSON string (encoding.TextUnmarshaler,[]byte). A mutation check fails the newtime.Timeand[]bytecases.ref_not_found, from the PR 3 advisory.What stays the same
erroranddetailtext, status codes,code, and theapplication/jsoncontent type.validateSyncBaseRepoandparseUpstreamConfignow return errors in place of message strings, with the same messages.Tests
TestInvalidParamsOnValidators(full server,pgandsqlite): 28 cases across the route families above, each asserting the exact item list, plus the negative controls (query parameter, both forms, force without target).assertProblemFieldsstill checks the member set and item shape on every full-server plain-error assertion.Dogfood
The same requests ran on a real local-mode server (SQLite + files) built from
origin/main(66f5c32) and from this branch (b2089fd), then through the stable Go and TS SDKs (1.17.1). The only difference isinvalid_paramson the five single-field failures. The cross-field rule carries none, and SDK output is identical. The clips are real-time asciinema replays rendered with agg.pr5-ab.mp4
Docs:
docs/reference/http-api.md(Errors) and the RFC 9457 row ofdocs/explanation/wire-compatibility.md.Refs #352
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high