vibesdk Feat: General agents + presentations

Summary

This PR refactors the agent architecture to separate behavior logic from infrastructure, enabling support for multiple agent behaviors (phasic and agentic modes).

Changes

Core Architecture

Renamed simpleGeneratorAgent.ts → baseAgent.ts with extracted base behavior class
Added AgentInfrastructure<TState> interface to decouple behavior from Durable Objects
Added BaseAgentBehavior<TState> abstract class with common agent functionality
Added worker/agents/core/phasic/behavior.ts (852 lines) - phasic behavior implementation
Added smartGeneratorAgent.ts - thin wrapper implementing AgentInfrastructure

State Management

Modified state.ts - split state into BaseProjectState, PhasicState, AgenticState
Modified types.ts - added BehaviorType, generic AgentInitArgs<TState>
Breaking: Replaced agentMode: 'deterministic' | 'smart' with behaviorType: 'phasic' | 'agentic'

Operations & Services

Modified 38 files in worker/agents/ to use ICodingAgent interface
Deleted ScreenshotAnalysisOperation.ts - screenshot handling moved/removed
Modified All tool files to accept ICodingAgent instead of concrete class
Modified PhaseImplementation.ts - simplified, moved logic to behavior
Modified GenerationContext.ts - added PhasicGenerationContext variant

Interface Changes

Modified ICodingAgent.ts - formalized interface for all agent behaviors
All tools now work through interface rather than concrete implementation

Motivation

The previous architecture tightly coupled agent behavior to Durable Objects infrastructure, making it difficult to:

Support multiple agent behaviors (deterministic phasic vs autonomous agentic)
Test agent logic without Durable Objects overhead
Reuse agent logic in different execution contexts

This refactoring enables:

Behavior Switching: Select phasic or agentic mode per session
Testing: Mock AgentInfrastructure for unit tests
Extensibility: Add new behaviors without modifying infrastructure

Testing

Manual Testing:

Create new session and verify phasic behavior works
Test all LLM tools (generate_files, deep_debug, etc.)
Verify state persistence across DO hibernation
Test user conversation flow and file regeneration

Areas Requiring Extra Attention:

State migration for existing sessions (old agentMode → new behaviorType)
Screenshot upload functionality (ScreenshotAnalysis operation removed)
Deep debugger integration with new interface

Breaking Changes

State Schema:

Removed: agentMode: 'deterministic' | 'smart'
Added: behaviorType: 'phasic' | 'agentic'

Impact: Existing Durable Object sessions may need migration logic or will default to phasic mode.

Related Issues

Part 1 of agent generalization effort
Enables future agentic behavior implementation
May affect #249 (screenshot workflow) due to ScreenshotAnalysisOperation removal

_{This PR description was automatically generated by Claude Code}

Nov 21 '25 21:11 AshishKumar4

Code Quality & Security Review

Recommendation: REQUEST CHANGES

This PR represents a substantial architectural refactoring to separate agent behavior from infrastructure. The overall design is sound, but there is 1 critical bug that must be fixed before merging.

Critical Issues

1. State Migration Bug - behaviorType Not Applied (CRITICAL)

Location: worker/agents/core/stateMigration.ts:154-164
Issue: The migration computes migratedBehaviorType but never applies it to newState
Impact: Existing Durable Objects with agentMode field will lose their behavior preference, causing sessions to default to undefined/unknown behavior
Fix: Apply migratedBehaviorType to newState.behaviorType before returning
Details: See inline comment

Code Quality

Previous Review Issues - Resolved:

Missing imports (AgenticAgentBehavior, PhasicAgentBehavior) have been fixed
Class names updated to AgenticCodingBehavior and PhasicCodingBehavior
Type guards properly implemented without any type

Architecture Review:

Agent behavior abstraction is well-designed with clear separation of concerns
AgentInfrastructure<TState> interface properly decouples behavior from Durable Objects
BaseCodingBehavior provides solid foundation for both phasic and agentic modes
State management cleanly split into BaseProjectState, PhasicState, AgenticState

Type Safety:

No violations of CLAUDE.md rule about any types detected in core files
Type guards are properly implemented (e.g., isAgenticBlueprint in AgenticProjectBuilder.ts:154-157)
Generic constraints properly used throughout (TState extends BaseProjectState)

Code Organization:

100 files changed is substantial but changes follow consistent patterns
Operations properly updated to use ICodingAgent interface
Tool files correctly accept interface instead of concrete implementation

Security Analysis

SQL Injection Risk: LOW

All SQL queries use parameterized queries (tagged template literals)
Pattern: this.sql\SELECT * FROM table WHERE id = ${id}`` is correct
No string concatenation in SQL queries detected

Authentication & Authorization: NO CHANGES

No modifications to auth/JWT handling in worker/api/
Session management unchanged
User ID handling follows existing patterns

Secrets Handling: SECURE

API keys properly accessed via env.VARIABLE
Legacy userApiKeys field correctly removed in migration (stateMigration.ts:73-81)
No secrets exposure detected

Data Exposure: NO ISSUES

State serialization properly controlled
Conversation history correctly isolated in separate SQLite tables
WebSocket messages use type-safe patterns

Input Validation: EXISTING PATTERNS MAINTAINED

User input handling unchanged in core logic
File path validation uses isPathSafe (codingAgent.ts:659)
Path normalization applied (codingAgent.ts:665)

Workflow Changes: ACCEPTABLE

Changes to .github/workflows/claude-reviews.yml are for the review process itself
No security implications from workflow modifications
Secrets passed to official actions (anthropics/, actions/) is required and safe

Summary

This is a well-architected refactoring that successfully decouples agent behavior from infrastructure. The code quality is high and follows project conventions from CLAUDE.md.

However, the critical state migration bug MUST be fixed before merging. Existing Durable Object sessions could lose their behavior configuration, causing runtime issues.

Approval Status: REQUEST CHANGES due to critical migration bug

Once the migration bug is fixed, this PR will be ready to merge.

Additional Notes

Breaking Changes:

State schema change (agentMode → behaviorType) is properly documented
Migration logic exists but has the critical bug noted above
Consider adding a migration test to verify old states are properly upgraded

Testing Recommendations:

Test state migration with a Durable Object that has old agentMode field
Verify behavior switching between phasic and agentic modes
Test state persistence across DO hibernation
Verify all LLM tools work with new interface abstraction

Dec 09 '25 04:12 github-actions[bot]

diffray code review

Free public review - Want AI code reviews on your PRs? Check out diffray.ai

Summary

Validated 33 issues: 23 kept (1 critical, 7 high, 13 medium, 2 low), 10 filtered (exaggerated claims or low value)

Issues Found: 24

See 10 individual line comment(s) for details.

Full issue list (click to expand)

🟠 HIGH - Debug console.log in production code

File: src/lib/api-client.ts:386

Category: quality

Description: Obvious debug console.log statement left in production code

Suggestion: Remove this debug console.log statement immediately

Confidence: 100%

Rule: fe_console_in_production

🟠 HIGH - Memory leak: setTimeout not cleaned up

File: src/routes/chat/hooks/use-chat.ts:613-632

Category: bug

Description: setTimeout for deployment timeout is created but never cleared, causing memory leak and potential stale closure execution

Suggestion: Store the timeout ID in a ref and clear it on component unmount or when deployment completes. Also clear it in the deployment success/error WebSocket message handlers.

Confidence: 100%

Rule: react_useeffect_missing_cleanup

🟡 MEDIUM - Console statement in production code

File: src/routes/chat/chat.tsx:53

Category: quality

Description: console.error left in production code without proper logging service

Suggestion: Replace with proper error logging using the logger utility from '@/utils/logger'

Confidence: 85%

Rule: fe_console_in_production

🟡 MEDIUM - Console statement in production code

File: src/routes/chat/chat.tsx:224

Category: quality

Description: console.error left in production code without proper logging service

Suggestion: Replace with proper error logging using the logger utility from '@/utils/logger'

Confidence: 85%

Rule: fe_console_in_production

🟡 MEDIUM - Console statement in production code

File: src/routes/chat/utils/handle-websocket-message.ts:176

Category: quality

Description: console.log left in production code - indicates debug code not cleaned up

Suggestion: Remove debug console.log or replace with proper logging using logger utility

Confidence: 90%

Rule: fe_console_in_production

🟡 MEDIUM - Console statement in production code

File: src/routes/chat/utils/handle-websocket-message.ts:214

Category: quality

Description: console.log left in production code - indicates debug code not cleaned up

Suggestion: Remove debug console.log or replace with proper logging using logger utility

Confidence: 90%

Rule: fe_console_in_production

🟡 MEDIUM - Console statement in production code

File: src/routes/chat/utils/handle-websocket-message.ts:349

Category: quality

Description: console.log with emoji left in production code - indicates debug code not cleaned up

Suggestion: Remove debug console.log or replace with proper logging using logger utility

Confidence: 90%

Rule: fe_console_in_production

🟡 MEDIUM - Console statement in production code

File: src/lib/api-client.ts:379

Category: quality

Description: console.log left in production code without proper logging service

Suggestion: Remove or replace with proper error logging using logger utility

Confidence: 90%

Rule: fe_console_in_production

🟡 MEDIUM - Console statement in production code

File: src/lib/api-client.ts:398

Category: quality

Description: console.error left in production code without proper logging service

Suggestion: Replace with proper error logging or remove if not needed

Confidence: 85%

Rule: fe_console_in_production

🟡 MEDIUM - Console statement in production code

File: src/routes/chat/utils/file-state-helpers.ts:59

Category: quality

Description: console.log left in production code - appears to be debug logging

Suggestion: Remove debug console.log or replace with proper logging utility

Confidence: 90%

Rule: fe_console_in_production

🟡 MEDIUM - Multiple console statements in production worker code

File: worker/agents/git/git.ts:67-125

Category: quality

Description: Multiple console.log statements left in Git implementation code

Suggestion: Replace with proper logger utility that's already available in the agent infrastructure

Confidence: 85%

Rule: fe_console_in_production

🟡 MEDIUM - Commented out console.error

File: src/routes/settings/index.tsx:168

Category: quality

Description: Commented code left in the file - should be removed

Suggestion: Remove commented console.error line

Confidence: 80%

Rule: quality_commented_code_blocks

🟡 MEDIUM - Console.warn in production code

File: src/routes/app/index.tsx:315

Category: quality

Description: console.warn left in production code without proper logging service

Suggestion: Replace with proper logging utility or handle error more gracefully

Confidence: 85%

Rule: fe_console_in_production

🟡 MEDIUM - File exceeds 300 lines (1771 lines)

File: worker/agents/core/behaviors/base.ts:1-1771

Category: quality

Description: File is too large and difficult to maintain. Should be split into focused modules.

Suggestion: Split into smaller modules: FileManagement, DeploymentManagement, CommandExecution, DebugOperations, UserInteraction modules

Confidence: 95%

Rule: ts_large_file

🟡 MEDIUM - File exceeds 300 lines (934 lines)

File: src/routes/chat/chat.tsx:1-934

Category: quality

Description: Component file is too large. Extract smaller focused components.

Suggestion: Split into smaller components: ChatContainer, ChatMessages, ChatSidebar, ChatModals as separate files

Confidence: 90%

Rule: ts_large_file

🟡 MEDIUM - File exceeds 300 lines (740 lines)

File: worker/agents/core/codingAgent.ts:1-740

Category: quality

Description: Agent file is too large with mixed concerns.

Suggestion: Extract WebSocket handling, conversation management, and git operations into separate modules

Confidence: 90%

Rule: ts_large_file

🟡 MEDIUM - File exceeds 300 lines (718 lines)

File: worker/agents/core/behaviors/phasic.ts:1-718

Category: quality

Description: Behavior class file too large for maintainability.

Suggestion: Extract phase generation, implementation, and review logic into separate operation classes

Confidence: 90%

Rule: ts_large_file

🟡 MEDIUM - Tight coupling - Direct instantiation of operations

File: worker/agents/core/behaviors/base.ts:72-77

Category: quality

Description: Operations are directly instantiated in constructor, making testing and swapping implementations difficult.

Suggestion: Pass operations via constructor (dependency injection) or use a factory pattern

Confidence: 75%

Rule: arch_tight_coupling

🟡 MEDIUM - Tight coupling - Multiple direct AppService instantiations

File: worker/agents/core/behaviors/base.ts:420-1724

Category: quality

Description: AppService instantiated directly 4 times throughout the class, preventing testability.

Suggestion: Inject AppService via constructor or create a single instance as a class property

Confidence: 85%

Rule: arch_tight_coupling

🟡 MEDIUM - SRP violation - executeCommands handles too many concerns

File: worker/agents/core/behaviors/base.ts:1330-1458

Category: quality

Description: Single method handles sanitization, chunking, retrying, AI regeneration, error handling, and history tracking.

Suggestion: Extract into CommandExecutor class with separate methods for validation, execution, retry logic, and history management

Confidence: 85%

Rule: arch_srp_violation

🟡 MEDIUM - SRP violation - captureScreenshot handles API calls and storage

File: worker/agents/core/behaviors/base.ts:1623-1769

Category: quality

Description: Method mixes screenshot capture API logic with image upload and database persistence.

Suggestion: Extract into ScreenshotService with separate methods for capture, upload, and persistence

Confidence: 80%

Rule: arch_srp_violation

🟠 HIGH - RegExp objects created inside loop - O(N*M) complexity

File: worker/services/code-fixer/fixers/ts2304.ts:170-240

Category: performance

Description: Multiple RegExp objects created per iteration in analyzeUsageContext function called within fileIssues loop. Creates ~20 RegExp objects per issue.

Suggestion: Hoist RegExp creation outside the function using a factory pattern or cache. Create RegExp objects once and reuse them with dynamic name insertion using string methods.

Confidence: 95%

Rule: perf_expensive_in_loop

🟡 MEDIUM - Inline function breaks React.memo memoization

File: src/routes/home.tsx:294

Category: performance

Description: AppCard is wrapped with React.memo but receives inline arrow function onClick={() => navigate(/app/${app.id})} in map loop, creating new function reference on every render and breaking memoization.

Suggestion: Use useCallback to create stable function reference: const handleAppClick = useCallback((id: string) => navigate(/app/${id}), [navigate]); then pass onClick={() => handleAppClick(app.id)} or refactor to pass app.id and handle navigation in AppCard.

Confidence: 85%

Rule: react_inline_function_props

🔵 LOW - Magic numbers without explanation

File: src/routes/home.tsx:448

Category: quality

Description: Values 420 and 100 lack semantic meaning

Suggestion: Extract to named constants like ARROW_MIN_VERTICAL_DISTANCE = 420

Confidence: 70%

Rule: general_magic_numbers

_{Powered by diffray - AI Code Review Agent}

Dec 10 '25 10:12 diffray-bot

Code Quality & Security Review

Recommendation: REQUEST CHANGES

This PR represents a substantial architectural refactoring to separate agent behavior from infrastructure. The overall design is sound and well-executed, but there is 1 critical bug that must be fixed before merging.

Critical Issues

1. State Migration Bug - behaviorType Not Applied (CRITICAL)

Location: worker/agents/core/stateMigration.ts:154-164
Issue: The migration computes migratedBehaviorType but never applies it to newState
Impact: Existing Durable Objects with agentMode field will lose their behavior preference, causing sessions to default to undefined/unknown behavior
Fix: Apply migratedBehaviorType to newState.behaviorType before returning
Details: See inline comment

Code Quality Issues

Console Statements in Production Code (MEDIUM)

Multiple console.log/console.error statements left in production code across:

src/lib/api-client.ts (4 instances)
src/routes/chat/utils/handle-websocket-message.ts (3 instances)
src/routes/chat/utils/file-state-helpers.ts (1 instance)
worker/agents/git/git.ts (11 instances)

Recommendation: Remove debug console statements or replace with proper logger utilities that are already available in the codebase.

Large Files (LOW PRIORITY)

Several files exceed recommended size limits:

worker/agents/core/behaviors/base.ts (1771 lines)
src/routes/chat/chat.tsx (934 lines)
worker/agents/core/codingAgent.ts (740 lines)
worker/agents/core/behaviors/phasic.ts (718 lines)

These are acceptable for this PR given the refactoring scope, but consider breaking them down in future iterations.

Architecture Review

Strengths:

Clean separation of behavior from infrastructure via AgentInfrastructure<TState> interface
Well-designed BaseCodingBehavior provides solid foundation for both phasic and agentic modes
State management cleanly split into BaseProjectState, PhasicState, AgenticState
Operations properly updated to use ICodingAgent interface
Type safety maintained throughout (no any violations detected in core files)

Observations:

Direct operation instantiation in constructors (lines 72-77 in base.ts) creates tight coupling, but acceptable for this architecture
Multiple AppService instantiations throughout could be consolidated, but not critical

Security Analysis

SQL Injection Risk: LOW

All SQL queries use parameterized queries (tagged template literals)
Pattern: this.sql\SELECT * FROM table WHERE id = ${id}`` is correct
No string concatenation in SQL queries detected

Authentication & Authorization: NO CHANGES

No modifications to auth/JWT handling
Session management unchanged
User ID handling follows existing patterns

Secrets Handling: SECURE

API keys properly accessed via env.VARIABLE
Legacy userApiKeys field correctly removed in migration (stateMigration.ts:73-81)
No secrets exposure detected

Data Exposure: NO ISSUES

State serialization properly controlled
Conversation history correctly isolated in separate SQLite tables
WebSocket messages use type-safe patterns

Input Validation: EXISTING PATTERNS MAINTAINED

User input handling unchanged in core logic
File path validation uses isPathSafe (codingAgent.ts:659)
Path normalization applied (codingAgent.ts:665)

Workflow Changes: NOT APPLICABLE TO SECURITY

Changes to .github/workflows/claude-reviews.yml are for the review process itself
This is the official Cloudflare repository
No security implications from workflow modifications

Breaking Changes Analysis

State Schema Change:

Old: agentMode: 'deterministic' | 'smart'
New: behaviorType: 'phasic' | 'agentic'

Migration Status: BROKEN (see critical issue above)

Author Note: PR author stated "Smart mode was never introduced", which may mean agentMode: 'smart' never existed in production. However, if agentMode: 'deterministic' existed, it should still be migrated to behaviorType: 'phasic' to prevent data loss.

Summary

This is a well-architected refactoring that successfully decouples agent behavior from infrastructure. The code quality is high and follows project conventions from CLAUDE.md.

However, the critical state migration bug MUST be fixed before merging. The bug will cause existing Durable Object sessions to lose their behavior configuration, potentially causing runtime issues.

Approval Status: REQUEST CHANGES due to critical migration bug

Once the migration bug is fixed, this PR will be ready to merge. The console.log cleanup is recommended but not blocking.

PR Description Status

Current PR Description: Comprehensive and well-formatted. No changes needed.

The description includes:

Clear summary of architectural changes
Detailed changelog with file locations
Motivation and benefits
Testing recommendations
Breaking changes documented
Related issues noted

The PR description meets all quality standards.

Dec 11 '25 22:12 github-actions[bot]

Claude finished @AshishKumar4's task in 2m 37s —— View job

Code Quality & Security Review

Recommendation: ✅ APPROVE

PR Summary

This PR refactors the agent architecture to separate behavior logic from infrastructure, enabling support for multiple agent behaviors (phasic and agentic modes). It introduces a clean separation via AgentInfrastructure<TState> interface and BaseCodingBehavior abstract class.

PR Description

Status: ✅ Comprehensive and well-formatted
Notes: No changes needed - includes clear summary, detailed changelog, motivation, testing recommendations, and breaking changes documentation

Addressed Issues

Part 1 of agent generalization effort
Enables future agentic behavior implementation
May affect #249 (screenshot workflow) due to ScreenshotAnalysisOperation removal

Critical Bug Fix Verified ✅

State Migration Bug - RESOLVED

Previous reviews identified that migratedBehaviorType was computed but never applied to newState. This has been fixed:

// stateMigration.ts:161-163
const newState: AgentState = {
    ...state,
    behaviorType: migratedBehaviorType,  // ✅ Now properly applied
    // ...
}

The migration now correctly:

Computes migratedBehaviorType from legacy agentMode field (lines 131-149)
Applies it to the new state (line 163)
Deletes the deprecated agentMode field (lines 179-181)

Code Quality

Severity	Issues
🔴 Critical	0
🟠 High	0
🟡 Medium	2 (recommendations)
🔵 Low	1

Medium Priority Recommendations:

Console statements in new feature hooks (src/features/*/index.tsx)
- Debug console.log statements in lifecycle hooks (onActivate/onDeactivate)
- Consider removing or gating behind development mode
Large files - As noted by previous reviewers, several files exceed 300 lines:
- worker/agents/core/behaviors/base.ts (1771 lines)
- worker/agents/core/codingAgent.ts (780 lines)
- worker/agents/core/behaviors/phasic.ts (728 lines)
These are acceptable for this refactoring scope but consider breaking down in future iterations.

Low Priority:

Type guard improvement in AgenticProjectBuilder.ts:154-157 - The type guard is correctly implemented without any casts. Minor suggestion: could use 'plan' in bp && Array.isArray((bp as AgenticBlueprint).plan) for extra safety, but current implementation is acceptable.

Security Analysis

Area	Status
SQL Injection	✅ Safe - Parameterized queries used throughout
Authentication	✅ No changes to auth flows
Secrets Handling	✅ API keys properly accessed via `env.VARIABLE`
Input Validation	✅ Existing patterns maintained
Path Traversal	✅ `isPathSafe` validation in place

Notable Security Improvements:

Legacy userApiKeys field correctly removed in migration (stateMigration.ts:73-81)
No secrets exposure detected
WebSocket messages use type-safe patterns

Architecture Review

Strengths:

✅ Clean separation via AgentInfrastructure<TState> interface
✅ Well-designed BaseCodingBehavior provides solid foundation
✅ State management cleanly split into BaseProjectState, PhasicState, AgenticState
✅ Operations properly updated to use ICodingAgent interface
✅ No any type violations in core files
✅ Memory leak for deployment timeout properly cleaned up (use-chat.ts:618-621)

New Feature System: The frontend feature plugin architecture (src/features/) is well-designed with:

FeatureModule interface for plugin registration
FeatureContext for shared state
Lazy loading support via FeatureLoader
Clean separation between app, presentation, and general features

Testing Recommendations

Manual Testing:

Create new session and verify phasic behavior works
Test state migration with existing Durable Objects that have old agentMode field
Verify behavior switching between phasic and agentic modes
Test all LLM tools (generate_files, deep_debug, etc.)
Verify state persistence across DO hibernation

Automated Testing:

Consider adding unit tests for StateMigration.migrateIfNeeded() to prevent future regressions

Final Recommendation

✅ APPROVE

The critical state migration bug has been fixed. The architecture is well-designed and the code quality is high. The remaining console.log statements are minor issues that don't block merging.

Summary of resolved issues from previous reviews:

[x] State migration bug - behaviorType now properly applied
[x] Memory leak - Deployment timeout properly cleaned up
[x] Type guard - No any cast in AgenticProjectBuilder
[x] Missing imports - PhasicCodingBehavior and AgenticCodingBehavior properly imported

Dec 14 '25 20:12 github-actions[bot]