Skip to content

Commit 2577b15

Browse files
rynfarrynfar
committed
docs: adopt DEF-1 and DEF-2, closing the deferred register
1 parent 5c0a807 commit 2577b15

1 file changed

Lines changed: 56 additions & 4 deletions

File tree

‎.agents/upstream-review.md‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,62 @@ Setup notes discovered during that pass, since they will bite the next session:
591591
that the accessibility tree does not expose are unreachable by gesture; deep
592592
links such as `pylon-code-dev://new/draft` are the reliable way in.
593593

594+
## 2026-08-11 (sixth batch) — deferred work adopted: `#4849`, `#6049`, `#6061`
595+
596+
**DEF-1 and DEF-2 adopted, closing the deferred register.** The developer chose
597+
to bring them in ahead of the recorded revisit gates, which had not come due —
598+
the date gate was 2026-08-25 and the churn check was still non-empty. That is a
599+
deliberate override, not a gate that passed, and it is recorded as such: the
600+
pull-requests surface may still move upstream, so expect follow-up commits.
601+
602+
No new upstream range is involved, so the cursor does not move. Every commit
603+
here sits at or before the current `reviewed-through`.
604+
605+
| Change set | Upstream | Decision | Pylon reference | Rationale or revisit condition |
606+
| ---------- | --------------------------------- | -------- | --------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
607+
| M1 | `cad2c9361` / `#4849` | adopted | (this branch) | Multi-provider pull requests page with in-app reviews: 143 files, 34,133 insertions, covering GitHub, GitLab, Bitbucket, and Azure DevOps providers plus the diff-comment review surface. Only **three** conflicts against a much-changed `pylon`, all resolved Pylon-first — see below. |
608+
| M2 | `a7b0366cb` / `#6049` | adopted | (this branch) | PR page header accounts for Windows window controls. Clean once M1 was in; it conflicted only because it patches a file M1 creates. |
609+
| M3 | `f5fce7416` / `#6061` (was DEF-2) | adopted | (this branch) | Routes self-hosted GitLab remotes through the pull-requests service. Clean once M1 supplied `PullRequestService.ts`, which is exactly why it was deferred with DEF-1 rather than skipped. |
610+
611+
Conflict resolutions, all Pylon-first:
612+
613+
- **`index.css`** — upstream reintroduces `--animate-status-pulse` alongside a
614+
new `--animate-ghost-pulse`. **Only `ghost-pulse` was taken.** The new
615+
`PullRequestGhosts.tsx` depends on it in six places, whereas
616+
`animate-status-pulse` is the continuously repainting dot Pylon deliberately
617+
replaced with DotMatrix, and `ServerUpdateAction.test.tsx` asserts its
618+
absence. Pylon's `-1s` skeleton delay was kept over upstream's undelayed one.
619+
- **`SidebarChrome.tsx`** — kept Pylon's usage Back button from K9 and added
620+
upstream's `pullRequestsSupported` Pull Requests entry. **This also completes
621+
`#6031`:** K9 took only its usage half because Pylon had no PR page, so the
622+
pull-requests Back button is now restored, giving that page the same way out.
623+
- **`RightPanelTabs.tsx`** — kept Pylon's "Pylon desktop app" wording and took
624+
upstream's new `terminal` disabled-reason key, which neither side had before.
625+
626+
Verification: server `pullRequest` and `sourceControl`, contracts
627+
`pullRequest`, and client-runtime `pullRequestDiffHttp` pass (496 tests); web
628+
`pullRequest`, `diffs`, `openPullRequestLink`, `rightPanelStore`,
629+
`reviewCommentContext`, `useLiveRefresh`, and `ServerUpdateAction` pass (198).
630+
Contracts, client-runtime, web, mobile, and server typecheck with no errors —
631+
the two `pullRequest` Effect diagnostics are upstream suggestions, not errors.
632+
Lint clean and format clean across all 145 changed files. The generated
633+
`routeTree.gen.ts` carries the new route.
634+
635+
Cheaper than the original DEF-1 assessment feared: **no lockfile change, no new
636+
external dependency, no `vite.config.ts` change, and no migrations.** The only
637+
manifest edit is a `./state/pull-requests` subpath export in
638+
`packages/client-runtime/package.json`.
639+
640+
**Not verified:** nothing here was exercised in a real client. This adds a whole
641+
product surface across web, desktop, and mobile, and its remote and
642+
multi-environment behavior is untested in Pylon.
643+
594644
## Deferred register
595645

646+
_The register is currently empty: DEF-1 and DEF-2 were adopted on 2026-08-11
647+
(see the sixth batch above). Entries are removed once adopted or skipped, so an
648+
empty register means nothing is waiting._
649+
596650
Upstream work that has been reviewed and consciously _not_ adopted yet, with
597651
the condition that should trigger a fresh look. Entries stay here until they
598652
are adopted or skipped outright — a deferral that nobody revisits is the same
@@ -602,7 +656,5 @@ Every review must read this register before reporting new candidates,
602656
re-evaluate each `Revisit when` against the current upstream head, and report
603657
the outcome. See Phase 2.5 of the `review-t3-upstream` skill.
604658

605-
| ID | Upstream | Deferred on | Revisit when | Why deferred |
606-
| ----- | ----------------------------------------------- | ----------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
607-
| DEF-1 | `cad2c9361` / `#4849` and `a7b0366cb` / `#6049` | 2026-08-11 | **Both** gates must hold, because an empty log alone is also what day one looks like: (1) the date is on or after **2026-08-25**, and (2) `git log --oneline --since="14 days ago" t3code-upstream/main -- apps/web/src/routes/_chat.pull-requests.tsx packages/contracts/src/pullRequest.ts packages/client-runtime/src/state/pullRequests.ts` is empty. Then review as its own dedicated batch with a real product decision, not as part of a range sync. | Multi-provider pull requests page with in-app reviews: **34,121 insertions across 143 files**, larger than the other 59 commits of the 2026-08-11 range combined. It is a new product surface, not a fix — an 800-line `pullRequest.ts` contract, ~149 lines of new RPC, new HTTP endpoints, new client-runtime state, new dependencies, a `vite.config.ts` change — and Pylon would own all of it across web, desktop, mobile, and every connection mode. Three reasons to wait rather than skip: (1) **it will churn** — in this same range upstream rewrote `UsagePage` (`#5823`) and deleted the project-settings routes it had just added (`#5923`) within days, and `#4849` landed one day before the reviewed head with `#6049` already following it; (2) it touches `openPullRequestLink` and `sourceControlPresentation`, the same area as the merged-badge/auto-settle fix made in the previous batch, so landing it now risks masking that; (3) there is no security, correctness, or provider-compatibility pressure. "Does Pylon want in-app PR review?" deserves its own product decision, not a line in a 60-commit sync. |
608-
| DEF-2 | `f5fce7416` / `#6061` | 2026-08-11 | Adopt together with DEF-1, as part of the same dedicated pull-requests batch. It has no standalone revisit condition: without the pull-requests surface there is no `PullRequestService.ts` to patch. | Routes self-hosted GitLab remotes through the pull-requests service. Two of its five files (`apps/server/src/pullRequest/PullRequestService.ts` and its test) do not exist in Pylon. The `SourceControlProviderRegistry` half could in principle be lifted out, but it exists to serve the PR service and carries no benefit alone. First application of the standing policy: work that depends on the deferred pull-requests surface is deferred with it, never skipped, so it is not lost when DEF-1 is eventually taken. |
659+
| ID | Upstream | Deferred on | Revisit when | Why deferred |
660+
| --- | -------- | ----------- | ------------ | ------------ |

0 commit comments

Comments
 (0)