Skip to content

Clean up seam drift, restructure the seams doc, and add seams and sync checks - #292

Merged
henrychu04 merged 9 commits into
coder-onlyfrom
t3/coder-seam-drift-sync-checks
Oct 31, 2026
Merged

henrychu04 merged 9 commits into
coder-onlyfrom
t3/coder-seam-drift-sync-checks

Conversation

@henrychu04

Copy link
Copy Markdown
Owner

Seam drift cleanup, a restructured seams doc with a check that keeps it honest, and sync tooling.

Fixes #228
Refs #231, #237

1. Seam drift (checked again against 80a2351)

  • Root vite.config.ts: the fork had kept an old copy of upstream's file, not a deliberate seam. I took upstream's file and cut only what needs uncarried code: the remote task cache and cached test tasks (infra/, scripts/lib/cached-tests.ts), the per-package test projects (vite.coder.config.ts extends this file, so it keeps the server's gitConfig.setup.ts), and the oxlint-plugin-t3code / @shadcn/lint rules. Each cut has a Coder: comment and a doc entry. vp lint still loads the config.
  • TerminalContextInlineChip.tsx: restored upstream. The contract's lineStart/lineEnd are required, so making them optional did nothing.
  • attachmentStore.ts: added the Coder: marker to the legacy legacy-<uuid>-<ext> comment.
  • MessageCopyButton.tsx, ui/menu.tsx, chatThreadActions.ts, threadNotifications.ts: restored upstream (props, the icon variant, and order). threadNotifications.ts keeps only its desktop-bridge cut, now marked.
  • Other drift found by the new check: restored types.ts's DEFAULT_RUNTIME_MODE (the contract default is also "full-access") and NoActiveThreadState.tsx (isElectron is the constant false). Added markers to useComposerResume/useThreadLifecycle (clearUsageLimitsFor), _chat.settings.tsx, and three unmarked server files: orchestrator/handlers.ts, ProviderSessionManager.ts, and persistence/Errors.ts. I think these plus the two testkit fixture files are the "5 server files"; the fixtures are tests, so the check skips them.

2. Stale doc entries

NavigationHistoryShortcuts is now AppSidebarLayout.tsx's unexported registerNavigationHistoryCommands. Removed useAgentOpenedPreviews, RightPanelTabs.browser.test.tsx, and the historical SelectedEnvironmentProviderSettings.

3. Seams doc restructure

"Upstream seams" went from about 1,150 lines to about 590: one entry per delta (file or symbol plus the delta), grouped by area. A new "Cut surfaces" section (about 100 lines) gathers the cut-surface lists and points at DELETED_PATHS. I dropped sentences that only described upstream parity (MCP Apps, agent secret requests, project icons, tool output images, and similar). "not carried" went from 72 to 10, "is upstream's/follows upstream" from 51 to 6. Other sections are unchanged. To check nothing was lost, I listed every backticked name in the old section that the new one doesn't mention and went through all 210. Each is a parity-only sentence, a cut file already in DELETED_PATHS, or one of the stale names; real seams among them went back in.

4. scripts/seams-check.mjs

Node only, no dependencies, runs in about 8 s. It fails when:

  • (a) a backticked path or name in "Upstream seams" or "Cut surfaces" exists in neither the tracked checkout nor the base tree (paths may skip src/, Markdown is excluded); or
  • (b) a modified non-test upstream file (git diff --diff-filter=M <base>) has no Coder: in its fork lines and no mention in coder-only.md. The merge-request wording swap is detected automatically (undoing the swap makes the lines equal upstream's), not kept as a hand-written allowlist.

Base defaults to SYNC_BASE (80a2351); sync.md §8 now bumps it. It passes on this branch, and before the cleanup it flagged exactly the four stale names. It is wired into sync.md §6 and §7.

5. Sync tooling

  • sync-resolve-deleted.mjs lists upstream files added under a deleted path that a carried file imports separately, under "Keep candidates", with their importers. In a trial merge of upstream 6eef681 (refactor(server): share a path check between file readers pingdotgg/t3code#28041) it listed auth/serverInternalPaths.ts with AssetAccess.ts, GitVcsDriverCore.ts, and WorkspaceFileSystem.ts. It also listed cli/up.ts because of the uncarried binCli.ts, which sync.md now explains. The §3 caveat is added, and the bulk-remove awk stops at the end of its section.
  • scripts/sync-check.mjs <PREV> <SHA> [--only …] replaces §6's inline marker-count, lost-upstream-line, both-sides-loss, and unused-locals checks. On the 80a2351 sync it prints exactly what the shell/Python versions print (23 / 265 / 3 lines). The unused-locals check runs TS 7's tsc via process.execPath.

6. #228

Removed GitLabCli.checkoutMergeRequest's unreachable branch field and --branch/--force handling, and the fork test for it. The --repo <url> -- <iid> selector stays.

7. #231 / #237

Verification

  • pnpm typecheck:web, typecheck:server, typecheck:helper, and the source-control filter typecheck pass.
  • node scripts/seams-check.mjs passes, vp fmt --check on changed files passes, and pnpm build passes.
  • pnpm test:coder: gateway/helper/coder-cli, codex app-server, contracts, server (3558 tests), providers, source-control, client-runtime, shared, and knip all pass.
    • One server test timed out under full-suite load: ClaudeAdapterV2 "persists a todo_list from inline Claude task-tool frames". It passed alone and with its whole file, and on the next test:server run.
    • test:web:coder had 1–2 different load-only failures per run (useInlineActionConfirm, AttachmentFilePreview, a localApi.test.ts hook timeout after a leaked happy-dom fetch). Each passes alone. The unchanged base commit 7b64141 fails the same way in the same environment (useInlineActionConfirm plus one other), so this predates the PR. The web Vitest config doesn't read the root config.
  • No regression tests were added. GitLab mr checkout: branch and force handling is unreachable #228 was checked with the updated GitLabCli.test.ts and the keep-candidate detection with a scratch merge.

🤖 Generated with Claude Code

henrychu04 and others added 7 commits October 31, 2026 12:12
MessageCopyButton's label/icon props, menu's "icon" checkbox variant,
TerminalContextInlineChip's required line range, the order of
hasExplicitComposerModelSelection and hasNotificationSound,
DEFAULT_RUNTIME_MODE (the contract default is also "full-access"), and
NoActiveThreadState's isElectron branch (the fork's env.ts reports false)
are upstream's again. threadNotifications.ts keeps only its desktop
bridge cut, now marked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fork kept an old copy of vite.config.ts. Take upstream's file and
cut only what depends on uncarried code: the remote task cache and
cached test tasks (infra/, scripts/lib/cached-tests.ts), the
per-package test projects (vite.coder.config.ts extends this file, so
it also loads the server's gitConfig setup), and the
oxlint-plugin-t3code and @shadcn/lint rules.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No caller passes branch or force to GitLabCli.checkoutMergeRequest, so
keep only the reachable --repo <url> -- <iid> selector.

Fixes #228

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #231

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "Upstream seams" section had grown to about 1,150 lines with cut
surfaces repeated across bullets and many sentences that described no
delta. Rewrite it as one short entry per real delta, grouped by area,
and move the cut surfaces into one "Cut surfaces" section that points at
DELETED_PATHS. Drop the stale NavigationHistoryShortcuts,
useAgentOpenedPreviews, RightPanelTabs.browser.test.tsx, and
SelectedEnvironmentProviderSettings entries, and name the modified files
the old text never listed.

scripts/seams-check.mjs fails when a backticked name in those sections
exists in neither the checkout nor the upstream base, or when a modified
upstream file has no Coder: marker in its fork lines, no mention in the
doc, and is not a pure merge-request wording swap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
scripts/sync-check.mjs runs the marker-count, lost-upstream-line,
both-sides-loss, and unused-locals checks that sync.md section 6 ran as
inline shell and Python; on the 80a2351 sync it prints the same lines.
sync-resolve-deleted.mjs now lists an upstream file added under a deleted
path apart, as a keep candidate, when a carried file imports it (upstream
pingdotgg#28041's auth/serverInternalPaths.ts). sync.md wires both scripts and the
seams check in, and section 8 bumps the check's recorded base.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
henrychu04 and others added 2 commits October 31, 2026 12:22
# Conflicts:
#	docs/internals/coder-only.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@henrychu04

Copy link
Copy Markdown
Owner Author

Merged coder-only (#290, #291) into this branch. The seams section conflicted as a whole; I kept the restructured section and re-applied the two seam edits from #290 (makeIdentity carried as is; bridge plan-mode refusal, cap before start, keep-until-published). seams-check then caught a stale MAX_RPC_MESSAGE_BYTES left by #291, now CODER_RPC_FRAME_MAX_BYTES. Checked types.ts: upstream's web default and the contracts default are both full-access, so the restore is right. Per-package runs: everything passes except the known #235 macOS /private/var pair in test:server and the web localApi flake (#236), which passes alone. Typechecks, knip, seams-check and build pass.

@henrychu04
henrychu04 merged commit 52c7e7a into coder-only Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitLab mr checkout: branch and force handling is unreachable

1 participant