feat!: adopt canonical change URIs for change requests by xytan0056 · Pull Request #264 · uber/tango

xytan0056 · 2026-07-30T16:39:03Z

Summary

Adopt the change-URI RFC format for change requests. The URI is the self-contained identity of a change: the host names the provider instance, the embedded head SHA pins the code state, and the URI alone determines the cache key. The separate proto commit field is removed.

The format is enforced by the bundled native orchestrator, not the gateway — the mapper passes URLs through opaquely, so custom Orchestrator implementations can accept formats of their own.

Accepted URI format

Provider	Format	Example
GitHub PR	`github://{host[:port]}/{org}/{repo}/pull/{pr}/{head_sha}`	`github://github.com/uber/tango/pull/123/c3a4b5d6e7f80912a3b4c5d6e7f80912a3b4c5d6`

phab:// and git:// are not supported yet. Violations are rejected, never normalized:

Component	Rule
`host`	required; lowercase only
`port`	optional; digits only
`org`, `repo`	verbatim, case-sensitive; org may span segments (`uber/frontend`)
`pr`	positive integer, no leading zeros
`head_sha`	full 40-char lowercase hex

No userinfo, query, fragment, or empty segments; parse → serialize round-trips byte-for-byte.

Changes

New core/changeuri package: canonical parser/validator
Proto: Request.commit removed; tangopb/ regenerated
Cache key hashes only the URI; native orchestrator rejects invalid URIs as ErrorUser
workspace.NewRequest parses the URI and threads PR number + head SHA to gitRequest

Breaking change

The native orchestrator rejects old-format URLs (github://org/repo/pull/N + commit field); callers must migrate. Old treehash-cache entries recompute; treehash-keyed graph entries are unaffected.

Test plan

CI
integration test

Change requests were identified by a loosely-formatted URL plus a separate commit field. Adopt the change-URI RFC format instead: github://{host[:port]}/{org}/{repo}/pull/{pr}/{head_sha} The URI is the self-contained identity of a change request: the host names the provider instance and the embedded head SHA pins the exact code state, so the URI alone determines the cache key. The URL format is orchestrator-specific: the gateway (mapper) passes it through opaquely, and the bundled native orchestrator validates and rejects non-canonical URIs as user errors. Custom Orchestrator implementations may accept formats of their own. - Add core/changeuri: canonical parser/validator (github:// scheme). Rejects non-canonical spellings (uppercase host, abbreviated or uppercase SHA, leading-zero PR number, query/fragment/userinfo) and enforces byte-for-byte round-tripping; never normalizes. - Remove the proto Request.commit field (reserved); regenerate tangopb. - Hash only the URI in GetReqURLsHash; the pinned SHA is embedded in it. - workspace.NewRequest parses the URI and threads the PR number and head SHA to gitRequest; the ancestor sanity check is now unconditional. - Update example client help text and README with the URI format. BREAKING CHANGE: for the bundled native orchestrator, request URLs in the old format (github://org/repo/pull/N with a separate commit field) are rejected; callers must send canonical change URIs.

yushan8 · 2026-07-30T22:36:56Z

+	u, err := url.Parse(raw)
+	if err != nil {
+		return PullRequest{}, fmt.Errorf("parse change URI: %w", err)
+	}
+	if u.Scheme != Scheme {
+		return PullRequest{}, fmt.Errorf("unsupported scheme %q: only %q is supported", u.Scheme, Scheme)
+	}
+	if u.Opaque != "" {
+		return PullRequest{}, fmt.Errorf("change URI must have an authority: %q", raw)
+	}
+	if u.User != nil {
+		return PullRequest{}, fmt.Errorf("change URI must not contain userinfo: %q", raw)
+	}
+	if u.RawQuery != "" || u.ForceQuery || u.Fragment != "" {
+		return PullRequest{}, fmt.Errorf("change URI must not contain a query or fragment: %q", raw)
+	}
+	if err := validateHost(u); err != nil {
+		return PullRequest{}, err
+	}


nit: should these lines be in a helper function validateURL. I think it's a little bit more readable that way so this function focuses on parsing the pull request string into the struct.

Parse now reads as parse -> validate -> extract path components.

yushan8 · 2026-07-30T22:53:45Z

@@ -0,0 +1,181 @@
+// Copyright (c) 2025 Uber Technologies, Inc.


Suggested change

// Copyright (c) 2025 Uber Technologies, Inc.

// Copyright (c) 2026 Uber Technologies, Inc.

Done in f4264d9.

yushan8 · 2026-07-30T22:54:25Z

-    string commit = 2;
+    // commit was the SHA to apply; it is now embedded in the URI.
+    reserved 2;
+    reserved "commit";


should we just remove it?

once remove and deploy the package, existing calls can fail

I think we should just remove it for now to keep the proto clean. This isn't used by anyone but us, I think we can update it until it's actually finalized and productionized. Once it's actually in production, I agree we should use that to deprecated the fields.

Stacked on #264. ## Summary `Apply` diffed against the floating `pull/{id}/head` ref, so two requests carrying the same change URI (pull/123) could materialize different trees under the same cache key if the PR has different HEADs. This diffs against the pinned head SHA from the URI instead, making the materialized tree deterministic for a given change URI. The ancestor check still rejects stale or foreign SHAs. ** This is a breaking change ## Tests - Mock test asserting the diff target is the pinned SHA, not the PR head - Real-git tests (local bare repo with `refs/pull/1/head`, no env gating): pinned content applies end-to-end; the tree is byte-identical across PR advancement; a SHA that exists but is not an ancestor of the PR head is rejected ## Test plan CI integration test

sbalabanov · 2026-07-30T20:25:23Z

-    // The commit SHA associated with the change request
-    string commit = 2;
+    // commit was the SHA to apply; it is now embedded in the URI.
+    reserved 2;


as long as we can afford to make breaking changes anyways, we may refrain from reserving a field numeric

sbalabanov · 2026-07-30T20:26:25Z

+    // e.g. github://github.com/uber/tango/pull/123/c3a4b5d6e7f80912a3b4c5d6e7f80912a3b4c5d6
+    // whose embedded head SHA pins the exact code state; custom
+    // orchestrators may accept formats of their own. The URL participates
+    // in cache keys as an opaque string.


important to mention that URI must be immutable to the content it represents, for any possible implementation of the orchestrator

sbalabanov · 2026-07-30T20:30:05Z

+// (https://github.com/uber/submitqueue/blob/main/doc/rfc/change-uri.md).
+//
+// A change URI is an RFC 3986 URI of the form scheme://{host[:port]}/{path}
+// whose path pins the change to an exact code state. Only the GitHub pull


to make future implementations more structured, should we place github implementation under changeuri/github folder? Or even adopt the package from submitqueue?

sbalabanov · 2026-07-30T20:33:16Z

+// Scheme is the URI scheme for GitHub pull requests.
+const Scheme = "github"
+
+// PullRequest is the parsed form of a canonical GitHub pull request URI.


the "canonical" Github only has PR number, not a hash I think?

xytan0056 requested review from a team as code owners July 30, 2026 16:39

xytan0056 force-pushed the change-uri-rfc branch 3 times, most recently from e4599bd to c6deb47 Compare July 30, 2026 16:54

xytan0056 force-pushed the change-uri-rfc branch from c6deb47 to 7f7eed7 Compare July 30, 2026 16:59

xytan0056 mentioned this pull request Jul 30, 2026

fix(workspace): pin applied content to the URI head SHA #265

Merged

yushan8 reviewed Jul 30, 2026

View reviewed changes

xytan0056 and others added 2 commits July 30, 2026 15:49

refactor(changeuri): extract URI-level checks into validateURL

511a9cc

Parse now reads as parse -> validate -> extract path components.

Merge branch 'main' into change-uri-rfc

c1dbf00

yushan8 reviewed Jul 30, 2026

View reviewed changes

yushan8 approved these changes Jul 30, 2026

View reviewed changes

xytan0056 added 2 commits July 30, 2026 16:02

chore(changeuri): fix copyright year

f4264d9

chore(proto): drop reserved statement for removed commit field

3739c32

xytan0056 merged commit 075e784 into main Jul 30, 2026
10 checks passed

sbalabanov reviewed Jul 31, 2026

View reviewed changes

sbalabanov mentioned this pull request Jul 31, 2026

fix(workspace): fetch PR refs from upstream and pin applied commit (audit #3) #257

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat!: adopt canonical change URIs for change requests - #264

feat!: adopt canonical change URIs for change requests#264
xytan0056 merged 5 commits into
mainfrom
change-uri-rfc

xytan0056 commented Jul 30, 2026 •

edited

Loading

Uh oh!

yushan8 Jul 30, 2026 •

edited

Loading

Uh oh!

xytan0056 Jul 30, 2026 •

edited

Loading

Uh oh!

yushan8 Jul 30, 2026

Uh oh!

xytan0056 Jul 30, 2026

Uh oh!

yushan8 Jul 30, 2026

Uh oh!

xytan0056 Jul 30, 2026 •

edited

Loading

Uh oh!

yushan8 Jul 30, 2026

Uh oh!

Uh oh!

sbalabanov Jul 30, 2026

Uh oh!

sbalabanov Jul 30, 2026

Uh oh!

sbalabanov Jul 30, 2026

Uh oh!

sbalabanov Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,181 @@
		// Copyright (c) 2025 Uber Technologies, Inc.

	// Copyright (c) 2025 Uber Technologies, Inc.
	// Copyright (c) 2026 Uber Technologies, Inc.

Uh oh!

Conversation

xytan0056 commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Accepted URI format

Changes

Breaking change

Test plan

Uh oh!

yushan8 Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xytan0056 Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xytan0056 Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xytan0056 commented Jul 30, 2026 •

edited

Loading

yushan8 Jul 30, 2026 •

edited

Loading

xytan0056 Jul 30, 2026 •

edited

Loading

xytan0056 Jul 30, 2026 •

edited

Loading