Prepare datamodel for multi-environment#1765
Open
juliusmarminge wants to merge 24 commits intot3code/pr-1708/web/atomic-store-refactorfrom
Open
Prepare datamodel for multi-environment#1765juliusmarminge wants to merge 24 commits intot3code/pr-1708/web/atomic-store-refactorfrom
juliusmarminge wants to merge 24 commits intot3code/pr-1708/web/atomic-store-refactorfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Redundant no-op identity map in effect pipeline
- Removed the no-op
Effect.map((option) => option)line that was a leftover from refactoring.
- Removed the no-op
- ✅ Fixed: Replay events enriched twice in subscription handler
- Moved
enrichProjectEventto only wrap the livestreamDomainEventsstream so already-enriched replay events skip the redundant enrichment pass.
- Moved
- ✅ Fixed: Selector creates unstable array references every call
- Added a per-projectScopedId reference-equality cache that returns the previous result array when the underlying scopedIds and sidebarMap references haven't changed.
Or push these changes by commenting:
@cursor push 481ff4254f
Preview (481ff4254f)
diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts
--- a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts
+++ b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts
@@ -748,7 +748,6 @@
"ProjectionSnapshotQuery.getActiveProjectByWorkspaceRoot:decodeRow",
),
),
- Effect.map((option) => option),
Effect.flatMap((option) =>
Option.isNone(option)
? Effect.succeed(Option.none<OrchestrationProject>())
diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -503,7 +503,10 @@
Effect.catch(() => Effect.succeed([] as Array<OrchestrationEvent>)),
);
const replayStream = Stream.fromIterable(replayEvents);
- const source = Stream.merge(replayStream, orchestrationEngine.streamDomainEvents);
+ const liveStream = orchestrationEngine.streamDomainEvents.pipe(
+ Stream.mapEffect(enrichProjectEvent),
+ );
+ const source = Stream.merge(replayStream, liveStream);
type SequenceState = {
readonly nextSequence: number;
readonly pendingBySequence: Map<number, OrchestrationEvent>;
@@ -515,43 +518,33 @@
return source.pipe(
Stream.mapEffect((event) =>
- enrichProjectEvent(event).pipe(
- Effect.flatMap((enrichedEvent) =>
- Ref.modify(
- state,
- ({
- nextSequence,
- pendingBySequence,
- }): [Array<OrchestrationEvent>, SequenceState] => {
- if (
- enrichedEvent.sequence < nextSequence ||
- pendingBySequence.has(enrichedEvent.sequence)
- ) {
- return [[], { nextSequence, pendingBySequence }];
- }
+ Ref.modify(
+ state,
+ ({
+ nextSequence,
+ pendingBySequence,
+ }): [Array<OrchestrationEvent>, SequenceState] => {
+ if (event.sequence < nextSequence || pendingBySequence.has(event.sequence)) {
+ return [[], { nextSequence, pendingBySequence }];
+ }
- const updatedPending = new Map(pendingBySequence);
- updatedPending.set(enrichedEvent.sequence, enrichedEvent);
+ const updatedPending = new Map(pendingBySequence);
+ updatedPending.set(event.sequence, event);
- const emit: Array<OrchestrationEvent> = [];
- let expected = nextSequence;
- for (;;) {
- const expectedEvent = updatedPending.get(expected);
- if (!expectedEvent) {
- break;
- }
- emit.push(expectedEvent);
- updatedPending.delete(expected);
- expected += 1;
- }
+ const emit: Array<OrchestrationEvent> = [];
+ let expected = nextSequence;
+ for (;;) {
+ const expectedEvent = updatedPending.get(expected);
+ if (!expectedEvent) {
+ break;
+ }
+ emit.push(expectedEvent);
+ updatedPending.delete(expected);
+ expected += 1;
+ }
- return [
- emit,
- { nextSequence: expected, pendingBySequence: updatedPending },
- ];
- },
- ),
- ),
+ return [emit, { nextSequence: expected, pendingBySequence: updatedPending }];
+ },
),
),
Stream.flatMap((events) => Stream.fromIterable(events)),
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts
--- a/apps/web/src/store.ts
+++ b/apps/web/src/store.ts
@@ -1305,22 +1305,36 @@
]
: undefined;
+const _threadIdsByProjectCache = new Map<
+ string,
+ { scopedIds: string[]; sidebarMap: Record<string, SidebarThreadSummary>; result: ThreadId[] }
+>();
+
export const selectThreadIdsByProjectId =
(projectId: ProjectId | null | undefined) =>
- (state: AppState): ThreadId[] =>
- projectId
- ? (
- state.threadScopedIdsByProjectScopedId[
- getProjectScopedId({
- environmentId: state.activeEnvironmentId,
- id: projectId,
- })
- ] ?? EMPTY_SCOPED_IDS
- )
- .map((scopedId) => state.sidebarThreadsByScopedId[scopedId]?.id ?? null)
- .filter((threadId): threadId is ThreadId => threadId !== null)
- : EMPTY_THREAD_IDS;
+ (state: AppState): ThreadId[] => {
+ if (!projectId) return EMPTY_THREAD_IDS;
+ const projectScopedId = getProjectScopedId({
+ environmentId: state.activeEnvironmentId,
+ id: projectId,
+ });
+ const scopedIds = state.threadScopedIdsByProjectScopedId[projectScopedId] ?? EMPTY_SCOPED_IDS;
+ const sidebarMap = state.sidebarThreadsByScopedId;
+
+ const cached = _threadIdsByProjectCache.get(projectScopedId);
+ if (cached && cached.scopedIds === scopedIds && cached.sidebarMap === sidebarMap) {
+ return cached.result;
+ }
+
+ const result = scopedIds
+ .map((scopedId) => sidebarMap[scopedId]?.id ?? null)
+ .filter((threadId): threadId is ThreadId => threadId !== null);
+
+ _threadIdsByProjectCache.set(projectScopedId, { scopedIds, sidebarMap, result });
+ return result;
+ };
+
export function setError(state: AppState, threadId: ThreadId, error: string | null): AppState {
return updateThreadState(state, state.activeEnvironmentId, threadId, (t) => {
if (t.error === error) return t;You can send follow-ups to the cloud agent here.
apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts
Outdated
Show resolved
Hide resolved
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Unreachable fallback values in
resolveServerUrl- Removed the redundant third and fourth arguments from firstNonEmptyString since resolvePrimaryEnvironmentBootstrapUrl() already contains the complete fallback chain and always returns a non-empty string.
- ✅ Fixed: Cache returns stale null for later-configured remotes
- Skip caching null identity results so directories without a git remote are re-resolved on subsequent calls, allowing later-configured remotes to be detected.
Or push these changes by commenting:
@cursor push ffbf2a1e3c
Preview (ffbf2a1e3c)
diff --git a/apps/server/src/project/Layers/RepositoryIdentityResolver.ts b/apps/server/src/project/Layers/RepositoryIdentityResolver.ts
--- a/apps/server/src/project/Layers/RepositoryIdentityResolver.ts
+++ b/apps/server/src/project/Layers/RepositoryIdentityResolver.ts
@@ -115,12 +115,14 @@
}
const resolved = yield* Effect.promise(() => resolveRepositoryIdentity(cwd));
- yield* Ref.update(cacheRef, (current) => {
- const next = new Map(current);
- next.set(cwd, resolved.identity);
- next.set(resolved.cacheKey, resolved.identity);
- return next;
- });
+ if (resolved.identity !== null) {
+ yield* Ref.update(cacheRef, (current) => {
+ const next = new Map(current);
+ next.set(cwd, resolved.identity);
+ next.set(resolved.cacheKey, resolved.identity);
+ return next;
+ });
+ }
return resolved.identity;
});
diff --git a/apps/web/src/lib/utils.ts b/apps/web/src/lib/utils.ts
--- a/apps/web/src/lib/utils.ts
+++ b/apps/web/src/lib/utils.ts
@@ -53,12 +53,7 @@
pathname?: string | undefined;
searchParams?: Record<string, string> | undefined;
}): string => {
- const rawUrl = firstNonEmptyString(
- options?.url,
- resolvePrimaryEnvironmentBootstrapUrl(),
- import.meta.env.VITE_WS_URL,
- window.location.origin,
- );
+ const rawUrl = firstNonEmptyString(options?.url, resolvePrimaryEnvironmentBootstrapUrl());
const parsedUrl = new URL(rawUrl);
if (options?.protocol) {You can send follow-ups to the cloud agent here.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Nullish coalescing collapses null into fallback incorrectly
- Replaced
environmentId ?? resolveCurrentEnvironmentId()withenvironmentId !== undefined ? environmentId : resolveCurrentEnvironmentId()so explicitnullis preserved.
- Replaced
- ✅ Fixed: Multiple exported functions are unused outside definitions
- Removed
scopeThreadSessionRef,resolveEnvironmentClient,tagEnvironmentValue,attachEnvironmentDescriptor, and their supporting types (EnvironmentClientRegistry,EnvironmentScopedRef) from client-runtime.
- Removed
Or push these changes by commenting:
@cursor push 4730c6f1c2
Preview (4730c6f1c2)
diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx
--- a/apps/web/src/routes/__root.tsx
+++ b/apps/web/src/routes/__root.tsx
@@ -531,7 +531,10 @@
try {
const snapshot = await api.orchestration.getSnapshot();
if (!disposed) {
- syncServerReadModel(snapshot, environmentId ?? resolveCurrentEnvironmentId());
+ syncServerReadModel(
+ snapshot,
+ environmentId !== undefined ? environmentId : resolveCurrentEnvironmentId(),
+ );
reconcileSnapshotDerivedState();
if (recovery.completeSnapshotRecovery(snapshot.snapshotSequence)) {
void runReplayRecovery("sequence-gap");
diff --git a/packages/client-runtime/src/knownEnvironment.ts b/packages/client-runtime/src/knownEnvironment.ts
--- a/packages/client-runtime/src/knownEnvironment.ts
+++ b/packages/client-runtime/src/knownEnvironment.ts
@@ -1,4 +1,4 @@
-import type { EnvironmentId, ExecutionEnvironmentDescriptor } from "@t3tools/contracts";
+import type { EnvironmentId } from "@t3tools/contracts";
export interface KnownEnvironmentConnectionTarget {
readonly type: "ws";
@@ -37,14 +37,3 @@
): string | null {
return environment?.target.wsUrl ?? null;
}
-
-export function attachEnvironmentDescriptor(
- environment: KnownEnvironment,
- descriptor: ExecutionEnvironmentDescriptor,
-): KnownEnvironment {
- return {
- ...environment,
- environmentId: descriptor.environmentId,
- label: descriptor.label,
- };
-}
diff --git a/packages/client-runtime/src/scoped.ts b/packages/client-runtime/src/scoped.ts
--- a/packages/client-runtime/src/scoped.ts
+++ b/packages/client-runtime/src/scoped.ts
@@ -2,20 +2,10 @@
EnvironmentId,
ProjectId,
ScopedProjectRef,
- ScopedThreadSessionRef,
ScopedThreadRef,
ThreadId,
} from "@t3tools/contracts";
-interface EnvironmentScopedRef<TId extends string> {
- readonly environmentId: EnvironmentId;
- readonly id: TId;
-}
-
-export interface EnvironmentClientRegistry<TClient> {
- readonly getClient: (environmentId: EnvironmentId) => TClient | null | undefined;
-}
-
export function scopeProjectRef(
environmentId: EnvironmentId,
projectId: ProjectId,
@@ -27,34 +17,7 @@
return { environmentId, threadId };
}
-export function scopeThreadSessionRef(
- environmentId: EnvironmentId,
- threadId: ThreadId,
-): ScopedThreadSessionRef {
- return { environmentId, threadId };
-}
-
-export function scopedRefKey(
- ref: EnvironmentScopedRef<string> | ScopedProjectRef | ScopedThreadRef | ScopedThreadSessionRef,
-): string {
- const localId = "id" in ref ? ref.id : "projectId" in ref ? ref.projectId : ref.threadId;
+export function scopedRefKey(ref: ScopedProjectRef | ScopedThreadRef): string {
+ const localId = "projectId" in ref ? ref.projectId : ref.threadId;
return `${ref.environmentId}:${localId}`;
}
-
-export function resolveEnvironmentClient<TClient>(
- registry: EnvironmentClientRegistry<TClient>,
- ref: EnvironmentScopedRef<string>,
-): TClient {
- const client = registry.getClient(ref.environmentId);
- if (!client) {
- throw new Error(`No client registered for environment ${ref.environmentId}.`);
- }
- return client;
-}
-
-export function tagEnvironmentValue<T>(
- environmentId: EnvironmentId,
- value: T,
-): { readonly environmentId: EnvironmentId; readonly value: T } {
- return { environmentId, value };
-}You can send follow-ups to the cloud agent here.
juliusmarminge
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
Author
Contributor
|
Bugbot Autofix prepared a fix for the issue found in the latest run.
Or push these changes by commenting: Preview (b666cde275)diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -58,7 +58,7 @@
import { APP_STAGE_LABEL, APP_VERSION } from "../branding";
import { isTerminalFocused } from "../lib/terminalFocus";
import { isLinuxPlatform, isMacPlatform, newCommandId, newProjectId } from "../lib/utils";
-import { getProjectScopedId, getThreadScopedId, useStore } from "../store";
+import { getProjectScopedId, useStore } from "../store";
import { selectThreadTerminalState, useTerminalStateStore } from "../terminalStateStore";
import { useUiStateStore } from "../uiStateStore";
import {
@@ -785,14 +785,8 @@
.filter((thread): thread is NonNullable<typeof thread> => thread !== undefined),
[sidebarThreadsByScopedId, threadScopedIdsByProjectScopedId],
);
- const sidebarThreadByScopedId = useMemo(
- () =>
- new Map(
- sidebarThreads.map((thread) => [
- getThreadScopedId({ environmentId: thread.environmentId, id: thread.id }),
- thread,
- ]),
- ),
+ const sidebarThreadById = useMemo(
+ () => new Map(sidebarThreads.map((thread) => [thread.id, thread] as const)),
[sidebarThreads],
);
const routeTerminalOpen = routeThreadId
@@ -1144,13 +1138,7 @@
if (clicked === "mark-unread") {
for (const id of ids) {
- let thread: SidebarThreadSummary | undefined;
- for (const t of sidebarThreadByScopedId.values()) {
- if (t.id === id) {
- thread = t;
- break;
- }
- }
+ const thread = sidebarThreadById.get(id);
markThreadUnread(id, thread?.latestTurn?.completedAt);
}
clearSelection();
@@ -1181,7 +1169,7 @@
deleteThread,
markThreadUnread,
removeFromSelection,
- sidebarThreadByScopedId,
+ sidebarThreadById,
selectedThreadIds,
],
);You can send follow-ups to the cloud agent here. |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Sidebar
renderedThreadIdsdiverges fromrenderedThreadsusage- Removed the dead
renderedThreadIdscomputation from therenderedProjectsmemo and updatedgetVisibleSidebarThreadIdsto derive thread IDs fromrenderedThreadsinstead.
- Removed the dead
Or push these changes by commenting:
@cursor push b3888a4824
Preview (b3888a4824)
diff --git a/apps/web/src/components/Sidebar.logic.test.ts b/apps/web/src/components/Sidebar.logic.test.ts
--- a/apps/web/src/components/Sidebar.logic.test.ts
+++ b/apps/web/src/components/Sidebar.logic.test.ts
@@ -326,14 +326,17 @@
expect(
getVisibleSidebarThreadIds([
{
- renderedThreadIds: [
- ThreadId.makeUnsafe("thread-12"),
- ThreadId.makeUnsafe("thread-11"),
- ThreadId.makeUnsafe("thread-10"),
+ renderedThreads: [
+ { id: ThreadId.makeUnsafe("thread-12") },
+ { id: ThreadId.makeUnsafe("thread-11") },
+ { id: ThreadId.makeUnsafe("thread-10") },
],
},
{
- renderedThreadIds: [ThreadId.makeUnsafe("thread-8"), ThreadId.makeUnsafe("thread-6")],
+ renderedThreads: [
+ { id: ThreadId.makeUnsafe("thread-8") },
+ { id: ThreadId.makeUnsafe("thread-6") },
+ ],
},
]),
).toEqual([
@@ -350,14 +353,17 @@
getVisibleSidebarThreadIds([
{
shouldShowThreadPanel: false,
- renderedThreadIds: [
- ThreadId.makeUnsafe("thread-hidden-2"),
- ThreadId.makeUnsafe("thread-hidden-1"),
+ renderedThreads: [
+ { id: ThreadId.makeUnsafe("thread-hidden-2") },
+ { id: ThreadId.makeUnsafe("thread-hidden-1") },
],
},
{
shouldShowThreadPanel: true,
- renderedThreadIds: [ThreadId.makeUnsafe("thread-12"), ThreadId.makeUnsafe("thread-11")],
+ renderedThreads: [
+ { id: ThreadId.makeUnsafe("thread-12") },
+ { id: ThreadId.makeUnsafe("thread-11") },
+ ],
},
]),
).toEqual([ThreadId.makeUnsafe("thread-12"), ThreadId.makeUnsafe("thread-11")]);
diff --git a/apps/web/src/components/Sidebar.logic.ts b/apps/web/src/components/Sidebar.logic.ts
--- a/apps/web/src/components/Sidebar.logic.ts
+++ b/apps/web/src/components/Sidebar.logic.ts
@@ -230,14 +230,16 @@
return [...ordered, ...remaining];
}
-export function getVisibleSidebarThreadIds<TThreadId>(
+export function getVisibleSidebarThreadIds<TThread extends { id: unknown }>(
renderedProjects: readonly {
shouldShowThreadPanel?: boolean;
- renderedThreadIds: readonly TThreadId[];
+ renderedThreads: readonly TThread[];
}[],
-): TThreadId[] {
+): TThread["id"][] {
return renderedProjects.flatMap((renderedProject) =>
- renderedProject.shouldShowThreadPanel === false ? [] : renderedProject.renderedThreadIds,
+ renderedProject.shouldShowThreadPanel === false
+ ? []
+ : renderedProject.renderedThreads.map((thread) => thread.id),
);
}
diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -1435,9 +1435,6 @@
const renderedThreads = pinnedCollapsedThread
? [pinnedCollapsedThread]
: visibleProjectThreads;
- const renderedThreadIds = pinnedCollapsedThread
- ? [pinnedCollapsedThread.id]
- : visibleProjectThreads.map((thread) => thread.id);
const showEmptyThreadState = project.expanded && projectThreads.length === 0;
return {
@@ -1447,7 +1444,6 @@
project,
projectStatus,
renderedThreads,
- renderedThreadIds,
showEmptyThreadState,
shouldShowThreadPanel,
isThreadListExpanded,You can send follow-ups to the cloud agent here.
- Persist a stable server environment ID and descriptor - Resolve repository identity from git remotes and enrich orchestration events - Thread environment metadata through desktop and web startup flows
- Add packages/client-runtime/package.json to the release smoke workspace list
- keep remote-host project/thread lookups separate from the active env - avoid double-enriching replayed project events
- Make active environment mandatory in store and read-model sync - Scope chat, sidebar, and event replay state by resolved environment - Update tests for environment-aware thread and project handling
- keep untouched environment sidebar entries and project thread indexes stable during local snapshot sync - avoid eagerly resolving bootstrap URLs when an explicit server URL is provided - tighten scoped helpers by removing unused environment/session utilities
- add TTL-backed positive and negative caching for repository identity resolution - refresh identities when remotes appear or change after cache expiry - cover late-remote and remote-change cases in tests
- Strip explicit ports from URL-style git remotes - Add regression coverage for HTTPS and SSH remotes
- Preserve persisted environment IDs on read failures - Scope sidebar thread lookups to the active environment - Treat empty server URLs as unset
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/remote-host-model
branch
from
314c455 to
54f905c
Compare
juliusmarminge
changed the base branch from
main
to
t3code/pr-1708/web/atomic-store-refactor
- Key web stores and routes by environment-aware thread refs - Update draft, terminal, and branch handling for scoped threads - Add tests for the new scoped selectors and routing
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Debug tool react-scan left in production HTML
- Removed the react-scan script tag from apps/web/index.html as it is a development-only profiling tool that should not be in production.
- ✅ Fixed: Draft thread errors silently swallowed by incorrect guard
- Changed the isCurrentServerThread guard to check
serverThread !== undefinedinstead ofactiveThread !== undefined, so draft thread errors correctly fall through to setLocalDraftErrorsByThreadId.
- Changed the isCurrentServerThread guard to check
Or push these changes by commenting:
@cursor push d45f9141fb
Preview (d45f9141fb)
diff --git a/apps/web/index.html b/apps/web/index.html
--- a/apps/web/index.html
+++ b/apps/web/index.html
@@ -12,7 +12,6 @@
rel="stylesheet"
/>
<title>T3 Code (Alpha)</title>
- <script crossorigin="anonymous" src="//unpkg.com/react-scan/dist/auto.global.js"></script>
</head>
<body>
<div id="root"></div>
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -1695,10 +1695,10 @@
if (!targetThreadId) return;
const nextError = sanitizeThreadErrorMessage(error);
const isCurrentServerThread =
- activeThread !== undefined &&
+ serverThread !== undefined &&
targetThreadId === routeThreadRef.threadId &&
- activeThread.environmentId === routeThreadRef.environmentId &&
- activeThread.id === routeThreadRef.threadId;
+ serverThread.environmentId === routeThreadRef.environmentId &&
+ serverThread.id === routeThreadRef.threadId;
if (isCurrentServerThread) {
setStoreThreadError(targetThreadId, nextError);
return;
@@ -1713,7 +1713,7 @@
};
});
},
- [activeThread, routeThreadRef, setStoreThreadError],
+ [serverThread, routeThreadRef, setStoreThreadError],
);
const focusComposer = useCallback(() => {You can send follow-ups to the cloud agent here.
| rel="stylesheet" | ||
| /> | ||
| <title>T3 Code (Alpha)</title> | ||
| <script crossorigin="anonymous" src="//unpkg.com/react-scan/dist/auto.global.js"></script> |
Contributor
There was a problem hiding this comment.
Debug tool react-scan left in production HTML
High Severity
The react-scan profiling/debugging script is loaded from an external CDN (//unpkg.com/react-scan/dist/auto.global.js) in the production index.html. This development-only tool adds a visual overlay highlighting re-renders and will impact page load time, runtime performance, and user experience in production. It also introduces an external dependency on unpkg.com availability.
Reviewed by Cursor Bugbot for commit 010f452. Configure here.
| rel="stylesheet" | ||
| /> | ||
| <title>T3 Code (Alpha)</title> | ||
| <script crossorigin="anonymous" src="//unpkg.com/react-scan/dist/auto.global.js"></script> |
Contributor
There was a problem hiding this comment.
🔴 Critical web/index.html:15
The react-scan debugging script is unconditionally loaded from //unpkg.com in index.html, so it executes in production builds without Subresource Integrity verification. If unpkg.com or the package is compromised, arbitrary JavaScript runs in users' browsers, and the performance overhead of component monitoring ships to all users.
- <script crossorigin="anonymous" src="//unpkg.com/react-scan/dist/auto.global.js"></script>🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/index.html around line 15:
The `react-scan` debugging script is unconditionally loaded from `//unpkg.com` in `index.html`, so it executes in production builds without Subresource Integrity verification. If unpkg.com or the package is compromised, arbitrary JavaScript runs in users' browsers, and the performance overhead of component monitoring ships to all users.
- Route composer drafts through `/draft/:threadId` - Persist logical project draft mappings and update store lookups - Adjust chat view, navigation, and tests for the new draft flow
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Tests use raw threadId instead of scoped key
- Replaced all five
draftsByThreadKey[newThreadId]/draftsByThreadKey[threadId]lookups withdraftsByThreadKey[threadKeyFor(...)]to use the correct scoped key format expected by the store.
- Replaced all five
Or push these changes by commenting:
@cursor push 690b18e97e
Preview (690b18e97e)
diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx
--- a/apps/web/src/components/ChatView.browser.tsx
+++ b/apps/web/src/components/ChatView.browser.tsx
@@ -2560,7 +2560,9 @@
);
const newThreadId = threadIdFromPath(newThreadPath);
- expect(useComposerDraftStore.getState().draftsByThreadKey[newThreadId]).toMatchObject({
+ expect(
+ useComposerDraftStore.getState().draftsByThreadKey[threadKeyFor(newThreadId)],
+ ).toMatchObject({
modelSelectionByProvider: {
codex: {
provider: "codex",
@@ -2613,7 +2615,9 @@
);
const newThreadId = threadIdFromPath(newThreadPath);
- expect(useComposerDraftStore.getState().draftsByThreadKey[newThreadId]).toMatchObject({
+ expect(
+ useComposerDraftStore.getState().draftsByThreadKey[threadKeyFor(newThreadId)],
+ ).toMatchObject({
modelSelectionByProvider: {
claudeAgent: {
provider: "claudeAgent",
@@ -2653,7 +2657,9 @@
);
const newThreadId = threadIdFromPath(newThreadPath);
- expect(useComposerDraftStore.getState().draftsByThreadKey[newThreadId]).toBe(undefined);
+ expect(useComposerDraftStore.getState().draftsByThreadKey[threadKeyFor(newThreadId)]).toBe(
+ undefined,
+ );
} finally {
await mounted.cleanup();
}
@@ -2695,7 +2701,9 @@
);
const threadId = threadIdFromPath(threadPath);
- expect(useComposerDraftStore.getState().draftsByThreadKey[threadId]).toMatchObject({
+ expect(
+ useComposerDraftStore.getState().draftsByThreadKey[threadKeyFor(threadId)],
+ ).toMatchObject({
modelSelectionByProvider: {
codex: {
provider: "codex",
@@ -2724,7 +2732,9 @@
(path) => path === threadPath,
"New-thread should reuse the existing project draft thread.",
);
- expect(useComposerDraftStore.getState().draftsByThreadKey[threadId]).toMatchObject({
+ expect(
+ useComposerDraftStore.getState().draftsByThreadKey[threadKeyFor(threadId)],
+ ).toMatchObject({
modelSelectionByProvider: {
codex: {
provider: "codex",You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 24ac444. Configure here.
apps/web/src/composerDraftStore.ts
Outdated
Comment on lines
+2585
to
+2595
| const draftThreadEntries = Object.entries(store.draftThreadsByThreadKey).flatMap( | ||
| ([threadKey]) => { | ||
| const threadRef = composerThreadRefFromKey(threadKey); | ||
| return threadRef?.threadId === threadId ? [threadRef] : []; | ||
| }, | ||
| ); | ||
| if (draftThreadEntries.length !== 1) { | ||
| return; | ||
| } | ||
| clearPromotedDraftThreadByRef(draftThreadEntries[0]!); | ||
| } |
Contributor
There was a problem hiding this comment.
🟢 Low src/composerDraftStore.ts:2585
When draftThreadEntries.length > 1 (multiple draft entries reference the same threadId under different keys), the function silently returns without clearing any of them. Since the function's purpose is to "clear a draft thread once the server has materialized the same thread id," having multiple draft entries for a promoted server thread should still result in cleanup, not silent abandonment. This could leave orphaned draft data.
+ const draftThreadEntries = Object.entries(store.draftThreadsByThreadKey).flatMap(
+ ([threadKey]) => {
+ const threadRef = composerThreadRefFromKey(threadKey);
+ return threadRef?.threadId === threadId ? [threadRef] : [];
+ },
+ );
+ for (const draftThreadRef of draftThreadEntries) {
+ clearPromotedDraftThreadByRef(draftThreadRef);
+ }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/composerDraftStore.ts around lines 2585-2595:
When `draftThreadEntries.length > 1` (multiple draft entries reference the same `threadId` under different keys), the function silently returns without clearing any of them. Since the function's purpose is to "clear a draft thread once the server has materialized the same thread id," having multiple draft entries for a promoted server thread should still result in cleanup, not silent abandonment. This could leave orphaned draft data.
Evidence trail:
apps/web/src/composerDraftStore.ts lines 2579-2596 (REVIEWED_COMMIT): The `clearPromotedDraftThread` function uses `if (draftThreadEntries.length !== 1) { return; }` which causes silent return without cleanup when multiple draft entries exist for the same threadId. The docstring at lines 2570-2576 states the purpose is to "Clear a draft thread once the server has materialized the same thread id."
- Route UI actions through environment-specific native APIs - Include environment IDs in git/react-query keys and callers - Update affected tests and timeline components
- Update web code and tests to use `localApi` and `environmentApi` - Rename related contracts and RPC helpers
- Move shared API wiring out of wsApi into localApi and environmentApi - Tighten environment client lookup to avoid cross-environment fallback - Update tests for renamed helpers and lookup behavior
- Track promoted drafts until the server thread starts - Redirect stale draft routes to the canonical server route - Update orchestration effects and draft-store persistence
- derive test server paths through Effect - keep the test config aligned with ServerConfigShape
- Re-expand bootstrap projects with scoped project keys - Keep thread errors tied to the current server thread - Add terminal state migration and thread-ref regression tests
- mark every matching draft thread when thread ids collide across environments - add regression coverage for shared thread ids and update lint directive syntax
- Correct oxlint and ESLint disable comment markers - Preserve existing behavior while restoring lint compatibility
| if (composerImages.length === 0) { | ||
| clearComposerDraftPersistedAttachments(threadId); | ||
| clearComposerDraftPersistedAttachments(routeThreadRef); | ||
| return; |
Contributor
There was a problem hiding this comment.
🟠 High components/ChatView.tsx:2544
In getPersistedAttachmentsForThread at line 2547, the code uses scopedThreadKey(routeThreadRef) to index into draftsByThreadKey, but for draft routes (routeKind === 'draft'), composer draft data is stored under the legacy plain threadId key. This causes the lookup to always return undefined for draft threads, so persisted attachments are never reused and fallbackPersistedAttachments is empty in the catch handler. When image serialization fails, syncComposerDraftPersistedAttachments(routeThreadRef, []) is called with an empty array, incorrectly clearing previously-persisted attachments. Consider using resolveComposerThreadRef to resolve the correct key before indexing.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around line 2544:
In `getPersistedAttachmentsForThread` at line 2547, the code uses `scopedThreadKey(routeThreadRef)` to index into `draftsByThreadKey`, but for draft routes (`routeKind === 'draft'`), composer draft data is stored under the legacy plain `threadId` key. This causes the lookup to always return `undefined` for draft threads, so persisted attachments are never reused and `fallbackPersistedAttachments` is empty in the catch handler. When image serialization fails, `syncComposerDraftPersistedAttachments(routeThreadRef, [])` is called with an empty array, incorrectly clearing previously-persisted attachments. Consider using `resolveComposerThreadRef` to resolve the correct key before indexing.
Evidence trail:
- apps/web/src/components/ChatView.tsx lines 637-639: `routeThreadRef = scopeThreadRef(environmentId, threadId)` always uses scoped format
- apps/web/src/components/ChatView.tsx line 665: Correct pattern `useComposerThreadDraft(routeKind === 'server' ? routeThreadRef : threadId)` - draft routes pass plain `threadId`
- apps/web/src/components/ChatView.tsx line 2547: Bug - `draftsByThreadKey[scopedThreadKey(routeThreadRef)]` always uses scoped key regardless of routeKind
- apps/web/src/composerDraftStore.ts lines 818-823: `composerThreadKey` returns just `threadId` for legacy environment
- apps/web/src/composerDraftStore.ts lines 840-866: `resolveComposerThreadRef` converts string threadId to legacy ref
- apps/web/src/composerDraftStore.ts line 829: `LEGACY_COMPOSER_ENVIRONMENT_ID = '__legacy__'`
- apps/web/src/components/ChatView.browser.tsx line 485: Test code confirms dual lookup pattern `draftsByThreadKey[threadKeyFor(threadId)] ?? draftsByThreadKey[threadId]`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
prep work for multi-environments
Summary
Testing
bun fmt- Not run.bun lint- Not run.bun typecheck- Not run.bun run test- Not run.Note
Medium Risk
Touches core server handshake/websocket payloads and read-model projection by adding environment descriptors and repository identity enrichment, which could affect client compatibility and event ordering. Repository identity resolution shells out to git and is cached, so failures/perf regressions are plausible in varied repo setups.
Overview
Adds first-class environment metadata to support multi-environment bootstrapping: the server now persists a stable
environmentIdto disk and exposes anExecutionEnvironmentDescriptor(label/platform/serverVersion/capabilities), which is included in server config and lifecyclewelcome/readyevents.Introduces a
RepositoryIdentityResolverthat derives a normalized repository identity from git remotes (with TTL caching), and threads that identity into orchestration projections (getSnapshot/getActiveProjectByWorkspaceRoot) and websocket event streams/replays for project events.Desktop plumbing is extended with a new IPC bridge method (
getLocalEnvironmentBootstrap) so the web UI can bootstrap the “Local environment” with its websocket URL; associated tests/timeouts are updated and additional test coverage is added for environment-id persistence, repository identity resolution, and event enrichment.Reviewed by Cursor Bugbot for commit 3e7db1d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Scope app state, routing, and APIs by environment to support multi-environment connections
@t3tools/client-runtimepackage withKnownEnvironment,ScopedProjectRef,ScopedThreadRef, and scoped key helpers used throughout the web app.AppStateinstore.tsto holdactiveEnvironmentIdand aenvironmentStateByIdmap, isolating projects, threads, and orchestration events per environment.NativeApi/WsRpcClientsingletons with a registry ofWsRpcClientEntryobjects keyed byKnownEnvironment, and introducesLocalApifor browser/OS interactions vs.EnvironmentApifor backend-scoped calls.ServerEnvironmentservice on the server that generates and persists a stableenvironmentId, and attaches anExecutionEnvironmentDescriptor(including platform, version, and capabilities) to startup, heartbeat, and WS snapshot payloads.project.createdandproject.meta-updatedorchestration events withrepositoryIdentityresolved from git remotes via a new cachedRepositoryIdentityResolver./_chat/$threadIdto/_chat/$environmentId/$threadIdfor server threads and/_chat/draft/$threadIdfor draft threads; route helpersbuildThreadRouteParams,resolveThreadRouteRef, andresolveThreadRouteTargetare added inthreadRoutes.ts.environmentId:threadId) instead of brandedThreadIdvalues; persisted composer drafts bump to schema v5 with migration.environmentIdsegment), breaking any bookmarked or deep-linked URLs.Macroscope summarized 583f97c.