diff --git a/.gitignore b/.gitignore index a4bb4665..8db1d9a9 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,5 @@ test-results/ # Claude Code — commit settings.json but exclude local settings (may contain secrets) .claude/settings.local.json -# Progress comment state file (ephemeral, written by ProgressMonitor) +# Progress comment state file (legacy — kept to ignore stale files from older runs) .cascade-progress-comment-id - diff --git a/src/backends/adapter.ts b/src/backends/adapter.ts index 1d1d8831..fa91b27a 100644 --- a/src/backends/adapter.ts +++ b/src/backends/adapter.ts @@ -21,7 +21,11 @@ import { withGitHubToken } from '../github/client.js'; import type { AgentInput, AgentResult, CascadeConfig, ProjectConfig } from '../types/index.js'; import { postProcessResult } from './postProcess.js'; import { createProgressMonitor } from './progress.js'; -import { augmentProjectSecrets, resolveGitHubToken } from './secretBuilder.js'; +import { + augmentProjectSecrets, + injectProgressCommentId, + resolveGitHubToken, +} from './secretBuilder.js'; import type { AgentBackend, AgentBackendInput } from './types.js'; /** @@ -115,6 +119,13 @@ async function buildBackendInput( // Build per-project secrets with CASCADE env var injections const projectSecrets = await augmentProjectSecrets(project, agentType, input); + // Inject pre-seeded progress comment ID so the subprocess finds it at startup + injectProgressCommentId( + projectSecrets, + cardId, + input.ackCommentId as string | number | undefined, + ); + // Override GITHUB_TOKEN in subprocess secrets with agent-scoped token if (gitHubToken && profile.needsGitHubToken) { projectSecrets.GITHUB_TOKEN = gitHubToken; diff --git a/src/backends/claude-code/env.ts b/src/backends/claude-code/env.ts index 31f2f2fe..73d4b24c 100644 --- a/src/backends/claude-code/env.ts +++ b/src/backends/claude-code/env.ts @@ -6,6 +6,8 @@ * server-side secrets from leaking into agent environments. */ +import { ENV_VAR_NAME as PROGRESS_COMMENT_ENV_VAR } from '../progressState.js'; + /** Exact variable names to pass through. */ export const ALLOWED_ENV_EXACT = new Set([ // System @@ -27,6 +29,9 @@ export const ALLOWED_ENV_EXACT = new Set([ // Squint 'SQUINT_DB_PATH', + // Progress comment state (pre-seeded ack comment ID) + PROGRESS_COMMENT_ENV_VAR, + // Node 'NODE_PATH', 'NODE_EXTRA_CA_CERTS', diff --git a/src/backends/claude-code/hooks.ts b/src/backends/claude-code/hooks.ts index afa48ad3..2cb2cd3a 100644 --- a/src/backends/claude-code/hooks.ts +++ b/src/backends/claude-code/hooks.ts @@ -8,7 +8,6 @@ import type { PreToolUseHookInput, SyncHookJSONOutput, } from '@anthropic-ai/claude-agent-sdk'; -import { STATE_FILE_NAME } from '../progressState.js'; import type { LogWriter } from '../types.js'; /** @@ -132,15 +131,7 @@ function checkUncommittedChanges(logWriter: LogWriter, repoDir: string): SyncHoo }).trim(); if (!status) return null; - // Filter out CASCADE internal state files that are not code changes - const meaningful = status - .split('\n') - .filter((line) => !line.endsWith(STATE_FILE_NAME)) - .join('\n') - .trim(); - if (!meaningful) return null; - - logWriter('WARN', 'Stop hook blocked: uncommitted changes', { status: meaningful }); + logWriter('WARN', 'Stop hook blocked: uncommitted changes', { status }); return { decision: 'block', reason: 'You have uncommitted changes. Stage, commit, then use cascade-tools github create-pr.', diff --git a/src/backends/progressMonitor.ts b/src/backends/progressMonitor.ts index 25336fb3..88981b85 100644 --- a/src/backends/progressMonitor.ts +++ b/src/backends/progressMonitor.ts @@ -73,7 +73,6 @@ export class ProgressMonitor implements ProgressReporter { ? new PMProgressPoster({ agentType: config.agentType, cardId: config.trello.cardId, - repoDir: config.repoDir, logWriter: config.logWriter, }) : null; @@ -125,13 +124,9 @@ export class ProgressMonitor implements ProgressReporter { commentId: this.config.preSeededCommentId, }); - // Write state file so PostComment gadget can find it - if (this.config.repoDir && this.config.trello) { - writeProgressCommentId( - this.config.repoDir, - this.config.trello.cardId, - this.config.preSeededCommentId, - ); + // Write env var so PostComment gadget can find it + if (this.config.trello) { + writeProgressCommentId(this.config.trello.cardId, this.config.preSeededCommentId); } } else if (this.pmPoster) { // Post initial comment immediately (fire-and-forget) @@ -148,12 +143,11 @@ export class ProgressMonitor implements ProgressReporter { stop(): void { this.scheduler.stop(); - // Clean up state file on stop (best-effort — stop() is called from finally - // blocks, so an rmSync failure must not mask the actual agent result) + // Clean up env var on stop (best-effort — stop() is called from finally blocks) try { - clearProgressCommentId(this.config.repoDir); + clearProgressCommentId(); } catch { - // State file cleanup is best-effort + // Env var cleanup is best-effort } } diff --git a/src/backends/progressState.ts b/src/backends/progressState.ts index 011bbc5b..6e85bc6c 100644 --- a/src/backends/progressState.ts +++ b/src/backends/progressState.ts @@ -1,83 +1,55 @@ /** - * File-based state bridge for sharing the progress comment ID between + * Env-var-based state bridge for sharing the progress comment ID between * the ProgressMonitor (which creates the initial comment) and the * PostComment gadget (which posts the final summary). * - * Uses a state file `.cascade-progress-comment-id` written to the repo - * working directory. This approach works for both the llmist backend - * (same process) and the Claude Code backend (subprocess), since both - * share the same filesystem. + * Uses the `CASCADE_PROGRESS_COMMENT_ID` environment variable following + * the existing `CASCADE_*` naming pattern. The env var format is + * `:`. * - * File format: `:` + * For the pre-seeded case (~90% of runs), the env var is injected into + * the Claude Code subprocess via `projectSecrets` before subprocess launch, + * so it is available from startup. For the dynamic case (ProgressMonitor + * `postInitial()`), `process.env` is updated in-process — same-process + * consumers see it immediately; cross-process visibility is an accepted gap. */ -import { existsSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { join } from 'node:path'; - -export const STATE_FILE_NAME = '.cascade-progress-comment-id'; +export const ENV_VAR_NAME = 'CASCADE_PROGRESS_COMMENT_ID'; /** - * Writes the progress comment ID to the state file in the given repo directory. + * Writes the progress comment ID to the env var. * - * @param repoDir - The working directory where the state file will be written. * @param workItemId - The work item ID (Trello card ID or JIRA issue key). * @param commentId - The comment ID returned by addComment(). */ -export function writeProgressCommentId( - repoDir: string, - workItemId: string, - commentId: string, -): void { - const filePath = join(repoDir, STATE_FILE_NAME); - writeFileSync(filePath, `${workItemId}:${commentId}`, 'utf-8'); +export function writeProgressCommentId(workItemId: string, commentId: string): void { + process.env[ENV_VAR_NAME] = `${workItemId}:${commentId}`; } /** - * Reads the progress comment state from the state file. + * Reads the progress comment state from the env var. * - * @param repoDir - Optional directory containing the state file. Defaults to - * `process.cwd()` if not provided. For cross-process usage - * (e.g., Claude Code subprocess), the caller should ensure - * `process.chdir(repoDir)` has been called, or pass `repoDir` - * explicitly. - * @returns `{ workItemId, commentId }` if the state file exists and is valid, + * @returns `{ workItemId, commentId }` if the env var is set and valid, * or `null` if not found or malformed. */ -export function readProgressCommentId( - repoDir?: string, -): { workItemId: string; commentId: string } | null { - const dir = repoDir ?? process.cwd(); - const filePath = join(dir, STATE_FILE_NAME); - - if (!existsSync(filePath)) return null; +export function readProgressCommentId(): { workItemId: string; commentId: string } | null { + const value = process.env[ENV_VAR_NAME]; + if (!value) return null; - try { - const content = readFileSync(filePath, 'utf-8').trim(); - const colonIndex = content.indexOf(':'); - if (colonIndex === -1) return null; + const colonIndex = value.indexOf(':'); + if (colonIndex === -1) return null; - const workItemId = content.slice(0, colonIndex); - const commentId = content.slice(colonIndex + 1); + const workItemId = value.slice(0, colonIndex); + const commentId = value.slice(colonIndex + 1); - if (!workItemId || !commentId) return null; + if (!workItemId || !commentId) return null; - return { workItemId, commentId }; - } catch { - return null; - } + return { workItemId, commentId }; } /** - * Deletes the progress comment state file. - * - * @param repoDir - Optional directory containing the state file. Defaults to - * `process.cwd()` if not provided. + * Clears the progress comment state by deleting the env var. */ -export function clearProgressCommentId(repoDir?: string): void { - const dir = repoDir ?? process.cwd(); - const filePath = join(dir, STATE_FILE_NAME); - - if (existsSync(filePath)) { - rmSync(filePath); - } +export function clearProgressCommentId(): void { + delete process.env[ENV_VAR_NAME]; } diff --git a/src/backends/progressState/pmPoster.ts b/src/backends/progressState/pmPoster.ts index 07b6a48a..18d67dff 100644 --- a/src/backends/progressState/pmPoster.ts +++ b/src/backends/progressState/pmPoster.ts @@ -2,7 +2,7 @@ * PM (Project Management) progress comment poster. * * Manages the create-once/update-in-place/fallback-to-new lifecycle - * for progress comments on Trello/JIRA work items. Handles state file + * for progress comments on Trello/JIRA work items. Handles env-var * coordination with the PostComment gadget subprocess. */ @@ -14,7 +14,6 @@ import type { LogWriter } from '../types.js'; export interface PMProgressPosterConfig { agentType: string; cardId: string; - repoDir?: string; logWriter: LogWriter; } @@ -38,12 +37,6 @@ export class PMProgressPoster { ); } - private maybeWriteStateFile(commentId: string | null): void { - if (this.config.repoDir && commentId) { - writeProgressCommentId(this.config.repoDir, this.config.cardId, commentId); - } - } - async postInitial(): Promise { const provider = getPMProviderOrNull(); if (!provider) return; @@ -55,8 +48,8 @@ export class PMProgressPoster { commentId: this.progressCommentId, }); - // Write state file so PostComment gadget can update this comment - this.maybeWriteStateFile(this.progressCommentId); + // Write env var so PostComment gadget can update this comment + writeProgressCommentId(this.config.cardId, this.progressCommentId); } async update(summary: string): Promise { @@ -66,11 +59,11 @@ export class PMProgressPoster { const { cardId } = this.config; if (this.progressCommentId) { - // If the PostComment gadget (subprocess) cleared the state file, + // If the PostComment gadget cleared the env var, // the agent has posted its final comment to this ID — do not overwrite. - const stateFile = readProgressCommentId(this.config.repoDir); - if (!stateFile) { - this.config.logWriter('DEBUG', 'State file cleared by agent — skipping progress update', { + const envVarState = readProgressCommentId(); + if (!envVarState) { + this.config.logWriter('DEBUG', 'Env var cleared by agent — skipping progress update', { commentId: this.progressCommentId, }); this.progressCommentId = null; @@ -94,8 +87,8 @@ export class PMProgressPoster { cardId, commentId: this.progressCommentId, }); - // Update state file with new comment ID - this.maybeWriteStateFile(this.progressCommentId); + // Update env var with new comment ID + writeProgressCommentId(cardId, this.progressCommentId); } } else { // First tick: create the comment and store its ID. @@ -106,8 +99,8 @@ export class PMProgressPoster { cardId, commentId: this.progressCommentId, }); - // Write state file so PostComment gadget can find this comment - this.maybeWriteStateFile(this.progressCommentId); + // Write env var so PostComment gadget can find this comment + writeProgressCommentId(cardId, this.progressCommentId); } } } diff --git a/src/backends/secretBuilder.ts b/src/backends/secretBuilder.ts index 2bed347a..bcc5af8e 100644 --- a/src/backends/secretBuilder.ts +++ b/src/backends/secretBuilder.ts @@ -4,6 +4,7 @@ import { getPersonaToken } from '../github/personas.js'; import { getJiraConfig } from '../pm/config.js'; import type { AgentInput, ProjectConfig } from '../types/index.js'; import { parseRepoFullName } from '../utils/repo.js'; +import { ENV_VAR_NAME } from './progressState.js'; /** * Resolve the GitHub token for profiles that need GitHub client access. @@ -62,3 +63,20 @@ export async function augmentProjectSecrets( return projectSecrets; } + +/** + * Inject the pre-seeded progress comment ID into project secrets so the + * Claude Code subprocess can find it via the CASCADE_PROGRESS_COMMENT_ID env var. + * + * Only injects when ackCommentId is a string (PM comment) and cardId is set. + * GitHub ack comments (numeric IDs) are handled separately via session state. + */ +export function injectProgressCommentId( + projectSecrets: Record, + cardId: string | undefined, + ackCommentId: string | number | undefined, +): void { + if (cardId && typeof ackCommentId === 'string' && ackCommentId) { + projectSecrets[ENV_VAR_NAME] = `${cardId}:${ackCommentId}`; + } +} diff --git a/tests/unit/backends/claude-code-hooks.test.ts b/tests/unit/backends/claude-code-hooks.test.ts index 8b21e668..5fd0430a 100644 --- a/tests/unit/backends/claude-code-hooks.test.ts +++ b/tests/unit/backends/claude-code-hooks.test.ts @@ -223,40 +223,6 @@ describe('buildStopHooks', () => { ); }); - it('ignores .cascade-progress-comment-id in uncommitted changes', async () => { - mockExecSync.mockReturnValueOnce('?? .cascade-progress-comment-id'); - mockExecSync.mockReturnValueOnce(''); // git log (no unpushed) - - const logWriter = makeLogWriter(); - const [matcher] = buildStopHooks(logWriter, '/tmp/repo'); - const [hook] = matcher.hooks; - - const result = await hook(makeStopInput(), undefined, { signal: AbortSignal.timeout(5000) }); - - expect(result).toEqual({ decision: 'approve' }); - expect(logWriter).not.toHaveBeenCalledWith('WARN', expect.anything(), expect.anything()); - }); - - it('still blocks when real changes exist alongside .cascade-progress-comment-id', async () => { - mockExecSync.mockReturnValueOnce('?? .cascade-progress-comment-id\n M src/index.ts'); - - const logWriter = makeLogWriter(); - const [matcher] = buildStopHooks(logWriter, '/tmp/repo'); - const [hook] = matcher.hooks; - - const result = await hook(makeStopInput(), undefined, { signal: AbortSignal.timeout(5000) }); - - expect(result).toEqual({ - decision: 'block', - reason: expect.stringContaining('uncommitted changes'), - }); - expect(logWriter).toHaveBeenCalledWith( - 'WARN', - 'Stop hook blocked: uncommitted changes', - expect.objectContaining({ status: 'M src/index.ts' }), - ); - }); - it('blocks when unpushed commits exist (no upstream tracking)', async () => { // git status --porcelain returns clean mockExecSync.mockReturnValueOnce(''); diff --git a/tests/unit/backends/pmPoster.test.ts b/tests/unit/backends/pmPoster.test.ts index d615f026..71e2aad5 100644 --- a/tests/unit/backends/pmPoster.test.ts +++ b/tests/unit/backends/pmPoster.test.ts @@ -47,7 +47,7 @@ const mockPMProvider = { }; beforeEach(() => { - // Default: state file exists + // Default: env var is set (progress state exists) mockReadProgressCommentId.mockReturnValue({ workItemId: 'card1', commentId: 'comment1' }); }); @@ -109,24 +109,14 @@ describe('PMProgressPoster — postInitial()', () => { ); }); - it('writes state file when repoDir is provided', async () => { + it('writes env var with cardId and commentId after posting', async () => { mockGetPMProvider.mockReturnValue(mockPMProvider as unknown as PMProvider); mockPMProvider.addComment.mockResolvedValue('initial-id'); - const poster = makePoster({ repoDir: '/tmp/repo' }); - - await poster.postInitial(); - - expect(mockWriteProgressCommentId).toHaveBeenCalledWith('/tmp/repo', 'card1', 'initial-id'); - }); - - it('does not write state file when repoDir is absent', async () => { - mockGetPMProvider.mockReturnValue(mockPMProvider as unknown as PMProvider); - mockPMProvider.addComment.mockResolvedValue('initial-id'); - const poster = makePoster(); // no repoDir + const poster = makePoster(); await poster.postInitial(); - expect(mockWriteProgressCommentId).not.toHaveBeenCalled(); + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card1', 'initial-id'); }); }); @@ -141,14 +131,14 @@ describe('PMProgressPoster — update()', () => { it('creates new comment when no existing comment ID (fallback branch)', async () => { mockGetPMProvider.mockReturnValue(mockPMProvider as unknown as PMProvider); mockPMProvider.addComment.mockResolvedValue('tick-id'); - const poster = makePoster({ repoDir: '/tmp/repo' }); + const poster = makePoster(); // No initial comment was posted await poster.update('First progress update'); expect(mockPMProvider.addComment).toHaveBeenCalledWith('card1', 'First progress update'); expect(poster.getCommentId()).toBe('tick-id'); - expect(mockWriteProgressCommentId).toHaveBeenCalledWith('/tmp/repo', 'card1', 'tick-id'); + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card1', 'tick-id'); }); it('updates existing comment when comment ID is set', async () => { @@ -167,9 +157,9 @@ describe('PMProgressPoster — update()', () => { expect(mockPMProvider.addComment).not.toHaveBeenCalled(); }); - it('skips update when state file has been cleared by agent subprocess', async () => { + it('skips update when env var has been cleared by agent subprocess', async () => { mockGetPMProvider.mockReturnValue(mockPMProvider as unknown as PMProvider); - mockReadProgressCommentId.mockReturnValue(null); // state file cleared + mockReadProgressCommentId.mockReturnValue(null); // env var cleared const poster = makePoster(); poster.setCommentId('existing-id'); @@ -183,7 +173,7 @@ describe('PMProgressPoster — update()', () => { mockGetPMProvider.mockReturnValue(mockPMProvider as unknown as PMProvider); mockPMProvider.updateComment.mockRejectedValue(new Error('Comment not found')); mockPMProvider.addComment.mockResolvedValue('fallback-id'); - const poster = makePoster({ repoDir: '/tmp/repo' }); + const poster = makePoster(); poster.setCommentId('deleted-id'); await poster.update('Fallback summary'); @@ -195,6 +185,6 @@ describe('PMProgressPoster — update()', () => { ); expect(mockPMProvider.addComment).toHaveBeenCalledWith('card1', 'Fallback summary'); expect(poster.getCommentId()).toBe('fallback-id'); - expect(mockWriteProgressCommentId).toHaveBeenCalledWith('/tmp/repo', 'card1', 'fallback-id'); + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card1', 'fallback-id'); }); }); diff --git a/tests/unit/backends/progress.test.ts b/tests/unit/backends/progress.test.ts index 92ed982e..6e20afc9 100644 --- a/tests/unit/backends/progress.test.ts +++ b/tests/unit/backends/progress.test.ts @@ -952,7 +952,7 @@ describe('ProgressMonitor — preSeededCommentId', () => { monitor.stop(); }); - it('writes state file for pre-seeded comment ID', async () => { + it('writes env var for pre-seeded comment ID', async () => { const monitor = new ProgressMonitor({ agentType: 'implementation', taskDescription: 'Test task', @@ -961,22 +961,17 @@ describe('ProgressMonitor — preSeededCommentId', () => { customModels: [], logWriter: vi.fn(), trello: { cardId: 'card1' }, - repoDir: '/tmp/test-repo', preSeededCommentId: 'router-ack-comment-42', }); monitor.start(); await vi.advanceTimersByTimeAsync(0); - expect(mockWriteProgressCommentId).toHaveBeenCalledWith( - '/tmp/test-repo', - 'card1', - 'router-ack-comment-42', - ); + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card1', 'router-ack-comment-42'); monitor.stop(); }); - it('does not write state file when repoDir is missing', async () => { + it('writes env var for pre-seeded comment ID even without repoDir', async () => { const monitor = new ProgressMonitor({ agentType: 'implementation', taskDescription: 'Test task', @@ -991,7 +986,7 @@ describe('ProgressMonitor — preSeededCommentId', () => { monitor.start(); await vi.advanceTimersByTimeAsync(0); - expect(mockWriteProgressCommentId).not.toHaveBeenCalled(); + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card1', 'router-ack-comment-42'); monitor.stop(); }); @@ -1028,8 +1023,8 @@ describe('ProgressMonitor — preSeededCommentId', () => { }); }); -describe('ProgressMonitor — state file integration', () => { - it('writes state file on initial comment when repoDir is provided', async () => { +describe('ProgressMonitor — env var integration', () => { + it('writes env var on initial comment', async () => { const logWriter = vi.fn(); const monitor = new ProgressMonitor({ agentType: 'planning', @@ -1038,7 +1033,6 @@ describe('ProgressMonitor — state file integration', () => { progressModel: 'test-model', customModels: [], logWriter, - repoDir: '/tmp/test-repo', trello: { cardId: 'card1' }, }); @@ -1048,15 +1042,11 @@ describe('ProgressMonitor — state file integration', () => { monitor.start(); await vi.advanceTimersByTimeAsync(0); - expect(mockWriteProgressCommentId).toHaveBeenCalledWith( - '/tmp/test-repo', - 'card1', - 'comment-id-initial', - ); + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card1', 'comment-id-initial'); monitor.stop(); }); - it('does not write state file when repoDir is not provided', async () => { + it('clears env var on stop()', () => { const monitor = new ProgressMonitor({ agentType: 'planning', taskDescription: 'Test task', @@ -1067,52 +1057,13 @@ describe('ProgressMonitor — state file integration', () => { trello: { cardId: 'card1' }, }); - mockGetPMProvider.mockReturnValue(mockPMProvider as unknown as PMProvider); - mockPMProvider.addComment.mockResolvedValue('comment-id-initial'); - monitor.start(); - await vi.advanceTimersByTimeAsync(0); - - expect(mockWriteProgressCommentId).not.toHaveBeenCalled(); monitor.stop(); - }); - it('clears state file on stop()', () => { - const monitor = new ProgressMonitor({ - agentType: 'planning', - taskDescription: 'Test task', - intervalMinutes: 5, - progressModel: 'test-model', - customModels: [], - logWriter: vi.fn(), - repoDir: '/tmp/test-repo', - trello: { cardId: 'card1' }, - }); - - monitor.start(); - monitor.stop(); - - expect(mockClearProgressCommentId).toHaveBeenCalledWith('/tmp/test-repo'); - }); - - it('clears state file on stop() even when repoDir not provided', () => { - const monitor = new ProgressMonitor({ - agentType: 'planning', - taskDescription: 'Test task', - intervalMinutes: 5, - progressModel: 'test-model', - customModels: [], - logWriter: vi.fn(), - trello: { cardId: 'card1' }, - }); - - monitor.start(); - monitor.stop(); - - expect(mockClearProgressCommentId).toHaveBeenCalledWith(undefined); + expect(mockClearProgressCommentId).toHaveBeenCalledWith(); }); - it('writes state file from first tick when postInitialComment() failed', async () => { + it('writes env var from first tick when postInitialComment() failed', async () => { const logWriter = vi.fn(); const monitor = new ProgressMonitor({ agentType: 'planning', @@ -1121,7 +1072,6 @@ describe('ProgressMonitor — state file integration', () => { progressModel: 'test-model', customModels: [], logWriter, - repoDir: '/tmp/test-repo', trello: { cardId: 'card1' }, }); @@ -1144,15 +1094,11 @@ describe('ProgressMonitor — state file integration', () => { await vi.advanceTimersByTimeAsync(1 * 60 * 1000); monitor.stop(); - // State file should be written from the else branch in postProgressToPM - expect(mockWriteProgressCommentId).toHaveBeenCalledWith( - '/tmp/test-repo', - 'card1', - 'comment-id-from-tick', - ); + // Env var should be written from the else branch in postProgressToPM + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card1', 'comment-id-from-tick'); }); - it('skips progress update when state file is cleared by agent subprocess', async () => { + it('skips progress update when env var is cleared by agent subprocess', async () => { const logWriter = vi.fn(); const monitor = new ProgressMonitor({ agentType: 'respond-to-planning-comment', @@ -1161,7 +1107,6 @@ describe('ProgressMonitor — state file integration', () => { progressModel: 'test-model', customModels: [], logWriter, - repoDir: '/tmp/test-repo', trello: { cardId: 'card1' }, }); @@ -1173,26 +1118,26 @@ describe('ProgressMonitor — state file integration', () => { monitor.start(); await vi.advanceTimersByTimeAsync(0); - // Simulate the PostComment gadget clearing the state file + // Simulate the PostComment gadget clearing the env var mockReadProgressCommentId.mockReturnValue(null); - // First tick fires at 1 minute — should detect cleared state file and skip + // First tick fires at 1 minute — should detect cleared env var and skip await vi.advanceTimersByTimeAsync(1 * 60 * 1000); monitor.stop(); - // updateComment should NOT have been called (state file was cleared) + // updateComment should NOT have been called (env var was cleared) expect(mockPMProvider.updateComment).not.toHaveBeenCalled(); // Should log the skip expect(logWriter).toHaveBeenCalledWith( 'DEBUG', - 'State file cleared by agent — skipping progress update', + 'Env var cleared by agent — skipping progress update', expect.objectContaining({ commentId: 'comment-id-initial' }), ); // progressCommentId should be cleared expect(monitor.getProgressCommentId()).toBeNull(); }); - it('updates state file when new comment is created after update failure', async () => { + it('updates env var when new comment is created after update failure', async () => { const logWriter = vi.fn(); const monitor = new ProgressMonitor({ agentType: 'planning', @@ -1201,7 +1146,6 @@ describe('ProgressMonitor — state file integration', () => { progressModel: 'test-model', customModels: [], logWriter, - repoDir: '/tmp/test-repo', trello: { cardId: 'card1' }, }); @@ -1220,10 +1164,6 @@ describe('ProgressMonitor — state file integration', () => { // writeProgressCommentId called for initial comment and for fallback comment expect(mockWriteProgressCommentId).toHaveBeenCalledTimes(2); - expect(mockWriteProgressCommentId).toHaveBeenLastCalledWith( - '/tmp/test-repo', - 'card1', - 'comment-id-fallback', - ); + expect(mockWriteProgressCommentId).toHaveBeenLastCalledWith('card1', 'comment-id-fallback'); }); }); diff --git a/tests/unit/backends/progressMonitor.test.ts b/tests/unit/backends/progressMonitor.test.ts index 0b394f7b..e5b202d5 100644 --- a/tests/unit/backends/progressMonitor.test.ts +++ b/tests/unit/backends/progressMonitor.test.ts @@ -219,16 +219,15 @@ describe('ProgressMonitor - start()', () => { expect(mockPMPosterPostInitial).not.toHaveBeenCalled(); }); - it('writes progress comment ID to state file when preSeededCommentId + repoDir + trello', () => { + it('writes progress comment ID to env var when preSeededCommentId + trello', () => { const config = makeConfig({ preSeededCommentId: 'seed-id', - repoDir: '/tmp/repo', trello: { cardId: 'card-1' }, }); const monitor = new ProgressMonitor(config); monitor.start(); - expect(mockWriteProgressCommentId).toHaveBeenCalledWith('/tmp/repo', 'card-1', 'seed-id'); + expect(mockWriteProgressCommentId).toHaveBeenCalledWith('card-1', 'seed-id'); }); it('posts initial comment when no preSeededCommentId and trello is configured', () => { @@ -260,11 +259,10 @@ describe('ProgressMonitor - stop()', () => { }); it('clears the progress comment ID state', () => { - const config = makeConfig({ repoDir: '/tmp/repo' }); - const monitor = new ProgressMonitor(config); + const monitor = new ProgressMonitor(makeConfig()); monitor.stop(); - expect(mockClearProgressCommentId).toHaveBeenCalledWith('/tmp/repo'); + expect(mockClearProgressCommentId).toHaveBeenCalledWith(); }); it('does not throw when clearProgressCommentId fails', () => { diff --git a/tests/unit/backends/progressState.test.ts b/tests/unit/backends/progressState.test.ts index db1861bf..dd97365d 100644 --- a/tests/unit/backends/progressState.test.ts +++ b/tests/unit/backends/progressState.test.ts @@ -1,118 +1,105 @@ -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { + ENV_VAR_NAME, clearProgressCommentId, readProgressCommentId, writeProgressCommentId, } from '../../../src/backends/progressState.js'; -const STATE_FILE_NAME = '.cascade-progress-comment-id'; - describe('progressState utilities', () => { - let tmpDir: string; - let origCwd: string; - beforeEach(() => { - tmpDir = join(tmpdir(), `cascade-test-${Date.now()}`); - mkdirSync(tmpDir, { recursive: true }); - origCwd = process.cwd(); - process.chdir(tmpDir); + // Ensure clean env var state before each test + delete process.env[ENV_VAR_NAME]; }); afterEach(() => { - process.chdir(origCwd); - rmSync(tmpDir, { recursive: true, force: true }); + // Clean up after each test + delete process.env[ENV_VAR_NAME]; }); describe('writeProgressCommentId', () => { - it('writes workItemId:commentId to state file in repoDir', () => { - writeProgressCommentId(tmpDir, 'card123', 'comment456'); - - const stateFile = join(tmpDir, STATE_FILE_NAME); - expect(existsSync(stateFile)).toBe(true); + it('writes workItemId:commentId to env var', () => { + writeProgressCommentId('card123', 'comment456'); - const content = readFileSync(stateFile, 'utf-8'); - expect(content).toBe('card123:comment456'); + expect(process.env[ENV_VAR_NAME]).toBe('card123:comment456'); }); - it('overwrites existing state file', () => { - writeProgressCommentId(tmpDir, 'card1', 'comment1'); - writeProgressCommentId(tmpDir, 'card2', 'comment2'); + it('overwrites existing env var', () => { + writeProgressCommentId('card1', 'comment1'); + writeProgressCommentId('card2', 'comment2'); + expect(process.env[ENV_VAR_NAME]).toBe('card2:comment2'); const result = readProgressCommentId(); expect(result).toEqual({ workItemId: 'card2', commentId: 'comment2' }); }); }); describe('readProgressCommentId', () => { - it('returns null when state file does not exist', () => { + it('returns null when env var is not set', () => { const result = readProgressCommentId(); expect(result).toBeNull(); }); - it('returns workItemId and commentId from state file in cwd', () => { - writeProgressCommentId(tmpDir, 'my-card', 'my-comment'); + it('returns workItemId and commentId from env var', () => { + writeProgressCommentId('my-card', 'my-comment'); const result = readProgressCommentId(); expect(result).toEqual({ workItemId: 'my-card', commentId: 'my-comment' }); }); - it('returns null for malformed state file (no colon)', () => { - writeFileSync(join(tmpDir, STATE_FILE_NAME), 'no-colon-here', 'utf-8'); + it('returns null for malformed env var (no colon)', () => { + process.env[ENV_VAR_NAME] = 'no-colon-here'; const result = readProgressCommentId(); expect(result).toBeNull(); }); - it('returns null for empty state file', () => { - writeFileSync(join(tmpDir, STATE_FILE_NAME), '', 'utf-8'); + it('returns null for empty env var', () => { + process.env[ENV_VAR_NAME] = ''; const result = readProgressCommentId(); expect(result).toBeNull(); }); - it('reads from explicit repoDir when provided', () => { - writeProgressCommentId(tmpDir, 'my-card', 'my-comment'); + it('handles commentId that contains colons (e.g. JIRA IDs)', () => { + writeProgressCommentId('PROJ-123', 'comment:with:colons'); - const result = readProgressCommentId(tmpDir); - expect(result).toEqual({ workItemId: 'my-card', commentId: 'my-comment' }); + const result = readProgressCommentId(); + expect(result).toEqual({ workItemId: 'PROJ-123', commentId: 'comment:with:colons' }); }); - it('handles commentId that contains colons (e.g. JIRA IDs)', () => { - writeProgressCommentId(tmpDir, 'PROJ-123', 'comment:with:colons'); + it('returns null when workItemId is empty', () => { + process.env[ENV_VAR_NAME] = ':comment-only'; const result = readProgressCommentId(); - expect(result).toEqual({ workItemId: 'PROJ-123', commentId: 'comment:with:colons' }); + expect(result).toBeNull(); }); - }); - describe('clearProgressCommentId', () => { - it('deletes state file from repoDir', () => { - writeProgressCommentId(tmpDir, 'card1', 'comment1'); - expect(existsSync(join(tmpDir, STATE_FILE_NAME))).toBe(true); + it('returns null when commentId is empty', () => { + process.env[ENV_VAR_NAME] = 'card-only:'; - clearProgressCommentId(tmpDir); - expect(existsSync(join(tmpDir, STATE_FILE_NAME))).toBe(false); + const result = readProgressCommentId(); + expect(result).toBeNull(); }); + }); - it('deletes state file from cwd when no repoDir provided', () => { - writeProgressCommentId(tmpDir, 'card1', 'comment1'); - expect(existsSync(join(tmpDir, STATE_FILE_NAME))).toBe(true); + describe('clearProgressCommentId', () => { + it('deletes the env var', () => { + writeProgressCommentId('card1', 'comment1'); + expect(process.env[ENV_VAR_NAME]).toBeDefined(); clearProgressCommentId(); - expect(existsSync(join(tmpDir, STATE_FILE_NAME))).toBe(false); + expect(process.env[ENV_VAR_NAME]).toBeUndefined(); }); - it('does not throw when state file does not exist', () => { - expect(() => clearProgressCommentId(tmpDir)).not.toThrow(); + it('does not throw when env var is not set', () => { + expect(() => clearProgressCommentId()).not.toThrow(); }); it('leaves readProgressCommentId returning null after clear', () => { - writeProgressCommentId(tmpDir, 'card1', 'comment1'); - clearProgressCommentId(tmpDir); + writeProgressCommentId('card1', 'comment1'); + clearProgressCommentId(); const result = readProgressCommentId(); expect(result).toBeNull(); diff --git a/tests/unit/backends/secretBuilder.test.ts b/tests/unit/backends/secretBuilder.test.ts index 6fc2a936..a27f7782 100644 --- a/tests/unit/backends/secretBuilder.test.ts +++ b/tests/unit/backends/secretBuilder.test.ts @@ -9,7 +9,12 @@ vi.mock('../../../src/github/personas.js', () => ({ })); import type { AgentProfile } from '../../../src/agents/definitions/profiles.js'; -import { augmentProjectSecrets, resolveGitHubToken } from '../../../src/backends/secretBuilder.js'; +import { ENV_VAR_NAME } from '../../../src/backends/progressState.js'; +import { + augmentProjectSecrets, + injectProgressCommentId, + resolveGitHubToken, +} from '../../../src/backends/secretBuilder.js'; import { getAllProjectCredentials } from '../../../src/config/provider.js'; import { getPersonaToken } from '../../../src/github/personas.js'; import type { AgentInput, ProjectConfig } from '../../../src/types/index.js'; @@ -164,3 +169,35 @@ describe('resolveGitHubToken', () => { ); }); }); + +describe('injectProgressCommentId', () => { + it('injects env var when cardId and string ackCommentId are provided', () => { + const secrets: Record = {}; + injectProgressCommentId(secrets, 'card-123', 'ack-comment-456'); + expect(secrets[ENV_VAR_NAME]).toBe('card-123:ack-comment-456'); + }); + + it('does not inject when ackCommentId is a number (GitHub ack comment)', () => { + const secrets: Record = {}; + injectProgressCommentId(secrets, 'card-123', 12345); + expect(secrets[ENV_VAR_NAME]).toBeUndefined(); + }); + + it('does not inject when cardId is undefined', () => { + const secrets: Record = {}; + injectProgressCommentId(secrets, undefined, 'ack-comment-456'); + expect(secrets[ENV_VAR_NAME]).toBeUndefined(); + }); + + it('does not inject when ackCommentId is undefined', () => { + const secrets: Record = {}; + injectProgressCommentId(secrets, 'card-123', undefined); + expect(secrets[ENV_VAR_NAME]).toBeUndefined(); + }); + + it('does not inject when ackCommentId is an empty string', () => { + const secrets: Record = {}; + injectProgressCommentId(secrets, 'card-123', ''); + expect(secrets[ENV_VAR_NAME]).toBeUndefined(); + }); +});