refactor: remove sessionRepositories and activitySinks maps; add issueTrackers TODOs (CYPACK-968) by cyrusagent · Pull Request #992 · cyrusagents/cyrus

cyrusagent · 2026-03-16T00:14:31Z

Summary

Removed redundant sessionRepositories: Map<string, string> from EdgeWorker — it was a lossy cache of session.repositories[0].repositoryId that only stored the primary repo
Deleted activitySinks: Map<string, IActivitySink> entirely — LinearActivitySink is a stateless wrapper around IIssueTrackerService, so sinks are now created on-the-fly via createActivitySink() helper at session registration time
Added TODO comments on issueTrackers (in both EdgeWorker.ts and CLAUDE.md) noting composite key needed for multi-platform support

Implementation approach

sessionRepositories removal

Each sessionRepositories.get(sessionId) call was replaced based on context:

Event handler callbacks: Used session from the event payload directly
Constructor callback and feedback delivery: Used this.agentSessionManager.getSession(id) to fetch the session first
Fallback resolution paths: Used firstSession.repositories[0]?.repositoryId
Repo deletion reverse-lookup: Used s.repositories.some() for correct multi-repo matching

activitySinks deletion

Removed initialization loop, repo addition sink creation, and repo deletion cleanup
Added createActivitySink(workspaceId) helper that looks up issueTracker and creates LinearActivitySink on-the-fly, falling back to NoopActivitySink
All session registration sites now call createActivitySink() instead of map lookup

Testing

All 628 edge-worker tests passing
Build succeeds across all packages
Linting clean, TypeScript type checking passes

Links

Linear: CYPACK-968

…K-968) Delete the redundant sessionRepositories: Map<string, string> from EdgeWorker, replacing all ~14 usage sites with direct reads from session.repositories[0]?.repositoryId. The reverse-lookup in repo deletion now uses session.repositories.some() which is more correct for multi-repo sessions. Also adds TODO comments documenting: - activitySinks multi-workspace limitation for cross-workspace sessions - issueTrackers composite key needed for multi-platform support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…PACK-968) Change activitySinks from Map<repoId, sink> to Map<workspaceId, sink>. This eliminates redundant sink instances (one per workspace instead of one per repo), decouples the lookup from which repo is "primary", and makes the data model honest about what it represents. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

LinearActivitySink is a thin, stateless wrapper around IIssueTrackerService. Instead of maintaining a pre-built map, create sinks on-the-fly at session registration time via createActivitySink() helper. Falls back to NoopActivitySink when no issue tracker is available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

PaytonWebber · 2026-03-16T16:58:13Z

 				const allActiveSessions = this.agentSessionManager.getActiveSessions();
-				const activeSessions = allActiveSessions.filter(
-					(s) => this.sessionRepositories.get(s.id) === repo.id,
+				const activeSessions = allActiveSessions.filter((s) =>
+					s.repositories.some((r) => r.repositoryId === repo.id),
 				);


This is also wrong now that CYPACK-911 has been merged. There is no longer a requirement for sessions to be linked to repos, now that we have support for surfaces like Slack. This will also need to be changed once multi-repo support is added

PaytonWebber · 2026-03-16T17:05:13Z

-				const repoId = this.sessionRepositories.get(childSessionId);
+				const childSession =
+					this.agentSessionManager.getSession(childSessionId);
+				const repoId = childSession?.repositories[0]?.repositoryId;


This will need to change once multi-repo session support is added. handleResumeParentSession should not require the repo now that CYPACK-911 has been merged. I presume it's needed to build the allowed tools for the session, but the repo config itself should not be passed to this method.

PaytonWebber · 2026-03-16T17:05:14Z

-				const repoId = this.sessionRepositories.get(sessionId);
+				const repoId = session.repositories[0]?.repositoryId;


Same thing with this.

PaytonWebber · 2026-03-16T17:05:14Z

-				const repoId = this.sessionRepositories.get(sessionId);
+				const repoId = session.repositories[0]?.repositoryId;


PaytonWebber · 2026-03-16T17:09:25Z

@cyrusagent look at the comments left on this PR as review and address them

cyrusagent-staging · 2026-03-16T17:09:30Z

This repository is not configured with Cyrus. Please add it at https://app.atcyrus.com/repos to start using Cyrus here.

ghost · 2026-03-16T17:09:30Z

This repository is not configured with Cyrus. Please add it at https://app.atcyrus.com/repos to start using Cyrus here.

ghost · 2026-03-16T17:09:31Z

I can't connect to your local environment because the Cyrus process isn't running. Run cyrus in your terminal and keep it running, then mention me again.

ghost · 2026-03-16T17:09:31Z

This repository is not configured with Cyrus. Please add it at https://app.atcyrus.com/repos to start using Cyrus here.

ghost · 2026-03-16T17:09:31Z

This repository is not configured with Cyrus. Please add it at https://app.atcyrus.com/repos to start using Cyrus here.

…internally (CYPACK-968) Move repo resolution out of event handler callbacks and into the handler methods themselves via a new resolveSessionRepo helper. This removes the requirement for sessions to be linked to repos at the call site, aligning with CYPACK-911 where sessions can exist without repos (e.g. Slack surfaces). Also adds multi-repo TODOs for removeDeletedRepositories and the helper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cyrusagent · 2026-03-16T17:14:13Z

Pushed. Here's a summary of the changes addressing all 4 review comments:

Removed repo parameter from handleResumeParentSession (line 325) — The _childRepo param was already unused; removed the repo lookup from the callback entirely.
Moved repo resolution into handleSubroutineTransition (line 349) — The method now resolves the repo internally instead of receiving it as a parameter.
Moved repo resolution into handleValidationLoopFixer (line 369) — Same treatment. Also applied to handleValidationLoopRerun for consistency.
Added multi-repo TODO on removeDeletedRepositories filter (line 2025) — Documented that when multi-repo sessions exist, we should detach only the removed repo rather than stopping the entire session.

All changes use a new resolveSessionRepo helper (with its own multi-repo TODO) that centralizes the session.repositories[0] pattern. Typecheck passes, all 628 edge-worker tests pass.

PaytonWebber · 2026-03-16T20:17:46Z

@@ -363,21 +346,12 @@ export class EdgeWorker extends EventEmitter {
 		this.agentSessionManager.on(
 			"validationLoopIteration",
 			async ({ sessionId, session, fixerPrompt, iteration, maxIterations }) => {
-				const repoId = this.sessionRepositories.get(sessionId);


Connoropolous · 2026-04-22T17:16:58Z

@cyrusagent merge main into this and then find out which of the changes we were trying to make in this PR are still relevant

cyrusagent-staging · 2026-04-22T17:21:36Z

I aborted the merge attempt — it would require substantial rework rather than mechanical conflict resolution. Here's the assessment:

Merge outcome

13 conflicts in EdgeWorker.ts + 1 trivial in CHANGELOG.internal.md. Not mergeable mechanically.

PR's 3 stated goals — still relevant

Remove sessionRepositories Map — still exists on main with ~12 call sites (grew from the original 14 as main added GitHub push, issue-state-change, GitLab, and warm-session code that also use it).
Remove activitySinks Map, create sinks on-the-fly — still exists on main. Main is in an inconsistent state: EdgeWorker.ts:197 comment says "keyed by Linear workspace ID" but EdgeWorker.ts:641 still sets by repoId. The PR's refactor would fix this.
Add issueTrackers composite-key TODO — still missing on main.

But the implementation is stale

Of the 6 PR commits, 4 touched code that main deleted in CYPACK-996 (PR #1018, skills refactor):

handleSubroutineTransition, handleValidationLoopFixer, handleValidationLoopRerun
All procedureAnalyzer references
The resolveSessionRepo helper was introduced specifically to serve those handlers

Related work already on main

Commit 97dde678 (PR #1120) stopped deleting activity sinks/trackers during repo removal — addresses a narrow slice of the same concerns but kept the maps.

Recommendation

Close this PR and redo the refactor fresh against current main. The goals still hold, but >50% of the diff targets deleted code, and the new code on main (push webhooks, issue-state-change, GitLab, warm sessions) has its own sessionRepositories usages that need the same treatment. A fresh pass is cleaner than resolving 13 conflicts in a file that's grown ~1000 lines since the PR was written.

Want me to proceed with a fresh refactor commit on this branch, or close the PR?

Resolves conflicts in CHANGELOG.internal.md (kept both CYPACK-968 and CYPACK-1101 entries) and EdgeWorker.ts. EdgeWorker.ts conflict resolution: - Preserved the PR's core refactor: removed sessionRepositories and activitySinks maps, added issueTrackers composite-key TODO, added resolveSessionRepo helper, on-the-fly createActivitySink - Discarded PR edits to handleSubroutineTransition, handleValidationLoopFixer, and handleValidationLoopRerun (those handlers were deleted by CYPACK-996) - Adopted main's updateLinearWorkspaceTokens, removed obsolete legacy issue tracker construction and workspace cleanup, adopted CYPACK-969 rename

cyrusagent · 2026-04-22T17:30:24Z

Merged main into this branch (commit 18d9534). Here's where each PR change landed:

Still relevant (kept):

Remove sessionRepositories map — replaced with resolveSessionRepo(session) helper using session.repositories[0].repositoryId
Remove activitySinks map — replaced with on-the-fly createActivitySink(workspaceId) lookup against issueTrackers
issueTrackers TODO noting composite-key ${platform}:${workspaceId} once we add GitHub/Jira issue trackers

Became obsolete (discarded):

All three edits to handleSubroutineTransition, handleValidationLoopFixer, and handleValidationLoopRerun — those handlers were deleted wholesale by CYPACK-996 (procedures→skills refactor), so there's nothing to modify

Already on main (no-op):

Re-keying activitySinks by workspace ID — main had already done this in Release v0.2.39 #1057 (commit 40e67b5), and we've now gone further by deleting the map entirely

Also adopted main's updateLinearWorkspaceTokens (which supersedes the PR's workspace-token update logic), dropped legacy issue-tracker construction, and picked up the CYPACK-969 createCyrusAgentSession rename. Typecheck + all 586 edge-worker tests pass.

cyrusagent · 2026-04-22T17:30:36Z

Merged main into cypack-968 (commit 18d9534), pushed, and posted the relevance summary to PR #992. Kept: sessionRepositories removal, activitySinks map deletion, issueTrackers TODO, resolveSessionRepo helper. Discarded: the three subroutine/validation handler edits (handlers were deleted by CYPACK-996).

Connoropolous · 2026-04-22T18:02:01Z

@cyrusagent update the PR description

Connoropolous · 2026-04-22T18:03:48Z

+				// TODO (multi-repo): When multi-repo session support is added, sessions may span
+				// multiple repos. Instead of stopping the entire session, we should detach only the
+				// removed repo and keep the session alive if it still has other repos. Sessions
+				// without any repos (e.g. Slack surfaces) are unaffected by repo removal.


@cyrusagent since we have multi repo now what should we think about doing here?

cyrusagent and others added 2 commits March 15, 2026 17:14

docs: update internal changelog for CYPACK-968

4f40c72

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cyrusagent marked this pull request as ready for review March 16, 2026 00:16

cyrusagent changed the title ~~refactor: remove sessionRepositories map; add multi-repo TODOs (CYPACK-968)~~ refactor: remove sessionRepositories; re-key activitySinks by workspace ID; add issueTrackers TODOs (CYPACK-968) Mar 16, 2026

cyrusagent and others added 2 commits March 15, 2026 17:38

docs: update internal changelog for activitySinks re-keying (CYPACK-968)

a554e66

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cyrusagent changed the title ~~refactor: remove sessionRepositories; re-key activitySinks by workspace ID; add issueTrackers TODOs (CYPACK-968)~~ refactor: remove sessionRepositories and activitySinks maps; add issueTrackers TODOs (CYPACK-968) Mar 16, 2026

PaytonWebber approved these changes Mar 16, 2026

View reviewed changes

PaytonWebber reviewed Mar 16, 2026

View reviewed changes

Connoropolous reviewed Apr 22, 2026

View reviewed changes

		const repoId = this.sessionRepositories.get(sessionId);
		const repoId = session.repositories[0]?.repositoryId;

Conversation

cyrusagent commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Implementation approach

sessionRepositories removal

activitySinks deletion

Testing

Links

Uh oh!

PaytonWebber Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

PaytonWebber Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

PaytonWebber Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

PaytonWebber Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

PaytonWebber commented Mar 16, 2026

Uh oh!

cyrusagent-staging Bot commented Mar 16, 2026

Uh oh!

ghost commented Mar 16, 2026

Uh oh!

ghost commented Mar 16, 2026

Uh oh!

ghost commented Mar 16, 2026

Uh oh!

ghost commented Mar 16, 2026

Uh oh!

cyrusagent Bot commented Mar 16, 2026

Uh oh!

PaytonWebber Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Connoropolous commented Apr 22, 2026

Uh oh!

cyrusagent-staging Bot commented Apr 22, 2026

Merge outcome

PR's 3 stated goals — still relevant

But the implementation is stale

Related work already on main

Recommendation

Uh oh!

cyrusagent commented Apr 22, 2026

Uh oh!

cyrusagent Bot commented Apr 22, 2026

Uh oh!

Connoropolous commented Apr 22, 2026

Uh oh!

Connoropolous Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cyrusagent commented Mar 16, 2026 •

edited

Loading