Skip to content

chore(stage): merge upstream main 78552a8c6 into stage - #278

Merged
bk-agent-01 merged 248 commits into
stagefrom
t3code/stage-upstream-20261006
Oct 6, 2026
Merged

bk-agent-01 merged 248 commits into
stagefrom
t3code/stage-upstream-20261006

Conversation

@bk-agent-01

@bk-agent-01 bk-agent-01 commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

stage was 240 upstream commits behind pingdotgg/t3code main (last sync aad732901, 2026-10-03). Tushar asked to bring stage up to date and deploy it.

What this PR does

It merges upstream main at 78552a8c6 into stage. All fork features stay. 77 files had conflicts. About 230 more fork files did not conflict but did not compile after the merge, so this PR fixes those too.

Decisions to review

Area Decision
MCP scope (pingdotgg#15219) The fork fields (principal, actorUserId, backgroundGrantHash) now sit on upstream's new thread / client / requestNamespace shape. External users and operators are thread-less clients with a full-access ceiling. They no longer get a made-up external-user:<id> thread id.
Pull request tools link_pull_request, unlink_pull_request, list_thread_pull_requests, watch_pull_request and unwatch_pull_request take upstream's threadId. The fork's sessionId parameter is gone. I found no caller that sends sessionId in bk-docs, the Linear bridge or Toolyard.
Scheduled-task tools pingdotgg#15219 lets these tools target other projects. The fork now checks project access for a target project that is not the caller's own (requireForkTargetProject).
ws.ts scope checks Upstream moved the RPC scope checks into middleware. The fork's MCP web-UI bridge calls handlers directly and skips that middleware, so the fork keeps its per-call scope checks.
Webhook MCP tools A caller with no thread and no user-wide scope is refused (confinedThreadId). It does not get "no restriction".
Migrations Upstream 057_ScheduledTaskWebhooks and 058_WebhookRelayDeliveries run at fork ids 1046 and 1047. No existing id changed.
Worktree branches Upstream changed the branch prefix to t3/ (pingdotgg#16220). New worktree branches are now t3/<codename>. Old t3code/* branches still count as taken.
Effect 4.0.1 (pingdotgg#16138) 160 fork files moved from effect/unstable/* to effect/*. One fork test changed how it injects a failure, because the 4.0.1 migrator treats a constraint error on the ledger insert as "locked" and returns no migrations.
node:crypto 18 fork files still use node:crypto. Each file now has an @effect-diagnostics suppression with a reason. Moving them to Effect Crypto (pingdotgg#16377) is a follow-up.
.gitmodules Upstream added two gitlinks under .repos/. A marked root .gitmodules declares them with update = none, so a full checkout does not fail.
New upstream RPCs orchestration.getTurnItem and secrets.answerRequest now require thread access, like the diff and projection RPCs next to them. A denied secret answer reports not_found. A fresh review found these two gaps.
PR watch credentials Watch groups are split by owner and source-control profile, so a read never uses another owner's GitHub login. This fixes a regression from the conflict resolution. A new runtime test covers it.
Ping/pong warning The fork's onPong warning in client-runtime/src/rpc/session.ts is gone. Upstream pingdotgg#16200 reports dropped connections, and Effect 4.0.1 removed the hook.

Follow-ups (not in this PR)

  1. Mobile ThreadDetailScreen.tsx can show two notices for a rejected queued message: the fork's failedOutboxDetail notice and upstream's ComposerErrorNotice (fix(mobile): a message that fails to send now says why in the thread pingdotgg/t3code#15807).
  2. Fork link handlers (fork/integratedBrowserLinks.tsx, fork/pullRequestBrowserLinks.ts) still use isPreviewSupportedInRuntime(), so they do not use upstream's server-hosted browser (feat(preview): run the browser on the environment server pingdotgg/t3code#15328).
  3. SidebarChrome.tsx uses min-h-8 instead of upstream's clipping h-8 wrapper, to keep the fork counters visible. This needs a visual check on stagebkt3.
  4. Server-browser stream, download and upload routes check scope only, not thread access. The fork's older preview RPCs are the same, so decide this as policy.
  5. Upstream DirectEndpoints.resolve() runs tailscale probes on every config read. Watch stage for slow connects. connectDiscoveryCache.expbkt3.ts caches the same probes for other paths.
  6. Move the 18 node:crypto fork files to Effect Crypto.
  7. The GitHub watch-fingerprint batch key does not include the source-control profile, so one batch can use the first owner's token. Today this mostly causes a full reread.

Checks run before push

  • Typecheck: contracts, shared, client-runtime, server, web, desktop, mobile and scripts all pass with 0 errors.
  • Lint and format pass on every changed file. The fork-marker check against upstream main passes.
  • CI round 1 found 2 test failures (fork mocks and an upstream test that mocks every atom). Both are fixed.
  • A fresh review of the conflict resolutions found the access and PR-watch gaps above. Both are fixed.
  • Tests: all of contracts (700), client-runtime (2,450) and mobile (2,079). Focused web (756) and desktop (117) tests. About 1,500 focused server tests for migrations, provider registry, worktrees, usage, MCP, webhooks, presence, Toolyard, auth, preview and orchestration-v2.

Lessons for fork add-ons (from this merge)

The full log of every difficulty is in the first PR comment. These are the main points:

  1. The conflict count understates the work by about half. 77 files conflicted. After resolution, about 230 more fork files failed the typecheck, and git reported no problem for any of them. The causes were deep imports into upstream folders that upstream moved, fork code on effect/unstable/*, and fork fields on an upstream interface that upstream reshaped.
  2. Import upstream through one fork adapter module, not by deep path. Five moved upstream modules broke 36 fork files.
  3. Do not add fork fields to core upstream types. Keep fork identity in a fork-owned service. The MCP scope reshape broke about 75 places for this reason.
  4. Use one gate, not copies. The fork pasted the same access check before 10 call sites, and upstream replaced all 10 call sites. One fork hook would have given 0–1 conflicts.
  5. State security boundaries explicitly, with a fork test. Upstream widened scheduled-task targets in hunks that merged cleanly. The fork boundary depended on the old implicit rule, and no conflict or type error showed this.
  6. Put fork code beside upstream code, not inside it. Re-indented upstream blocks, renamed upstream functions and copied upstream pipelines each conflicted with small upstream edits.
  7. Do not rely on upstream side effects or internals. A fork read depended on an upstream cache write that upstream moved. Only a return-type change exposed it.
  8. Every new upstream RPC is ungated in the fork until someone adds the gate. Audit new RPCs and routes on every merge, or make the fork gate the default for all handlers.
  9. Run lint, format, the import scan and the fork failure-path tests locally after every merge. Each one found problems that CI would otherwise find one 33-minute round at a time.

Model: Claude Opus 5.5 (orchestrator plus Opus subagents), Claude Code harness in T3 Code.

🤖 Generated with Claude Code

jakeleventhal and others added 30 commits October 3, 2026 09:45
…ingdotgg#15254)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com>
…ge histories (pingdotgg#15149)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ingdotgg#14992)

Chat stays centered while the workspace card fits beside it with 32px to spare. When it does not fit, chat moves left only as far as needed, narrows only after it reaches the left padding, and the card becomes a popover below a 640px chat. The card is lighter: 280px wide, 32px rows, no section labels, no "Project folder" hint.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…gdotgg#15330)

Takes over pingdotgg#14876.

Co-authored-by: tris203 <admin@snappeh.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ngdotgg#15333)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… you (pingdotgg#15346)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…otgg#15009)

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…itch (pingdotgg#15355)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…own (pingdotgg#15388)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…dotgg#15334)

Co-authored-by: scratchyone <11479077+scratchyone@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…" in release builds (pingdotgg#15142)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: saphid <saphid@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
saphid and others added 16 commits October 6, 2026 04:19
…g#13217)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ingdotgg#16400)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Yash Singh <saiansh2525@gmail.com>
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
…#16515)

Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Merges 240 upstream commits (aad7329..78552a8, 2026-10-03 → 2026-10-06)
into stage. 77 files conflicted; fork features are kept.

Beyond the conflicts, the merge also carries:
- Effect 4.0.1 (pingdotgg#16138): fork files move from `effect/unstable/*` to `effect/*`.
- Server modules flattened (pingdotgg#16295) and layers renamed (pingdotgg#16282): fork
  imports and tests follow the new paths and names.
- MCP scope reshape (pingdotgg#15219): the fork's principal/actor fields sit on
  upstream's thread/client/requestNamespace shape; external callers are
  thread-less clients; pull-request tools take upstream's `threadId`.
- Upstream migrations 057/058 run at fork ids 1046/1047.
- New lint rules (prefer-catch-tags, suppression reasons) applied to fork code;
  `node:crypto` in fork files is suppressed with a reason (follow-up: Effect Crypto).
- New worktree branches use upstream's `t3/` prefix (`t3/<codename>`).
- Root .gitmodules declares upstream's two vendored gitlinks (update = none).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Merge difficulties log (full)

Every place this merge cost time, with a lesson for how fork add-ons should be built. D1–D13 in the order they were found.

D1. Upstream flattened Services/ + Layers/ folders (pingdotgg#16295, 191 files) — silent breakage

  • 42 fork-owned files, 51 relative imports, point at paths that no longer exist.
    Git reports NO conflict for any of them (the fork files are new files; only the
    imported path moved). Only typecheck finds them.
  • Same family: refactor: layer variables are named layer or layerXyz pingdotgg/t3code#16282 renamed layer variables to layer/layerXyz, refactor(server): import service modules as namespaces, not aliased layers pingdotgg/t3code#16267 made
    service imports namespaces. Fork code that names FooLive breaks the same silent way.
  • Lesson for add-ons: a fork module that deep-imports upstream internals by path is
    coupled to upstream's folder layout. Prefer importing through one fork-owned
    adapter module per upstream service (fork/upstreamServices.ts re-exporting what
    fork code needs), so a layout refactor is a one-file fix, not 42.
  • Lesson for the merge procedure: after any merge, run a "relative import resolves?"
    scan over the tree before typecheck. It takes 2 s and gives the exact list.
  • Measured fix: only 5 old module paths were involved (persistence/Layers/Sqlite,
    persistence/{Layers,Services}/OrchestrationEventStore, persistence/Layers/OrchestrationCommandReceipts,
    provider/Services/ProviderRegistry, provider/Services/ProviderAuthService) but they were imported
    from 36 fork files. 36 edits for 5 moved modules = the cost of deep imports.
  • Tooling trap hit during the fix: a scripted path rewrite produced ./Layerspersistence/Sqlite.ts for
    files that sit inside the moved folder (relative spec shorter than the module key). The import
    re-scan caught it. Always re-run the resolve scan after a scripted rewrite.

D2. Dependencies both sides added independently

  • mermaid: fork pinned 11.16.1; upstream later added ^11.17.2 itself. Same package, two owners.
    Took upstream's. Lesson: when upstream adopts a dependency the fork already had, drop the fork pin
    in the next merge, or the conflict repeats.
  • third-party-licenses.config.json: both sides appended to the same array tail, and both added a
    khroma entry (fork: bare; upstream: full notice). JSON cannot carry markers, so the fork block is
    invisible to the marker check. Lesson: keep fork license entries in a separate fork-owned file that
    the license tool merges, if the tool allows it; otherwise this conflicts every merge.
  • pnpm-lock.yaml: took upstream's lock and ran pnpm install --lockfile-only (58 s). Side effect:
    5 fork-only transitive packages moved (@platejs/core|slate|utils → 53.3.15, slate-react →
    0.127.1, slate-hyperscript → 0.127.0). Fork pins are on the top-level platejs only, so its
    transitive deps float on every regenerate. Risk: the chat-comments editor. Lesson: pin the editor
    stack through a fork-owned catalog entry or pnpm.overrides if exact versions matter.

D3. New upstream lint rules apply to fork code too

D4. Effect went stable (4.0.0-rc.115 → 4.0.1, upstream pingdotgg#16138): module paths moved, no conflicts

  • Upstream rewrote all 977 effect/unstable/<x> imports to effect/<x> (plus httpapi → http-api,
    effect/Encoding split into effect/encoding/*). Upstream files took this automatically.
  • 160 fork-owned files still imported effect/unstable/... (82 of them sql/SqlClient). Git showed no
    conflict for any of them; the old paths simply stop existing in 4.0.1. Fixed with one sed
    (effect/unstable/ → effect/) because the fork used only the 15 paths that map 1:1.
  • Lesson for add-ons: fork code depends on the exact framework version upstream pins. A framework
    upgrade upstream is a fork-wide change. Two cheap defences: (1) a post-merge scan that every bare
    import specifier still resolves (same as the relative scan in D1, but for packages), and (2) keep
    fork code on the same import style upstream uses, so upstream's own codemod commit can be replayed
    on fork files (git show 194c73f3f gives the full mapping).
  • Measured (non-conflicted files): 12 lint errors, all prefer-catch-tags, in 4 files
    (auth/http.ts 9, auth/ExchangeIdentity.ts, ProviderTurnStartService.ts, managerGuard.expbkt3.ts).
    The rule has no auto-fix, so a small rewrite script did catchTag("X", h) → catchTags({ X: h }),
    then vp fmt on the 4 files. Fork code in upstream files (auth/http.ts holds the Clerk login path)
    carries upstream's lint debt too, because upstream fixed only its own lines.

D5. Contracts / client-runtime / desktop / mobile (13 files) — resolver report

D6. Web client (12 files) — resolver report + typecheck findings

D7. Server core (12 files) — resolver report

D8. Orchestration-v2 / persistence / provider (18 files + 4 structural) — resolver report

D9. MCP server (14 files) — resolver report — the biggest design cost of this merge

  • Upstream feat(server): T3 MCP tools take explicit thread and project targets pingdotgg/t3code#15219 reshaped the MCP invocation scope (flat threadId/providerSessionId/
    providerInstanceId → thread?, client, requestNamespace). The fork had added principal,
    actorUserId, backgroundGrantHash to the SAME interface and faked external callers as synthetic
    threads (threadId: "external-user:<id>"). Result: the scope had to be redesigned, and every fork
    consumer of scope.threadId broke — ~75 typecheck errors in ~30 files git reported as clean.
    Lesson: keep fork identity in a separate fork-owned context service (like
    CurrentOrchestrationActorUserId), and read scope fields through one fork accessor. Never extend a
    core upstream interface with fork fields, and never fake upstream data (synthetic thread ids).
  • OrchestratorMcpService.ts (12 hunks): the fork pasted the same 3-line access check before every
    loadProjection(scope.threadId); upstream replaced every such call site with loadCaller helpers
    → 10 near-identical hunks. Lesson: ONE fork gate (decorate the read service, or hook the shared
    caller loader) → 0–1 hunks.
  • Pull-request tools: the fork added a sibling sessionId parameter; upstream added threadId for
    the same purpose. Resolved by adopting upstream's threadId (API change on stage; no caller in
    bk-docs / bridge / toolyard passes sessionId). Lesson: extend upstream's parameters, never add a
    parallel one; put only the authorization in fork code.
  • SILENT SECURITY RISK: feat(server): T3 MCP tools take explicit thread and project targets pingdotgg/t3code#15219 also widened scheduled-task tools beyond the caller's own project in
    hunks that merged cleanly. The fork's team boundary had relied on upstream's implicit "calling
    project only" rule. The resolver added explicit project-access checks (requireForkTargetProject).
    No conflict and no type error would ever have shown this. Lesson: state fork security boundaries
    explicitly in one policy hook, with a fork test that asserts "outside project → refused"; that test
    turns red the next time upstream widens a target.
  • node:crypto: upstream made nodeBuiltinImport a typecheck error and moved to Effect Crypto
    (refactor: Effect code gets UUIDs and SHA-256 from Effect's Crypto pingdotgg/t3code#16377). 17 fork files used node:crypto; suppressed with a reason (upstream's own pattern), the
    real migration is a follow-up.
  • Server typecheck after resolution: 214 errors in ~60 files, ~95% in fork-owned files or fork
    tests that git merged "cleanly". The conflict count (77 files) understates the work by about half.

D10. Upstream changed a shared constant the fork builds on (behaviour change, no conflict, no type error)

  • feat: new branches use the shorter t3/ prefix pingdotgg/t3code#16220 changed WORKTREE_BRANCH_PREFIX from t3code to t3. The fork's codename allocator
    (worktreeNaming/allocateWorktreeDirectory.ts) builds branch names from that constant, so new stage
    worktree branches become t3/<codename> instead of t3code/<codename>. Existing t3code/*
    branches still count as taken (the allocator strips any prefix), so codenames do not collide.
  • 6 fork test assertions hard-coded t3code/${codename} and would fail only at test time.
  • Lesson: fork tests should derive expectations from the same constant the code uses
    (${WORKTREE_BRANCH_PREFIX}/…), and a fork feature that depends on an upstream constant should
    say so in a comment at the use site so a reviewer of the next merge looks for changes to it.
  • Tooling outside this repo that parses t3code/ branch names (bridge, scripts, skills) may need
    the t3/ form too. Not checked in this merge.

D11. Server typecheck fix-up, orchestration group (15 files)

  • Renamed upstream test helpers (refactor: layer variables are named layer or layerXyz pingdotgg/t3code#16282: makeLayer, makeSingleLayer, workerLive, makeTestLayer,
    makeOrchestratorV2ReplayLayerWithRegistry) used directly by ~8 fork tests. Fixing the one fork
    testkit (ForkCompatibility.expbkt3.ts) cleared the errors in 4 other files at once — proof that a
    single fork testkit is the right seam. Lesson: route all fork tests through fork testkits.
  • NEAR-SILENT BREAK: fork UsageService.readThreadUsage relied on a SIDE EFFECT of upstream
    readFileRecords (it wrote the scan cache). Upstream perf(usage): cut warm usage scans from seconds to milliseconds on large histories pingdotgg/t3code#15149 moved that write to the caller. TypeScript
    caught it only because the return type also changed; had it stayed an array, per-thread usage reads
    would have quietly stopped warming the cache. Lesson: never rely on upstream side effects; add a
    fork test that asserts the behaviour you rely on ("a thread read warms the scan cache").
  • Fork imported an internal upstream helper (addTotals) that upstream's perf rewrite deleted.
    Lesson: a fork module owns its tiny helpers instead of importing upstream internals.
  • Fork-added REQUIREMENTS on upstream services (clerkAuth, SqlClient for ownership backfill,
    SessionArchiveSweeper at startup; SourceControlProfileService in the PR watch reactor;
    actorUserId/upstreamServers on McpProviderSessionConfig) break EVERY new upstream test that
    builds those services. Lesson: wire fork extras inside the fork layer, or make them optional
    (Effect.serviceOption, optional fields with defaults), so upstream requirement types do not change.

D12. Dependency behaviour change broke a fork test's failure injection (found only by running tests)

  • Fork test reconcileV2PreviewMigration.test.ts ("rolls back schema and ledger together on failure")
    injected a failure with a SQLite trigger RAISE(ABORT) on the migrations ledger insert. Effect
    4.0.1's Migrator now maps ANY constraint error on that insert to MigrationError{kind:"Locked"}
    and returns [] (it assumes a concurrent run). RAISE(ABORT) is a constraint error, so the run
    "succeeded" with nothing applied, and the test failed. Fixed by injecting a plain SQL error
    (trigger writes to a missing table). No type error, no conflict; only running the test showed it.
  • Real-world note: with Effect 4.0.1, a constraint error on the migrations ledger insert no longer
    fails startup — it is treated as "another process is migrating" and silently skipped.
  • Lesson: fork tests that simulate failures depend on the exact error classification of the
    framework. After a framework upgrade, run the fork's failure-path tests locally before CI.

D13. Repository plumbing upstream does not need but the fork does

  • Upstream chore(refs): sync Effect and Alchemy references to 4.0.1 and beta.80 pingdotgg/t3code#16170 added two gitlinks (.repos/alchemy-effect/submodules/{distilled,floci}) with no
    root .gitmodules. Upstream CI sparse-checks-out without .repos/, so it never notices. The fork's
    deploy-stage.yml checks out the full tree, and (per the 2026-09-15 merge) actions/checkout
    fails on unmapped gitlinks. Added a marked root .gitmodules with update = none.
  • webUi MCP tool-count tests (226 → 230 RPCs, 31 → 30 streams): upstream added 7 RPCs and removed
    3 (preview automation). Verified by counting Rpc.make( (173 → 177) before bumping. These counts
    drift on every merge. Lesson: assert the count against WsRpcGroup.requests.size only (the test
    already does that too) and drop the hard-coded number, or keep it in one fork constant.
  • Fork test passed a fake turn input ({ threadId } as never); upstream's new metrics read
    input.modelSelection.model → runtime TypeError. Lesson: as never fixtures hide exactly the
    fields upstream starts to read; build fixtures from real constructors.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL labels Oct 6, 2026
stage moved to 668aab6 (PR #277: expbkmain and the Claude rotation port)
while the upstream merge was in progress. Two conflicts: one import line in
ClaudeAdapterV2.test.ts, and the restored claudeUsageLimits test, which git
moved out of provider/Layers/ (flattened upstream). publish-bk-desktop-dmg.ts
follows the new prefer-catch-tags lint rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. label Oct 6, 2026
bk-agent-01 and others added 5 commits October 6, 2026 19:20
RestartContinuation now reads turn items and recovers delegated tasks, and
upstream's thread-search test mocks every atom, which the fork's offline
search reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er owner

- orchestration.getTurnItem and secrets.answerRequest (new upstream RPCs) now
  require thread access, like the neighbouring diff and projection RPCs.
- PR watch groups split by owner and source-control profile, so a group's
  reads never use another owner's credentials (regression from the merge).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mping the nightly version

Upstream's server browser tests (pingdotgg#15328) need a headless Chromium, which
upstream CI installs. A new upstream CLI test reads the release channel from
the package version, so the stage job now tests against the committed versions,
as upstream CI does, and stamps the nightly version only for the build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bk-agent-01
bk-agent-01 merged commit 8fc28c6 into stage Oct 6, 2026
24 of 26 checks passed
@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Merge difficulties log, part 2 (D14–D18)

Found after the first comment. D14–D18, in the order they were found.

D14. The base branch moved during the merge

D15. CI round 1 and the fresh review found what local checks missed

  • CI (Test Server 2, Test Web) found 2 failures in tests that local focused runs did not include:
    • RestartContinuation.manager.expbkt3.test.ts: fork mocks lacked what upstream's new restart
      logic reads (turnItems, recoverDelegatedTask, message text/attachments). as never
      fixtures again.
    • Upstream's new queries.threadSearch.test.tsx mocks useAtomValue for EVERY atom; the fork's
      offline search inside the same hook reads environment atoms → crash. Fork code inside an
      upstream hook is exercised by upstream's tests with upstream's mocks.
  • Fresh-context review found 3 real gaps no test or type caught:
    • NEW upstream RPCs (orchestration.getTurnItem, secrets.answerRequest) arrived without the
      fork's per-thread access check. Every upstream RPC added after the fork's access model was
      written is ungated by default. Fixed with requireThreadAccess.
    • PR watch reads were grouped by project+PR only, so two owners shared the first owner's
      credentials — a regression created during conflict resolution. Fixed: groups split by owner
      and source-control profile; new runtime test "reads a watched pull request once per thread owner".
    • Follow-ups only: server-browser stream routes and GitHub fingerprint batch keys.
  • Test-isolation trap: the new runtime test left two watches open in a store shared by the whole
    suite, which made a LATER upstream test count 3 reads instead of 1. Clean up shared state in
    fork tests added to upstream suites.
  • Marker trap: re-wrapping an upstream Pick<…> union onto several lines counts as editing
    upstream lines. Add fork fields with & Pick<…> inside BEGIN/END markers instead.
  • Lesson: the fork's access model is "opt-in per handler". A merge that adds RPCs/routes needs an
    explicit audit: list git diff <base> <upstream> -- packages/contracts/src/rpc.ts additions and
    check each new handler for the fork gate. Better: make the gate the default (one wrapper that
    every handler goes through, with an explicit allowlist for public ones).

D16. Upstream CI assumes runners the fork does not have

  • This merge carries native mobile changes, so upstream's ci.yml job "Mobile Native Static Analysis"
    ran instead of skipping. It needs blacksmith-6vcpu-macos-26 (upstream's paid Blacksmith runner),
    which this repository does not have, so it stays queued forever, and the aggregate check job
    (needs every job) never finishes. Earlier PRs into stage skipped that job, so nobody noticed.
  • The stage deploy is unaffected: deploy-stage.yml (validate, ubuntu-latest) is the only gate the
    deploy timer reads.
  • Lesson: after a merge, diff runs-on: labels in .github/workflows/ against the runners the fork
    has; either map them in a fork-owned override or mark those jobs as not applicable.
  • Second Blacksmith-only job: "Transfer report artifact" (blacksmith-2vcpu-ubuntu-2404, added by
    upstream ci: run the transfer report job on Blacksmith pingdotgg/t3code#16178) also queues forever on this repository.

D17. The stage gate runs all suites on one runner; upstream's own timeouts assume it does not

  • Stage validate failed: MessagesTimeline.test.tsx beforeAll timed out at 30 s while importing
    the component. The CI "Test Web" job ran the same file on the same commit and passed. Upstream had
    already raised that hook to 30 s; this merge made the import heavier (mermaid diagrams, shell
    syntax highlighting), and deploy-stage.yml runs every package's tests at once on one runner.
    Raised to 120 s with a marker. Cost: one more 35-minute CI round.
  • Lesson: the fork's deploy gate is a different test environment from upstream CI (one runner, all
    suites in parallel). Either run the stage gate's tests with the same sharding as upstream CI, or
    expect upstream's import-heavy tests to flake there.
  • Third CI round: Test Server 6 failed in upstream's AcpRegistryAdapterV2.test.ts (replay status
    file read mid-write: "Unexpected end of JSON input"). Files are identical to upstream and passed on
    the previous head → upstream flake. gh run rerun --failed is refused because the CI run never
    finishes (the Blacksmith jobs stay queued), so single-job reruns are impossible on this repository
    while those jobs exist. Merge evidence instead: previous head green on that shard + stage
    validate re-runs the whole server suite.

D18. The fork's deploy workflow drifts from upstream CI's runner setup

  • Stage validate (round 3) failed on 2 server tests that passed in the regular CI jobs:
    • cli/invocation.test.ts (new upstream): expects sudo npx t3 browser setup, got
      t3@nightly …, because deploy-stage.yml stamped the nightly version into package.json
      BEFORE running tests. Upstream CI tests against committed versions.
    • preview/ServerBrowserPage.test.ts (new upstream, feat(preview): run the browser on the environment server pingdotgg/t3code#15328): needs headless Chromium, which
      upstream's ci.yml installs (playwright-core/cli.js install --with-deps --only-shell chromium)
      and the fork workflow did not.
  • Fixed in the fork-owned workflow: install Chromium; stamp versions after tests.
  • Lesson: the fork's deploy workflow copies part of upstream CI. Each merge must diff upstream's
    ci.yml setup steps (git diff <base> <upstream> -- .github/workflows/ci.yml) and port new runner
    prerequisites to deploy-stage.yml. Better: have the deploy workflow reuse upstream's test job
    (or needs: the CI workflow's result) instead of re-running a hand-maintained copy.
  • Each such miss costs one ~35-minute CI round. This merge needed 4 rounds.

@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Merged by bk-agent-01 at head 4790aa334 (all 24 runnable checks green, including stage validate). Go: Tushar asked in the T3 session "Deploy Latest T3Code Upstream to Stage" (2026-10-06) to pull the latest upstream main into stage and deploy it. The Blacksmith-only jobs (Mobile Native Static Analysis, Transfer report artifact) cannot run on this repository and were not treated as blockers.

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.