Skip to content

Upstream sync 2026-07-27: absorb 34 commits through v0.0.29 - #26

Merged
NoahHendrickson merged 38 commits into
customfrom
claude/sync-2026-07-27
Jul 28, 2026
Merged

Upstream sync 2026-07-27: absorb 34 commits through v0.0.29#26
NoahHendrickson merged 38 commits into
customfrom
claude/sync-2026-07-27

Conversation

@NoahHendrickson

Copy link
Copy Markdown
Owner

Summary

Merges upstream/main (476d69c, through v0.0.29) into custom — 34 upstream commits. Merged from upstream/main directly rather than origin/main because the mirror is 13 commits stale (known fork-sync-mirror token-permission failure on upstream's release.yml change).

What upstream changed (highlights)

Conflicts resolved (2)

Both were fork fork-workspace-header styling colliding with new upstream behavior; resolved per manifest intent — behavior adopted, fork presentation kept:

  1. ChatHeader.tsx — upstream made the project breadcrumb a "new thread" button with favicon + tooltip. Kept the click behavior, tooltip, and a11y label; kept fork presentation (no favicon, semibold project name, semibold /). Fence comment updated to record the adoption. Upstream's ProjectFavicon import stays out.
  2. ProjectScriptsControl.tsx — upstream made the imports menu controlled (close-on-edit, fix: close actions dropdown when editing pingdotgg/t3code#4660). Took upstream's open/onOpenChange and kept the fork's data-fork-pill on the trigger.

Customization rework

  • phosphor-duotone-icons — guard caught two unmapped lucide icons from the new features: BookmarkIcon (stash) → BookmarkSimple, PictureInPicture2 (PiP) → PictureInPicture, both duotone.

Verification

  • Fork guard suite: 19 files, 152 tests passing
  • CLAUDE.md symlink blob verified as the fork's 9-byte no-newline target
  • Fence counts across all watched files match the pre-merge baseline
  • ChatHeader.test.ts (4 tests) passing; web tsgo --noEmit clean; .fork/lint-owned.mjs clean (46 files, no warnings)

🤖 Generated with Claude Code

t3dotgg and others added 30 commits July 26, 2026 19:48
…otgg#4556)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…tgg#4397)

Co-authored-by: codex <codex@users.noreply.github.com>
…queue (pingdotgg#4453)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Prepare Relay production infrastructure for the PlanetScale PS_20 HA topology, upgrade Effect and Alchemy to compatible betas, reconcile migration state, and preserve patched MCP session termination behavior.\n\nCo-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
…ts down (pingdotgg#4531)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
…tgg#4670)

Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
KrzysztofMoch and others added 6 commits July 27, 2026 23:25
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	apps/web/src/components/ProjectScriptsControl.tsx
#	apps/web/src/components/chat/ChatHeader.tsx
Upstream's prompt stash (pingdotgg#4453) and preview picture-in-picture (pingdotgg#4397)
introduced two lucide icons the shim did not export; the phosphor-duotone
guard caught both. Bookmark maps to BookmarkSimple and PictureInPicture2
to PictureInPicture, both at the default duotone weight.

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

cursor Bot commented Jul 28, 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 28, 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 real features, but several landings fail the maintainability bar: unjustified growth of already-huge modules, two first-party files crossing 1k lines, duplicated client heuristics instead of a projection contract, and an incomplete stash extraction that left ~400 lines of orchestration inside ChatComposer.

Fork conflict resolutions (ChatHeader, ProjectScriptsControl) and the Phosphor shim mappings look clean. The structural debt is in the upstream feature landings being absorbed as-is.

(Inline comments skipped — GitHub rejects reviews on this PR because the diff exceeds the 300-file limit. Anchors below are HEAD line numbers.)


1. Structural regression — PiP/frame-capture in a 3.6k-line god module

apps/desktop/src/preview/Manager.ts3043 → 3679; Manager.test.ts +771.

frameCaptureSessionsRef / pictureInPictureSessionsRef (~L485–498), withTabLifecycleLock (~L529), and openPictureInPicture (~L2264) are bolted into makeNativeOperations.

Extract PiP + frame-capture (+ session maps / locks / aspect-ratio state) into dedicated modules. Keep Manager as thin orchestration. Growing this file further makes every preview change unreadable. Once extracted, own tab-generation / closingTabIds / semaphore checks as one lifecycle policy — not ad-hoc predicates in every create/close/register path.

2. Missed code-judo — stash orchestration left in ChatComposer

apps/web/src/components/chat/ChatComposer.tsx2905 → 3353.

Store/menu/badge/compression were extracted, then ~400 lines of restore/delete/stash/shortcut/pulse orchestration stayed in the composer (~L1936–2126+).

Pull into useComposerPromptStash (or similar) so ChatComposer only wires the hook. Incomplete extraction leaves every future composer change entangled with stash.

3. Boundary / duplicated magic — WS payload projection

apps/server/src/orchestration/ActivityPayloadProjection.ts ~L176–181.

collectChangedFiles is effectively copied from apps/web/src/session-logic.ts. The comment admits both clients still walk path-like keys. Don't encode the client heuristic twice — project an explicit contract field (typed files / summary shape) and teach web/mobile to prefer it so the dual unknown-walkers can die.

4. 1k crossing — scripts/dev-runner.test.ts

749 → 1184. Extracting scripts/lib/dev-share.ts was the right implementation move; the test surface didn't follow. Split port/single-origin policy tests (isBrowserAllowedPort ~L622) from --share integration cases; lean on dev-share.test.ts.

5. 1k crossing — apps/server/src/cloud/http.ts

963 → 1034 via releaseManagedTunnelOnShutdown (~L634). Coherent feature, wrong home. Move release (+ CLI-desired/managed gating) into a small adjacent module next to link reconcile — don't let cloud HTTP keep absorbing every environment lifecycle concern.


Not blocking (positive)

  • Prompt stash store + compression + menu/badge are well-bounded
  • Web mini-player split (ThreadPreviewMiniPlayer, store, layout helpers) is fine
  • Tailscale --share extraction to dev-share.ts is good judo
  • PR-branch scoping (resolveBranchToolbarPrBranch) simplifies rather than adds
  • Fork presentation keeps on conflict files are intentional and clean

Approval bar: not met — clear structural regressions, unjustified file-size crossings, duplicated heuristic projection, and an obvious decomposition left on the table.

Open in Web View Automation 

Sent by Cursor Automation: Thermo-nuclear PR review

Copy link
Copy Markdown
Owner Author

Review — upstream sync 2026-07-27

Static review of d3b1bb54..1b928384. I re-derived the sync's own invariants rather than reading the summary: fence integrity across every watched file, the icon-shim closure, where merged upstream lines landed relative to fork fences, and whether each guard still fails for the reason it claims to. There is no node_modules in my environment, so the test / typecheck / lint results in the description are taken on trust — none of the findings below depend on running them.

The merge mechanics are clean. The problem is on the ChatHeader side, and it is the specific thing this fork's whole safety model rests on: after this PR, the customization it touched is less guarded than before it.


1. The workspace-header guard now passes on the wrong element

apps/web/src/__fork_guards__/forkWorkspaceHeader.test.ts:87

expect(chatHeader).toMatch(/font-semibold text-muted-foreground/u);

Before this PR, that matched the project name (ChatHeader.tsx, base line 102: max-w-40 truncate text-sm font-semibold text-muted-foreground). The resolution moved the colour onto upstream's trigger button, so the name is now max-w-40 truncate text-sm font-semibold (line 119) and the only substring match left in the file is line 123 — the / separator:

119:  <span className="max-w-40 truncate text-sm font-semibold">{activeProjectName}</span>
123:  <span aria-hidden className="text-sm font-semibold text-muted-foreground">

Concretely: revert the project name to upstream's font-medium today and this test stays green, because the separator still satisfies the regex. The test's own comment — "Reverting either half gives two competing bold runs, which is what upstream had" — is now false for the project half. This is the same failure the sibling test in this file was rewritten to eliminate ("a tripwire dressed up as a proof"); it has reappeared two tests down.

Fix: anchor the assertion to the name, not to a loose class-pair — e.g. match /text-sm font-semibold">\{activeProjectName\}/ and assert the muted colour separately on the trigger button.

2. Nothing asserts the behaviour that was deliberately adopted

This is the only file in the merge where upstream's new lines landed inside a fork fence — I checked all six fenced components:

file fences added lines added inside a fence
ChatComposer.tsx 17 448 0
ChatView.tsx 5 52 0
SidebarV2.tsx 10 14 0
AppSidebarLayout.tsx 1 21 0
ProjectScriptsControl.tsx 1 15 0
ChatHeader.tsx 1 22 20

That is the right call for a hunk the fork owns presentationally — but it has a consequence the PR doesn't close. onClick, aria-label and the tooltip are now fork-fenced, so every future sync treats them as fork-owned and will not re-apply upstream changes to them. Meanwhile:

  • the guard asserts only weight + not.toContain("ProjectFavicon") — nothing about the click behaviour;
  • .fork/customizations.yaml#fork-workspace-header's intent still describes only the weight swap and the removed favicon. The fence comment records the adoption, but the manifest header says intent is what the daily sync routine reads before resolving a conflict — the code comment is not that contract.

So dropping onNewThreadInProject in a future sync silently loses pingdotgg#4638 with CI green and the manifest still "accurate". Two lines fix it: a sentence in intent saying the breadcrumb is an upstream new-thread button whose behaviour tracks upstream, and one expect(chatHeader).toContain("onClick={onNewThreadInProject}")-shaped assertion.

3. Dead class in the fork's own hunk

ChatHeader.tsx:114 keeps upstream's gap-1.5 on the trigger button. The favicon was the second child; the button now wraps a single span, so the gap (and items-center) spaces nothing. One token.


Inherited from upstream — not blockers, but you now own them

Raw NUL bytes in ChatComposer.tsx. Upstream's stash snapshot keys use literal U+0000 as a delimiter inside template literals — 6 of them, at byte offsets 75934, 75953, 76330, 76354, 80449, 80459 (e.g. `${image.mimeType}\0${image.sizeBytes}\0${image.name}`). git diff still treats the file as text (the NULs sit past git's 8000-byte sniff window, which is why the stat reads a normal 448+/0-), but grep classifies it as binary:

$ grep -n "fork:begin" apps/web/src/components/chat/ChatComposer.tsx
Binary file apps/web/src/components/chat/ChatComposer.tsx matches

Zero lines, exit 0. That file carries 17 fenced hunks — the densest in the fork. .fork/detect-drift.mjs, .fork/lint-owned.mjs and the guards all use readFileSync, so automated checks are unaffected; the hazard is every ad-hoc grep/rg fence audit a human or agent runs during a sync, which will silently report nothing in the file that matters most. Worth a .gitattributes entry or a line in .fork/AGENTS.md.

allowedHosts is now [".ts.net", ...T3CODE_DEV_ALLOWED_HOSTS] on every vp run dev (apps/web/vite.config.ts), not gated on --share, alongside the new /ws proxy entry. Vite's allowedHosts is DNS-rebinding protection, and a bare suffix trusts every name under that public suffix rather than this tailnet's. The inline comment argues tailnet DNS is attacker-proof; that's upstream's call, but it is now the fork's default dev posture — flagging so it's an inherited decision rather than an unnoticed one.

Nit, upstream's: ProjectScriptsControl's actionsMenuOpen is a two-key record whose handlers cross-clear the other menu — but the two Menus are in mutually exclusive branches of the same ternary, so the cross-clear can never fire and a single boolean would do. The fork's diff against upstream here is exactly the two data-fork-pill attributes and nothing else; noting it only so nobody later "fixes" it as fork code.


Verified clean

  • Icon shim is closed. I re-ran the guard's scan independently: 146 distinct named bindings imported from lucide-react across apps/web/src + packages, all 146 exported by the shim, zero missing after the BookmarkIcon / PictureInPicture2 additions, and no default/namespace/dynamic import forms. Three exports (LucideProps, CirclePlusIcon, LaptopIcon) now have no importer — harmless.
  • Fence integrity: no watched file lost or gained a fork:begin/fork:end across the merge.
  • No shadows in use (apps/web/src/overrides/ holds only README.md), so there is no stale-override-copy risk this sync — the usual worst failure mode for a fork this size does not apply.
  • release-upstream-only gate intact on both entry jobs; upstream's only release.yml change is --archive=tgz.
  • CLAUDE.md blob is exactly 9 bytes, AGENTS.md, no trailing newline.
  • No conflict markers and no .orig/.rej anywhere in the tree.
  • patchedDependenciespatches/ fully consistent after the effect beta.78 → beta.102 rename — all 13 entries resolve.
  • fix(web): scope PR state to the thread branch pingdotgg/t3code#4460 composes with sidebar-v2-card-rows. resolveThreadPr losing hasDedicatedWorktree narrows PR resolution to gitStatus.refName === thread.branch, which drops the badge for local threads parked off the checked-out branch. showsMetaRow reads the same prBadge/prUnknown the render reads, so those rows lose the third line and the 90px hint together — the contain-intrinsic-size claim can't drift. Worth a visual pass on a repo with several non-worktree threads, since more rows will now render at 68px than before, but the invariant holds.

Fix 1 and 2 and this is good to land; 3 is a token.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Follow-up: CI Test is red, and it isn't a fork guard

Closing the loop on the one thing my review couldn't check — run 30323510546:

FAIL  unit  src/lib/stashImageCompression.test.ts > compressImageForStash
            > reports too-large when even the smallest encoding overflows the budget
Error: Test timed out in 15000ms.

Test Files  1 failed | 201 passed (202)
      Tests  1 failed | 1791 passed (1792)

Every fork guard passed. The failure is in an upstream file that arrived with the prompt stash (pingdotgg#4453), so the description's focused-run results were accurate as far as they went — this test simply wasn't in the focused set. Check, Release Smoke and Mobile Native Static Analysis are all green; this one job is what has the PR at mergeable_state: blocked.

It is not flake. The stub returns a constant 8 MB for every encode, so nothing ever fits the budget and the give-up path runs to completion:

  • 3 dimension passes — [1, ...FALLBACK_SCALE_STEPS] = [1, 0.75, 0.55]
  • each pass: 1 WebP probe + 4 QUALITY_STEPS = 5 encodes
  • 15 encodes × 8 MB, and encodeToDataUrl calls blobToDataUrl on every one

blobToDataUrlbytesToBase64 builds each result with String.fromCharCode(...chunk) at 32 KB a chunk — ~244 spread calls per encode, ~3,660 in total — then btoas an 8 MB string and concatenates its way to a ~10.7 M-char data URL. That's ~120 MB pushed through spread-argument fromCharCode and ~160 MB of string building, all to answer "still too big". Deterministic work, just a lot of it, which is why 15 s is marginal rather than random.

The underlying inefficiency is real and upstream's: encodeWithinBudget compares encoded.dataUrl.length against budgetChars after materializing the base64. blob.size is known before that, and dataUrl.length ≈ blob.size × 4/3 + header — so an encoding that cannot possibly fit is knowable without ever building the string. Rejecting on blob.size first would make the give-up path cheap and the test fast, and it's a real win in the browser too: this runs on the ⌘S path with the composer waiting on it.

The fork-specific part: ci-runners puts this fork on ubuntu-latest because Blacksmith's app isn't installed here, while upstream runs the same suite on Blacksmith. A test whose cost is this close to its timeout will fail preferentially on the slower runner — so "green upstream" is not evidence it will be green here, and this is likely to recur on every sync that touches that path until the byte-size check lands.

Cheapest unblock is a testTimeout on that one case; the right fix is the blob.size short-circuit. Either way it's worth deciding deliberately rather than re-running and hoping — a re-run may well pass, which is the trap.


Generated by Claude Code

NoahHendrickson and others added 2 commits July 27, 2026 23:09
Review fixes for the 2026-07-27 sync PR:

- Anchor the semibold assertion to the project name itself; the loose
  class-pair match was satisfied by the '/' separator alone, so reverting
  the name to upstream's font-medium stayed green. Assert the muted colour
  separately on the breadcrumb trigger where it now lives.
- Guard the adopted pingdotgg#4638 behaviour: the new-thread click and tooltip sit
  inside the fork fence, so a future sync will not re-apply them from
  upstream; without an assertion, dropping them is silent. Record the same
  contract in the fork-workspace-header manifest intent, which is what the
  sync routine reads.
- Drop the dead gap-1.5 on the breadcrumb trigger (spaced the favicon the
  fork removed).
- Raise the give-up-path timeout in stashImageCompression.test.ts to 60s,
  fenced under ci-runners: 15 encodes of an 8 MB stub through chunked
  base64 is deterministic work at the margin of 15s on GitHub-hosted
  runners, which are slower than the Blacksmith runners upstream tunes
  against. Test-only on purpose — the byte-size short-circuit is upstream's
  fix to make, and diverging in hot upstream logic costs a conflict on
  every sync. Losing the hunk turns CI red by itself, which is the
  enforcement; the manifest entry records the policy.
- Document the ChatComposer.tsx raw-NUL grep hazard in .fork/AGENTS.md:
  plain grep classifies the fork's densest fenced file as binary and
  reports zero matches with exit 0; fence audits must use grep -a or rg.

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

Copy link
Copy Markdown
Owner Author

Review fixes landed — 5d58ee9

Also merged origin/custom back in (0ff236e), picking up #27's mirror deploy-key fix.

@NoahHendrickson's review

1. Guard anchored to the right element — the semibold assertion now matches text-sm font-semibold">{activeProjectName} (the name itself), with a separate assertion that the muted colour sits on the breadcrumb trigger. Reverting the name to font-medium now fails the guard; the separator can no longer satisfy it. The test comment records why the loose class-pair was wrong.

2. Adopted behaviour is now asserted and manifested — new guard case asserts onClick={onNewThreadInProject} and the tooltip, with a comment explaining the fence-ownership consequence. The fork-workspace-header intent in .fork/customizations.yaml now states the breadcrumb is upstream's pingdotgg#4638 button whose click/aria-label/tooltip must track upstream, with only the presentation fork-owned — so the sync routine reads the contract, not just the code comment.

3. Dead gap-1.5 removed.

CI red — decided, not re-run: took the fenced per-test timeout (60s) on the give-up-path case, not the blob.size short-circuit. Rationale, recorded in the manifest under ci-runners (whose slow-runner premise is exactly why this fires here and not upstream): the short-circuit rewrites hot upstream product logic that upstream will iterate on, buying a conflict on every pingdotgg#4453 follow-up, and the real fix is upstream's to make. The fence is self-guarding — a sync that drops it turns CI red by itself. ci-runners now watches the test file so the drift detector flags upstream churn there.

NUL-byte hazard — adopted. New "Auditing fences with grep" section in .fork/AGENTS.md. I hit this exact trap during the sync (mid-merge fence audit reported 0 fences in ChatComposer.tsx; diagnosed as binary-sniffing then) — it deserved the writedown.

allowedHosts widening — acknowledged as an inherited upstream decision; leaving as-is this sync. If we want the fork to gate .ts.net on --share, that's a deliberate Tier-4 divergence for a separate PR.

Verification: 156/156 guards green (includes #27's new relay guard), stashImageCompression.test.ts 8/8, web typecheck clean, lint-owned clean (47 files), drift detector parses the updated manifest and flags the new watch entry as expected.

Cursor "thermo-nuclear" review

Declining the structural asks (Manager.ts extraction, useComposerPromptStash, WS projection contract, test-file splits) — not because they're wrong, but because they're wrong here. This fork's sync model absorbs upstream as-is and confines divergence to custom/, overrides/, and small fenced hunks; refactoring upstream modules inside a sync PR converts every future sync of those files into a manual conflict-resolution exercise, which is precisely the failure mode the .fork/ machinery exists to prevent. The review itself notes the fork-owned resolutions are clean. The structural debt belongs to upstream's tree and should be fixed there, arriving here through a later sync.


Generated by Claude Code

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

Code review — sync merge resolution + fork rework

Scope: the fork-authored substance of this sync (the two conflict resolutions, the icon-shim and guard-hardening commits, and the manifest/docs changes), not the 34 upstream commits. 8 finder angles, one verification pass per surviving candidate — most verified mechanically (mutating the merged source and re-running the guard assertions, recomputing the base64 budget arithmetic, checking vitest 4.1.9's type overloads).

The core merge is sound. All 146 lucide identifiers imported across the merged aliased graph resolve in the Phosphor shim (including the two new ones, with the right export names for upstream's bindings); onNewThreadInProject is declared and wired from ChatView; the actionsMenuOpen controlled-menu merge reproduces upstream pingdotgg#4660 faithfully including close-on-edit; ProjectFavicon is fully gone (no unused import); ChatComposer's fences survived the stash feature's no-conflict merge intact; the release-upstream-only gate still wraps every release job; and the vendored subtrees were synced with the effect beta.78→beta.102 bump.

The findings below are all about the durability of the fork machinery, not broken behavior. Ranked by severity.


1. The guard hardening dropped the only tripwire for the separator restyle — apps/web/src/__fork_guards__/forkWorkspaceHeader.test.ts:93 [CONFIRMED, test-coverage]

The old /font-semibold text-muted-foreground/u assertion's only match in the merged file was the / separator span's class list — so it doubled as the tripwire for the fork's separator restyle (upstream has text-muted-foreground/40). The two replacement assertions anchor to the project name and the breadcrumb trigger; neither touches the separator.

Demonstrated end-to-end: swapping the merged separator for upstream's <span aria-hidden className="text-muted-foreground/40">/</span> leaves all six new guard assertions green, while the removed assertion would have failed. A future sync that keeps upstream's separator merges with CI green and the fork presentation silently reverts — the exact silent-loss class this commit says it was written to eliminate. Fix: add a separator-anchored assertion (e.g. toContain('font-semibold text-muted-foreground">') scoped to the separator, or not.toContain("text-muted-foreground/40")).

2. The ci-runners timeout fence has no guard asserting it — .fork/customizations.yaml:60 [CONFIRMED, conventions]

.fork/AGENTS.md ("Every customization requires") mandates a guard test in __fork_guards__/ "so an upstream sync that silently drops the customization turns CI red instead of passing quietly." ciRunners.test.ts only regex-checks runner labels in ci.yml; nothing reads the newly watched stashImageCompression.test.ts (detect-drift is advisory-only; customizationsManifest.test.ts checks the converse direction — a wholesale upstream replacement passes all four of its assertions).

The manifest's substitute rationale — "losing one turns CI red by itself" — is undercut by this PR's own commit message calling the test "at the margin of 15s": a sync that takes upstream's version can pass on a fast run, and the loss resurfaces later as apparent upstream flake instead of a flagged fork regression. One assertion in ciRunners.test.ts (fence marker or timeout: 60_000 present in the watched file) closes it.

3. Second data-fork-pill carrier is unfenced and unguarded — apps/web/src/components/ProjectScriptsControl.tsx:433 [CONFIRMED, conventions]

The file's only fence (337–342) wraps the Group-branch carrier; the importable-scripts MenuTrigger's data-fork-pill at line 433 sits outside any fence (.fork/AGENTS.md rule 4: inline edits must be fenced), and the pill guard is a single toContain("data-fork-pill") per file — satisfied by the other carrier. A future sync that rewrites the imports-branch MenuTrigger drops its pill with CI green, and the "Add action"-only state renders as an upstream button.

The unfenced placement pre-exists on custom, but this hunk was one of the two declared conflicts and the resolution hand-rewrote exactly these lines — this was the moment to fence it or strengthen the guard.

4. The fence swallows upstream's pingdotgg#4638 behavior lines — apps/web/src/components/chat/ChatHeader.tsx:91 [CONFIRMED, altitude]

The resolution placed upstream's entire new-thread breadcrumb — button element, aria-label, onClick, Tooltip/TooltipPopup, ~50 lines arrived verbatim from upstream — inside fork:begin fork-workspace-header. The sync machinery now treats upstream-owned behavior as fork-owned; the manifest concedes the cost ("port upstream's intent by hand when that hunk drifts") and the new guard pins today's snapshot of upstream's text, not currency with upstream. When upstream iterates on the breadcrumb, the fork silently ships the frozen copy with CI green.

The deeper mechanism already exists three files over in this same customization: data-fork-pill + theme.custom.css. Every fork delta here except the favicon removal (font weights, gaps, separator color) is pure CSS — a data-fork attribute on the trigger plus Tier-1 rules would leave upstream's behavior lines byte-identical and auto-merging, shrinking the fence to the one-line favicon removal. (CSS-hiding the favicon is not equivalent — ProjectFavicon would still mount and fetch via useAssetUrl — so that one line legitimately stays a code edit.)

5. Muted-colour guard regex false-fails on a prop reorder — forkWorkspaceHeader.test.ts:97 [PLAUSIBLE, test-coverage]

Measured: the trigger's text-muted-foreground sits 142 chars after the aria-label anchor; sorting the JSX props so className precedes aria-label moves it to 414 — past the [\s\S]{0,400} window — so a formatting-only edit turns the guard red pointing at nothing. (The false-green path via the separator is numerically out of reach: 619 chars, only ~156 strippable.) Strictly stronger and window-free: toContain("text-muted-foreground transition-colors hover:text-foreground") — occurs exactly once (only the trigger has hover:text-foreground) and fails precisely when the muted colour is lost.

6. The 60s timeout raise is unnecessary — apps/web/src/lib/stashImageCompression.test.ts:128 [CONFIRMED, efficiency]

The give-up path's 15 encodes are fixed by constants (3 dimension passes × [1 WebP probe + 4 quality steps]), not stub size. A ~1.1 MB stub (stubCanvasPipeline(() => 1_100_000) + makeFile(1_100_000)) encodes to ~1,466,691 data-URL chars — over the 1,300,000 budget on every attempt — driving the identical 15 encodes, the same {ok:false, reason:"too-large"}, and the same finally-block close(), at ~16.5 MB of base64 work instead of ~120 MB, comfortably inside the default 15s. Both fixes live inside the same fenced it() block, so the sync-conflict footprint is identical — but the timeout raise burns 10–15s of CI every run and, if the loop ever regresses into a hang, delays failure reporting to 60s.

7. The timeout rewrite re-indents the whole test body — stashImageCompression.test.ts:127 [CONFIRMED, simplification]

The options-object signature re-indented every body line (the -w diff confirms indentation is the only body change), so any upstream edit inside this test now produces a whole-hunk conflict on every future sync — contradicting the commit's own minimize-divergence rationale. @vitest/runner@4.1.9 still declares the trailing-number overload (name, fn?, options?: number) first-class, no @deprecated tag: appending , 60_000 before the closing paren is the same timeout as a 1-line delta.

8. Dead classes on the new-thread button — ChatHeader.tsx:115 [CONFIRMED, simplification]

min-w-0 is inert (the parent span is shrink-0, no width pressure ever reaches the button) and items-center renders identically with the single text child. Both came from upstream's two-child (favicon + text) button — and the same commit already pruned gap-1.5 on exactly the removed-favicon reasoning, so the remaining two are an oversight, not a choice. Fork-fenced lines every future resolution re-reasons about; two fewer tokens to carry.

9. Fence comment duplicates the manifest prose — ChatHeader.tsx:103 [CONFIRMED, simplification]

The fence comment's three paragraphs (weight inversion, favicon removal, pingdotgg#4638 adoption) are near-verbatim copies of the manifest's fork-workspace-header intent — against the manifest's own "narrative there, contract here" division, and against house style elsewhere (main.tsx, SidebarV2.tsx, ChatComposer.tsx use a bare pointer or short local note). Two independently edited copies will drift; the manifest is what the sync routine reads. Shrink the in-file comment to a one-liner pointing at the anchor the fence header already cites. (The ci-runners fence comment in the stash test was checked for the same issue and is fine — it carries local detail the manifest doesn't.)


Also checked and clean: guard regexes match the merged source character-for-character and fail under regression simulation (except as noted above); it(name, {timeout}, fn) is valid under vitest 4.1.9; the new watch: entry satisfies the manifest parser and fence-claim invariant; the ChatComposer NUL-byte claim in .fork/AGENTS.md is factually true (6 raw NULs); --share exists in the merged dev-runner; the phosphor guard's completeness mechanism genuinely scans all lucide import forms.

🤖 Generated with Claude Code

@NoahHendrickson
NoahHendrickson merged commit 1137d8d into custom Jul 28, 2026
10 checks passed
@NoahHendrickson
NoahHendrickson deleted the claude/sync-2026-07-27 branch July 28, 2026 13:35
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.

8 participants