Surface execution environment and repository identity metadata#1765
Surface execution environment and repository identity metadata#1765juliusmarminge wants to merge 2 commits intomainfrom
Conversation
- 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
|
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}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
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.
Reviewed by Cursor Bugbot for commit 410dbe8. Configure here.
| deletedAt: row.deletedAt, | ||
| }), | ||
| ), | ||
| Effect.map((option) => option), |
There was a problem hiding this comment.
Redundant no-op identity map in effect pipeline
Low Severity
Effect.map((option) => option) is a no-op identity transformation that does nothing. This appears to be a leftover from refactoring where Effect.map(Option.map(...)) was replaced — the Effect.map wrapper wasn't removed when the inner logic moved into the subsequent Effect.flatMap.
Reviewed by Cursor Bugbot for commit 410dbe8. Configure here.
| orchestrationEngine.readEvents(fromSequenceExclusive), | ||
| ).pipe( | ||
| Effect.map((events) => Array.from(events)), | ||
| Effect.flatMap(enrichOrchestrationEvents), |
There was a problem hiding this comment.
Replay events enriched twice in subscription handler
Low Severity
In subscribeOrchestrationDomainEvents, replay events are enriched with repository identity twice — first via enrichOrchestrationEvents at collection time, then again individually via enrichProjectEvent inside the Stream.mapEffect. The per-event enrichment in the stream is needed for live events from streamDomainEvents, but replay events have already been enriched. The caching in RepositoryIdentityResolver mitigates the cost, but the double resolution (including a potential getReadModel() call for project.meta-updated) is unnecessary work.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 410dbe8. Configure here.
| ) | ||
| .map((scopedId) => state.sidebarThreadsByScopedId[scopedId]?.id ?? null) | ||
| .filter((threadId): threadId is ThreadId => threadId !== null) | ||
| : EMPTY_THREAD_IDS; |
There was a problem hiding this comment.
Selector creates unstable array references every call
Medium Severity
selectThreadIdsByProjectId now performs .map().filter() on every invocation, always returning a new array reference even when the underlying data hasn't changed. The previous implementation returned a stable stored reference from threadIdsByProjectId. When used as a Zustand selector (e.g., useStore(selectThreadIdsByProjectId(id))), the new behavior defeats Object.is equality checks and causes unnecessary React re-renders on every unrelated store update.
Reviewed by Cursor Bugbot for commit 410dbe8. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| function resolveTargetEnvironmentId( | ||
| state: AppState, | ||
| environmentId?: EnvironmentId | null, | ||
| ): EnvironmentId | null { | ||
| return environmentId ?? state.activeEnvironmentId ?? null; | ||
| } |
There was a problem hiding this comment.
🟢 Low src/store.ts:645
In resolveTargetEnvironmentId, using environmentId ?? state.activeEnvironmentId treats null as a missing value. Since null is a valid environment identifier used throughout the codebase (e.g., environmentId: null in threads/projects), explicitly passing null incorrectly resolves to activeEnvironmentId when set, causing operations to target the wrong environment when the "null environment" was intended.
| function resolveTargetEnvironmentId( | |
| state: AppState, | |
| environmentId?: EnvironmentId | null, | |
| ): EnvironmentId | null { | |
| return environmentId ?? state.activeEnvironmentId ?? null; | |
| } | |
| function resolveTargetEnvironmentId( | |
| state: AppState, | |
| environmentId?: EnvironmentId | null, | |
| ): EnvironmentId | null { | |
| return environmentId !== undefined ? environmentId : state.activeEnvironmentId ?? null; | |
| } |
Also found in 1 other location(s)
apps/web/src/components/Sidebar.tsx:1106
The
getActiveEnvironmentThreadfunction usesactiveEnvironmentIdto build the scoped lookup key, but threads rendered in the sidebar may belong to projects with a differentenvironmentId. When the user right-clicks on such a thread,handleThreadContextMenuat line 1106 callsgetActiveEnvironmentThread(threadId)which returnsundefined, causing the context menu to silently fail to appear. The same issue affectshandleMultiSelectContextMenuat line 1198 where the "Mark unread" action will silently skip threads from non-active environments.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around lines 645-650:
In `resolveTargetEnvironmentId`, using `environmentId ?? state.activeEnvironmentId` treats `null` as a missing value. Since `null` is a valid environment identifier used throughout the codebase (e.g., `environmentId: null` in threads/projects), explicitly passing `null` incorrectly resolves to `activeEnvironmentId` when set, causing operations to target the wrong environment when the "null environment" was intended.
Evidence trail:
apps/web/src/store.ts lines 645-649 (resolveTargetEnvironmentId function using `??`); apps/web/src/store.ts lines 192-240 (mapThread and mapProject using environmentId parameter directly); apps/web/src/store.ts lines 656-673 (syncServerReadModel using targetEnvironmentId for mapping); packages/contracts/src/baseSchemas.ts line 23 (EnvironmentId is a branded string type, distinct from null)
Also found in 1 other location(s):
- apps/web/src/components/Sidebar.tsx:1106 -- The `getActiveEnvironmentThread` function uses `activeEnvironmentId` to build the scoped lookup key, but threads rendered in the sidebar may belong to projects with a different `environmentId`. When the user right-clicks on such a thread, `handleThreadContextMenu` at line 1106 calls `getActiveEnvironmentThread(threadId)` which returns `undefined`, causing the context menu to silently fail to appear. The same issue affects `handleMultiSelectContextMenu` at line 1198 where the "Mark unread" action will silently skip threads from non-active environments.
ApprovabilityVerdict: Needs human review This PR introduces a substantial new feature for environment and repository identity tracking with new services, a new package, and significant state management refactoring. The complexity of environment-scoped entity handling plus unresolved comments about selector stability and null-handling semantics warrant human review. You can customize Macroscope's approvability policy. Learn more. |
- Add packages/client-runtime/package.json to the release smoke workspace list
1 participant
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
Medium risk due to cross-cutting contract/schema changes (
ServerConfig+ lifecycle payloads now requireenvironment) and new event enrichment/state scoping that can affect websocket streams, store selectors, and existing clients/tests.Overview
Surfaces execution-environment metadata end-to-end. The server now generates and persists a stable
EnvironmentIdand exposes anExecutionEnvironmentDescriptor(label/platform/version/capabilities), threading it intoserverGetConfigpluswelcome/readylifecycle events.Adds repository identity resolution and propagation. A new
RepositoryIdentityResolverderives a normalizedRepositoryIdentityfrom git remotes (with caching), and the server attaches it to project snapshot data and toproject.created/project.meta-updatedevents during replay and live subscriptions.Updates desktop + web client bootstrapping and state for multi-environment. Desktop exposes
getLocalEnvironmentBootstrapover IPC; web addsenvironmentBootstrap+ new@t3tools/client-runtimehelpers and refactors the Zustand store/sidebar to scope projects/threads byactiveEnvironmentId, including snapshot syncing and event application per environment.Reviewed by Cursor Bugbot for commit 95c2115. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Surface execution environment and repository identity metadata across server and client
ServerEnvironmentservice that persists a stable environment ID in the state directory and exposes anExecutionEnvironmentDescriptor(platform, server version,repositoryIdentitycapability) included inwelcomeandreadylifecycle events andserverGetConfigresponses.RepositoryIdentityResolverservice that inspectsgit remote -voutput to resolve aRepositoryIdentity(normalized canonical key, optional GitHub owner/repo) for a working directory, with an in-memory cache keyed by repo top-level.project.createdandproject.meta-updatedevents returned over RPC withrepositoryIdentitywhen resolvable.<environmentId>:<localId>), so snapshot syncs, event application, and selectors are scoped to the active environment.@t3tools/client-runtimepackage with utilities for constructingKnownEnvironmentobjects, scoped refs, and environment-aware client resolution.getLocalEnvironmentBootstrap()toDesktopBridge(IPC + preload) so the renderer can retrieve the local environment label and WebSocket URL.ServerConfig,ServerLifecycleWelcomePayload, andServerLifecycleReadyPayloadschemas now require anenvironmentfield — existing consumers that construct these objects (e.g. tests, mocks) must be updated.Macroscope summarized 95c2115.