Conversation
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| (normalized.includes("could not resolve to a pullrequest") || | ||
| normalized.includes("repository.pullrequest") || | ||
| normalized.includes("no pull requests found for branch") || | ||
| normalized.includes("http 404") || |
There was a problem hiding this comment.
🟠 High vcs/VcsProcess.ts:96
An inaccessible GitHub resource is classified as not-found, so an expired or insufficiently scoped token causes GitHubCli to raise GitHubPullRequestNotFoundError and getPullRequestStack to return null, discarding the existing stack and entering the non-stack merge flow. The generic http 404 match also covers GitHub's authorization-scoped 404 responses; remove it or restrict it to an explicit pull-request-not-found message so authentication failures remain actionable.
- normalized.includes("http 404") ||🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/vcs/VcsProcess.ts around line 96:
An inaccessible GitHub resource is classified as `not-found`, so an expired or insufficiently scoped token causes `GitHubCli` to raise `GitHubPullRequestNotFoundError` and `getPullRequestStack` to return `null`, discarding the existing stack and entering the non-stack merge flow. The generic `http 404` match also covers GitHub's authorization-scoped 404 responses; remove it or restrict it to an explicit pull-request-not-found message so authentication failures remain actionable.
There was a problem hiding this comment.
Fixed in bedb2a1. The fallback now applies only to the stacks listing and verifies PR-read access before returning null. It probes GET /repos/{owner}/{repo}/pulls/{number}/commits?per_page=1 with the same host and credentials; every probe failure propagates. Unlike the basic repository/PR endpoints, listing PR commits requires the same pull-request read permission as listing stacks.
A not-found error fetching an already discovered stack's details is no longer caught by the fallback either. This keeps access failures actionable and preserves the previous stack instead of clearing it.
Added regression coverage for access-probe 404/401/403, signed-out credentials, 429/503, and known-stack detail failures. All 312 focused tests pass, along with server typecheck and targeted lint. The read-only GHES check still returns a stacks 404 with a successful permission probe, preserving the original fix.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a focused two-file bug fix that restores the existing no-stacks fallback through one Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughGitHub CLI HTTP 404 failures now use the ChangesGitHub CLI stack handling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant GitHubPullRequestCli
participant VcsProcess
participant GitHubAPI
GitHubPullRequestCli->>VcsProcess: Read stacks preview
VcsProcess->>GitHubAPI: Request stacks data
GitHubAPI-->>VcsProcess: Return 404
GitHubPullRequestCli->>VcsProcess: Check pull request commits
VcsProcess->>GitHubAPI: Request commits with --silent
GitHubAPI-->>VcsProcess: Return success or typed failure
VcsProcess-->>GitHubPullRequestCli: Return null or propagate failure
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:S
The tracked upstream PR list held bare numbers. The reports showed each
PR's intake status, but not why the fork was waiting on it or when the
entry could be removed.
Each entry in `.github/upstream-tracked-prs.json` is now `{ "pr": 123,
"reason": "..." }`, and the decoder rejects an entry without a reason.
`upstream-queue.ts status` prints the reason under each PR, and the
tracked PR report in the Upstream lag report and promotion summaries has
a new "Why tracked" column.
List changes:
- Removed `pingdotgg#9511`, `pingdotgg#9753`, `pingdotgg#9773` and `pingdotgg#9807`, which
are already recorded as imported.
- Added the GitHub stack merge chain: `pingdotgg#10839`,
`pingdotgg#10870`, `pingdotgg#10875` and `pingdotgg#11486`, plus the open follow-up `pingdotgg#12645`. The
fork's merge button uses GitHub's legacy merge endpoint, which GitHub
documents as unable to merge stacked PRs; `pingdotgg#10875` adds a merge stack
action and replaces the fork's stack section.
- Wrote reasons for the other existing entries from the investigations
that added them.
The runbook now says to remove an entry once the report shows it
recorded or once its reason no longer applies, and that a reason writes
a fork PR as "fork #123" while a bare number means an upstream PR.
## Validation
- Ran `node scripts/upstream-queue.ts status` and `node
scripts/upstream-tracked-prs-report.ts` against freshly fetched fork and
upstream refs. All 13 entries show their reason; 11 are pending and
`pingdotgg#10845` and `pingdotgg#12645` are open upstream.
- Decoder tests cover a valid entry, a bare number, a string PR number,
a duplicate PR, and a missing or blank reason. The tracked PR and intake
tests pass (16), along with the scripts typecheck and targeted lint.
---
Written by an agent (Claude Code, claude-opus-5-5).
This branch has not been deployed
1 participant
What Changed
Recognize
HTTP 404inghstderr asnot-foundinVcsProcess, so hosts without the stacks API can reach the existing no-stack fallback.Limit that fallback to the stacks listing and verify access before returning no stack. A listing 404 triggers a read-only request for one commit on the same PR, using the same host and credentials. The commit-list endpoint requires the same pull-request read permission as the stacks API. If the probe fails, its error propagates; a 404 while reading an already discovered stack's details also stays an error.
Replace the pre-classified fallback mock with raw process-output tests through
VcsProcess,GitHubCli, andGitHubPullRequestCli. Cover both observed 404 formats, denied/missing PR access, invalid credentials, rate limits, and service failures.Why
On GitHub Enterprise Server, the stacks endpoint returns
gh: Not Found (HTTP 404). The process boundary currently classifies this ascommand-failed, so the existing not-found fallback never runs. An otherwise mergeable PR stays stuck on Retry stack lookup, with the normal merge action hidden.GitHub also uses 404 for inaccessible resources. Verifying PR-read access prevents an authorization-scoped 404 from being silently treated as an unstacked PR. A repository-metadata or PR-detail probe would be weaker: those can succeed without pull-request read permission. The client-side stack guard and merge-permission checks are unchanged.
Validation
VcsProcess.test.ts,GitHubCli.test.ts,GitHubPullRequestCli.test.ts,PullRequestSyncReactor.test.ts, andpullRequestDetail.logic.test.ts.git diff --checkpass.No UI components or layouts change. Screenshots from the local validation contain private enterprise repository details, so they are not attached.
Checklist
Summary by CodeRabbit