fix: Phase 6 review follow-up — shared util, GET side-effect, dead branch, test gap#13
fix: Phase 6 review follow-up — shared util, GET side-effect, dead branch, test gap#13
Conversation
…effect, fix dead branch, add needs-answer test
There was a problem hiding this comment.
Code Review
This pull request updates the development workflow documentation in CLAUDE.md, refactors the projectGroupFromDirectory function into a shared utility, and removes project auto-creation logic from the agent tree API to prevent side effects during GET requests. Additionally, it simplifies a redundant style property in the AgentCanvas component and adds a test case for session status overrides. Feedback suggests removing an unused import in agent.ts and optimizing the new utility function by hoisting the bucketPrefixes set to a constant to avoid repeated allocations.
| import { getAllCanvasNodes, getAllProjects, getAllSessionRelations, getAllTaskInvocations, getForkRelationMap } from '../db/index.js' | ||
| import { opencodeAdapter } from '../opencode/index.js' | ||
| import { getPendingPermissions, getPendingQuestions } from '../sse/broadcaster.js' | ||
| import { projectGroupFromDirectory } from '../utils/projectGroup.js' |
| export function projectGroupFromDirectory(directory: string): string { | ||
| const marker = '/workspace/' | ||
| const index = directory.indexOf(marker) | ||
| const normalized = index >= 0 ? directory.slice(index + marker.length) : directory.replace(/^\/+/, '') | ||
| const parts = normalized.split('/').filter(Boolean) | ||
| if (parts.length === 0) return 'workspace' | ||
| const bucketPrefixes = new Set(['apps', 'research', 'pypi_lib', 'libs', 'infra', 'skills', 'mcps', 'anal-repo']) | ||
| if (parts.length >= 2 && bucketPrefixes.has(parts[0])) { | ||
| return `${parts[0]}/${parts[1]}` | ||
| } | ||
| return parts[0] | ||
| } |
There was a problem hiding this comment.
The bucketPrefixes Set is currently allocated on every call to projectGroupFromDirectory. Since this function is called within loops (e.g., in tree.ts), moving this set to a constant outside the function scope will avoid unnecessary allocations and improve performance.
| export function projectGroupFromDirectory(directory: string): string { | |
| const marker = '/workspace/' | |
| const index = directory.indexOf(marker) | |
| const normalized = index >= 0 ? directory.slice(index + marker.length) : directory.replace(/^\/+/, '') | |
| const parts = normalized.split('/').filter(Boolean) | |
| if (parts.length === 0) return 'workspace' | |
| const bucketPrefixes = new Set(['apps', 'research', 'pypi_lib', 'libs', 'infra', 'skills', 'mcps', 'anal-repo']) | |
| if (parts.length >= 2 && bucketPrefixes.has(parts[0])) { | |
| return `${parts[0]}/${parts[1]}` | |
| } | |
| return parts[0] | |
| } | |
| const BUCKET_PREFIXES = new Set(['apps', 'research', 'pypi_lib', 'libs', 'infra', 'skills', 'mcps', 'anal-repo']) | |
| export function projectGroupFromDirectory(directory: string): string { | |
| const marker = '/workspace/' | |
| const index = directory.indexOf(marker) | |
| const normalized = index >= 0 ? directory.slice(index + marker.length) : directory.replace(/^\/+/, '') | |
| const parts = normalized.split('/').filter(Boolean) | |
| if (parts.length === 0) return 'workspace' | |
| if (parts.length >= 2 && BUCKET_PREFIXES.has(parts[0])) { | |
| return `${parts[0]}/${parts[1]}` | |
| } | |
| return parts[0] | |
| } |
Summary
Follow-up fixes from Phase 6 code review (PR #12).
projectGroupFromDirectoryto shared util (src/server/utils/projectGroup.ts) — removes 3x duplication betweenagent.tsandtree.tson the server side/api/agent/tree—setCanvasNodeProjectwas called on every GET request; removed entirely; auto-assignment remains inGET /api/tree(tree.ts)background: sseStatus === 'disconnected' ? '#1c1917' : '#1c1917'collapsed tobackground: '#1c1917'needs-answerstatus override test —agent.test.tswas missing coverage for thequestion.asked→needs-answerpathTest plan
grep setCanvasNodeProject src/server/routes/agent.ts→ 0 matchessrc/server/utils/projectGroup.tscreated and imported in agent.ts + tree.ts