Skip to content

[5/n][sl][github][gh stacks] wire up gh stacks to sl pr submit - #1396

Open
raydatray wants to merge 5 commits into
facebook:mainfrom
raydatray:pr1396
Open

[5/n][sl][github][gh stacks] wire up gh stacks to sl pr submit#1396
raydatray wants to merge 5 commits into
facebook:mainfrom
raydatray:pr1396

Conversation

@raydatray

@raydatray raydatray commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown

ctx

actually links the submitted PRs into a native github stack and deals with the various constraints github puts on stacked diffs

changes made

  • after submit, diffs are linked via the stacks API: create the stack on first submit, then append new diffs when the local stack extends it at the top
  • the remote stack is queried up front, before any mutations, and submit fails closed:
    • if the query fails, abort (pushing blind against an unknown stack state could corrupt it)
    • if it diverged from the local stack (reorder, dropped diffs, or a new diff inserted below the top - stacks can only grow at the top), abort before updating bases or pushing. new --restack flag dissolves and recreates it (since the gh API has no reorder operation). tell the user to do this as a ui hint
    • --restack refuses to dissolve a stack it could not recreate (fewer than 2 diffs), and aborts if dissolution is only partial (merged/queued diffs stay behind)
    • closed stacks are treated as no stack
  • base branches of diffs already in a stack are never updated via the API (since gh rejects that, the stack manages bases on its own). body/title rewrites use the "base-less" mutation from [2/n][sl][github][gh stacks] add support for stack endpoint #1393
  • new diffs are created directly against the head branch below them instead of being created against main and re-based afterwards (this is impossible to do once diffs are stacked on gh). falls back to default base for forks, where gh native stacks are not supported and a warning is printed (i wanted to submit this stack with my patch to show it works but alas i cant :( )

test plan

  • test-ext-github-pr-submit-stacked.t covers: the initial submit, extending an existing stack, diverged stack with/without --restack and with/without local changes to push, mid-stack insertion with/without --restack, stacks API query failure, partial unstack (abort up front / warn during sync), --restack with fewer than 2 diffs, closed stack, fork fallback
  • test-ext-github-pr-submit-stacked-placeholder.t: placeholder strategy + stacked
  • new mocks derive commit hashes from the test repo at runtime (reposetup) instead of hardcoding them, and use the consumption check from [1/n][sl][github][gh stacks] support headers and list params #1392 so unexercised expectations fail the test
  • tested live on my own machine/repo
sl config --local extensions.github=/Users/ray/sapling/eden/scm/sapling/ext/github/__init__.py
sl config --local extensions.signing_shim=/Users/ray/.sl-signing-shim.py
sl config --local github.pr-workflow=stacked
  • create a stack here [1/n][stacks] test raydatray/rusty-mcrouter#197
  • submit 197 and 198 - it creates a stack
  • then add 200 locally and submit again - it extends the stack
  • then add 202 locally and submit again - it extends the stack
  • go down to 198 and amend the diff locally. restack locally and resubmit - modifies only 198 and the stack is updated
Screenshot 2026-07-31 at 3 42 08 PM

Stack created with Sapling. Best reviewed with ReviewStack.

This was referenced Jul 31, 2026
Open
Open
Open
Open
Open
Open
@meta-cla meta-cla Bot added the CLA Signed label Jul 31, 2026
@meta-codesync

meta-codesync Bot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request has been imported. If you are a Meta employee, you can view this in D114402728. (Because this pull request was imported automatically, there will not be any future comments.)

@raydatray raydatray mentioned this pull request Jul 31, 2026
Open
@facebook-github-tools

facebook-github-tools Bot commented Aug 13, 2026

Copy link
Copy Markdown

@raydatray has updated the pull request. You must reimport the pull request before landing.

@raydatray
[1/n][sl][github][gh stacks] support headers and list params
a201672 
### ctx
github added native stacked pull requests, it would be nice to add native support to it for sapling!

this stack lets `sl pr submit` and `sl pull` via a new `github.pr.workflow = stacked` mode

### changes made
- `github_gh_cli.make_request` accepts custom headers (`-H`, needed for the `X-GitHub-Api-Version` preview header)
- `_format_param` supports list values using the `gh api` repeated-field syntax (`pull_requests[]=101`)
- an empty list is passed explicitly as `key[]` without a value (per the gh api manual) rather than dropped, since an empty array and a missing field can mean different things to an endpoint
- mock_utils: `MockGitHubServer` now tracks which expectations were consumed; tests can opt in via `wrap_with_consumption_check` to fail when an expected request silently stops happening (closes the old TODO)

### test plan
doctests in `_format_param` (registered in test-doctest.py, including the empty-array case), existing github .t suite. verified the empty-array wire format empirically: `gh api -F "pull_requests[]" --verbose` sends `{"pull_requests": []}`
@raydatray
[2/n][sl][github][gh stacks] add support for stack endpoint
39097a0 
### ctx
basic plumbing for github native stacks REST API

### changes made
- `gh_submit`: `StackDetails` dataclass + `get_stack_for_pull_request`,  `create_stack`, `add_prs_to_stack`, `unstack`, all pinned to the `2026-03-10` preview API version
- `unstack` returns the remaining stack when dissolution is partial (merged/queued diffs cannot be unstacked) so callers can react instead of assuming success
- `update_pull_request` now takes `base: Optional[str]`: github rejects `updatePullRequest` mutations that include `baseRefName` for PRs in a  native stack, so `base=None` uses a new mutation variant  (`GRAPHQL_UPDATE_PULL_REQUEST_NO_BASE`) that only touches title/body

### test plan
CI (doctest on `_parse_stack_from_dict`), and dogfooded on my own computer via a patch  raydatray/rusty-mcrouter#198
<img width="905" height="468" alt="Screenshot 2026-07-31 at 3 20 04 PM" src="https://github.com/user-attachments/assets/bbe461a5-4128-4c75-b603-8ef4e012bb05" />
@raydatray
[3/n][sl][github][gh stacks] add stacked workflow for submit
f79b0f3 
### ctx
introduces the new native github stack workflow

### changes made
- new `SubmitWorkflow.STACKED` variant, selected via `github.pr-workflow=stacked`
- chains each PR's base to the head branch of the PR below it (like `single`), factored into `SubmitWorkflow.uses_chained_bases()`
- hardened chained-base selection everywhere it happens (base-update loop, body rewrites, serial and placeholder creation): closed/merged diffs are skipped when picking the base below (their head branches would break the chain), and forks never chain (fork head branches cannot be bases on the upstream repo, so fall back to the default branch)

### test plan
- new test-ext-github-pr-submit-stacked.t (initial submit)
- test-ext-github-pr-submit-closed.t: open diffs stacked on a closed one chain past it to main
- test-ext-github-pr-submit-placeholder-issue.t: placeholder strategy on a fork creates diffs against the upstream default branch
@raydatray
[4/n][sl][github][gh stacks] update submit body for stacks
6d6375d 
### ctx
github renders native stacks in the PR UI itself so the sapling footer would be redundant - lets remove it when submitting via native stacks

### changes made
- `create_pull_request_title_and_body` takes a `stack_list` flag; the stacked workflow omits the footer, other workflows are unchanged

### test plan
updated mocks + test-ext-github-pr-submit-stacked.t
@raydatray
[5/n][sl][github][gh stacks] wire up gh stacks to sl pr submit
28ddf2e 
### ctx
actually links the submitted PRs into a native github stack and deals with the various constraints github puts on stacked diffs

### changes made
- after submit, diffs are linked via the stacks API: create the stack on first submit, then append new diffs when the local stack extends it at the top
- the remote stack is queried up front, before any mutations, and submit fails closed:
  - if the query fails, abort (pushing blind against an unknown stack state could corrupt it)
  - if it diverged from the local stack (reorder, dropped diffs, or a new diff inserted below the top - stacks can only grow at the top), abort before updating bases or pushing. new `--restack` flag dissolves and recreates it (since the gh API has no reorder operation). tell the user to do this as a ui hint
  - `--restack` refuses to dissolve a stack it could not recreate (fewer than 2 diffs), and aborts if dissolution is only partial (merged/queued diffs stay behind)
  - closed stacks are treated as no stack
- base branches of diffs already in a stack are never updated via the API (since gh rejects that, the stack manages bases on its own). body/title rewrites use the "base-less" mutation from facebook#1393
- new diffs are created directly against the head branch below them instead of being created against main and re-based afterwards (this is impossible to do once diffs are stacked on gh). falls back to default base for forks, where gh native stacks are not supported and a warning is printed (i wanted to submit this stack with my patch to show it works but alas i cant :( )
### test plan
- test-ext-github-pr-submit-stacked.t covers: the initial submit, extending an existing stack, diverged stack with/without --restack and with/without local changes to push, mid-stack insertion with/without --restack, stacks API query failure, partial unstack (abort up front / warn during sync), --restack with fewer than 2 diffs, closed stack, fork fallback
- test-ext-github-pr-submit-stacked-placeholder.t: placeholder strategy + stacked
- new mocks derive commit hashes from the test repo at runtime (reposetup) instead of hardcoding them, and use the consumption check from facebook#1392 so unexercised expectations fail the test
- tested live on my own machine/repo
```
sl config --local extensions.github=/Users/ray/sapling/eden/scm/sapling/ext/github/__init__.py
sl config --local extensions.signing_shim=/Users/ray/.sl-signing-shim.py
sl config --local github.pr-workflow=stacked
```
  - create a stack here raydatray/rusty-mcrouter#197
  - submit 197 and 198 - it creates a stack
  - then add 200 locally and submit again - it extends the stack
  - then add 202 locally and submit again - it extends the stack
  - go down to 198 and amend the diff locally. restack locally and resubmit - modifies only 198 and the stack is updated

<img width="906" height="465" alt="Screenshot 2026-07-31 at 3 42 08 PM" src="https://github.com/user-attachments/assets/16287bdb-fafa-4bf9-9427-5265875f9acf" />
@facebook-github-tools

facebook-github-tools Bot commented Aug 13, 2026

Copy link
Copy Markdown

@raydatray has updated the pull request. You must reimport the pull request before landing.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

CLA Signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@raydatray