chore(sync): absorb upstream main through 12391bd0d - #120
Conversation
…tgg#10173) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ing (pingdotgg#10210) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ingdotgg#10211) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
…#9561) Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
…dotgg#10247) Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
…ts (pingdotgg#10244) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ingdotgg#10257) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ngdotgg#10705) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…here else (pingdotgg#10712) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…#10713) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…not drift (pingdotgg#10714) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…them (pingdotgg#10501) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Pixel Perfect <me@pixp.cc>
Co-authored-by: Pixel Perfect <me@pixp.cc>
…gg#10790) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Merge pingdotgg/t3code main (779 commits since b21d872) into custom. Resolved 35 conflicted files, porting every fork customization on top of upstream's changes: sidebar drag/favicon/file-drop work under the V3 cards, the resting-composer machinery under the fork's ComposerShell, the release workflow gate onto resolve_commit, and the desktop Linux identity onto upstream's reverse-DNS desktop entry name. Decisions recorded in .fork/customizations.yaml: - upstream's resting composer layout (pingdotgg#7855) is neutralized in the composerFooterLayout shadow; the fork keeps one compact layout. - the vite-plus 0.3.0 lint rules that fired across fork-owned code are listed as DEFERRED_RULES in .fork/lint-owned.mjs for a follow-up. - the sidebar keeps its two DndContexts instead of upstream's single-list drop verbs; the project scope set stays session-scoped. Override drift ported for ComposerPendingUserInputPanel, ModelPickerContent and composerFooterLayout; the lucide shim gained the icons upstream started importing plus a createLucideIcon implementation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
NoahHendrickson
left a comment
There was a problem hiding this comment.
Reviewed the merge resolutions and fork integration at 87706006f, concentrating on the sidebar/draft flow, composer and model-picker shadows, preview integration, desktop identity, and CI changes. Found two issues that should be fixed before merging; details below.
Validation: traced the affected code paths in both merge parents and the result, and inspected the failed Check job log. No local test suites or browser verification were run. This was a focused integration review, not an exhaustive re-review of all 779 upstream commits or vendored references.
Posted as a comment review because the authenticated account owns this PR and cannot request changes on its own PR.
Codex (GPT-6) via Codex.
[P2] Handle draft targets before enabling file drops on every row
Source: apps/web/src/components/Sidebar.tsx:3993
This callback is also passed to the fork's unsent draft rows. Dropping a file on one queues a ScopedThreadRef, but navigateToThread correctly navigates that row to /draft/$draftId; handleThreadFileDrop then compares the landed URL against /$environmentId/$threadId and clears the queued files. Independently, ChatView's consumer returns immediately when composerDraftTarget is a string (a DraftId), so an already-open draft cannot consume the drop either. The row accepts/highlights the drop and opens the draft, but no attachment arrives. Add a draft-aware delivery path, or withhold the file-drop handler for draft rows until that path is supported.
[P2] Adapt the Knip configuration to the fork before gating CI
Source: .github/workflows/ci.yml:58
The newly enabled step already fails on this PR: Check job reports 21 unused files (including the active ModelPickerContent, ComposerPendingUserInputPanel, and PreviewPanel overrides and their custom dependencies) plus msw, then exits 1 before the remaining Check job steps execute. knip.jsonc only adds scripts/warm-dep-cache.ts as a web entry and does not account for the fork's build-time override resolution. Teach Knip about those entry points/resolution and resolve the dependency finding before enabling this gate; otherwise this sync leaves CI permanently red despite the reported focused checks passing.
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
There was a problem hiding this comment.
Thermo-nuclear review of the absorb, not the 779 upstream commits. Product decisions are fine: refuse resting, keep two DndContexts, defer the vite-plus rules. The merge still made the fork architecture worse in three places, so this does not meet the approval bar.
GitHub will not attach inline comments on a 14k-file diff; line references are below.
1. Structural — composerFooterLayout became a full-copy of dead resting code
apps/web/src/overrides/components/composerFooterLayout.ts lines 53–56 (the file is 36→234).
A 36-line breakpoint shadow became a verbatim copy of upstream’s resting module so ChatComposer typechecks, then a fake flag so none of it runs:
const forkAdoptsRestingComposerLayout: boolean = false;
if (!forkAdoptsRestingComposerLayout) {
return false;
}The comment is inverted: copying the predicate “so the next sync ports cleanly” is what creates the next port. overrides/README.md already says wrap (~upstream/) and own only the part you changed.
Keep the fork compact breakpoints and the two compact predicates. Re-export the resting helpers from ~upstream/components/composerFooterLayout. Own only shouldUseRestingComposerLayout() { return false }. That deletes ~180 lines and stops the next absorb from conflict-porting helpers this fork never executes.
2. Kill-switch is leaky — new consumer still asserts isResting === true
apps/web/src/components/chat/useComposerFocusState.test.tsx lines 65–73.
The absorb already knew tests resolve through the shadow. It fenced composerFooterLayout.test.ts and rewired MessagesTimeline.test.tsx to read isComposerScrollCollapsed. This new file still does:
await act(() => composer.setIsComposerScrollCollapsed(true));
expect(isResting).toBe(true); // shadowed predicate is falseIt is not on fork-composer-shell watch:. Treat it like the timeline test: assert the hook’s scroll-collapse flag and stop deriving isResting from a neutralized export. A kill-switch that has to be patched into every new caller will rot the same way on the next sync.
3. File-size — ModelPickerContent crossed 1k
apps/web/src/overrides/components/chat/ModelPickerContent.tsx lines 117–130 (927→1043; upstream original is 957).
shouldOfferModelPickerSetup, the antigravity default-model resolution, and the setup footer are real absorb work, but they landed in a file that already restacks the entire render. Crossing 1k needs a stronger reason than “upstream grew and we keep a full copy.”
Move the fork render into custom/ as a presentational view. Keep this shadow as the upstream-shaped state/logic port, and re-export pure helpers from ~upstream/components/chat/ModelPickerContent. The next logic-port should be a 3-way against ~957 lines, not another 80 on top of 1043.
Refuse the resting layout. Do not own upstream’s resting module to do it.
Sent by Cursor Automation: Thermo nuke 4.6
Upstream now rejects unused files, dependencies and exports (pingdotgg#9962, pingdotgg#10282). Declare the override shadow tree as knip entries, exempt the vendored design-mode engine like upstream's _generated/, drop the export keyword on fork symbols only used in their own module, delete the two stage-backdrop exports the fork never rendered, and remove the msw devDependency upstream already retired. The sidebar's stale-scope cleanup now waits for every project snapshot, which is what upstream's snapshot-readiness hook was for. The pre-ready platform test asserts upstream's Linux desktop entry name and display name; fenced to the fork's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
NoahHendrickson
left a comment
There was a problem hiding this comment.
Reviewed at 8e6ffbc40 (the knip + desktop-entry fix is in). Each item below was verified against the PR head in a local worktree; CI evidence is from run 34279376903 on the previous commit.
Blocking: one test failure is still hidden from CI
apps/web/src/components/chat/useComposerFocusState.test.tsx:67 (new upstream test) calls shouldUseRestingComposerLayout through the alias and hits the fork shadow's fenced return false. Reproduced locally on 8e6ffbc40:
× can collapse again on the next scroll after returning to the end
AssertionError: expected false to be true
The previous CI run never showed it because the Test job's vp run --parallel … test aborted on the desktop failure before reaching apps/web. Now that the desktop test is fixed, this one will surface. The sibling MessagesTimeline.test.tsx:396 fence (read isComposerScrollCollapsed in the probe instead of the neutralized predicate) is the pattern to mirror, plus a watch: entry under fork-composer-shell.
Behavior regressions
PreviewChromeRow.tsxrenders{leadingActions}twice. The PR body records the fork's fence as dropped because upstream now owns the slot, but the fenced render at line 122 survived next to upstream's at line 178. The design-mode layers toggle and the profile badge both appear twice in the preview header. The guard assertstoContainonly, so it stays green.ModelPickerContent.tsx:563gatesproviderSetupEntrieson upstream's!isSearching, but the fork'ssearchOpenstate (search toggled open, no query) already swaps the list to the whole catalogue. The previously selected tab's setup CTA renders beneath a cross-provider list. Gate on!searchVisible.
Performance: resting is refused, but its measurement pipeline stays live
The shadow neutralizes the predicate, yet three upstream pieces that exist only to feed it still run on every desktop thread. None of it changes what the user sees. Inline comments on ChatComposer.tsx:4037 and ChatView.tsx:3375 / :8437 have the specifics.
An alternative worth weighing: fence isComposerResting = false at its one consumer in ChatComposer instead of in the shadow. The shadow could then export * from "~upstream/components/composerFooterLayout" and keep only the fork's breakpoint constants and the two compact-footer predicates. That deletes ~150 hand-copied lines, the two Tier 4 test fences, and the blocking item above. The ~upstream/ escape hatch already exists in overrideResolver.ts and is documented in overrides/README.md; nothing uses it yet.
Fork hygiene
Sidebar.tsx:3-17:autoAnimate,PointerSensor,closestCenter,arrayMove, andverticalListSortingStrategyare now fork-only imports (upstream dropped them forSidebar.pointer), but they sit outside any fence. The manifest'ssidebar-v2-list-animationentry says the fence carries the import; it carries only the call site.Sidebar.tsx:596-607,:720-727,:773-779: new fork prose recording refusals of upstream pingdotgg#9731/pingdotgg#9750/pingdotgg#10378/pingdotgg#9759 is unfenced. A later sync resolving toward upstream loses the notes silently.- The new
fork-composer-shellinvariant ("shouldUseRestingComposerLayout always returns false") has no guard in__fork_guards__/. Its only assertion is a fence inside upstream'scomposerFooterLayout.test.ts, which iswatch:-listed, notverify:-listed. forkSubagentSpawnCard.test.tsdropped theisTerminalSubagentStatusassertion without replacing it with one oncoordinatorStatus: workflowGroup?.workflow.status, now the sole carrier of the coordinator-authoritative liveness rule.
Lower priority
.fork/lint-owned.mjsDEFERRED_RULESis an open set: 61 warnings pass today and 200 would pass tomorrow. A per-rule baseline count that fails on growth would keep the burn-down honest.new-task-flow-provider.tsx:928(mobile): the fence deliberately ignores upstream's newoptions.currentCheckoutBranchand the manifest documents why, but the parameter is now an unused-parameter lint warning. Rename to_optionsor reference it inside the fence.docs/user/thread-sidebar.md:34-53documents cross-section drag verbs and active-list reordering the fork's sidebar refuses.Sidebar.drag.ts,Sidebar.pointer.ts, and their tests have no importers in the fork. Fine to carry for the next sync, but a manifest note would stop someone wiring them by accident.ComposerShell.tsx:73:[[data-with-context]_&]:pb-2spells the same dependencyComposerSurface.tsx:44already spells asgroup-data-with-context/composer-surface:. Use the named group variant so a rename is one grep.
Checked and found fine: ProjectFavicon call sites, deriveAgentSpawnSummary port, ~/pendingUserInput value convention, sessionAtom read in SentPreviewResolution, canvasResolutions error-name check, release workflow gate on resolve_commit, linuxDesktopEntryName port, the loosened guard regexes, the new knip entries and vendor exemption, and the lucide shim's createLucideIcon (node attrs carry key).
Inline notes
GitHub refuses inline comments on this PR (the diff exceeds its file cap), so the per-line detail is here.
apps/web/src/components/chat/useComposerFocusState.test.tsx:67
Blocking. Resolves through the alias to the fork's composerFooterLayout shadow, where the predicate is fenced to return false. Fails on 8e6ffbc40 (expected false to be true); line 73 fails the same way. CI has not shown it yet because the parallel test run aborted on the desktop failure before apps/web ran.
Mirror MessagesTimeline.test.tsx:396: read state.isComposerScrollCollapsed for isResting inside a fork-composer-shell fence, and add this file to that entry's watch: list.
apps/web/src/components/preview/PreviewChromeRow.tsx:178
{leadingActions} is rendered here by upstream and again at line 122 inside the fork-design-mode fence. Both ForkPreviewLayersToggle and the profile badge from PreviewView.tsx:740 appear twice in the chrome row. The PR body says the fork fence was dropped for this slot; delete the one at 122 and consider tightening the guard from toContain to a single-occurrence assertion.
apps/web/src/overrides/components/chat/ModelPickerContent.tsx:563
Upstream hides the setup block whenever the list stops being tab-scoped, which for upstream is isSearching. The fork has a second unscoped state: searchOpen with no query renders the whole catalogue (line 492). With a provider tab selected that needs setup, clicking the search icon shows every provider's models with that tab's status message and "Open provider setup" still underneath. Gate on !searchVisible.
apps/web/src/components/chat/ChatComposer.tsx:4037
Performance. This document-level capture wheel listener runs on every wheel event in every desktop thread and, once the gesture crosses the threshold, calls setIsComposerScrollCollapsed(true). That re-renders the whole composer and registers the window focus listener at line 4018, but isComposerResting can never become true in the fork, so nothing visible changes. Short-circuit canTrackComposerScrollGesture on the fork constant (or fence isComposerResting = false at line 3904 and let the shadow re-export upstream, see the summary).
apps/web/src/components/ChatView.tsx:8437
Performance. MessagesTimeline measures content overflow in a useLayoutEffect on every rows.length change plus a rAF re-measure from the viewport ResizeObserver on every timeline resize, and each flip re-renders ChatView through setTimelineOverflows. The only consumer is timelineOverflows in shouldUseRestingComposerLayout, which returns before reading it. reportContentOverflow early-returns when the callback is absent, so dropping this prop is enough.
apps/web/src/components/ChatView.tsx:3375
Performance. hostsRestingComposerControls: routeKind === "server" keeps an invisible off-flow BranchToolbar mounted for every server thread on a non-git project purely so the composer can measure resting controls it never relocates. That toolbar runs useLabelsOverflow measurement in a layout effect on every render plus a ResizeObserver and document.fonts listener, and useRestingComposerControlsLayout attaches a second observer on the empty host. Pass false here in the fork; the mobile collapsed-strip path is unaffected because that uses showComposerContextStrip.
apps/web/src/overrides/components/composerFooterLayout.ts:53
This shadow now hand-copies ~150 lines of upstream's resting helpers (resolveRestingComposerControlsLayout, resolveComposerTimelineInset, resolveScrollToEndClearance, etc.) that are still live for the mobile strip and ChatView, so every upstream fix to pingdotgg#7855 machinery is a manual port. Consider fencing isComposerResting = false at its one consumer in ChatComposer.tsx:3904 instead, then export * from "~upstream/components/composerFooterLayout" here and keep only the fork's two breakpoint constants and the compact-footer predicates. That also removes the fences in composerFooterLayout.test.ts:111, MessagesTimeline.test.tsx:396, and the failing useComposerFocusState.test.tsx.
apps/web/src/components/Sidebar.tsx:3
Fork hygiene. autoAnimate (and below it PointerSensor, closestCenter, arrayMove, verticalListSortingStrategy) are fork-only imports now: upstream removed them in favour of Sidebar.pointer. They sit outside any fence, and the sidebar-v2-list-animation manifest entry says its fence carries the import when it only carries the call site. A future sync that takes upstream's import block drops these with no drift signal.
apps/web/src/components/Sidebar.tsx:601
Fork hygiene. This block (and the notes at 720-727 and 773-779) is new fork prose recording refusals of upstream code, but none of the three is fenced. .fork/AGENTS.md rule 4 wants inline edits to upstream files fenced so a later sync resolving toward upstream cannot lose them silently.
apps/web/src/__fork_guards__/forkSubagentSpawnCard.test.ts:36
The dropped isTerminalSubagentStatus assertion was the guard for the coordinator-authoritative liveness rule. That rule now lives only in AgentSpawnCtaRow.tsx:84 passing coordinatorStatus: workflowGroup?.workflow.status to the helper. Assert that argument here, or a sync that drops it makes live fall back to working > 0 while the suite stays green.
.fork/lint-owned.mjs:85
An open set never ratchets: new react(refs) or set-state-in-effect violations in custom/ or overrides/ pass CI indefinitely. A per-rule baseline (Map<rule, maxCount> from today's counts) that fails on growth and nags when it can shrink would keep the deferral honest.
apps/mobile/src/features/threads/new-task-flow-provider.tsx:928
Nit. The fork fence at line 994 deliberately ignores options.currentCheckoutBranch (the manifest documents the trade-off), which leaves this parameter unread: vp lint reports eslint(no-unused-vars): Parameter 'options' is declared but never used. Rename to _options or mention it inside the fence.
docs/user/thread-sidebar.md:34
This paragraph and the reorder paragraph below document upstream's cross-section drag verbs (Pin / Unpin / Settle / Un-settle / Wake) and active-list reordering. The fork's sidebar keeps only pinned-card reordering and refused the single-list drop verbs, so shipped docs now describe features a fork user cannot trigger.
apps/web/src/custom/ComposerShell.tsx:73
Nit. ComposerSurface.tsx:44 spells the same dependency as group-data-with-context/composer-surface:; using the named group variant here keeps one spelling to grep when upstream renames the attribute, and stays inside Tailwind's variant ordering.
Claude Fable 5.1 via Claude Code.
- Sidebar draft rows no longer accept file drops: upstream's row drop navigates to the server thread route and clears the queued files when it does not land there, so a drop on a draft opened it and delivered nothing. - The composerFooterLayout shadow re-exports upstream's module and owns only the fork breakpoints, the two compact predicates, and the resting verdict, instead of carrying a full copy of the resting helpers it never runs. - useComposerFocusState.test.tsx reads the hook's scroll-collapse flag, the way the timeline test already does, rather than the neutralized predicate. - theme.custom.css reformatted for the oxfmt in vite-plus 0.3.0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Review follow-ups landed in 8e6ffbc and 148f899: Codex review
Cursor review
|
Upstream's project icons (pingdotgg#9137) load lucide icons by name through `lucide-react/dynamic`. The fork's bare `lucide-react` alias rewrote that subpath into the shim file, which broke the web build and the two project-icon test suites. A dynamic shim now serves DynamicIcon, iconNames and IconName from the shim's own table, so a project icon draws the same Phosphor glyph the app uses for that name and the picker only offers names that render. The alias is listed ahead of the bare one in vite and mirrored in tsconfig; rocket and workflow joined the table so every popular pick resolves. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Preview chrome rendered its leadingActions slot twice (fork fence beside upstream's); the fork render is gone and the guard now asserts a single occurrence. - ModelPickerContent gates the provider-setup footer on searchVisible, so an open-but-empty search (already showing the whole catalogue) hides it. - The resting composer's measurement pipeline no longer runs for a layout the fork refuses: the shadow exports FORK_ADOPTS_RESTING_COMPOSER_LAYOUT, ChatComposer skips the document wheel listener, ChatView skips timeline overflow measurement and mounts the hidden measuring strip only on phone viewports where the collapsed composer uses it. - Sidebar's fork-only dnd-kit/auto-animate imports and its refusal notes are fenced under their manifest ids. - A composer-shell guard asserts the shadow refuses the resting predicate; the spawn-card guard asserts the coordinator-status argument. - DEFERRED_RULES carries per-rule baselines: growth fails, shrinkage nags. - The mobile outbox fence reads upstream's send-time branch argument after the live stream, clearing the unused-parameter warning. - User docs describe the fork's pinned-card and project-header drags instead of upstream's cross-section drop verbs. - ComposerShell uses the named group variant ComposerSurface already spells. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Second Codex review addressed in 148f899, 18d224f and 66b2d89:
|
Switching to Manual reused a leftover project order, new-agent drafts landed at the bottom, and a few composer/sidebar surfaces missed the design. Snapshot the on-screen list, restamp draft createdAt, stack settled copy, and let more wallpaper through the glass. Co-authored-by: Cursor <cursoragent@cursor.com>


Merge
pingdotgg/t3codemain through 12391bd (779 commits since b21d872) intocustom.35 conflicted files resolved with every fork customization ported on top of upstream's changes: the sidebar drag, favicon, and file-drop work under the V3 cards; the resting-composer machinery under the fork's ComposerShell; the release workflow gate onto the new
resolve_commitjob; the desktop Linux identity onto upstream's reverse-DNS desktop entry name. Override drift ported for ComposerPendingUserInputPanel, ModelPickerContent, and composerFooterLayout. The lucide shim gained the icons upstream started importing plus acreateLucideIconimplementation.Decisions recorded in
.fork/customizations.yamlDEFERRED_RULESin.fork/lint-owned.mjsand reported without failing the gate. Follow-up: burn them down.Verification
leadingActions, the mobile resolver import) or deleted the tests (stage-backdrop render assertions).build-desktop-artifactcross-arch Windows case on Apple Silicon.Claude Fable 5.1 via Claude Code.
🤖 Generated with Claude Code