Skip to content

feat: add query length validation with 20k character limit#275

Merged
AshishKumar4 merged 1 commit intonightlyfrom
feat/max-query-length
Dec 18, 2025
Merged

feat: add query length validation with 20k character limit#275
AshishKumar4 merged 1 commit intonightlyfrom
feat/max-query-length

Conversation

@AshishKumar4
Copy link
Collaborator

@AshishKumar4 AshishKumar4 commented Dec 18, 2025

Summary

Add query length validation with a 20,000 character limit to prevent performance issues from extremely large prompts. Validation is applied on both client and server sides for defense-in-depth.

Changes

  • Add MAX_AGENT_QUERY_LENGTH constant (20,000 characters) in worker/api/controllers/agent/types.ts
  • Add server-side validation in CodingAgentController returning 413 error with SecurityError for oversized queries
  • Add client-side validation in src/routes/home.tsx and src/routes/chat/hooks/use-chat.ts with user-friendly toast messages
  • Truncate oversized queries during state migration to prevent performance issues with existing sessions
  • Export MAX_AGENT_QUERY_LENGTH from src/api-types.ts for frontend usage
  • Suppress toast notifications in createAgentSession API to avoid duplicate error messages (client validates first)

Motivation

Large queries can cause performance degradation and excessive resource consumption during code generation. This change enforces a reasonable limit to protect the system while providing clear feedback to users.

Testing

  • Manually test submitting prompts exceeding 20k characters on home page and chat page
  • Verify 413 error response from API when bypassing client validation
  • Verify existing sessions with large queries are handled gracefully during state migration

Breaking Changes

None - this is a new validation that protects against edge cases. Existing normal usage is unaffected.

- Add MAX_AGENT_QUERY_LENGTH constant (20,000 characters) to agent types
- Validate query length in agent controller and return 413 error if exceeded
- Add client-side validation in home route and chat hook with user-friendly error messages
- Truncate oversized queries during state migration to prevent performance issues
- Export MAX_AGENT_QUERY_LENGTH from api-types for frontend usage
- Pass skipRateLimitCheck flag in createAgentSession API
@AshishKumar4 AshishKumar4 merged commit 8927bf0 into nightly Dec 18, 2025
4 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Dec 18, 2025

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


Code Quality & Security Review (Critical Path)

Recommendation: APPROVE


PR Summary

This PR adds query length validation with a 20,000 character limit to prevent performance issues from extremely large prompts. It implements defense-in-depth validation on both client and server sides.

PR Description

  • Updated: No changes needed
  • Notes: Description is comprehensive with clear summary, changes, motivation, and testing instructions

Addressed Issues

  • None explicitly linked (appears to be a proactive security/performance improvement)

Code Quality

Critical: None

High: None

Medium:

  • State Migration Truncation (worker/agents/core/stateMigration.ts): The existing inline review comment correctly identifies that silently truncating queries during state migration can cause data loss for existing sessions. While the warning log is helpful for debugging, users have no visibility into this modification. Consider:
    • Adding a user-facing notification when their session loads with a truncated query
    • Storing original length for audit purposes

Low:

  • The error message format is slightly different between client (Prompt too large (X characters). Maximum allowed is Y characters.) and server (same format but wrapped in SecurityError) - this is acceptable as both convey the same information
  • Minor: console.log("Came here"); at line 385 in api-client.ts appears to be debug code that should be cleaned up (pre-existing, not from this PR)

Security

Critical: None

High: None

Medium: None

Low:

  • The 20,000 character limit is a reasonable default for preventing resource exhaustion attacks
  • Server-side validation properly returns 413 status with SecurityError type, maintaining consistency with the security error handling pattern

Architecture Analysis

Aspect Assessment
Defense-in-depth Good - Client validates first (better UX), server validates as fallback
DRY principle Good - MAX_AGENT_QUERY_LENGTH constant defined once, imported where needed
Type safety Good - No any types introduced
Error handling Good - Proper HTTP 413 status code, SecurityError with INVALID_INPUT type
Pattern consistency Good - Follows existing patterns for API client, error types, and controller responses

Changes Summary

File Change
worker/api/controllers/agent/types.ts Defines MAX_AGENT_QUERY_LENGTH = 20_000
worker/api/controllers/agent/controller.ts Server-side validation with 413 SecurityError response
worker/agents/core/stateMigration.ts Truncates oversized queries during state migration
src/api-types.ts Exports constant for frontend usage
src/lib/api-client.ts Suppresses toast for createAgentSession to avoid duplicates
src/routes/home.tsx Client-side validation with toast message
src/routes/chat/hooks/use-chat.ts Client-side validation with toast and error message

Testing

  • Manual testing recommended per PR description:
    • Submit prompts exceeding 20k characters on home page and chat page
    • Verify 413 error response from API when bypassing client validation
    • Verify existing sessions with large queries are handled gracefully

Final Recommendation

APPROVE - This is a well-implemented security/performance feature that follows the project's established patterns. The defense-in-depth approach with client and server validation is the right design. The existing inline review comment about state migration truncation is valid but not blocking - it's a reasonable trade-off to prevent system degradation from legacy sessions with excessively large queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant