diff --git a/src/gadgets/sessionState.ts b/src/gadgets/sessionState.ts index d1c5bf20..0128d830 100644 --- a/src/gadgets/sessionState.ts +++ b/src/gadgets/sessionState.ts @@ -22,46 +22,40 @@ export interface InitSessionStateOptions { initialHeadSha?: string; } -// Session-level state accessible to all gadgets -let sessionState = { - agentType: null as string | null, - baseBranch: 'main' as string, - projectId: null as string | null, - workItemId: null as string | null, - workItemUrl: null as string | null, - workItemTitle: null as string | null, - initialHeadSha: null as string | null, - hooks: {} as SessionHooks, - readOnlyFs: false, - prCreated: false, - prUrl: null as string | null, - reviewSubmitted: false, - reviewUrl: null as string | null, - reviewBody: null as string | null, - reviewEvent: null as string | null, - initialCommentId: null as number | null, -}; +interface SessionStateData { + agentType: string | null; + baseBranch: string; + projectId: string | null; + workItemId: string | null; + workItemUrl: string | null; + workItemTitle: string | null; + initialHeadSha: string | null; + hooks: SessionHooks; + readOnlyFs: boolean; + prCreated: boolean; + prUrl: string | null; + reviewSubmitted: boolean; + reviewUrl: string | null; + reviewBody: string | null; + reviewEvent: string | null; + initialCommentId: number | null; +} -export function initSessionState(options: InitSessionStateOptions): void { - const { - agentType, - baseBranch, - projectId, - workItemId, - hooks, - workItemUrl, - workItemTitle, - initialHeadSha, - } = options; - sessionState = { - agentType, - baseBranch: baseBranch ?? 'main', - projectId: projectId ?? null, - workItemId: workItemId ?? null, - workItemUrl: workItemUrl ?? null, - workItemTitle: workItemTitle ?? null, - initialHeadSha: initialHeadSha ?? null, - hooks: hooks ?? {}, +/** + * Injectable SessionState class. Encapsulates all mutable session-level state + * accessible to gadgets. Use `createSessionState()` to create isolated instances + * in tests, or `setDefaultSessionState()` to inject a custom instance. + */ +export class SessionState { + private state: SessionStateData = { + agentType: null, + baseBranch: 'main', + projectId: null, + workItemId: null, + workItemUrl: null, + workItemTitle: null, + initialHeadSha: null, + hooks: {}, readOnlyFs: false, prCreated: false, prUrl: null, @@ -71,35 +65,171 @@ export function initSessionState(options: InitSessionStateOptions): void { reviewEvent: null, initialCommentId: null, }; + + init(options: InitSessionStateOptions): void { + const { + agentType, + baseBranch, + projectId, + workItemId, + hooks, + workItemUrl, + workItemTitle, + initialHeadSha, + } = options; + this.state = { + agentType, + baseBranch: baseBranch ?? 'main', + projectId: projectId ?? null, + workItemId: workItemId ?? null, + workItemUrl: workItemUrl ?? null, + workItemTitle: workItemTitle ?? null, + initialHeadSha: initialHeadSha ?? null, + hooks: hooks ?? {}, + readOnlyFs: false, + prCreated: false, + prUrl: null, + reviewSubmitted: false, + reviewUrl: null, + reviewBody: null, + reviewEvent: null, + initialCommentId: null, + }; + } + + getBaseBranch(): string { + return this.state.baseBranch; + } + + getProjectId(): string | null { + return this.state.projectId; + } + + getWorkItemId(): string | null { + return this.state.workItemId; + } + + setReadOnlyFs(readOnly: boolean): void { + this.state.readOnlyFs = readOnly; + } + + getWorkItemUrl(): string | null { + return this.state.workItemUrl; + } + + getWorkItemTitle(): string | null { + return this.state.workItemTitle; + } + + recordPRCreation(prUrl: string): void { + this.state.prCreated = true; + this.state.prUrl = prUrl; + } + + recordReviewSubmission(reviewUrl: string, body?: string | null, event?: string | null): void { + this.state.reviewSubmitted = true; + this.state.reviewUrl = reviewUrl; + this.state.reviewBody = body ?? null; + this.state.reviewEvent = event ?? null; + } + + recordInitialComment(commentId: number): void { + this.state.initialCommentId = commentId; + } + + /** + * Clear the initial comment ID from session state without performing a deletion. + * + * Called by the backend adapter when the sidecar signals that the subprocess + * already deleted the comment (ackCommentDeleted: true), so that the + * GitHubProgressPoster post-agent callback does not attempt a redundant delete. + */ + clearInitialComment(): void { + this.state.initialCommentId = null; + } + + /** + * Delete the initial ack comment from the PR and clear it from session state. + * + * Called by gadgets (e.g. CreatePRReview) immediately after a significant event + * to clean up the stale ack/progress comment as soon as possible. + * Wrapped in a try-catch so failures don't propagate to the caller. + */ + async deleteInitialComment(owner: string, repo: string): Promise { + const commentId = this.state.initialCommentId; + if (!commentId) return; + + // Clear state first so the post-agent callback sees null and short-circuits + this.state.initialCommentId = null; + + try { + const { githubClient } = await import('../github/client.js'); + await githubClient.deletePRComment(owner, repo, commentId); + } catch { + // Best-effort: restore the id so post-agent callback can retry + this.state.initialCommentId = commentId; + } + } + + getSessionState(): SessionStateData { + return { ...this.state }; + } +} + +/** + * Create an isolated SessionState instance. Use this in tests to avoid + * state bleeding between parallel test cases. + */ +export function createSessionState(): SessionState { + return new SessionState(); +} + +// Module-level default instance — shared by all module-level wrapper functions +let _defaultInstance: SessionState = new SessionState(); + +/** + * Replace the module-level default instance. Useful in tests or DI scenarios + * where a custom SessionState should be injected for all wrapper functions. + */ +export function setDefaultSessionState(instance: SessionState): void { + _defaultInstance = instance; +} + +// --------------------------------------------------------------------------- +// Backward-compatible module-level wrapper functions +// All 17 consumers continue to work without import changes. +// --------------------------------------------------------------------------- + +export function initSessionState(options: InitSessionStateOptions): void { + _defaultInstance.init(options); } export function getBaseBranch(): string { - return sessionState.baseBranch; + return _defaultInstance.getBaseBranch(); } export function getProjectId(): string | null { - return sessionState.projectId; + return _defaultInstance.getProjectId(); } export function getWorkItemId(): string | null { - return sessionState.workItemId; + return _defaultInstance.getWorkItemId(); } export function setReadOnlyFs(readOnly: boolean): void { - sessionState.readOnlyFs = readOnly; + _defaultInstance.setReadOnlyFs(readOnly); } export function getWorkItemUrl(): string | null { - return sessionState.workItemUrl; + return _defaultInstance.getWorkItemUrl(); } export function getWorkItemTitle(): string | null { - return sessionState.workItemTitle; + return _defaultInstance.getWorkItemTitle(); } export function recordPRCreation(prUrl: string): void { - sessionState.prCreated = true; - sessionState.prUrl = prUrl; + _defaultInstance.recordPRCreation(prUrl); } export function recordReviewSubmission( @@ -107,14 +237,11 @@ export function recordReviewSubmission( body?: string | null, event?: string | null, ): void { - sessionState.reviewSubmitted = true; - sessionState.reviewUrl = reviewUrl; - sessionState.reviewBody = body ?? null; - sessionState.reviewEvent = event ?? null; + _defaultInstance.recordReviewSubmission(reviewUrl, body, event); } export function recordInitialComment(commentId: number): void { - sessionState.initialCommentId = commentId; + _defaultInstance.recordInitialComment(commentId); } /** @@ -125,7 +252,7 @@ export function recordInitialComment(commentId: number): void { * GitHubProgressPoster post-agent callback does not attempt a redundant delete. */ export function clearInitialComment(): void { - sessionState.initialCommentId = null; + _defaultInstance.clearInitialComment(); } /** @@ -136,21 +263,9 @@ export function clearInitialComment(): void { * Wrapped in a try-catch so failures don't propagate to the caller. */ export async function deleteInitialComment(owner: string, repo: string): Promise { - const commentId = sessionState.initialCommentId; - if (!commentId) return; - - // Clear state first so the post-agent callback sees null and short-circuits - sessionState.initialCommentId = null; - - try { - const { githubClient } = await import('../github/client.js'); - await githubClient.deletePRComment(owner, repo, commentId); - } catch { - // Best-effort: restore the id so post-agent callback can retry - sessionState.initialCommentId = commentId; - } + return _defaultInstance.deleteInitialComment(owner, repo); } export function getSessionState() { - return { ...sessionState }; + return _defaultInstance.getSessionState(); } diff --git a/tests/unit/gadgets/sessionState.test.ts b/tests/unit/gadgets/sessionState.test.ts index e77076cb..758b1360 100644 --- a/tests/unit/gadgets/sessionState.test.ts +++ b/tests/unit/gadgets/sessionState.test.ts @@ -12,6 +12,9 @@ vi.mock('../../../src/github/client.js', () => ({ })); import { + SessionState, + clearInitialComment, + createSessionState, deleteInitialComment, getBaseBranch, getProjectId, @@ -23,8 +26,13 @@ import { recordInitialComment, recordPRCreation, recordReviewSubmission, + setDefaultSessionState, } from '../../../src/gadgets/sessionState.js'; +// --------------------------------------------------------------------------- +// Module-level backward-compatible function tests (original suite) +// --------------------------------------------------------------------------- + describe('initSessionState', () => { beforeEach(() => { vi.resetAllMocks(); @@ -325,3 +333,260 @@ describe('getSessionState', () => { expect(freshState.prUrl).toBeNull(); }); }); + +// --------------------------------------------------------------------------- +// SessionState class interface tests +// --------------------------------------------------------------------------- + +describe('SessionState class', () => { + let ss: SessionState; + + beforeEach(() => { + ss = new SessionState(); + }); + + it('starts with default state', () => { + const state = ss.getSessionState(); + expect(state.agentType).toBeNull(); + expect(state.baseBranch).toBe('main'); + expect(state.projectId).toBeNull(); + expect(state.workItemId).toBeNull(); + expect(state.prCreated).toBe(false); + expect(state.prUrl).toBeNull(); + expect(state.reviewSubmitted).toBe(false); + expect(state.initialCommentId).toBeNull(); + expect(state.hooks).toEqual({}); + }); + + it('init() sets all fields correctly', () => { + ss.init({ + agentType: 'implementation', + baseBranch: 'develop', + projectId: 'proj-1', + workItemId: 'card-1', + workItemUrl: 'https://trello.com/c/abc', + workItemTitle: 'My Card', + initialHeadSha: 'sha123', + hooks: { requiresPR: true }, + }); + const state = ss.getSessionState(); + expect(state.agentType).toBe('implementation'); + expect(state.baseBranch).toBe('develop'); + expect(state.projectId).toBe('proj-1'); + expect(state.workItemId).toBe('card-1'); + expect(state.workItemUrl).toBe('https://trello.com/c/abc'); + expect(state.workItemTitle).toBe('My Card'); + expect(state.initialHeadSha).toBe('sha123'); + expect(state.hooks).toEqual({ requiresPR: true }); + }); + + it('init() resets mutable fields on each call', () => { + ss.init({ agentType: 'implementation' }); + ss.recordPRCreation('https://github.com/pr/1'); + ss.recordInitialComment(42); + ss.init({ agentType: 'review' }); + const state = ss.getSessionState(); + expect(state.prCreated).toBe(false); + expect(state.prUrl).toBeNull(); + expect(state.initialCommentId).toBeNull(); + }); + + it('getBaseBranch() returns current base branch', () => { + ss.init({ agentType: 'implementation', baseBranch: 'feature/foo' }); + expect(ss.getBaseBranch()).toBe('feature/foo'); + }); + + it('getProjectId() returns project id', () => { + ss.init({ agentType: 'implementation', projectId: 'p-123' }); + expect(ss.getProjectId()).toBe('p-123'); + }); + + it('getWorkItemId() returns work item id', () => { + ss.init({ agentType: 'implementation', workItemId: 'w-999' }); + expect(ss.getWorkItemId()).toBe('w-999'); + }); + + it('getWorkItemUrl() returns null when not set', () => { + ss.init({ agentType: 'implementation' }); + expect(ss.getWorkItemUrl()).toBeNull(); + }); + + it('getWorkItemTitle() returns null when not set', () => { + ss.init({ agentType: 'implementation' }); + expect(ss.getWorkItemTitle()).toBeNull(); + }); + + it('setReadOnlyFs() updates readOnlyFs', () => { + ss.init({ agentType: 'implementation' }); + expect(ss.getSessionState().readOnlyFs).toBe(false); + ss.setReadOnlyFs(true); + expect(ss.getSessionState().readOnlyFs).toBe(true); + ss.setReadOnlyFs(false); + expect(ss.getSessionState().readOnlyFs).toBe(false); + }); + + it('recordPRCreation() sets prCreated and prUrl', () => { + ss.init({ agentType: 'implementation' }); + ss.recordPRCreation('https://github.com/owner/repo/pull/7'); + const state = ss.getSessionState(); + expect(state.prCreated).toBe(true); + expect(state.prUrl).toBe('https://github.com/owner/repo/pull/7'); + }); + + it('recordReviewSubmission() sets review fields', () => { + ss.init({ agentType: 'review' }); + ss.recordReviewSubmission('https://github.com/pr/1#review-1', 'LGTM', 'APPROVE'); + const state = ss.getSessionState(); + expect(state.reviewSubmitted).toBe(true); + expect(state.reviewUrl).toBe('https://github.com/pr/1#review-1'); + expect(state.reviewBody).toBe('LGTM'); + expect(state.reviewEvent).toBe('APPROVE'); + }); + + it('recordReviewSubmission() defaults body/event to null', () => { + ss.init({ agentType: 'review' }); + ss.recordReviewSubmission('https://github.com/pr/1#review-1'); + const state = ss.getSessionState(); + expect(state.reviewBody).toBeNull(); + expect(state.reviewEvent).toBeNull(); + }); + + it('recordInitialComment() stores comment id', () => { + ss.init({ agentType: 'implementation' }); + ss.recordInitialComment(7777); + expect(ss.getSessionState().initialCommentId).toBe(7777); + }); + + it('clearInitialComment() sets initialCommentId to null', () => { + ss.init({ agentType: 'implementation' }); + ss.recordInitialComment(7777); + ss.clearInitialComment(); + expect(ss.getSessionState().initialCommentId).toBeNull(); + }); + + it('deleteInitialComment() calls github client and clears comment id', async () => { + vi.resetAllMocks(); + mockDeletePRComment.mockResolvedValue(undefined); + ss.init({ agentType: 'implementation' }); + ss.recordInitialComment(55); + await ss.deleteInitialComment('owner', 'repo'); + expect(mockDeletePRComment).toHaveBeenCalledWith('owner', 'repo', 55); + expect(ss.getSessionState().initialCommentId).toBeNull(); + }); + + it('deleteInitialComment() restores id on error', async () => { + vi.resetAllMocks(); + mockDeletePRComment.mockRejectedValue(new Error('fail')); + ss.init({ agentType: 'implementation' }); + ss.recordInitialComment(55); + await ss.deleteInitialComment('owner', 'repo'); + expect(ss.getSessionState().initialCommentId).toBe(55); + }); + + it('deleteInitialComment() does nothing when no comment id', async () => { + vi.resetAllMocks(); + ss.init({ agentType: 'implementation' }); + await ss.deleteInitialComment('owner', 'repo'); + expect(mockDeletePRComment).not.toHaveBeenCalled(); + }); + + it('getSessionState() returns a copy, not a reference', () => { + ss.init({ agentType: 'implementation' }); + const s1 = ss.getSessionState(); + const s2 = ss.getSessionState(); + expect(s1).not.toBe(s2); + expect(s1).toEqual(s2); + }); + + it('mutations to returned state do not affect internal state', () => { + ss.init({ agentType: 'implementation' }); + const state = ss.getSessionState(); + (state as Record).prCreated = true; + expect(ss.getSessionState().prCreated).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// createSessionState() factory tests +// --------------------------------------------------------------------------- + +describe('createSessionState()', () => { + it('returns a new SessionState instance', () => { + const a = createSessionState(); + const b = createSessionState(); + expect(a).toBeInstanceOf(SessionState); + expect(b).toBeInstanceOf(SessionState); + expect(a).not.toBe(b); + }); + + it('instances are isolated — mutations on one do not affect the other', () => { + const a = createSessionState(); + const b = createSessionState(); + a.init({ agentType: 'implementation' }); + a.recordPRCreation('https://github.com/pr/1'); + b.init({ agentType: 'review' }); + expect(a.getSessionState().prCreated).toBe(true); + expect(b.getSessionState().prCreated).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// setDefaultSessionState() / DI tests +// --------------------------------------------------------------------------- + +describe('setDefaultSessionState()', () => { + it('replaces the default instance used by module-level functions', () => { + const custom = createSessionState(); + custom.init({ agentType: 'custom-agent', projectId: 'custom-proj' }); + + setDefaultSessionState(custom); + + expect(getProjectId()).toBe('custom-proj'); + expect(getBaseBranch()).toBe('main'); + }); + + it('module-level mutations affect the injected instance', () => { + const custom = createSessionState(); + custom.init({ agentType: 'implementation' }); + setDefaultSessionState(custom); + + recordPRCreation('https://github.com/pr/99'); + + expect(custom.getSessionState().prCreated).toBe(true); + expect(custom.getSessionState().prUrl).toBe('https://github.com/pr/99'); + }); + + it('restores isolation after resetting to fresh instance', () => { + const first = createSessionState(); + first.init({ agentType: 'first', projectId: 'first-proj' }); + setDefaultSessionState(first); + expect(getProjectId()).toBe('first-proj'); + + const second = createSessionState(); + second.init({ agentType: 'second', projectId: 'second-proj' }); + setDefaultSessionState(second); + expect(getProjectId()).toBe('second-proj'); + + // first should be unchanged + expect(first.getProjectId()).toBe('first-proj'); + }); +}); + +// --------------------------------------------------------------------------- +// clearInitialComment module-level wrapper test +// --------------------------------------------------------------------------- + +describe('clearInitialComment (module-level)', () => { + beforeEach(() => { + // Reset to a fresh default instance to avoid cross-test contamination + setDefaultSessionState(createSessionState()); + initSessionState({ agentType: 'implementation' }); + }); + + it('clears initialCommentId via module function', () => { + recordInitialComment(888); + expect(getSessionState().initialCommentId).toBe(888); + clearInitialComment(); + expect(getSessionState().initialCommentId).toBeNull(); + }); +});