Skip to content

fix(api): add ref_guard_failed problem members to the notes 409s - #573

Merged
ThomasK33 merged 1 commit into
mainfrom
568-notes-ref-guard
Oct 27, 2026
Merged

ThomasK33 merged 1 commit into
mainfrom
568-notes-ref-guard

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

A stale expected_notes_ref_sha on POST /api/repos/{repo_name}/notes/append and on the DELETE .../notes query parameter form now answers 409 with upstream's NotesWriteRefGuardProblemDetails body: the note members as before, plus the problem members with code ref_guard_failed, guard and current_sha. This is PR 2 of 2 and completes #568.

What

  • Both forms already answered 409 with the note envelope (sha, notes_ref, target_ref, base_commit, new_ref_sha, result). The body keeps those members unchanged and adds type, title, status, code: ref_guard_failed, detail, error, instance, guard: expected_notes_ref_sha and current_sha.
  • current_sha is the notes ref's tip, or 40 zeros when the notes ref does not exist. new_ref_sha already repeated that value. A request that sends the deprecated expected_ref_sha gets the standard guard name, as upstream documents.
  • detail, error and result.message are the same text as before.
  • Unchanged: the body forms (POST .../notes and the deprecated DELETE body) keep 412 precondition_failed with no problem members (desk decision on api: a stale expected_*_sha on commit-pack and diff-commit answers 412; the upstream spec lists only 409 #533), and the content type stays application/json (the api: problem-details later tiers (specific codes, instance, result envelope, git/LFS writers, problem+json) — held on SDK trigger #352 SDK gate has not fired).
  • errors.go: the problem builder now returns the envelope, so the notes body embeds the same checked members that every other problem body uses.
  • Docs: http-api.md (error codes, notes) and wire-compatibility.md (the RFC 9457 row and the notes row).

Why

The upstream docs OpenAPI of 2026-10-27 (c494c01b, vendored by #571) documents this body for a stale notes guard. It also covers item 3 of #352 for these two routes only.

Proof

  • Tests first. On main's notes handler, the changed subtests fail on each backend (PostgreSQL and SQLite): the append stale guard and the DELETE query-form stale guard. The body has no problem members there. With the fix they pass.
  • A shared helper checks the whole body. It checks the note members (sha, notes_ref, target_ref, new_ref_sha, a failed conflict result whose message equals detail) and the guard members. Then it strips the note members and runs the shared problem oracle, assertProblemFields, on the rest. TestProblemMembersMatchUpstream pins the note members to NotesWriteRefGuardProblemDetails in the vendored spec.
  • New cases: an unborn notes ref gives current_sha 40 zeros, and the deprecated expected_ref_sha gives guard: expected_notes_ref_sha. The append body's instance is the path. The DELETE body form keeps 412 with no code member.
  • Mutation checks: 8 mutants, all killed. They covered the old envelope, the stale guard as current_sha, a wrong guard name, the note members dropped, problem members on the body forms, a result.message that differs from detail, an extra member in the spec schema, and an oracle without the notes message.
  • Gates on this tree: go test -race ./internal/api/... ok (PostgreSQL and SQLite), golangci-lint v2.15.0 0 issues, and the docs gate (0 broken links, the link-checker tests pass).
  • The patch also drops a doubled "and" in the RFC 9457 row that feat(api)!: answer failed merge guards with 409 ref_guard_failed #571 added.

Fixes #568


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

Refs #568

_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_

Change-Id: I4bc70a1df227e6121e34473aa8832ee047b13e0e
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T23:00:20.661151Z 0518b33 Manual request
🔒 Security Review ✅ Completed 2026-10-27T23:01:41.685671Z 0518b33 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 0518b33f7e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 0518b33f7e

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upstream drift (spec 2026-10-27): failed ref guards answer 409 ref_guard_failed with guard and current_sha; vendor c494c01b

1 participant