Skip to content

sync: absorb upstream v0.0.31 (34 commits) - #38

Open
NoahHendrickson wants to merge 36 commits into
customfrom
claude/sync-2026-07-30
Open

sync: absorb upstream v0.0.31 (34 commits)#38
NoahHendrickson wants to merge 36 commits into
customfrom
claude/sync-2026-07-30

Conversation

@NoahHendrickson

Copy link
Copy Markdown
Owner

Absorbs upstream/main through 9dd425b22 — the v0.0.31 release plus early v0.0.32-nightly work: content-based thread search (pingdotgg#4959), sidebar title regeneration (pingdotgg#4810), pasted-screenshot compression (pingdotgg#4967), the ~300MB desktop size reduction (pingdotgg#4824), native resource diagnostics (pingdotgg#2679), the T3 Connect sign-in restore (pingdotgg#4809), the claude-opus-5 model bump (pingdotgg#4832), and ~20 web/mobile fixes.

Conflict resolutions (per manifest intents)

  • SidebarV2.tsx — kept the fork's three-line card body (sidebar-v2-card-rows) and ported upstream fix(web): restore sidebar v2 thread actions and terminal icon pingdotgg/t3code#4712's two card features into it: the terminal-status glyph now arrives as a new terminalSlot on the fork-owned SidebarV2ThreadCardMeta (repo line, after the branch cluster, mirroring upstream's reading order), and the title-regeneration sr-only status joins the fork's title line. Tooltip takes the union: fork's devServerLabel + real-last-error rows alongside upstream's new terminal-process row. Dropped upstream's re-added SquarePenIcon import (its usage site doesn't exist in the fork; the lint gate would flag it).
  • ChatComposer.tsx — kept the fork's composerPrimaryActionSlot shell (fork-composer-shell) and threaded upstream's new required sendDisabledReason prop through the slot and the mobile pending-actions call site.
  • build-desktop-artifact.ts — kept the fork's no3y-Code-* artifact name (fork-app-identity), took upstream's new electronLanguages/files exclusions (the 300MB shrink applies to fork builds too).
  • pnpm-lock.yaml — regenerated from upstream's side via vp i; the fork's Geist/Phosphor deps re-resolved.

Sync fallout fixed

  • Phosphor shim: mapped the seven lucide icons upstream's resource diagnostics introduced (Activity→Pulse, Battery→BatteryMedium, Cpu, Database, Gauge, HardDrive, MemoryStick→Memory).
  • ci-runners manifest paths follow upstream's stashImageCompressionimageCompression rename (the fenced timeout hunk survived the rename intact).

Verified

  • Fork guard suite: 24 files / 186 tests green.
  • Focused tests: SidebarV2ThreadCardMeta (18), imageCompression (30 incl. the fenced give-up path), build-desktop-artifact, desktop Clerk suite (12, incl. registrar suppression).
  • tsgo clean on apps/web and apps/desktop; .fork/lint-owned.mjs: 55 files, no warnings; vp fmt clean.
  • Clerk launch-resilience fences in main.ts/DesktopClerk.ts verified intact next to upstream's T3 Connect sign-in restore.

Also enabled Issues on this repo — the mirror's drift job had been failing its [fork-drift] issue POST with HTTP 410 (Issues disabled, GitHub's fork default).

Model: Claude Fable 5 · Harness: Claude Code

🤖 Generated with Claude Code

PixPMusic and others added 30 commits July 29, 2026 10:59
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…otgg#4853)

Co-authored-by: Simon Doba <simon.doba@orbit.de>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…it (pingdotgg#4949)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…oring (pingdotgg#4967)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
t3dotgg and others added 6 commits July 30, 2026 04:45
…g#4807)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	apps/web/src/components/SidebarV2.tsx
#	apps/web/src/components/chat/ChatComposer.tsx
#	pnpm-lock.yaml
#	scripts/build-desktop-artifact.ts
Upstream pingdotgg#2679 introduced seven lucide icons the Phosphor shim had no
mapping for, and the ci-runners timeout carrier moved with upstream's
stashImageCompression -> imageCompression rename. Guard suite is green
again on both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 30, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL labels Jul 30, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thermo-nuclear code quality review — REQUEST CHANGES

This sync lands several structural regressions on custom: three brand-new files already past the 1k-line bar, a resource-telemetry stack split across parallel mutable state machines, and more feature branches bolted into already oversized reactor/settings/sidebar modules. The fork conflict resolutions mostly preserve intent — that is not enough to clear the maintainability bar for what lands here.

Ordered findings:

  1. Structural regression — resource telemetry arrives as a sprawling cross-layer stack (native binary + NativeTelemetryClient + aggregator + desktop publisher + background policy + diagnostics UI) instead of clear ownership boundaries.
  2. Missed code-judo — thread title regeneration should be a dedicated reactor path, not another mode inside ProviderCommandReactor.
  3. Spaghetti growthSettingsPanels and SidebarV2 keep absorbing feature UI/action branches despite already being megafiles.
  4. Boundary / type cleanliness — background subscription scopes rely on module globals and casts instead of a typed adapter.
  5. 1k-line blockersResourceTelemetryDiagnostics.tsx (1268), native/resource-monitor/src/main.rs (1160), and NativeTelemetryClient.ts (1024) all land above 1k without a decomposition story.
  6. Fork seamterminalSlot is inserted next to fenced sidebar customizations without the same fence/manifest discipline.

Not approving. Decompose the 1k+ newcomers and pull title-regeneration / background-activity UI out of the megafiles before this merges.

Open in Web View Automation 

Sent by Cursor Automation: Thermo-nuclear PR review

const crypto = yield* Crypto.Crypto;
const config = yield* ServerConfig;
const initializedAt = yield* DateTime.now;
const state = yield* Ref.make(initialState);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this lands as a 1k-line service factory that owns spawn lifecycle, protocol decoding, request registries, health publication, collection control, retry state, and supervision. that is too many state machines in one closure. split the protocol client from the sidecar supervisor and the pending-request registry before this lands.

root_pid: u32,
sample_interval_ms: u64,
#[serde(default)]
external_processes: Vec<ExternalProcess>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this new binary is already 1160 lines and mixes protocol definitions, collection, history retention, command dispatch, main loop, and tests. can we decompose this into protocol/collector/history/runtime modules first? a native sidecar is exactly where we need boring ownership boundaries, not a monolith.

);
}

export function ResourceTelemetryDiagnostics() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pushes a brand-new settings page to 1268 lines. formatters, history charting, process tables, source health cards, side effects, retry/kill actions, and confirmation UI should not all live in one component file. decompose this before landing; the 1k-line bar is not met.

const start: ProviderCommandReactorShape["start"] = Effect.fn("start")(function* () {
const processEvent = Effect.fn("processEvent")(function* (event: OrchestrationEvent) {
if (
(event.type === "thread.meta-updated" && event.payload.regenerateTitle === true) ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this adds another special-case event predicate into an already broad provider command reactor. title regeneration is metadata/text-generation work, not provider session command orchestration. can we move this behind a dedicated title-regeneration reactor so this switch/gate does not keep absorbing unrelated features?

@@ -524,6 +681,272 @@ export function useSettingsRestore(onRestored?: () => void) {
};
}

function BackgroundActivityAdvancedDialog({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this settings file was already huge, and the advanced background activity dialog adds another large feature island directly into it. extract the dialog and its repeated interval-row/update logic into a focused module; this works, but it makes the settings megafile more spaghetti.

if (observation.method !== WS_METHODS.subscribeVcsStatus) {
return null;
}
const input = observation.input as { readonly cwd?: unknown };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this boundary need to cast subscription input to { cwd?: unknown }? scope retention is now global mutable state keyed off loosely typed observations. make subscription-to-background-scope a typed adapter/helper instead of normalizing cast-heavy shape checks in the reporter.

@@ -1161,6 +1218,7 @@ const SidebarV2Row = memo(function SidebarV2Row(props: {
/* fork:begin sidebar-v2-dev-server-pulse — see .fork/customizations.yaml#sidebar-v2-dev-server-pulse */
devServerPort={devServerPort}
/* fork:end sidebar-v2-dev-server-pulse */
terminalSlot={terminalStatusIcon}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this prop is inserted into the fork-owned card meta seam right beside fenced fork hunks, but it is unfenced. for this fork, inline edits in upstream files need the same fence/manifest discipline or the next sync cannot distinguish intentional custom glue from upstream drift.

Copy link
Copy Markdown
Owner Author

Review: sync #38 (upstream v0.0.31, 315 files / +26,935 −4,680)

Reviewed the fork-owned surface rather than the upstream commits, since those were reviewed upstream. Everything below was checked against the tree, not the PR description.

Verified independently — these hold up

  • No customization was silently dropped. Fence inventory at cf8c815b vs 9ed90310 is identical for all 23 ids, count-for-count (fork-app-identity 33, fork-composer-shell 25, sidebar-v2-project-grouping 12, …). That's the strongest signal a sync this size can give.
  • build-desktop-artifact.ts is a clean resolution. git diff 9dd425b2 9ed90310 -- scripts/build-desktop-artifact.ts is only the fork-app-identity fences — the electronLanguages, files, mac/linux asarUnpack drop, and the resource-monitor staging are upstream's verbatim. The fork took the 300MB shrink without diverging in it.
  • The ci-runners carrier survived its rename intact. Fence is at imageCompression.test.ts:125 with the give-up-path prose unchanged, and zero stashImageCompression references remain anywhere in the tree.
  • Icon completeness is genuinely guarded, not trusted. phosphorIcons.test.ts scans every lucide-react binding across apps/web/src + packages/ (plus default/namespace/dynamic import forms) and asserts a shim export exists — so the seven new mappings are enforced rather than eyeballed.
  • No double render of the terminal glyph. terminalStatusIcon at SidebarV2.tsx:924 is inside the slim variant's early return; :1221 is the card variant. Distinct paths.

Findings

1. terminalSlot is a new fork customization with neither a manifest entry nor a guard. .fork/AGENTS.md — "Every customization requires an entry in .fork/customizations.yamland a guard test in apps/web/src/__fork_guards__/ asserting the outcome." The only manifest change in this PR is the imageCompression rename. terminalSlot appears in exactly two files repo-wide, both source (SidebarV2.tsx:1221, SidebarV2ThreadCardMeta.tsx:62,212); none of sidebarV2CardRows.test.ts's twelve assertions touch it.

2. …and it's declared optional, while its sibling slot is required. prSlot: ReactNode is deliberately required so every call site must pass an explicit null; terminalSlot?: ReactNode is not. Together with #1 that's a two-lock failure with both locks open: the next sync that rewrites the <SidebarV2ThreadCardMeta …> block can drop terminalSlot={terminalStatusIcon} and both tsgo and the full guard suite stay green. The glyph then vanishes from the card variant while the slim shelf keeps it — which reads as a design decision, not a regression. Make it readonly terminalSlot: ReactNode and add one assertion to sidebarV2CardRows.test.ts.

3. The call site at SidebarV2.tsx:1221 is unfenced, alone among its siblings. In the same JSX block, projectTitleHidden, hasWorktree, and devServerPort each carry fork:begin/fork:end. terminalSlot={terminalStatusIcon} carries nothing. The fence is the marker the next conflict resolution reads — an unfenced fork line inside a block upstream rewrites is precisely the line that gets lost. It wants a fence under sidebar-v2-card-rows and a sentence in that entry's intent, which is also the fix for #1.

4. .fork/customizations.yaml:1252 now cites a file this very sync deleted. The t3-connect-official-config intent reads "The values are public identifiers, not secrets (docs/cloud/t3-connect-clerk.md)" — upstream pingdotgg#4807 (9dd425b2) deleted that path in this diff; the content now lives at docs/internals/t3-connect.md. That citation is the evidence backing a security-relevant claim about four baked Clerk keys, so it's the one reference in the manifest that most needs to resolve. customizationsManifest.test.ts validates files:/watch: paths but not paths cited in intent: prose, which is why nothing caught it. Worth extending that guard to scan intent prose for docs/**/*.md — upstream just demonstrated it relocates docs wholesale.

5. ActivityIcon is the only bold glyph among the seven new mappings. icon("activity", PhPulse, "bold") vs duotone for the other six. They render adjacent at size-3.5 in one stat list — ResourceTelemetryDiagnostics.tsx:990–1031 is Cpu, MemoryStick, Activity, HardDrive, Database, Gauge in sequence. Bold is legitimate in the shim (41 of 153 entries), but here it makes exactly one row of a homogeneous list read heavier. Either make it duotone or say in the comment why it isn't. While there: BatteryIconBatteryMedium is the section header for "Host state" (:1084), a section that reports the real power source in the rows beneath it — a half-full battery glyph reads as a charge level rather than a category label.

6. Nit — the new imports break the file's alphabetical order. Memory lands before MagnifyingGlass, Pulse before Plus. The rest of the list is alphabetical.

7. CI is not green yet. On 9ed9031, combined status is pending: Test and Mobile Native Static Analysis are still in_progress. Check and Release Smoke passed. The verification in the description is a local run — worth waiting for Test before merging, given the size.

Non-issues, noted so they aren't re-litigated

Base is custom, correctly. Dropping upstream's re-added SquarePenIcon import from SidebarV2.tsx is right — the shim still exports it and the three real consumers (CommandPalette, Sidebar, MessagesTimeline) are unaffected. sendDisabledReason is threaded end to end: ChatView:5961ChatComposer:425/452/542/2812ComposerPrimaryActions:25/79/232, with the shell passing it through rather than recomputing a disabled predicate of its own. And upstream has moved past the absorbed tip (origin/main is at 1877237c) — expected, just means a short next sync.

Verdict: the mechanical resolution is good and the risky file (build-desktop-artifact.ts) was resolved exactly right. #1#3 are one commit together and should land before merge — they're the difference between a customization that's protected and one that merely happens to work today. #4 is a one-line fix. #5#6 are polish.


Generated by Claude Code

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

Labels

size:XXL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.