Skip to content

chore: sync with upstream/main (ec80933ac) - #58

Merged
Fryuni merged 66 commits into
mainfrom
fryuni/sync-upstream-main-2
Oct 9, 2026
Merged

Fryuni merged 66 commits into
mainfrom
fryuni/sync-upstream-main-2

Conversation

@Fryuni

@Fryuni Fryuni commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Problem

The fork is 65 commits behind upstream/main. This sync must preserve the features and trade-offs recorded in docs/adr while using upstream implementations wherever possible.

Change

Merged pingdotgg/t3code main at ec80933ac8cd02fec5c97b342462ccc9567cdb1e into the fork, retaining upstream's provider package extraction and all other upstream changes.

The only content conflicts were in PullRequestService.test.ts and SourceControlProviderRegistry.test.ts. Both now retain the fork's Forgejo identity/discovery coverage and upstream's GitHub Enterprise coverage and credential fixture. No fork-specific runtime changes were needed.

Reviewed all 11 ADRs against this upstream revision. None of the 10 active decisions is newly implemented upstream. ADR 0013 remains superseded: its note now explains upstream's local desktop preview default and the action to move a tab to the environment-hosted browser. All ADR comparison revisions are updated. The fork release define and publishing guard remain intact, and PR #57's worktree-startup cancellation fix is preserved.

Scope and approval

Maintainer-requested upstream sync. Review the conflict resolutions and integration of existing fork behavior. Problems already present on either Fryuni/t3code main at de1c50e28 or upstream main at ec80933ac are outside this PR's scope.

Verification

  • Server, web, and mobile typechecks passed.
  • 33 focused test files, 1,281 tests passed: Forgejo identity/discovery and PR routing; public URL/session policy; worktree launch, remote selection, MCP handoff and startup cancellation; model qualifiers; project settings; contracts; moved orchestration instructions.
  • Targeted lint and formatting passed for both conflict resolutions; ADR formatting passed.
  • The committed remerge diff contains only the two test conflict resolutions and ADR updates. No custom UI behavior was added; upstream UI changes were retained. No browser/manual UI pass was run.
  • Repository-wide checks are left to CI.

Model and harness: GPT-6 Astra (ultra), Codex in T3 Code.

t3dotgg and others added 30 commits October 8, 2026 03:01
…#17152)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…pos (pingdotgg#15946)

Co-authored-by: PR Batch Tester <agent@local.test>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…0439)

Co-authored-by: Yash Singh <saiansh2525@gmail.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…id (pingdotgg#17211)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rs (pingdotgg#17077)

Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
…otgg#17214)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…otgg#14314)

Co-authored-by: PR Batch Tester <agent@local.test>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…otgg#17271)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…17291)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…gg#11059)

Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
…dflared (pingdotgg#17275)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…pingdotgg#16998)

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

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ges (pingdotgg#17299)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…gg#17300)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…gg#17302)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…otgg#17307)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Yash-Singh1 and others added 21 commits October 9, 2026 00:52
…Pi, and testing (pingdotgg#17375)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…age (pingdotgg#17345)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…age (pingdotgg#17354)

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

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ilscale (pingdotgg#17158)

Co-authored-by: PR Batch Tester <agent@local.test>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T13:09:48.488100Z 43f6f3a Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Fryuni

Fryuni commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@fryuni-s-proval Please review this upstream sync at 43f6f3a and explicitly confirm approval if there are no in-scope findings.

For both reviewers: focus on merge resolution and interaction between upstream changes and the fork's ADR features. The merge parents are fork main de1c50e and upstream main ec80933. The two test conflicts retain both Forgejo and GitHub Enterprise coverage; the ADR notes describe the remaining differences. No feature was newly superseded upstream, and ADR 0013 continues using upstream's preview implementation.

Please do not expand this sync to fix problems already present on either parent/main. For any suspected regression, distinguish an integration defect from unchanged fork code or code carried directly from upstream. Local verification: 1,281 focused tests and server/web/mobile typechecks passed; CI is running.

@fryuni-s-proval

Copy link
Copy Markdown

Thanks for the work on this update, especially the session-lifecycle hardening.

Overview

Summary

The inspected changes add thread-search RPCs, preview profile reporting, and WebSocket termination when an authenticated session is revoked or expires. Preview sessions also gain confirmation dialogs for external-protocol navigation.

This review was limited to the authentication, WebSocket wiring, HTTP sandbox policy, and desktop preview-session diffs; it does not constitute comprehensive approval of the 867-file PR. No review-unit handoffs were supplied, so the remaining changes still need review before merge.

Good points

🟢 apps/server/src/auth/SessionStore.ts:1058 subscribes to session changes before checking the stored credential, covering revocation that races with connection setup.
🟢 apps/server/src/auth/SessionStore.test.ts:118 adds coverage for invalidating multiple connections through revocation, replacement, and bulk revocation, while apps/server/src/auth/SessionStore.test.ts:149 checks expiration at the credential deadline.

Main Issues

No actionable defects were verified in the inspected portions. The merge recommendation remains pending review of the broader change set.

@fryuni-s-proval

Copy link
Copy Markdown

@Fryuni

Approved for the requested upstream-sync scope; I found no in-scope integration defects in the inspected diffs and head snapshot. This is approval expressed in this comment, not a formal GitHub approval submission—the available tools do not support submitting one.

The key merge/integration checks were:

  • Both sides of the test conflicts are retained. Forgejo instance-path isolation remains in apps/server/src/pullRequest/PullRequestService.test.ts:519–611, alongside the added Enterprise discovery/routing test at :614–660. The registry retains the requested-authority/cache test at apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts:315–357, adds Enterprise cases at :415–485, and defaults the GitHub credential mock to “not signed in” at :105–109, avoiding accidental claims of custom hosts in existing fixtures.
  • Provider extraction does not replace fork launch behavior. The ThreadLaunchService.ts diff only changes imports (apps/server/src/orchestration-v2/ThreadLaunchService.ts:49–52); existing-branch validation and the startup-cancellation cleanup remain, including clearing workspace metadata only after removal succeeds (:650–692). Forgejo CLI's change is likewise an import relocation (apps/server/src/sourceControl/ForgejoCli.ts:19).
  • PR checkout preference and model qualifiers remain wired in. The shared opener still prefers the thread's matching project within its environment (apps/web/src/lib/openPullRequestLink.ts:175–198). Web rows/search and mobile options still use getModelProviderLabel (apps/web/src/components/chat/ModelListRow.tsx:47, apps/web/src/components/chat/modelPickerSearch.ts:23, apps/mobile/src/lib/modelOptions.ts:194).
  • Public-URL auth and fork release safeguards remain intact. Session construction still passes publicUrl into cookie-policy resolution (apps/server/src/auth/SessionStore.ts:501–510). The release define remains at apps/server/vite.config.ts:116–118, and the publishing workflow still supplies the repository and checks the executable bundle before publishing (.github/workflows/server-linux-release.yml:60,106). Automatic worktree cleanup still removes without force, as ADR 0012 requires (apps/server/src/storageCleanup.ts:398–399).
  • ADR 0013's revised explanation matches the preview changes. Remote desktop previews default to the local browser, with an alternate server runtime available (apps/web/src/browser/previewRuntime.ts:20–43), consistent with docs/adr/0013-connection-localhost-url-templates.md:8–18.

I did not turn unchanged fork behavior or unrelated imported changes into findings. This was a static review: I could not independently run the reported 1,281 tests/typechecks, inspect CI status, or generate a parent-specific remerge diff with the available tools. The reported local verification is therefore supporting evidence, not verification I performed.

@Fryuni

Fryuni commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

@fryuni-s-proval Your inspected portions have no actionable defects. Please finish the review of this merge's integration scope and give an explicit approve/request-changes recommendation on 43f6f3aad823f5fc5269be54d7cc8bb05b4101ef.

This is a maintainer-requested sync, not an 867-file feature implementation. The maintainer explicitly excludes problems already on either main. Most files come verbatim from upstream. For example, apps/server/src/http.ts and apps/desktop/src/preview/BrowserSession.ts are identical to upstream ec80933ac; SessionStore's only fork delta is passing publicUrl to cookie-name selection. Please avoid blocking on a comprehensive re-review of upstream's own changes.

Concrete review units:

  1. Manual merge resolutions. git show --remerge-diff 43f6f3aad shows only two test files plus the 11 ADR updates. PullRequestService.test.ts keeps both case-sensitive Forgejo instance cache tests and upstream's stored-unknown GitHub Enterprise listing test. SourceControlProviderRegistry.test.ts keeps TestClock, the fork detector import, and upstream's Redacted; the merged fixture retains the upstream default missing-GitHub-credential mock and fork login injection. These suites pass.
  2. Automatic integration of existing ADR features. ADRs 0001–0004: Forgejo discovery/identity and checkout-preferred PR opening; upstream's new GitHub Enterprise discovery should still compose with the fork cache. ADRs 0007–0009: slug labels, existing-branch launches, project base and remote choices; verify upstream's package moves and model/settings changes retain these. In particular fork launch guidance moved into packages/provider-core/src/server/orchestrationInstructions.ts. ADRs 0010–0012: public URL, fork release define/publish guard, ignored-file cleanup. No new runtime logic was added to resolve this merge. We independently audited these groups and exercised their focused tests.
  3. ADR accuracy. No additional fork feature is implemented upstream in this range. ADR 0013 remains superseded and now describes upstream fix(web): desktop opens remote environments' browser tabs locally pingdotgg/t3code#17316's local desktop default plus the move-to-environment-browser action. Keep upstream's implementation.

Use the two parent snapshots (de1c50e28 and ec80933ac) to distinguish integration regressions from pre-existing issues. An actual interaction regression is in scope even if it auto-merged. Additional test coverage or unrelated cleanup is not required absent such a defect.

@fryuni-s-proval

Copy link
Copy Markdown

@Fryuni

Recommendation: APPROVE 43f6f3aad823f5fc5269be54d7cc8bb05b4101ef for the maintainer-defined upstream-sync integration scope. I found no actionable integration regression and have no request-changes findings. I am not making approval contingent on re-reviewing upstream’s imported implementation, unrelated cleanup, or additional test coverage.

I completed the remaining targeted static checks:

  • Manual test resolutions: Both case-sensitive Forgejo cache tests remain at apps/server/src/pullRequest/PullRequestService.test.ts:519–611, alongside the stored-unknown Enterprise listing test at :614–660. The registry fixture preserves both the default missing-GitHub-credential mock and fork login injection (apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts:105–114), with the requested-authority/cache test retained at :315–357 and Enterprise discovery cases added at :415–485.
  • Discovery/cache composition: Refinement still caches the full remote identity, including baseUrl and requestedHost (apps/server/src/sourceControl/SourceControlProviderRegistry.ts:225–265). Enterprise discovery operates through the existing managed-provider refinement path, preserves the supplied base URL, and skips an empty base URL (apps/server/src/sourceControl/GitHubSourceControlProvider.ts:264–282; apps/server/src/sourceControl/SourceControlProviderDiscovery.ts:299–334). I found no loss of the fork’s authority separation in that integration.
  • Provider extraction, launch guidance, and settings: The moved instructions retain existing-branch launch semantics and startFromRemote precedence (packages/provider-core/src/server/orchestrationInstructions.ts:21–25), and the provider adapters reference the new module. ThreadLaunchService changes only imports; its existing-branch path still suppresses remote fetching (apps/server/src/orchestration-v2/ThreadLaunchService.ts:49–52,342–346,432–442). Origin fallback and explicit-upstream failure behavior remain in apps/server/src/git/GitWorkflowService.ts:304–380. The project-only base setting survives the settings extraction (packages/contracts/src/settings.ts:987–1018), remains preserved by scoped writes (apps/web/src/components/settings/scopedSettings.ts:316–328), and remains consumed by web and mobile launch selection (apps/web/src/hooks/useHandleNewThread.ts:152,276–287; apps/mobile/src/features/threads/new-task-flow-provider.tsx:1103–1127). Model qualifier labels remain wired into web rows/search and mobile options (apps/web/src/components/chat/ModelListRow.tsx:47; apps/web/src/components/chat/modelPickerSearch.ts:23; apps/mobile/src/lib/modelOptions.ts:194,249).
  • Remaining safeguards and ADRs: Public URL still reaches cookie policy (apps/server/src/auth/SessionStore.ts:501–510); the fork release define and pre-publication bundle guard remain (apps/server/vite.config.ts:116–118; .github/workflows/server-linux-release.yml:60,106); ignored-file cleanup remains non-forced (apps/server/src/storageCleanup.ts:398–399). All ten active ADR diffs only update the comparison revision. ADR 0013 remains superseded, and its revised local-desktop-default/environment-browser explanation matches apps/web/src/browser/previewRuntime.ts:20–43 (docs/adr/0013-connection-localhost-url-templates.md:8–18).

Verification boundary: This is a static-review recommendation, not a formal GitHub approval submission. The available tools cannot check out arbitrary parent snapshots, run git show --remerge-diff, execute tests, or inspect CI. I therefore did not independently verify the parent-to-head identity claims or rerun the reported passing suites. Those limitations are not findings or reasons to request changes.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 43f6f3aad8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Fryuni
Fryuni merged commit 2e2b14e into main Oct 9, 2026
9 checks passed
@Fryuni
Fryuni deleted the fryuni/sync-upstream-main-2 branch October 9, 2026 13:12
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.