Skip to content

chore(sync): absorb upstream main through 018d7f277 (v0.0.34–v0.0.35 cycles) - #100

Merged
NoahHendrickson merged 204 commits into
customfrom
claude/sync-2026-08-28
Aug 28, 2026
Merged

chore(sync): absorb upstream main through 018d7f277 (v0.0.34–v0.0.35 cycles)#100
NoahHendrickson merged 204 commits into
customfrom
claude/sync-2026-08-28

Conversation

@NoahHendrickson

Copy link
Copy Markdown
Owner

Absorbs 202 upstream commits (last sync point: 82b8a93, Aug 18) through 018d7f2 (Aug 28), spanning the v0.0.34 and v0.0.35 release cycles. Notable arrivals: attached composer state drawers + tasks drawer (pingdotgg#7150, pingdotgg#7740), the skills redesign in the $ and / menus (pingdotgg#8009, pingdotgg#7737), image uploads before send + HEIC (pingdotgg#8048, pingdotgg#8161), PDF/ZIP uploads to 50MB (pingdotgg#8235), Claude context compaction (pingdotgg#8144), the appearance contrast control (pingdotgg#7906), unified workspace navigation (pingdotgg#7153) + project-settings back button (pingdotgg#8168), PR↔thread linking (pingdotgg#8160) + PR detail refresh (pingdotgg#7148), settle/restore and pin keyboard shortcuts (pingdotgg#8089), pinned threads that settle now landing in the settled section (pingdotgg#7969), the macOS launchd background service (pingdotgg#6286), macOS preview DMGs from a PR label (pingdotgg#8182), sharded server tests + a Rust CI split (pingdotgg#7286), OpenCode skill discovery (pingdotgg#3154), tab mute (pingdotgg#7252), terminal close confirm (pingdotgg#7592), Codex 0.150 multi-agent/plan support, and the usage insights redesign (pingdotgg#7147).

Resolution highlights

  • 12 conflicted files (27 last time). Fences re-seated per the manifest; upstream structure wins wherever the fork carries only an overlay (PanelLayoutControls trigger shape, composerProviderState planModeEnabled, index.css contrast-aware sidebar border).
  • ChatComposer: upstream's render body adopted wholesale — top drawer (approval / questions / plan), tasks drawer, task/stash shoulder tabs, attachment upload chrome, DISCONNECTED_COMPOSER_PLACEHOLDER — with the fork shell re-seated on top: ComposerShell vessel around everything, box/surface/drag-over markers, the px-2 py-2.5 prompt row, ComposerPromptRow carrying the primary action (upstream's inline task/stash badges now ride that cluster), "Ask anything", design-change pills, and no bottom toolbar. The context-usage meter takes upstream's compact action in the readout slot; ComposerFooterPrimaryActions drops the meter props it no longer renders. NUL-byte stash keys intact.
  • Sidebar: fork card system kept. Ported: per-environment provider entries (fix(web): resolve sidebar provider icons from the thread's own environment pingdotgg/t3code#7292), linked-PR-aware changeRequestAutoSettles / snapshot inputs (feat: link pull requests to threads pingdotgg/t3code#8160), the jump-hint visibility hook + terminal-focus context (fix(web): delay thread shortcut hints by 200 ms pingdotgg/t3code#8172, fix(web): hide thread jump hints while the terminal is focused pingdotgg/t3code#7277), the pinned block in its own li > ul so a drop reorders inside one list (fix(web): prevent pinned threads reshuffling after drop pingdotgg/t3code#7676; uses the fork list gap), the un-settle tooltip (fix(web): give sidebar un-settle button a tooltip pingdotgg/t3code#7796) on the fork's action class, releaseComposerDraftUploads on draft discard (feat(web): upload image attachments before sending pingdotgg/t3code#8048), isPinned from pinnedAt (fix(clients): move settled pinned threads into the settled section pingdotgg/t3code#7969). Upstream's hoisted pin marker (an unpin button in every row) becomes a passive "Pinned" glyph on slim rows only — unpin stays a card hover action + context menu per the manifest.
  • SidebarChrome: upstream's SidebarUtilityMenu split (new consumer: SettingsSidebarNav) adopted; T3Wordmark stays gone.
  • MessagesTimeline: working row adopts upstream's bordered text-sm restyle + thinking row, with the pixel rain re-seated ahead of the label; design-mode extraction and upstream's isImageAttachment filter both kept.
  • ChatView: resumeCompaction banner adopted; liveness stays on the context-strip pill (upstream's backgroundLiveness banner not rendered); chat-composer-glass-shell-attached and the fork's with-context class both applied; stray file-drop guard kept beside upstream's externalComposerDrawerAttached.
  • CI: three new upstream jobs (test_server shards, rust, mobile_native_changes) fenced to hosted runners; the macOS lint keeps upstream's change gate. release.yml: relay_public_config and build_wsl_node_pty now run alongside preflight upstream, so they carry the repository gate themselves (guard expectation widened).
  • Desktop artifact script: fork icon assets kept; upstream's isDesktopPreviewVersion adopted (it has a caller); resolveDesktopWebAssetBrand stays deleted.
  • Icons: six new lucide bindings added to the Phosphor shim — Volume2/VolumeOff → SpeakerHigh/SpeakerSlash, Square, PackageIcon → Package, UserRoundIcon → UserCircle, PackagePlusIcon → DownloadSimple (Phosphor has no package-plus).
  • Glass vibrancy: the two new contrast-derived sidebar tokens (--contrast-sidebar-foreground, --contrast-sidebar-muted-foreground) pinned opaque under the glass panel, as the guard requires.
  • threadSidebarWidth.test.ts: upstream pruned it (test: prune trivial error and layout tests pingdotgg/t3code#8400). The fork's two fenced assertions moved into guards (narrowWorkspaceLayout already covered the width math; the brand-truncation check now lives in forkAppIdentity); manifest watch entries removed. The shadow follows upstream's un-export of THREAD_SIDEBAR_DEFAULT_WIDTH.
  • Guards retuned to the new changeRequestAutoSettles signature and to WorkspacePageHeader owning the drag strip (feat(web): unify workspace navigation pingdotgg/t3code#7153). All 357 pass.

Judgment calls to review

  1. Shoulder tabs vs. context strip (visual): upstream's task/stash shoulder tabs hang off the top edge of the main surface's relative wrapper, which now sits inside the fork vessel directly under the fork's context chips. Upstream's pt-7 on the form was sized for its own layout. Worth a look with a non-empty stash queue or a tasks drawer.
  2. Questions strip (fork-pending-user-input): the panel now renders in upstream's top drawer, which is already transparent, so the fork's surface-scoped strip-clearing rule is dormant. The data-fork-pending-user-input-strip hook marks the drawer wrappers so the fence and guard stay; intent updated to say so.
  3. ComposerPendingUserInputPanel shadow drift (declined): upstream restyled its own card for the drawer (feat(web): attach composer state drawers pingdotgg/t3code#7150). The fork owns the Questions card design, so nothing was ported — check it reads well inside the drawer.
  4. Contrast slider under glass: pinning the two contrast tokens opaque means the new appearance-contrast control does not move sidebar text while Cool Darker vibrancy is on; it still applies everywhere else.
  5. desktop-macos-preview.yml (new upstream): PR-label triggered, Blacksmith runners, not covered by the ci-runners guard. Left untouched — it only fires if someone labels a PR preview:mac, where it would queue forever here. Gate it or fence it if that ever matters.
  6. Mobile queued-task branch: unchanged from last sync's note (fork resolver kept).
  7. scripts/build-desktop-artifact.test.ts cross-arch Windows case fails on Apple Silicon with pristine upstream's script + test too (verified), so it is host-dependent and pre-existing; CI on Linux should be green.

Verification

  • vp test run src/__fork_guards__ — 42 files, 357 tests green
  • Web typecheck green; desktop typecheck green (Effect suggestions only); .fork/lint-owned.mjs clean (187 files); vp fmt --check clean on every hand-edited file
  • src/custom + fork/ resolver + ComposerPendingUserInputPanel + sidebar / PanelLayoutControls / composerProviderState tests — 23 files, 190 tests green
  • Mobile branch-resolver tests — 5 green
  • scripts/build-desktop-artifact.test.ts — 51/52 (the 1 failure is item 7 above)
  • No browser pass was done; the composer drawer layout and the pinned block are the two surfaces most worth eyeballing.

Merge and CI takes the full suite from here.

Resolved by Claude Fable 5 via Claude Code.

🤖 Generated with Claude Code

aoright and others added 30 commits August 18, 2026 13:38
…in GitHubPullRequestCli (pingdotgg#7385)

Signed-off-by: aoright <102943475+aoright@users.noreply.github.com>
Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com>
Co-authored-by: maria <254055478+maria-rcks@users.noreply.github.com>
Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com>
Co-authored-by: maria <254055478+maria-rcks@users.noreply.github.com>
…t full speed (pingdotgg#7460)

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

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…tgg#6286)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: GPT-5.6 <noreply@openai.com>
colonelpanic8 and others added 19 commits August 27, 2026 14:40
Co-authored-by: Yash Singh <saiansh2525@gmail.com>
Co-authored-by: Julius Marminge <julius@mac.lan>
Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com>
Recent scheduled nightlies have been delayed or skipped.

Move the nightly cron from minute 7 to minute 38. Keep the existing three-hour interval. This tests a different point in each three-hour window without claiming it will fix GitHub schedule delivery.

Authored by GPT-5.6 Sol with the Codex harness.
Add a mod+shift+p shortcut that pins or unpins the active thread. Ignore the shortcut during terminal focus and when the server does not support thread pinning.
Co-authored-by: codex <codex@users.noreply.github.com>
Absorbs 202 upstream commits (82b8a93..018d7f2, v0.0.34–v0.0.35
cycles). 12 conflicted files resolved per .fork/customizations.yaml;
fork guards, web/desktop typecheck, fork lint green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 12.6 KiB 13.3 KiB +658 B (+5.1%) 15.1 KiB
Codex Thread snapshot wire 6.3 KiB 6.9 KiB +620 B (+9.6%) 7.3 KiB
Codex Live turn WebSocket wire 6.3 KiB 6.4 KiB +38 B (+0.6%) 7.8 KiB
Codex Live turn WebSocket decoded 51.8 KiB 55.5 KiB +3.7 KiB (+7.1%) 66.4 KiB
Codex Live turn messages 16 9 −7 (−43.8%) 21
Claude Total thread wire 12.6 KiB 13.3 KiB +760 B (+5.9%) 15.1 KiB
Claude Thread snapshot wire 6.3 KiB 6.9 KiB +642 B (+10.0%) 7.3 KiB
Claude Live turn WebSocket wire 6.3 KiB 6.4 KiB +118 B (+1.8%) 7.8 KiB
Claude Live turn WebSocket decoded 52.7 KiB 56.4 KiB +3.7 KiB (+7.1%) 66.4 KiB
Claude Live turn messages 16 10 −6 (−37.5%) 21

Baseline: cae8062 · PR result: 2dd06b7 · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@NoahHendrickson NoahHendrickson left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Review complete. I found two actionable merge-resolution issues that should be addressed before landing.

onDragOverCapture={composerMentionDragHandlers.onDragOver}
onDragLeaveCapture={onComposerMentionDragLeaveCapture}
onDropCapture={composerMentionDragHandlers.onDrop}
className={cn("mx-auto w-full min-w-0 max-w-3xl", hasShoulderTab && "pt-7")}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[P2] Reserve shoulder-tab clearance below the context strip

When contextStrip is present, ComposerShell renders it before the vessel, while the task/stash tabs are absolutely positioned at -top-7 inside the later main-surface wrapper. This pt-7 pads the form before both elements, so it does not create any space between them: on a git thread with a stash or visible tasks, the 32px tab extends into the context row and overlaps the branch/environment/liveness controls. Put the shoulder clearance between the context row and the main-surface wrapper (or otherwise account for it inside the vessel) instead.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Confirmed, and this is the one finding on the PR that is a real merge-introduced regression — my review below missed it.

Verified structure at HEAD:

  • pt-7 on the form (line 3150) is upstream's (HEAD^2:2972); the fork parent had no shoulder tabs at all.
  • ComposerShell renders the context row ([data-fork-composer-context-row], pb-2) before the vessel, and the tabs are absolute -top-7 inside the relative main-surface wrapper (ComposerTasksBadge.tsx:119, ComposerStashBadge.tsx:69, rendered at 3302/3312). So the 28px rise lands on the 8px context row, not on the form's padding.
  • showShoulderTabs (2556) does not consider contextStrip, and the strip is present on every git thread, so the trigger is just: git thread + stash entry or visible task steps.
  • No fork CSS repositions .chat-composer-shoulder-tab (no hits in theme.custom.css / ComposerShell.css).

Smallest fix that keeps upstream's pt-7 untouched: have ComposerShell accept a shoulderClearance (or reuse hasShoulderTab) and put the pt-7 on the vessel wrapper instead of the form when context is present — or drop the form padding via the fork fence and always pad the vessel. Either way the clearance needs to sit between the context row and the surface.

— Claude Fable 5 via Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7. ComposerShell now takes shoulderClearance; with a context row it pads that row pb-9 (8px + the 28px tab rise) so the tabs sit between the chips and the surface. The form's upstream pt-7 is fenced to apply only when there is no context row, so the clearance is never doubled.

github.event.pull_request.head.repo.full_name == github.repository &&
contains(github.event.pull_request.labels.*.name, 'preview:mac') &&
(github.event.action != 'labeled' || github.event.label.name == 'preview:mac')
runs-on: blacksmith-12vcpu-macos-26

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[P2] Move the preview workflow off unavailable Blacksmith runners

This fork does not have the Blacksmith GitHub app; .fork/customizations.yaml#ci-runners explicitly records that blacksmith-* jobs queue forever here. Applying preview:mac therefore leaves this build permanently queued, and the publish/cleanup jobs repeat the same unavailable runner at lines 128 and 292, making the imported preview workflow unusable. Convert all three jobs to GitHub-hosted runners and include this workflow in the ci-runners manifest/guard coverage.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Confirmed. desktop-macos-preview.yml is new in this PR and uses blacksmith-12vcpu-macos-26 (32) and blacksmith-8vcpu-ubuntu-2404 (128, 292). Its if: gates only on same-repo head + the preview:mac label — there is no github.repository == 'pingdotgg/t3code' gate like release.yml carries — so labeling a fork PR queues the build forever. The ci-runners guard (ciRunners.test.ts:22) only scans ci.yml and mobile-fingerprint-check.yml, so nothing flags it.

For completeness, the other Blacksmith hits in this sync:

  • publish-aur.yml (also new) is Blacksmith too, but it is only reachable via release.yml's publish_aur job (needs: [preflight, release], both upstream-gated) or a manual workflow_dispatch — effectively covered by release-upstream-only.
  • web-preview.yml:40 was already on custom before this PR, not a sync regression.

So the fix here is either (a) convert the three jobs to GitHub-hosted runners and add the file to ci-runners' watch: + the guard's it.each list, or (b) add the upstream-repository gate so it never runs on the fork. (a) matches how ci.yml was handled if the fork wants mac preview builds; (b) is smaller.

— Claude Fable 5 via Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7, taking option (b): build and cleanup now carry github.repository == 'pingdotgg/t3code' under release-upstream-only fences (publish only runs after build, so it skips with it). releaseUpstreamOnly.test.ts gained a case that parses this workflow and asserts exactly those two jobs are gated, and the manifest watches the file. Kept the Blacksmith labels rather than converting — the fork's own path is fork-release.yml, and a preview build that publishes to a rolling release is upstream's release identity.

@NoahHendrickson NoahHendrickson left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Review: upstream sync through 018d7f2

Verified the resolution mechanically and semantically before commenting:

  • Fork guard suite (42 files / 357 tests), apps/web typecheck, .fork/lint-owned.mjs, and the conflict-resolved tests (TraitsPicker, PreviewView, imageCompression, DesktopAppIdentity) all pass. build-desktop-artifact.test.ts is 51/52 with the one failure being the known host-dependent Windows cross-arch case on Apple Silicon (fails on pristine upstream too).
  • Every fork:begin/fork:end pair is balanced, every fence id exists in the manifest, no manifest path is missing, and per-id fence counts only grew (ci-runners 4→7, release-upstream-only 2→4 for upstream's new root jobs).
  • Every non-fenced hunk in the conflicted files diffs clean against upstream with whitespace ignored. Upstream's additions that were intentionally relocated (background-liveness banner → composerContextStrip.tsx, bottom toolbar → ComposerShell control row, favicon dropped per fork-workspace-header) all match the manifest's recorded intent. Upload wiring from pingdotgg#8048/pingdotgg#8235, the thread.pin keybinding, and the project-settings back button are all present.
  • Shadows: threadSidebarWidth.ts correctly follows pingdotgg#8400 (default made module-private; no consumer imported it). ComposerPendingUserInputPanel upstream changes were class-only; the override's prop surface is identical. The lucide→Phosphor shim covers all 204 icon names imported anywhere in apps/web/src.
  • Desktop bridge: fork:set-sidebar-vibrancy channel agrees across preload, main, and web; contracts' DesktopBridge carries no fork members.

No correctness regressions found. The inline comments below are all low-severity: one loosened guard, one stale manifest watch: entry, and a handful of duplication / no-op-fence cleanups introduced by the resolution. None block merging; the guard tightening (sidebarV2CardRows.test.ts) and the manifest watch: entry are the two I'd fix before the next sync since they affect what future syncs catch.

Checked and deliberately not flagged (pre-existing on custom, not introduced here): the double drag-over mechanism in ChatComposer.tsx/theme.custom.css:515, the unfenced labelSeparator test in TraitsPicker.test.ts, brand configuration living in scripts/build-desktop-artifact.ts rather than scripts/lib/brand-assets.ts, and the collapsed-mobile inline badges being mounted inside the hidden prompt wrapper (upstream's toolbar has the same shape). Also a nit not worth its own thread: PackagePlusIcon wraps PhDownloadSimple duotone while DownloadIcon already exports the same glyph bold — export const PackagePlusIcon = DownloadIcon; would keep one weight per pictogram.

Reviewed by Claude Fable 5 via Claude Code (/code-review high).

@@ -42,7 +42,7 @@ const upstreamCss = readSibling("../index.css");

describe("fork guard: sidebar-v2-card-rows", () => {
it("keeps upstream wake and monitoring semantics in the customized row", () => {
expect(sidebarV2).toContain("!changeRequestAutoSettles(prState, props.autoSettleOnMerge)");
expect(sidebarV2).toContain("!changeRequestAutoSettles(pr, {");

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Guard loosened. The old assertions (prState !== "merged" / prState !== "closed") proved the fork's card row actually applied the settle check. The replacement only proves a call to changeRequestAutoSettles(pr, { exists — it no longer proves autoSettleOnMerge: props.autoSettleOnMerge (or the new thread activity anchor from pingdotgg#7454) is forwarded.

Today Sidebar.tsx:701-704 does forward both, so nothing is broken. But changeRequestAutoSettles treats an omitted autoSettleOnMerge as true, so a future resolution that lands !changeRequestAutoSettles(pr, { thread }) would auto-settle every merged-PR thread out of the card rows with the user's setting off, and this guard would stay green.

Suggest asserting the options too:

expect(sidebarV2).toContain("!changeRequestAutoSettles(pr, {");
expect(sidebarV2).toContain("autoSettleOnMerge: props.autoSettleOnMerge,");

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — the guard now also asserts autoSettleOnMerge: props.autoSettleOnMerge, is forwarded.

Comment thread .fork/customizations.yaml
@@ -1676,12 +1674,6 @@
- apps/web/src/components/ChatView.tsx

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Stale watch: after porting the guard to pingdotgg#7153. The comment still calls ChatView.tsx "one of the two title-bar drag owners", but the drag strip now lives in apps/web/src/components/WorkspacePageHeader.tsx (electron && "drag-region"), which the updated guard reads (narrowWorkspaceLayout.test.ts:307-310) but the manifest neither watches nor mentions — WorkspacePageHeader appears nowhere in this file.

detect-drift.mjs only reviews watch: paths, so an upstream change to how the header applies drag-region (moving it outside [data-slot="sidebar-inset"], or switching to an inline -webkit-app-region style) would not be flagged before a sync lands; the fork's release rule would silently stop matching and the floating sidebar's press would be swallowed by the window drag strip again on Electron.

Add apps/web/src/components/WorkspacePageHeader.tsx here and reword the ChatView.tsx comment to what it still owns (the measured column).

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7WorkspacePageHeader.tsx added to narrow-workspace-layout's watch: with the drag-strip note, and the ChatView.tsx comment now claims only the measured column.

row variant. Unpin stays a card hover action (below) and a context-menu
item here, so the slim shelves draw the passive marker only: a settled or
snoozed thread still shows it is pinned without growing a 12px target. */
const pinIndicator = props.isPinned ? (

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Hoisted here, but the card row still carries its own copy. This pinIndicator is rendered at the slim site (line 1196) only; the card branch inside the sidebar-v2-card-rows fence (~line 1395) inlines a byte-identical <PinIcon aria-label="Pinned" role="img" className="size-3 shrink-0 text-muted-foreground/65" />. Upstream hoisted pinIndicator precisely so both variants share it (HEAD^2 renders it at both 1272 and 1435); aria-label="Pinned" went from one occurrence in each parent to two in the merge.

Rendering {pinIndicator} at the card site drops one fenced hunk (95→94) and means the next upstream tweak to the marker can't silently miss the card. The card-rows guard regex (sidebarV2CardRows.test.ts:124) would need to point at {pinIndicator} near {title} instead of the inline <PinIcon.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — the card renders {pinIndicator} too, the fence comment says both variants share it, and the card-rows guard matches {pinIndicator} after the title-line comment instead of the inline <PinIcon.


{isComposerCollapsedMobile && activePendingApproval ? (
</div>
) : !isComposerCollapsedMobile && pendingUserInputs.length > 0 ? (
<div

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

A <div> whose only job is to keep a fence alive. In the fork parent this attribute rode upstream's styled vessel wrapper (rounded-t-[19px] border-b … bg-muted/20). Upstream pingdotgg#7150 removed that wrapper and renders ComposerPendingUserInputPanel directly in the top drawer — which is a sibling rendered before [data-fork-composer-surface] (line 3334), so the rule this attribute feeds (theme.custom.css:1807, [data-fork-composer-surface] [data-fork-pending-user-input-strip]) can no longer match. The manifest entry itself calls the rule "dormant" (customizations.yaml:2771).

The resolution re-created a bare wrapper element to hang the attribute on. Net cost: an extra DOM node on every pending-questions render, a fenced hunk in the hottest file in the repo, a dead CSS rule, a paragraph explaining the deadness, and forkPendingUserInput.test.ts:64-81 asserting all of it.

Simpler: render <ComposerPendingUserInputPanel> directly as upstream does, drop the attribute from both wrappers (here and the collapsed-controls div at ~3224), delete the theme.custom.css:1807 rule and its guard case, remove ChatComposer.tsx from that entry's watch:, and cut the "dormant" paragraph. If a panel ever renders inside the surface again, re-add it then.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — both wrappers gone (the panel renders directly, as upstream does), the theme.custom.css rule and its guard case deleted, ChatComposer.tsx dropped from that entry's watch:, and the intent paragraph now just records that upstream's drawer is already transparent.

/>
{/* fork:end fork-composer-shell */}

{/* fork:begin fork-composer-shell — see .fork/customizations.yaml#fork-composer-shell

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Comment-only fence. This fork:begin/fork:end pair wraps nothing but prose, and the same sentence already lives in the manifest (#fork-composer-shell: "Upstream's bottom toolbar is not rendered at all — the control row owns those controls"). It wasn't in the fork parent; the merge added it where upstream's toolbar used to sit.

An empty fence is one more hunk that conflicts on every sync for zero code. Suggest deleting the four lines, or folding one sentence into the existing ComposerShell fence comment at line 3153.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — the four lines are gone; the manifest sentence is the record.

variant="ghost"
size="sm"
disabled={!rightPanelAvailable}
disabled={!terminalAvailable}
>
{/* fork:begin fork-workspace-header — see .fork/customizations.yaml#fork-workspace-header */}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

No-op fences. Both fenced hunks here (52-54 and 79-81) wrap <PanelBottomIcon className="size-4" /> / <PanelRightIcon className="size-4" />, which are byte-identical to upstream (git diff HEAD^2 HEAD -- this file shows only the four comment lines). The manifest's intent for fork-workspace-header says the glyph swap happens "via the Phosphor shim rather than at the call site", and forkWorkspaceHeader.test.ts:136-137 guards the shim, not this file.

The fence is the only reason this file was in the conflict list at all, and it will conflict again on every upstream touch of these lines while protecting nothing. Delete the four comment lines; the watch: entry can stay so drift is still reported.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — fences removed; the file is byte-identical to upstream again and stays on the watch: list.

@@ -137,7 +136,10 @@ describe("fork guard: narrow-workspace-layout", () => {
// The shadow owns the whole module, so an unrelated value drifting in it
// is a silent behaviour change nothing else would catch.
expect(THREAD_SIDEBAR_WIDTH_STORAGE_KEY).toBe("chat_thread_sidebar_width");
expect(THREAD_SIDEBAR_DEFAULT_WIDTH).toBe(16 * 16);
// Module-private since upstream #7153; read from source so it still cannot drift.
expect(readSibling("../overrides/components/threadSidebarWidth.ts")).toContain(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Pins the constant's spelling rather than its behavior. toContain("const THREAD_SIDEBAR_DEFAULT_WIDTH = 16 * 16;") goes red on 256, a reformat, or a rename with zero behavior change, and stays green if resolveInitialThreadSidebarWidth stopped using the constant. The override's public API already exposes the default and is imported at the top of this file:

// null stored width → default, uncapped by a wide viewport
expect(resolveInitialThreadSidebarWidth(null, 10_000)).toBe(16 * 16);

That matches how the rest of this it block checks THREAD_SIDEBAR_MIN_WIDTH and THREAD_SIDEBAR_WIDTH_STORAGE_KEY by value, and the two readSibling("../overrides/components/threadSidebarWidth.ts") calls in this and the next test could then become one module-level read next to upstreamWidths.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — asserts resolveInitialThreadSidebarWidth(null, 10_000) is 16 * 16, and the override source is read once at module level next to upstreamWidths.

expect(chatView).toContain("drag-region relative flex h-[var(--workspace-topbar-height)]");
// Upstream #7153 moved the strip out of ChatView into the shared header.
const pageHeader = readSibling("../components/WorkspacePageHeader.tsx");
expect(pageHeader).toContain('electron && "drag-region"');

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Minor: h-[var(--workspace-topbar-height)] is a fork-workspace-header concern, not a drag-region one — this test is about the strip the overlay releases. It adds a false-red surface (upstream retuning the header height) to a guard whose real claim is on the line above. Suggest dropping this assertion and keeping only the "drag-region" check.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — dropped; the test keeps only the drag-region claim.

// truncate) rather than hiding the way upstream's wordmark link did. This
// used to ride a fenced hunk in upstream's threadSidebarWidth.test.ts,
// which upstream pruned in #8400.
expect(chrome).toContain("sidebar-brand ml-auto h-6 w-fit min-w-0 shrink-0");

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Eight utility tokens in fixed order from a high-churn upstream file. The intent in the comment is "truncates (min-w-0 + truncate) rather than hiding", but the assertion pins sidebar-brand ml-auto h-6 w-fit min-w-0 shrink-0 verbatim from SidebarChrome.tsx:97. A class-sort pass, h-6h-7, or an ml-auto layout change breaks the guard while the fork's outcome is intact — and SidebarChrome is one of the most-touched files per sync.

Assert the intent instead, e.g. expect(chrome).toMatch(/className="[^"]*\bsidebar-brand\b[^"]*\bmin-w-0\b/) alongside the existing truncate check.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 2dd06b7 — now toMatch(/className="[^"]*\bsidebar-brand\b[^"]*\bmin-w-0\b/u) beside the truncate check.

the default contrast (base 100%, boost 0%): the palette's own opaque
foregrounds. Under glass the contrast slider therefore does not move
sidebar text; it still applies everywhere else. */
--contrast-sidebar-foreground: #e8e8e8;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Nit: these two literals are exactly this palette's own --sidebar-foreground (#e8e8e8, line 131) and --sidebar-muted-foreground (#d0d4d6, line 139), declared on the same [data-sidebar-version="v2"] element — the comment says as much ("the palette's own opaque foregrounds"). Unlike --sidebar-icon-color above, no color-mix arithmetic is involved, so var(--sidebar-foreground) / var(--sidebar-muted-foreground) resolve to the same opaque colors without a second copy that drifts the first time Cool Darker's foregrounds are retuned.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Keeping the literals, deliberately: forkCoolDarkerSidebarVibrancy.test.ts:435 requires every token index.css derives from --sidebar to be pinned as an opaque #rrggbb under glass (toMatch(/^#[0-9a-f]{6}$/)), so a var() would fail the guard. That rule is the fork's flatten-alpha doctrine and I'd rather not loosen it in a sync. The drift risk you describe is real; noted in the comment above the two values so a Cool Darker retune knows to touch them.

@NoahHendrickson

Copy link
Copy Markdown
Owner Author

Amendment to my review above. Two inline comments already on this PR from another reviewer hold up under verification and are more important than anything in my list — I've confirmed both in-thread:

  1. Shoulder tabs overlap the context strip (ChatComposer.tsx:3150). Upstream's pt-7 clearance sits on the form, but ComposerShell renders the fork's context row between the form edge and the surface the tabs hang off, so on a git thread with a stash or task steps the tab climbs into the branch/environment/liveness controls. This is a merge-introduced visual regression; my "no correctness regressions" line should read "no logic regressions".
  2. desktop-macos-preview.yml runs on Blacksmith with no upstream-repository gate; applying preview:mac on the fork queues forever, and the ci-runners guard does not scan it.

I'd treat those two as the merge blockers; my nine comments remain low-severity follow-ups.

— Claude Fable 5 via Claude Code

- ComposerShell reserves the shoulder-tab clearance between the context
  row and the vessel; the form's pt-7 only applies without a context row
- desktop-macos-preview.yml gated on the upstream repository at build and
  cleanup; releaseUpstreamOnly guard covers it; manifest watches it
- session-logic 20k benchmark budget raised under a ci-runners fence
  (106 ms on a hosted runner)
- Pin marker shared by card and slim rows; card-rows guard pins the
  forwarded autoSettleOnMerge
- Dormant pending-user-input strip hook removed (ChatComposer wrappers,
  theme rule, guard case, manifest watch)
- Comment-only and no-op fences dropped; guards assert outcomes instead
  of class spellings; WorkspacePageHeader watched for the drag strip;
  PackagePlusIcon reuses DownloadIcon

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

Copy link
Copy Markdown
Owner Author

Addressed the review in 2dd06b7.

Blockers: shoulder-tab clearance now lives in ComposerShell between the context row and the vessel (form pt-7 only without a context row); desktop-macos-preview.yml is gated on the upstream repository at build and cleanup, with a new releaseUpstreamOnly guard case and a manifest watch entry.

CI: the red Test job was session-logic.test.ts > updates 20,000 ordered tool activities within 100 ms at 106 ms on the hosted runner — the ci-runners case the manifest already describes. Raised to 250 ms under a ci-runners fence and added the file to that entry's watch: and intent.

Cleanups: all nine applied except the palette var() nit, which the vibrancy guard forbids (it pins opaque hex on purpose) — replied in-thread. PackagePlusIcon now reuses DownloadIcon.

Verified: fork guards 42/357, web typecheck, fork lint, src/custom + resolver + session-logic + ComposerPendingUserInputPanel tests (64 files / 612 green), fmt clean.

— Claude Fable 5 via Claude Code

@NoahHendrickson
NoahHendrickson merged commit 3835c60 into custom Aug 28, 2026
20 checks passed
@NoahHendrickson
NoahHendrickson deleted the claude/sync-2026-08-28 branch August 28, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. 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.