Merge dev to main

src/agents/prompts/templates/respond-to-planning-comment.eta

@@ -1,10 +1,10 @@
 You are a senior software architect responding to a user's comment on a planning <%= it.workItemNoun || 'card' %>.
 
 CRITICAL:
-1. **PLANNING ONLY** - Your ONLY job is to make targeted updates to the plan. DO NOT implement, edit source files, write code, or make any changes to the codebase.
-2. **READ THE COMMENT CAREFULLY** - The user's comment is your primary instruction. Understand exactly what they're asking for before making changes.
-3. **SURGICAL UPDATES** - Default to targeted, minimal changes to the <%= it.workItemNoun || 'card' %> description/checklists. Only do a full rewrite if the user clearly asks for one.
-4. DO NOT JUST OUTPUT TEXT - You MUST use UpdateWorkItem, AddChecklist, and PostComment gadgets.
+1. **PLANNING ONLY** - Your primary role is to update the plan and answer questions about it. DO NOT implement, edit source files, write code, or make any changes to the codebase.
+2. **READ THE COMMENT CAREFULLY** - The user's comment is your primary instruction. Understand exactly what they're asking for before taking action.
+3. **CLASSIFY THE COMMENT** - Determine whether the comment requires plan changes, a conversational reply, or both (see Comment Classification below).
+4. **SURGICAL UPDATES** - When plan changes are needed, default to targeted, minimal changes to the <%= it.workItemNoun || 'card' %> description/checklists. Only do a full rewrite if the user clearly asks for one.
 5. COMMUNICATE WITH THE USER OVER <%= it.pmName || 'Trello' %> EXCLUSIVELY - Use PostComment and UpdateWorkItem.
 6. DO NOT MANAGE LABELS - Labels (PROCESSING, PROCESSED, etc.) are handled automatically by the system.
 
@@ -29,6 +29,16 @@ You are running in a cloned copy of the project repository. Before updating the
 
 <%~ include("partials/squint-exploration") %>
 
+## Comment Classification
+
+Before acting, classify the user's comment into one of three categories:
+
+| Category | Signals | Action |
+|----------|---------|--------|
+| **A: Question / Clarification** | "Why?", "Can you explain?", "How does X relate to Y?", "What's the risk of…?", "Is there a reason…?" | Explore the codebase to ground your answer → `PostComment` only. Do NOT call `UpdateWorkItem` or modify checklists. |
+| **B: Plan Update** *(default)* | "Add error handling", "Remove step 3", "Split into phases", "The path should be X", "Change the approach to…" | Update the plan → `PostComment` summarizing changes. **This is the default when intent is ambiguous.** |
+| **C: Both** | "Can you explain step 3? Also add error handling to it.", "Why this approach? And please add a migration step." | Update the plan AND answer the question in the same `PostComment`. |
+
 ## Codebase Pattern Analysis
 
 **Your updates MUST align with existing codebase conventions.** Before proposing any changes:
@@ -49,9 +59,12 @@ You are running in a cloned copy of the project repository. Before updating the
 
 1. **Read the triggering comment** — it's provided in the user prompt below
 2. **Read the current <%= it.workItemNoun || 'card' %>** using ReadWorkItem to understand the existing plan
-3. **Explore the codebase** as needed to ground your changes in reality
-4. **Make surgical updates** to the <%= it.workItemNoun || 'card' %> description and/or checklists based on the user's request
-5. **Post a reply comment** via PostComment explaining what you changed and why
+3. **Classify the comment** — determine Category A, B, or C (see Comment Classification above)
+4. **Explore the codebase** as needed to ground your response in reality
+5. **Act based on the category:**
+   - **Category A (Question):** Research the answer using codebase exploration, then `PostComment` with a thorough, grounded reply. Do NOT call `UpdateWorkItem` or modify checklists.
+   - **Category B (Plan Update):** Make surgical updates to the <%= it.workItemNoun || 'card' %> description and/or checklists, then `PostComment` summarizing what changed.
+   - **Category C (Both):** Update the plan AND answer the question in the same `PostComment`.
 
 ## Updating the Plan and Checklists
 
@@ -68,7 +81,7 @@ When the user asks to narrow scope, focus on a subset, or drop items from the pl
 
 When updating the <%= it.workItemNoun || 'card' %>, preserve the existing format with **emoji section headers** and **bold key terms**. Only modify the sections that need to change.
 
-After making updates, post a reply comment:
+**For plan updates (Category B or C)**, post a reply comment:
 
 ```
 📝 **Plan Updated**
@@ -80,6 +93,14 @@ Based on your comment, I've made the following changes:
 [Any additional context or rationale]
 ```
 
+**For question-only replies (Category A)**, post a reply comment:
+
+```
+💬 **Re: [brief topic]**
+
+[Thorough, codebase-grounded answer with specific file/function references]
+```
+
 <%~ include("partials/environment") %>
 
 ## Gadgets Available
@@ -104,10 +125,12 @@ Based on your comment, I've made the following changes:
 - ALWAYS read the triggering comment carefully before doing anything
 - ALWAYS use `ReadWorkItem` to understand the current state of the <%= it.workItemNoun || 'card' %>
 - ALWAYS explore the codebase when the user's request requires understanding code structure
-- ALWAYS use `UpdateWorkItem` to save your changes - DON'T JUST OUTPUT TEXT
-- ALWAYS post a reply comment via `PostComment` explaining what you changed
+- Use `UpdateWorkItem` to save plan changes when they are needed (Category B or C) — DON'T JUST OUTPUT TEXT when the user requests changes
+- NEVER modify the plan when the comment is purely a question or request for clarification (Category A) — reply with `PostComment` only
+- ALWAYS post a reply comment via `PostComment` — for updates, explain what changed; for questions, provide a thorough codebase-grounded answer
 - ALWAYS preserve existing formatting (emoji headers, bold terms, markdown links)
 - ALWAYS use markdown link syntax `[title](url)` when referencing other <%= it.workItemNounPlural || 'cards' %>
 - DEFAULT to surgical, targeted changes — don't rewrite sections that don't need changing
-- Ground your changes in actual code exploration
+- DEFAULT to plan updates (Category B) when intent is ambiguous
+- Ground your responses in actual code exploration
 - Be specific about file paths and function names

src/agents/shared/taskPrompts.ts

@@ -37,7 +37,7 @@ ${commentText}
 ---
 
 The work item data (title, description, checklists, attachments, comments) has been pre-loaded above.
-Read the user's comment carefully and respond accordingly. Default to surgical, targeted updates unless they clearly ask for a full rewrite.`;
+Read the user's comment carefully and classify it: if they ask a question or request clarification, reply with a thorough answer via PostComment (do not modify the plan). If they request plan changes, make surgical, targeted updates. If the comment contains both a question and a change request, do both. Default to plan updates when intent is ambiguous.`;
 }
 
 // ============================================================================

src/backends/secretBuilder.ts

@@ -16,13 +16,7 @@ export async function resolveGitHubToken(
 ): Promise<string | undefined> {
 	if (!profile.needsGitHubToken) return undefined;
 
-	try {
-		return await getPersonaToken(projectId, agentType);
-	} catch {
-		// Fall back to legacy GITHUB_TOKEN for projects not yet migrated
-		const secrets = await getAllProjectCredentials(projectId);
-		return secrets.GITHUB_TOKEN;
-	}
+	return getPersonaToken(projectId, agentType);
 }
 
 /**

src/config/provider.ts

@@ -124,6 +124,14 @@ export async function getIntegrationCredential(
 		return process.env[envKey];
 	}
 
+	// Worker context: all credentials set by router, this one doesn't exist
+	if (process.env.CASCADE_CREDENTIAL_KEYS) {
+		throw new Error(
+			`Integration credential '${category}/${role}' not found for project '${projectId}'`,
+		);
+	}
+
+	// Router/dashboard context: resolve from DB
 	const value = await resolveIntegrationCredential(projectId, category, role);
 	if (value) return value;
 
@@ -146,6 +154,12 @@ export async function getIntegrationCredentialOrNull(
 		return process.env[envKey];
 	}
 
+	// Worker context: all credentials set by router, this one doesn't exist
+	if (process.env.CASCADE_CREDENTIAL_KEYS) {
+		return null;
+	}
+
+	// Router/dashboard context: resolve from DB
 	return resolveIntegrationCredential(projectId, category, role);
 }
 
@@ -166,6 +180,12 @@ export async function getOrgCredential(
 		return process.env[envVarKey];
 	}
 
+	// Worker context: all credentials set by router, this one doesn't exist
+	if (process.env.CASCADE_CREDENTIAL_KEYS) {
+		return null;
+	}
+
+	// Router/dashboard context: resolve from DB
 	const orgId = await getOrgIdForProject(projectId);
 	return resolveOrgCredential(orgId, envVarKey);
 }
@@ -181,6 +201,19 @@ export async function getOrgCredential(
  * 3. Merges integration credentials over org defaults
  */
 export async function getAllProjectCredentials(projectId: string): Promise<Record<string, string>> {
+	// Worker context: reconstruct from individual env vars set by the router
+	const keyList = process.env.CASCADE_CREDENTIAL_KEYS;
+	if (keyList) {
+		const result: Record<string, string> = {};
+		for (const key of keyList.split(',')) {
+			if (key && process.env[key]) {
+				result[key] = process.env[key];
+			}
+		}
+		return result;
+	}
+
+	// Router/dashboard context: resolve from DB (has CREDENTIAL_MASTER_KEY)
 	const orgId = await getOrgIdForProject(projectId);
 
 	const [integrationCreds, orgCreds] = await Promise.all([

src/router/worker-manager.ts

@@ -63,15 +63,16 @@ async function buildWorkerEnv(job: Job<CascadeJob>): Promise<string[]> {
 		`LOG_LEVEL=${process.env.LOG_LEVEL || 'info'}`,
 	];
 
-	// Resolve project credentials in the router and pass as JSON.
-	// Workers cache these on startup, then scrub from env.
+	// Resolve project credentials in the router and set as individual env vars.
 	// NOTE: CREDENTIAL_MASTER_KEY is intentionally NOT passed to workers.
 	const projectId = await extractProjectIdFromJob(job.data);
 	if (projectId) {
 		try {
 			const secrets = await getAllProjectCredentials(projectId);
-			env.push(`CASCADE_CREDENTIALS=${JSON.stringify(secrets)}`);
-			env.push(`CASCADE_CREDENTIALS_PROJECT_ID=${projectId}`);
+			for (const [key, value] of Object.entries(secrets)) {
+				env.push(`${key}=${value}`);
+			}
+			env.push(`CASCADE_CREDENTIAL_KEYS=${Object.keys(secrets).join(',')}`);
 		} catch (err) {
 			console.warn('[WorkerManager] Failed to resolve credentials for project:', {
 				projectId,
@@ -93,7 +94,7 @@ async function spawnWorker(job: Job<CascadeJob>): Promise<void> {
 	const containerName = `cascade-worker-${jobId}`;
 
 	const workerEnv = await buildWorkerEnv(job);
-	const hasCredentials = workerEnv.some((e) => e.startsWith('CASCADE_CREDENTIALS='));
+	const hasCredentials = workerEnv.some((e) => e.startsWith('CASCADE_CREDENTIAL_KEYS='));
 
 	console.log('[WorkerManager] Spawning worker:', {
 		jobId,

src/utils/envScrub.ts

@@ -15,16 +15,13 @@ const SENSITIVE_ENV_KEYS = [
 	'DATABASE_URL',
 	'DATABASE_SSL',
 	'REDIS_URL',
-	'CASCADE_CREDENTIALS',
-	'CASCADE_CREDENTIALS_PROJECT_ID',
 ] as const;
 
 /**
  * Remove sensitive environment variables from process.env.
  *
  * Call this AFTER:
  * - Database connection pool is initialized (getDb())
- * - Project credentials are set as individual env vars (loadRouterCredentials())
  *
  * After scrubbing:
  * - Database pool continues to work (uses cached connection string)

src/worker-entry.ts

@@ -88,20 +88,6 @@ type DashboardJobData = ManualRunJobData | RetryRunJobData | DebugAnalysisJobDat
 
 type JobData = TrelloJobData | GitHubJobData | JiraJobData | DashboardJobData;
 
-function loadRouterCredentials(): void {
-	const credentialsJson = process.env.CASCADE_CREDENTIALS;
-	if (!credentialsJson) return;
-	try {
-		const secrets = JSON.parse(credentialsJson) as Record<string, string>;
-		for (const [key, value] of Object.entries(secrets)) {
-			process.env[key] = value;
-		}
-		logger.info('[Worker] Set credentials as env vars from router');
-	} catch (err) {
-		logger.warn('[Worker] Failed to parse CASCADE_CREDENTIALS', { error: String(err) });
-	}
-}
-
 async function processDashboardJob(jobId: string, jobData: DashboardJobData): Promise<void> {
 	const { loadProjectConfigById } = await import('./config/provider.js');
 
@@ -177,18 +163,15 @@ async function main(): Promise<void> {
 	const config = await loadConfig();
 	logger.info('[Worker] Loaded projects config', { projects: config.projects.map((p) => p.id) });
 
-	// Set credentials as individual env vars (passed as JSON in CASCADE_CREDENTIALS).
-	// Router resolves and decrypts credentials before spawning workers, so workers
-	// never need the CREDENTIAL_MASTER_KEY.
-	if (process.env.CASCADE_CREDENTIALS) {
-		loadRouterCredentials();
-	} else {
+	// Credentials are set as individual env vars by the router (Docker env).
+	// CASCADE_CREDENTIAL_KEYS lists the key names for reconstruction.
+	if (!process.env.CASCADE_CREDENTIAL_KEYS) {
 		logger.error('[Worker] No credentials passed from router - job will likely fail', {
 			jobType: jobData.type,
 		});
 	}
 
-	// SECURITY: Scrub sensitive env vars (DATABASE_URL, CASCADE_CREDENTIALS, etc.)
+	// SECURITY: Scrub sensitive env vars (DATABASE_URL, etc.)
 	// before agent execution. Subprocesses (Tmux, etc.) will not inherit these secrets.
 	scrubSensitiveEnv();
 	logger.info('[Worker] Scrubbed sensitive env vars');

tests/unit/agents/prompts.test.ts

@@ -77,6 +77,38 @@ describe('system prompts content', () => {
 		expect(prompt).toContain('Tmux');
 		expect(prompt).toContain('conventional commits');
 	});
+
+	it('respond-to-planning-comment prompt includes comment classification', () => {
+		const prompt = getSystemPrompt('respond-to-planning-comment');
+		expect(prompt).toContain('Comment Classification');
+		expect(prompt).toContain('Question / Clarification');
+		expect(prompt).toContain('Plan Update');
+	});
+
+	it('respond-to-planning-comment prompt includes both response formats', () => {
+		const prompt = getSystemPrompt('respond-to-planning-comment');
+		expect(prompt).toContain('Plan Updated');
+		expect(prompt).toContain('Re: [brief topic]');
+	});
+
+	it('respond-to-planning-comment prompt instructs no plan changes for questions', () => {
+		const prompt = getSystemPrompt('respond-to-planning-comment');
+		expect(prompt).toContain('NEVER modify the plan when the comment is purely a question');
+		expect(prompt).toContain('PostComment');
+	});
+
+	it('respond-to-planning-comment prompt defaults to plan updates for ambiguous comments', () => {
+		const prompt = getSystemPrompt('respond-to-planning-comment');
+		expect(prompt).toContain('DEFAULT to plan updates (Category B) when intent is ambiguous');
+	});
+
+	it('respond-to-planning-comment prompt includes classify step in task flow', () => {
+		const prompt = getSystemPrompt('respond-to-planning-comment');
+		expect(prompt).toContain('Classify the comment');
+		expect(prompt).toContain('Category A (Question)');
+		expect(prompt).toContain('Category B (Plan Update)');
+		expect(prompt).toContain('Category C (Both)');
+	});
 });
 
 describe('resolveIncludes', () => {

tests/unit/agents/shared/taskPrompts.test.ts

@@ -30,7 +30,7 @@ describe('buildCommentResponsePrompt', () => {
 		expect(prompt).toContain('@alice');
 	});
 
-	it('instructs surgical updates by default', () => {
+	it('instructs surgical updates for plan changes', () => {
 		const prompt = buildCommentResponsePrompt('card-1', 'Fix the typo', 'bob');
 		expect(prompt).toContain('surgical');
 	});
@@ -39,6 +39,23 @@ describe('buildCommentResponsePrompt', () => {
 		const prompt = buildCommentResponsePrompt('card-1', 'Update docs', 'carol');
 		expect(prompt).toContain('pre-loaded');
 	});
+
+	it('instructs to classify the comment', () => {
+		const prompt = buildCommentResponsePrompt('card-1', 'Why this approach?', 'dave');
+		expect(prompt).toContain('classify');
+	});
+
+	it('instructs question-only replies via PostComment without plan modification', () => {
+		const prompt = buildCommentResponsePrompt('card-1', 'Why this approach?', 'dave');
+		expect(prompt).toContain('question');
+		expect(prompt).toContain('PostComment');
+		expect(prompt).toContain('do not modify the plan');
+	});
+
+	it('defaults to plan updates when intent is ambiguous', () => {
+		const prompt = buildCommentResponsePrompt('card-1', 'Some comment', 'eve');
+		expect(prompt).toContain('Default to plan updates when intent is ambiguous');
+	});
 });
 
 describe('buildReviewPrompt', () => {

tests/unit/backends/secretBuilder.test.ts

@@ -157,19 +157,11 @@ describe('resolveGitHubToken', () => {
 		expect(mockGetPersonaToken).toHaveBeenCalledWith('project-id', 'implementation');
 	});
 
-	it('falls back to GITHUB_TOKEN from credentials when getPersonaToken throws', async () => {
+	it('propagates error when getPersonaToken throws', async () => {
 		mockGetPersonaToken.mockRejectedValue(new Error('persona not found'));
-		mockGetAllProjectCredentials.mockResolvedValue({ GITHUB_TOKEN: 'fallback-token' });
 		const profile = makeProfile({ needsGitHubToken: true });
-		const token = await resolveGitHubToken(profile, 'project-id', 'implementation');
-		expect(token).toBe('fallback-token');
-	});
-
-	it('returns undefined from fallback when GITHUB_TOKEN credential is missing', async () => {
-		mockGetPersonaToken.mockRejectedValue(new Error('persona not found'));
-		mockGetAllProjectCredentials.mockResolvedValue({});
-		const profile = makeProfile({ needsGitHubToken: true });
-		const token = await resolveGitHubToken(profile, 'project-id', 'implementation');
-		expect(token).toBeUndefined();
+		await expect(resolveGitHubToken(profile, 'project-id', 'implementation')).rejects.toThrow(
+			'persona not found',
+		);
 	});
 });