Skip to content

Enhance token generation by including isPullRequest flag in xetWriteToken and related functions for better handling of pull requests#1746

Merged
mishig25 merged 1 commit intomainfrom
fix_xet_pull_request
Sep 13, 2025
Merged

Enhance token generation by including isPullRequest flag in xetWriteToken and related functions for better handling of pull requests#1746
mishig25 merged 1 commit intomainfrom
fix_xet_pull_request

Conversation

@mishig25
Copy link
Collaborator

@mishig25 mishig25 commented Sep 12, 2025 •
edited
Loading

I was getting error when trying to create pull request on repos that I do not own:

Error: Forbidden: pass `create_pr=1` as a query parameter to create a Pull Request. URL: https://huggingface.co/api/models/reach-vb/TinyLlama-1.1B-Chat-v1.0-q4_k_m-GGUF/xet-write-token/main

I was using new API from #1718

// Edit first kB of file
await commit({
  repo,
  accessToken: "hf_...",
  operations: [{
    type: "edit",
    originalContent: originalFile,
    edits: [{
      start: 0,
      end: 1000,
      content: new Blob(["blablabla"])
    }]
  }]
})

Let me know if this PR is the right way to handle this issue

@mishig25
Enhance token generation by including isPullRequest flag in xetWriteT…
3d81ba9 
…oken and related functions for better handling of pull requests
coyotte508
coyotte508 approved these changes Sep 12, 2025
View reviewed changes
Copy link
Member

@coyotte508 coyotte508 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @assafvayner for viz

@mishig25 mishig25 marked this pull request as ready for review September 12, 2025 15:12
@mishig25
Copy link
Collaborator Author

mishig25 commented Sep 12, 2025

updated the description

@coyotte508
Copy link
Member

coyotte508 commented Sep 12, 2025

this should be the right way @mishig25

@coyotte508
Copy link
Member

coyotte508 commented Sep 12, 2025 •
edited
Loading

hmm maybe this should be taken into account in https://github.com/huggingface-internal/moon-landing/pull/14572 too

@seanses
Copy link

seanses commented Sep 12, 2025

hmm maybe this should be taken into account in huggingface-internal/moon-landing#14572 too

Probably not.. To git push LFS files to a repo clients first need to communicate with the LFS Batch API, and that requires write access.

@coyotte508
Copy link
Member

coyotte508 commented Sep 12, 2025

hmm maybe this should be taken into account in huggingface-internal/moon-landing#14572 too

Probably not.. To git push LFS files to a repo clients first need to communicate with the LFS Batch API, and that requires write access.

@seanses There are multiple cases in moon-landing's current behavior:

  • If discussions are disabled for the repo, the user needs write access to a repo
  • If a ref is specified in the LFS protocol, the user needs write access to that ref (eg if it's a ref they opened a PR with)
  • If no ref is specified in the LFS protocol (eg opening a PR) the user need read access to the repo only

The JS lib has this snippet in their LFS call:

huggingface.js/packages/hub/src/lib/commit.ts

Lines 330 to 334 in bc98deb

...(!params.isPullRequest && {
ref: {
name: params.branch ?? "main",
},
}),

My guess if the user only has read access, eg they don't specify a ref, then the xet-write-token url the git-xet client needs to use to refresh the token needs to have a ?create_pr=1 query param. If they specify a ref, we should forward the ref to the xet-write-token url given to the client.

(will re-review https://github.com/huggingface-internal/moon-landing/pull/14572 monday)

cc @Wauplin

@seanses
Copy link

seanses commented Sep 12, 2025

@coyotte508 I see. Thanks for explaining this!

@mishig25 mishig25 merged commit 5607688 into main Sep 13, 2025
3 of 5 checks passed
@mishig25 mishig25 deleted the fix_xet_pull_request branch September 13, 2025 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coyotte508 coyotte508 coyotte508 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mishig25 @coyotte508 @seanses