[BUG]: (possibly) discrepancy between updateRef docs and implementation

[keilin-anz]

What happened?

Summary

Calling updateRef and passing a ref value of the form refs/tags/foo/v1.2.3 results in a "ref doesn't exist" error. (it does exist)

Passing a ref property of the form tags/foo/v1.2.3 does however work - but shouldn't since it doesn't start with refs

Details

From the docs:

If the value doesn't start with refs and have at least two slashes, it will be rejected

From the plugin-rest-endpoint-methods.js repo:

updateRef: ["PATCH /repos/{owner}/{repo}/git/refs/{ref}"],

(emphasis my own)

Why it might be a bug

ℹ️ disclaimer: it's late, and whilst I've attempted to check my facts I could have missed something glaringly obvious 😅

• What I did:
  • Called updateRef (via github-script@v6) with a ref property of the form refs/tags/foo/v1.2.3
• Expected behaviour:
  • specified ref is updated
• Actual behaviour:
  • error is returned claiming the ref doesn't exist

Analysis

Upon inspection of the error, I noticed the underlying endpoint url contained refs/refs/tags/...

• What I did:
  • (troubleshooting) changed my ref property in the call to exclude the refs/ prefix (ie. tags/foo/v1.2.3 instead of refs/tags/foo/v1.2.3)
• Expected behaviour:
  • (per documentation) call rejects because the ref doesn't start with refs and doesn't contain at least two slashes
• *Actual behaviour:
  • it updated my ref as originally expected, ie. my argument tags/foo/v1.2.3 updated refs/tags/foo/v1.2.3

Things which may (or may not!) be involved in fixing

Presumedly one of the following:

1. Update the docs to reflect the actual behaviour
2. Update the endpoint config to exclude the refs/ prefix
3. Strip the refs/ prefix if present (probably not great, though potentially less potentially breaking than (2) and more consistent than (1)
4. Point out that I'm a dunce and that I should commence exuberant facepalming 🤦

And if going with (2) or (3), presumedly:

• Update updateRef to reject refs which don't start with refs

Versions

• actions/github-script@v6
• whichever version of octokit/rest.js the above uses
• whichever version of plugin-rest-endpoint-methods.js the above uses

Relevant log output

Note: I have replaced the org and repo names with OWNER_ORG and REPO_NAME below, as well as the tag name and the SHA

RequestError [HttpError]: Reference already exists
    at /actions-runner/install/_work/_actions/actions/github-script/v6/dist/index.js:6842:21
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async eval (eval at callAsyncFunction (/actions-runner/install/_work/_actions/actions/github-script/v6/dist/index.js:15143:16), <anonymous>:14:3)
    at async main (/actions-runner/install/_work/_actions/actions/github-script/v6/dist/index.js:15236:20) {
  status: 422,
  response: {
    url: 'https://api.github.com/repos/OWNER_ORG/REPO_NAME/git/refs',
    status: 422,
    headers: {
      'access-control-allow-origin': '*',
      'access-control-expose-headers': 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset',
      'content-length': '117',
      'content-security-policy': "default-src 'none'",
      'content-type': 'application/json; charset=utf-8',
      date: 'Wed, 02 Aug 2023 14:25:10 GMT',
      'referrer-policy': 'origin-when-cross-origin, strict-origin-when-cross-origin',
      server: 'GitHub.com',
      'strict-transport-security': 'max-age=31536000; includeSubdomains; preload',
      vary: 'Accept-Encoding, Accept, X-Requested-With',
      'x-content-type-options': 'nosniff',
      'x-frame-options': 'deny',
      'x-github-api-version-selected': '2022-11-28',
      'x-github-media-type': 'github.v3; format=json',
      'x-github-request-id': '1326:1D98:CB10B9:D71500:64CA6746',
      'x-ratelimit-limit': '15000',
      'x-ratelimit-remaining': '14943',
      'x-ratelimit-reset': '1690987679',
      'x-ratelimit-resource': 'core',
      'x-ratelimit-used': '57',
      'x-xss-protection': '0'
    },
    data: {
      message: 'Reference already exists',
      documentation_url: 'https://docs.github.com/rest/git/refs#create-a-reference'
    }
  },
  request: {
    method: 'POST',
    url: 'https://api.github.com/repos/OWNER_ORG/REPO_NAME/git/refs',
    headers: {
      accept: 'application/vnd.github.v3+json',
      'user-agent': 'actions/github-script octokit-core.js/3.6.0 Node.js/16.16.0 (linux; x64)',
      authorization: 'token [REDACTED]',
      'content-type': 'application/json; charset=utf-8'
    },
    body: '{"ref":"refs/tags/TAG_NAME/v0.1.3","sha":"lots of SHA numbers"}',
    request: { agent: [TunnelingAgent], hook: [Function: bound bound register] }
  }
}

Code of Conduct

• I agree to follow this project's Code of Conduct

[github-actions]

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

[wolfy1339]

There could be a bug in how we pass in the data parameters. That would be in the underlying modules @octokit/request and @octokit/endpoint

We don't have any special implementation on our side, we just simply wrap a request to the API.
All the endpoints are auto-generated from GitHub's OpenaAPI specs

[saiya]

We don't have any special implementation on our side, we just simply wrap a request to the API.
All the endpoints are auto-generated from GitHub's OpenaAPI specs

The generated code has inconsistency between createRef method and other ref related methods (getRef, updateRef, deleteRef):

• ref parameter of the createRef method: Takes full ref (e.g. refs/tags/my-tag)
  • The method use the value as a part of request body. Octokit don't add ref/ prefix in this case
• Other ref related methods (getRef, updateRef, deleteRef): Takes ref without ref/ prefix (e.g. tags/my-tag)
  • The method use the value as a part of URL path (ref/{ref}). Octockit's code adds ref/ prefix because it the URL's template is ref/{ref}

This difference should be caused by request body V.S. path (ref/{ref}). Ideally, Octokit's code should avoid adding duplicated ref/ prefix when constructing URL path of the ref related APIs (getRef, updateRef, deleteRef).

I assume Octokit cannot change this behavior in the all 2.x releases because changing this makes breaking change.
But the current tricky behavior should be documented at least...

[gr2m]

The generated code has inconsistency between createRef method and other ref related methods (getRef, updateRef, deleteRef):

I think the inconsistency is in how the REST API is defined in the OpenAPI spec: https://github.com/github/rest-api-description/

See the docs, which is generated from the same OpenAPI spec

1. Create a reference: POST /repos/{owner}/{repo}/git/refs - ref must include the refs prefix. (https://docs.github.com/en/rest/git/refs?apiVersion=2022-11-28#create-a-reference)
2. Update a reference: PATCH /repos/{owner}/{repo}/git/refs/{ref} - ref must not include the refs prefix. (https://docs.github.com/en/rest/git/refs?apiVersion=2022-11-28#update-a-reference). It's the same to get or delete a ref.

You can see how that makes sense from the REST API design perspective. But when using named methods with Octokit it's confusing.

I don't know what thy POST /repos/{owner}/{repo}/git/refs endpoint requires the ref request body argument to start with refs/, but here we are. It would be great if the refs/ prefix was not required in POST /repos/{owner}/{repo}/git/refs, we could try to find out why that is