Graphite Community

Hey <@U04RUSU99GX>! Can you describe more of what's happening here?

It was an issue with our VPN - working for me now!

I have a post-rewrite git hook that exits with a nonzero exit code when the rebased branch fails to compile/pass a test suite. This hook gets run each time graphite rebases a branch, but graphite seems to ignore the error code and continues on. I think graphite should stop rebasing a stack when the post-rewrite git hook fails.

image.png

Thanks for flagging! Will bring this up with our notifs team

you should be able to submit comments on your own pr the same way that a reviewer can — I may not be understanding the issue correctly?

Might be having a moment but I couldn't see the review UI for the PR

<@U02V54PQUGP> were you able to figure this out? happy to help!

Regarding 2, had similar problems the last few days but mostly with 502 error :thinking_face:

Hey <@U031D24164U>! You shouldn't have to be authenticating that frequently...going to loop <@U02RREE9GKH> here for some CLI help

When you say you need to reauthenticate, what exactly have you been needing to do?

Sorry, should've explained the issue completely. The CLI prompts me to enter my github username and my PAT when I try `gt ss`

Ah, this is a git issue, not related to graphite :slightly_smiling_face: If you clone via HTTPS, you'll need to cache your credentials — <https://docs.github.com/en/get-started/getting-started-with-git/caching-your-github-credentials-in-git>

You can also switch to a SSL URL if your repo allows: <https://docs.github.com/en/get-started/getting-started-with-git/managing-remote-repositories#switching-remote-urls-from-https-to-ssh>

Oh. Thanks, will try out the solutions you've mentioned. But this wasn't an issue earlier. Did this crop up recently?

i'm trying to look up what weird characters `^W` and `^u` are

is `^W` a delete character? i forget

my hunch is when we read from a user prompt, we're getting some garbage characters in there for some reason from the CLI

google is being really unhelpful when i search for `"^W"` even in quotes

but i've seen this bug before when trying to edit some `read` input from bash, in general

i would prefer if our user input prompts used `readline` under the hood so you could get the nice vi/emacs bindings when editing the input

and i wonder if that would clear up any weird terminal input bugs that can arise, like when you hit the `delete` key and it just inputs a `^W` style code instead

<https://stackoverflow.com/questions/19818752/bash-script-arrow-keys-produces-weird-characters>

Yeah I tried to look on Google as well about those char and didn't find anytying meaningful :sweat_smile:

The commit messages were written in vim: for one case a branch was created with `gt branch create` then amended with `git commit -a` and message written in vim.
The PR was squashed and merged. I checked the "original" commit: it doesn't contain the non-printable character. However the trunk commit (created from the PR title) has it.
So the character ended up in the PR title and because it was created with a `gt ss` we strongly suspect it's graphite but don't know for sure

arrow keys when entering the PR title? :thinking_face:  I will ask!

&gt; The commit messages were written in vim: for one case a branch was created with `gt branch create` then amended with `git commit -a` and message written in vim.
and then when they did `gt ss`, did they just accept the commit's message as the PR title, as-is, without editing it?

e.g. the `git commit` message that was made in Vim just became the PR title, straight up?

also would there be a way to just dump the raw `git commit` message that produced the bad PR title into a `txt` file and share it? in case there are secret hidden characters
or if `git` is doing something smart and hiding them?

and can you dump the raw commit message of the PR merge commit in the `main` branch, so i can see that weird character?

^ is usually use as short form of Ctrl, so ^W usually is Ctrl+W

yeah i just don't remember what that stands for

Do those people maybe use `nano` as editor usually?
It e.g. uses `^U` as "Paste" action. Maybe they try to past with that, then recognize they are not within `nano` and use the proper paste action, already having typed the `^U` character?

I just noticed that the developer edited the commit title (trying to understand how)
Here is the git log for the commit in the PR:
```git log -1 4972cf157a326f4b77b1cbda2b3b10557442cbcf                                    
commit 4972cf157a326f4b77b1cbda2b3b10557442cbcf
Author: XXXXXX
Date:   Tue May 23 10:48:14 2023 +0200

    Role names are singular
    
    Also, change XXXXX for consistency with other existing roles.```
and here is the one in the trunk (so the squashed and merged PR), showing the non-printable character in the title (first line):
```git log -1 d2017b3                                                                                       
commit d2017b3f19476233f376b689d18c3cf28cc4fb27
Author: XXXXXXXX
Date:   Tue May 23 14:46:42 2023 +0200

    ^UMinor adjustments to role name (#38926)
    
    Role names are singular.
    Also, change XXXXX for consistency with other existing roles.```
trying to understand with the dev what could have happened

hm, i wonder if it's in our request logs somewhere, from the `gt ss` action

i know they could also have edited PR title via the Github UI, or the Graphite UI

they edited using the cli, interesting point mentioned by the dev:
&gt; In traditional UNIX CLIs, ^U would mean "clear current line and restart" (but that would be swallowed by the line discipline in the tty driver and would not be seen by application code). Now I don't think I actually used that keypress (I wouldn't swear, but I don't think I did), but maybe there's something fishy with some component synthesizing it when replacing the default title with something else? (Or maybe I did use it and don't remember correctly).
&gt; 
&gt; In any case I think the specific use case of editing the default PR title in the context of a stack submit might well be narrowing towards the root cause.
Also ^W is "delete word left of cursor"

Maybe those are included as characters in the readline?

Ah, right, ^U is the counterpart to ^K (former to delete from caret to start of line, latter from caret to end of line)

thanks, this is all good context!
sometimes i find the shell gets in a weird state where these characters start creeping in (but other times they don't!) so i will try to reproduce

Hey <@U03RGJEDWH5>! This is a known issue - we're unable to render certain images on GitHub in Graphite due to privacy protections (GitHub doesn't want to leak images from PRs). We're going to show better error messaging here though

cc <@U05597DUYDD>, I thought we still include links to images in this case tho, no?

you click the broken image, and it brings up a zoomed image modal with an equally broken image :laughing:

the only workaround right now is to click edit comment, and manually copy the URL out of the comment

i would like to fix this though, so maybe we can chat in design hours tomorrow

What browser are you using? Has this been happening consistently or just recently?

rather consistently. I’m using the Arc browser

Ah yeah - known issue of infinite sharpening pencils/long wait times here due to some issues with Graphite+Arc compatibility (I use arc too). Thanks to <@U02PSKGK4TS>, we're going to try to make some changes here

Arc sometimes freezes tabs (assuming for performance) and if it freezes a Graphite tab, a new instance of Graphite (either a new tab or in peek) will also freeze

<@U059J4BPJN5> do you by chance have a Graphite tab pinned to your sidebar? If so, then when you experience this issue, you can click that tab (or other Graphite tab in the sidebar) to unfreeze the tab and unblock the IndexedDB operations. 

<@U02PSKGK4TS>, thanks. I think that helped. I”m going to avoid pinning Graphite tabs for now

OOF so glad I stumbled on this thread, this was so confusing! thanks for the workaround, its working great again :relieved:

I'm sorry I did not take a screenshot and cannot remember exactly.
Some about graphite using indexedDB.

The error message has a link to <https://bugzilla.mozilla.org/show_bug.cgi?id=1639542>

Also just seen it on Chrome.
With this in the console:
```main.deb6dca2.js:2 Failed to connect to recovery DB DOMException: Internal error opening backing store for indexedDB.open.
(anonymous) @ main.deb6dca2.js:2
main.deb6dca2.js:2 Uncaught (in promise) DOMException: Internal error opening backing store for indexedDB.open.
main.deb6dca2.js:2 DOMException: Internal error opening backing store for indexedDB.open.```
Just reloading the page fixed it here.

Does this happen if you do a `gt branch track` without the -p flag?

```$ gt branch track
ERROR: Command failed with error exit code 1:
git merge-base th/coverage-threshold-poc-1 edo/update-user-dual-write```
Gives me a different base branch, which threw me off (how does it pick that base branch?) and prompted me to specify `master` as the base

Yup, known issue! See this thread:
<https://graphite-community.slack.com/archives/C02DRNRA9RA/p1684871108668409>

To be clear my report is that it incorrectly references `gt add` as a valid graphite command.

Ah I see. It used to work, seems like a regression... cc <@U02RREE9GKH>

this does work, it just displays that message

so you know it’s passing through to git!

Oh interesting, it seems that the comment got submitted, but is now referencing a _different line_. On top of that, the comment starts with `[RE: lines 47 to 47]` which is where I actually placed the comment in the graphite UI.

<@U05597DUYDD> for vis who has been cataloging a few bugs to fix in our commenting experience.

<@U054ZV6BNG0> can you share the steps you took, so we can attempt to reproduce it?

Yeah it's a bit from memory, but I think it was this:
1. triple dot menu top-right on file =&gt; `Show all lines`
2. scroll up to a line that _wasn't_ touched in the PR
3. add comment
4. observe that the comment "disappears" in graphite UI
5. observe that comment appears on other line in github UI 

<@U054ZV6BNG0> The comment shouldn’t disappear when you’re on Graphite (that sounds like a bug).

As for the placement of the comment on GitHub: GitHub does not allow for commenting on unchanged lines. Graphite enables you to comment on unchanged lines and renders your comments on in the right place if you’re viewing the PR on <http://app.graphite.dev|app.graphite.dev> however, if you’re viewing it on GitHub, you will see the comment pinned to the closest commentable line with the actual lines mentioned in the comment.

Okay, honestly I might misremember, this is all from memory. Maybe I just double checked github, and was confused that the comment was somewhere else.

I've had to merge this PR but this seems to be happening on all my PRs today

hm looks like you've merged the pr already so we've lost that state

we've started calculating required codeowners on Graphite which is why you're seeing that message now. If you see this again can you let me know? It may have been a problem with us missing some data on our side at the time. On our side now it does look like the pr has all owners.

also question, how was reviewing on Graphite with codeowners required before? we didn't respect it at all so I'm sure our mergeability calculations were never right for you?

Oh! Really sorry. Yeah, I'll ping you next time I see it happen

Before, it just didn't let me merge I thought, though I might have missed it letting me. I usually monitor who's approved before I press merge

That said... We also have a bot that tells you the minimum set of reviewers you need for a PR (codeowners get... Complicated real fast, and our codeowners file is quite large and uses some weird github behaviour related to how they evaluate codeowners)

before, if you had minimum number of approvals you could try to merge on graphite but if we were missing required codeowners it would fail doing the merge, which is the reason we wanted to start enforcing codeowners on the graphite side. codeowners does get complicated quickly, especially for prs with lots of files with lots of different codeowners. Hopefully our new codeowner checks work for you but definitely let us know if you see something weird again!

Ok, I've got <https://github.com/wayveai/WayveCode/pull/32981?no-redirect=1|another PR>

That I can merge in github but not graphite

Oh I see the problem now! Looks like our codeowners calculation is right but we don't have data for those teams so we couldn't link the reviewers to the team codeowners. We should have this data so looking into why it's missing!

There's a fix going out now that should fix this going forward! Thanks for flagging and let me know if you see anything unexpected still

Oooh that's interesting! Is it just that you don't have access to our owners groups int the integration? But thanks for looking into this!

we should have permission there was just a bug in our teams logic. codeowners is also tricky since there isn't an api for it so we need to rederive everything ourselves

My apologies for "reopening" this thread, but I'm still experiencing this problem - I'm not sure if it's the same problem or if it's just similar/related.

We auto-assign two reviewers to each PR, but we only require approval from a single code-owner before merging.

What happens is: one reviewer approves, and I'm able to merge in GitHub - but Graphite still shows: `Needs approvals from code owners` and requires me to check the 'use admin privileges' box to merge.

For our workflow this is fine; we only need one approving review.

<@U058B0B8YF4> do you have an example pr where this is happening?

I'll get one set up - just waiting for the CI checks to finish.

feel free to dm when you do and I can take a look!

Graphite - Notifications are down ​*[Investigating]*​ Notifications have not been sent over the last 20 minutes, we are currently investigating!
 ​*Affected:*​
Notifications
 View incident button

Known issue! We have some folks looking into it now, thanks for flagging!!

Will update this thread when resolved :))

<@U047KHH8JH3> just merged a fix for this, should kick in soon!

I agree that it's not easy, maybe cause it takes more discipline to remember to run the things in the right order. <https://graphite.dev/docs/collaborating-on-a-stack|This page helps some>, but it'd be cool if there was more info, maybe an example or video...

The flow just isn't perfect right now, and that page describes what we currently support — we're working on some improvements to this flow this summer!

&gt; maybe an example or video...
coming soon!

graphite approved.png

Thanks for reporting, sorry about the bad experience. Could you share your PR org/repo/number via DM? I know what the issue is &amp; how to fix, and hope to do so by early next week.

hey! should be safe to just ctrl-c out of this when it happens, it's a known issue that seems to happen in certain repos. we're going to be adding more granular debugging to a future cli version to figure it out

(the cache being persisted here is just used to help with performance)

i’ve been attempting to sync this particular since last night and still no luck today :weary:. do i need to nuke my repo and start over?

"Persisting cache" happens after all the syncing, so ctrl-C here will be as if the command completed successfully

oh got it! so i can proceed as normal and pretend it all worked as long as i get to this spot?

Screen Recording 2023-05-26 at 11.02.02 AM.mov

cc <@U05597DUYDD> who is working on this feature

<@U04BX08MCBV> for now, does either
• paste to upload
• toolbar &gt; click the add image button
work as a workaround?

<@U04BX08MCBV> i merged a fix for drag &amp; drop in firefox, whenever our build pipeline gets it out it should be live!

DragonDrop! Thanks for the quick fix :heart:

I'm out for the rest of the day so I can't check, sorry :upside_down_face:

all good, i sanity checked it today and it's working great

multi file drop is working as well now, as a new feature

hi! it seems like you might somehow be running graphite on an old node version — how did you install the CLI?

Ahh, that could be the cause of it. I had changed to a older node version because of another project but haven't changed it back. Will change it back to the latest one

Works after changing node to latest version. Thanks!

we do try to check for this, but something weird might have happened

Any hint about node being the cause in the error message and I would've changed it back :sweat_smile:

yeah unfortunately this is thrown by node, before it reaches our code (because its some JS syntax that wasn't supported until node 14) — there is a version check script that we run on install which is the best we can do, but it seems to not have caught the issue in your case

Hello! Sorry this happened, I know this can be super frustrating :disappointed:

cc <@U02RREE9GKH> for advice here

hmm, thanks for reporting — we should be saving the description here for you even if that happens, I'll try to repro and fix!

Hi there! Sorry you're running into this, we'll investigate!

Can you DM me the expected behavior for a given PR?

<https://github.com/Uniswap/interface/pull/6675|uniswap/interface/pull/6675>

this repo has one approval but requires 2 approvals to merge, and i can't get the PR to appear in either my "approved" or "waiting for reviewers" sections with the following defs

<@U04HQKF7TSN> is this enough info? appreciate the response!

Thanks, will take a quick look now to see if anything is off and a deeper look a bit later!

This *should* be in "Waiting for reviewers" I think.

Do you mind if I reset your filters to defaults?

i'm a 1-day-old user so i don't have much personalized

Ah no worries, let me know if you see this again!

<@U04HQKF7TSN> i have some PRs with this behavior again, would you mind taking another look?

as an example, <https://github.com/Uniswap/interface/pull/6695|this one> has only 1 of the 2 required approvals and it's being counted in my "approved" count but doesn't actually appear in that section

we do have a CODEOWNERS file, but that's not included in the requirements for merging a PR to the `main` branch

Can you leave one or two of them unchanged?

actually i can't control my teammates but there are a few here so hopefully they don't all get approved

It's really strange, the DB state I see shouldn't put them in approved state

... looks like there were missing parentheses in the sql

seems like it's working :handshake:  thanks ~!!

chrome_ew04WiwcZK.gif

Also the search is not finding the PRs hidden behind the "Show more"

The API call for `/v1/graphite/section/prs` when pressing the "Show more" button succeeds, but only returns the already displayed 10 PRs

And if I sort alplhabetically it suddenly shows all 22

Thanks for the context! We'll take a look at what's going on here

We were having a bit of an issue with our sync yesterday, this might have been related

<https://github.com/spockframework/spock/pull/1671>

<https://app.graphite.dev/github/pr/spockframework/spock/1671>

Sorry about that! Looks like we had a blip in our notification sending system — thanks for reporting

Sorry you ran into this! It’s possible there was a secondary rate limit from github. If you re-run `gt ss`, does it correct?

I just now tried with `gt bs -f` and that did not help

Ah, yeah, `--always` not `--force`, one moment

And then let <@U05597DUYDD> ask me again why I don't trust Graphite. :smile:

Thanks for bearing with us! I know there are some rough edges, but your feedback helps and you can trust we’re working hard to improve graphite as fast as possible!

Yep, with `--always` dashboard and GH comment were properly fixed, thanks

glad! Yeah, I think you got bit by a secondary rate limit. We see this with tall (10+) stacks sometimes. Its undocumented in Github exactly how they throttle. We could update our system to catch and retry with backoff :thinking_face:

~i wonder if we should pause every N PRs in a stack for some sleep amount~
er, catching would probably be smoother

Yeah, retry with backoff on failure sounds like a good idea, maybe after vaildating the action really didn't happen. Failure could also happen after the action was done.

Btw. regarding "hit by secondary rate limite", that is unlikely as it was a 5xx error which should usually be a server error, not a request error like invalid arguments or rate limits. Also the output of gt cli is different:

```[...]
verify-method-in-method-invocation: <https://app.graphite.dev/github/pr/spockframework/spock/1681> (created)
ERROR: Failed to submit PR for verify-method-reflection-in-method-invocation: Looks like you've triggered a GitHub API secondary rate limit 🥞 Nice stack!
Please wait a moment and then try your submit operation again.
$ gt ss --pr-mode gt --publish --no-edit
[...]
verify-feature-in-specification-context: <https://app.graphite.dev/github/pr/spockframework/spock/1685> (created)
ERROR: Unexpected server response (502).

Response: {
  "_response": {}
}```