Fix CodeRabbit issues: implement validation middleware and improve error handling

[devin-ai-integration]

Fix CodeRabbit issues: implement validation middleware and improve error handling

Summary

This PR addresses 93 CodeRabbit issues from PR #279 by implementing a comprehensive body validation middleware system and fixing several security/reliability issues:

🔧 Key Changes:

• New validation middleware (/lib/validation/) with type-safe request body validation for all /sessions, /tutorials, and /users API endpoints
• Fixed config import issues by replacing dynamic imports with static imports in tutorial-state/route.ts
• Enhanced KV persistence error handling with proper success checks and error bubbling
• Added path traversal protection for tutorial routes to prevent directory traversal attacks
• Removed implicit any types on request body parameters with proper TypeScript interfaces
• Standardized parseInt usage to Number.parseInt for consistency
• Improved error responses with detailed validation messages and 400 status codes

🛡️ Security Improvements:

• Tutorial ID validation prevents path traversal (.., /, \ characters blocked)
• Snippet loading now has error handling and path validation
• Request body validation prevents malformed/malicious payloads

📝 Type Safety:

• Uses existing types from /app/chat/types.ts (Session, Message, TutorialData, etc.)
• New comprehensive validation functions for all request types
• Eliminates implicit any types on request bodies

Review & Testing Checklist for Human (4 items - 🔴 High Risk)

• Test all API endpoints manually - Verify POST/PUT/DELETE requests to /api/sessions/*, /api/tutorials/*, and /api/users/tutorial-state still work correctly with the new validation middleware
• Check frontend compatibility - Ensure the new error response format ({ error, details }) doesn't break existing frontend error handling
• Verify validation rules match expectations - Review that validation in lib/validation/middleware.ts matches the actual data structures your frontend sends (especially Message, Session types)
• Test edge cases - Try invalid tutorial IDs, malformed JSON bodies, missing required fields to ensure validation works as expected

Notes

This change touches many API endpoints simultaneously and introduces new validation logic that could affect existing functionality. While the validation is based on existing types from /app/chat/types.ts, there could be mismatches between what the frontend actually sends vs. what the types expect.

The KV error handling improvements should make the API more resilient to storage failures, but the behavior changes could surface errors differently than before.

Requested by: @afterrburn (srith@agentuity.com)
Devin Session: https://app.devin.ai/sessions/df50e03078644f8cbe96f8c1227b902c

Summary by CodeRabbit

• New Features
  • Unified input validation across session, tutorial, and tutorial-state APIs with consistent, readable error responses.
  • Sessions list returns an empty set with pagination when no data is available.
• Bug Fixes
  • Safer tutorials: validated IDs/steps, range checks, and sanitized snippet paths.
  • Snippet load failures are logged and skipped instead of failing requests.
  • Clearer store errors with proper 404 vs other failures.
  • Enforced session ID match on update.
  • More reliable tutorial progress reset with explicit persistence handling.

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Introduces a reusable validation module and updates multiple API routes to use centralized JSON parsing and schema-based validation. Adds input sanitization, structured error responses, and explicit timestamp normalization. Adjusts sessions and tutorials routes to validate payloads and params before processing, with minor error message changes and safer KV interactions.

Changes

Cohort / File(s)	Summary
Validation framework lib/validation/middleware.ts, lib/validation/types.ts	Adds a schema-based validation system, helper types, reusable validators, request parsing with standardized error responses, and typed request/response interfaces for sessions, messages, tutorials, and pagination.
Sessions API app/api/sessions/route.ts, app/api/sessions/[sessionId]/route.ts, app/api/sessions/[sessionId]/messages/route.ts	Replaces ad-hoc body parsing with parseAndValidateJSON; validates sessions and messages; normalizes timestamps; adds sessionId mismatch check (400); distinguishes 404 vs other errors in listing; maintains downstream persistence/streaming logic.
Tutorials API app/api/tutorials/route.ts, app/api/tutorials/[id]/steps/[stepNumber]/route.ts	Adds param validation (tutorial id, step number), path sanitization for snippet reads, guarded range checks, structured validation errors, and resilient snippet loading; listing now skips unsafe entries.
User tutorial state app/api/users/tutorial-state/route.ts	Centralizes POST/DELETE validation; persists reset via KV with explicit error handling; keeps GET unchanged; replaces dynamic imports with static ones.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant R as Route Handler
  participant V as parseAndValidateJSON
  participant S as Store/Services

  C->>R: HTTP Request (JSON)
  R->>V: Parse + Validate (schema/validator)
  alt Validation fails
    V-->>R: { success:false, response }
    R-->>C: HTTP 400 (standardized errors)
  else Validation succeeds
    V-->>R: { success:true, data }
    R->>S: Process with validated data
    S-->>R: Result / Stream / KV response
    R-->>C: HTTP 2xx JSON
  end
  note over R,V: Centralized error formatting and type-safe data

Loading

sequenceDiagram
  autonumber
  participant C as Client
  participant R as Tutorials Step Route
  participant V as Validators (id/step)
  participant FS as File System

  C->>R: GET /api/tutorials/:id/steps/:stepNumber
  R->>V: validateTutorialId(id)
  alt Invalid id
    R-->>C: 400 Validation error
  else Valid id
    R->>V: validateStepNumber(stepNumber)
    alt Invalid step
      R-->>C: 400 Validation error
    else In range?
      alt Out of range
        R-->>C: 404 Step not found
      else OK
        R->>FS: Read MDX + snippets (sanitize paths)
        alt Read error
          R->>R: Log warning, skip snippet
        end
        R-->>C: 200 JSON (metadata, mdx, snippets, totals)
      end
    end
  end
  note over R,FS: Skips entries with ".." or "\" to prevent traversal

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

• rblalock
• jhaynie

Poem

I thump my paw at structured cheer,
New schemas bloom—no chaos here.
With whiskered wisdom, I validate,
Guarding paths from sneaky fate.
Timestamps tidy, sessions neat—
Hippity-hop, our APIs beat!
(ʘ‿ʘ)／🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 34.78% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly identifies the primary work — implementing validation middleware and improving error handling — which matches the PR objectives and the majority of changed files. It is concise, specific, and does not contain noisy elements like file lists or vague wording. A reviewer scanning PR history would understand the main intent from this title.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	docs	4f11153	Sep 14 2025, 05:29 PM

[afterrburn]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

app/api/sessions/[sessionId]/route.ts (1)

219-234: Normalize timestamp here too to keep data contract consistent

messages/route.ts converts message.timestamp to ISO, but this POST path appends the raw timestamp. This divergence can break consumers expecting uniform ISO strings.

Apply before constructing updatedSession:

-    const { message } = validation.data;
+    const { message } = validation.data;
+    if (message.timestamp) {
+      message.timestamp = toISOString(message.timestamp);
+    }

app/api/tutorials/[id]/steps/[stepNumber]/route.ts (1)

7-9: Fix route params typing and remove unnecessary await.

params is not a Promise in Next.js route handlers; awaiting it forces an incorrect type and can mask real typing issues.

Apply:

-interface RouteParams {
-  params: Promise<{ id: string; stepNumber: string }>;
-}
+interface RouteParams {
+  params: { id: string; stepNumber: string };
+}
@@
-    const { id, stepNumber } = await params;
+    const { id, stepNumber } = params;

Also applies to: 13-13

🧹 Nitpick comments (17)

app/api/tutorials/route.ts (1)

20-22: Harden path traversal defense: normalize + allow‑list slug check

includes(".."), "/", "" can be bypassed via encodings or alternate separators. Safer to resolve to an absolute path and verify it remains under tutorialRoot, and also enforce a slug allow‑list.

Apply this diff within the loop:

-      if (entry.includes('..') || entry.includes('/') || entry.includes('\\')) {
-        continue;
-      }
+      // Stronger validation: only accept sluggy names and ensure resolved path stays under tutorialRoot
+      const isSlug = /^[A-Za-z0-9._-]+$/.test(entry);
+      const resolvedPath = resolve(tutorialRoot, entry);
+      if (!isSlug || !resolvedPath.startsWith(tutorialRoot + sep)) {
+        // optionally: console.warn(`Skipping unsafe tutorial entry: ${entry}`);
+        continue;
+      }

And update imports:

// at top
-import { join } from 'path';
+import { join, resolve, sep } from 'path';

lib/validation/types.ts (3)

1-1: Remove unused type and use path alias for consistency

TutorialData isn’t used here, and other files import via '@/...' alias. Align to avoid fragile relative paths and unused import lint errors.

-import { Session, Message, TutorialData } from '../../app/chat/types';
+import { Session, Message } from '@/app/chat/types';

---

31-36: Prefer unknown over any in generic defaults

unknown is safer and pushes callers to narrow types.

-export interface ApiResponse<T = any> {
+export interface ApiResponse<T = unknown> {
   success: boolean;
   data?: T;
   error?: string;
   message?: string;
 }

---

46-49: Avoid duplicating payload beside ApiResponse; use ApiResponse properly

Current SessionsResponse both extends ApiResponse and adds its own fields, leading to two payload shapes (data vs sessions). Wrap the payload in ApiResponse instead.

-export interface SessionsResponse extends ApiResponse {
-  sessions: Session[];
-  pagination: PaginationInfo;
-}
+export interface SessionsPayload {
+  sessions: Session[];
+  pagination: PaginationInfo;
+}
+export type SessionsResponse = ApiResponse<SessionsPayload>;

app/api/sessions/[sessionId]/messages/route.ts (2)

69-88: Validation flow reads clean; minor: coerce and default explicitly

Body shape check + validateMessage is solid. Small nit: explicitly narrow processWithAgent using === true to avoid truthy surprises.

-      const processWithAgent = body.processWithAgent !== undefined ? Boolean(body.processWithAgent) : true;
+      const processWithAgent = body.processWithAgent === true ? true : true;
+      // or simply: const processWithAgent = body.processWithAgent !== false;

---

284-349: SSE parsing may drop/garble events across chunk boundaries

Splitting decoded text by “\n” per chunk without buffering leftover partial lines risks JSON parse errors and lost deltas/finish events. Use a rolling buffer and TextDecoder with stream: true.

Example helper you can reuse here (and in title-gen):

function createSSELineReader() {
  const decoder = new TextDecoder();
  let buffer = '';
  return {
    push(chunk: Uint8Array): string[] {
      buffer += decoder.decode(chunk, { stream: true });
      const lines = buffer.split('\n');
      buffer = lines.pop() ?? '';
      return lines;
    },
    flush(): string[] {
      const lines = buffer ? [buffer] : [];
      buffer = '';
      return lines;
    }
  };
}

Then inside the streaming loop, replace line splitting with the helper:

const sse = createSSELineReader();
...
const lines = sse.push(value);
for (const line of lines) { /* existing data: handling */ }
...
// after the loop
for (const line of sse.flush()) { /* finalize any remaining line */ }

app/api/sessions/route.ts (1)

23-27: Nit: specify radix when parsing integers

Explicit base avoids edge cases and linters complaining.

-    const parsedLimit = Number.parseInt(searchParams.get('limit') ?? String(DEFAULT_SESSIONS_LIMIT));
-    const parsedCursor = Number.parseInt(searchParams.get('cursor') ?? '0');
+    const parsedLimit = Number.parseInt(searchParams.get('limit') ?? String(DEFAULT_SESSIONS_LIMIT), 10);
+    const parsedCursor = Number.parseInt(searchParams.get('cursor') ?? '0', 10);

app/api/tutorials/[id]/steps/[stepNumber]/route.ts (6)

25-31: Remove redundant falsy check on validated stepIndex.

After a successful validation, data is defined and ≥1; the extra 400 path is unreachable.

-    const stepIndex = stepValidation.data;
-    if (!stepIndex) {
-      return NextResponse.json(
-        { success: false, error: 'Invalid step number' },
-        { status: 400 }
-      );
-    }
+    const stepIndex = stepValidation.data;

---

2-2: Prep for safer path resolution in snippet loader.

You’ll need resolve/relative to harden snippet path checks (see next comment).

-import { join } from 'path';
+import { join, resolve, relative } from 'path';

---

78-81: Guard invalid range requests for snippets.

If to < from, the slice is empty; bail early to avoid misleading output.

-        const startIdx = Math.max(0, (desc.from ? desc.from - 1 : 0));
-        const endIdx = Math.min(lines.length, desc.to ? desc.to : lines.length);
+        const startIdx = Math.max(0, (desc.from ? desc.from - 1 : 0));
+        const endIdx = Math.min(lines.length, desc.to ?? lines.length);
+        if (endIdx < startIdx) return;

---

89-107: Make the CodeFromFiles tag regex multiline-safe.

([^>]*?) won’t match attributes split across lines. Use a dot‑all class.

-    const filesTagRegex = /<CodeFromFiles\s+([^>]*?)\/>/g;
+    const filesTagRegex = /<CodeFromFiles\s+([\s\S]*?)\/>/g;

---

153-164: Align totalSteps with the step list actually used.

You filter index out for stepSlugs but return pages.length. This yields inconsistent pagination.

-        totalSteps: pages.length
+        totalSteps: stepSlugs.length

---

36-40: Optional: return 404 when child meta is missing.

A missing meta.json implies a missing tutorial; consider 404 instead of 500.

lib/validation/middleware.ts (4)

100-108: Tighten enum validation to enforce string type.

Schema constrains enumValues to strings; ensure the value is a string before includes.

-    case 'enum':
-      if (!schema.enumValues || !schema.enumValues.includes(value)) {
+    case 'enum':
+      if (typeof value !== 'string' || !schema.enumValues || !schema.enumValues.includes(value)) {
         return { 
           field: fieldName, 
           message: `must be one of: ${schema.enumValues?.join(', ')}`, 
           received: value 
         };
       }

---

176-181: Validate timestamp format (basic ISO-8601 check).

Prevents arbitrary strings in timestamp.

-  timestamp: { type: 'string', required: true },
+  timestamp: { 
+    type: 'string', 
+    required: true,
+    customValidator: (v, field) =>
+      Number.isFinite(Date.parse(v)) ? null : { field, message: 'must be a valid ISO-8601 date string', received: v }
+  },

---

289-292: Consistency: reuse createValidationError for invalid JSON.

Aligns 400 responses with the new { error, details } shape.

-    return {
-      success: false,
-      response: NextResponse.json({ error: 'Invalid JSON body' }, { status: 400 })
-    };
+    return {
+      success: false,
+      response: createValidationError('Invalid JSON body', [{ field: 'body', message: 'invalid JSON' }])
+    };

---

141-160: Optional: support unknown-key policy.

If you need stricter schemas, add an option to reject/strip unknown fields in validateObject.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61aaac7 and 67e7160.

📒 Files selected for processing (8)

• app/api/sessions/[sessionId]/messages/route.ts (2 hunks)
• app/api/sessions/[sessionId]/route.ts (4 hunks)
• app/api/sessions/route.ts (4 hunks)
• app/api/tutorials/[id]/steps/[stepNumber]/route.ts (5 hunks)
• app/api/tutorials/route.ts (2 hunks)
• app/api/users/tutorial-state/route.ts (3 hunks)
• lib/validation/middleware.ts (1 hunks)
• lib/validation/types.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-09-10T14:24:52.800Z

Learnt from: afterrburn
PR: agentuity/docs#279
File: agent-docs/src/agents/agent-pulse/types.ts:2-7
Timestamp: 2025-09-10T14:24:52.800Z
Learning: The Agentuity SDK (agentuity/sdk) only exports specific types: AgentRequest, AgentResponse, AgentContext, and VectorUpsertParams. It does not export general message types like ConversationMessage or ChatMessage.

Applied to files:

• lib/validation/types.ts

📚 Learning: 2025-07-23T12:40:34.834Z

Learnt from: CR
PR: agentuity/docs#0
File: agent-docs/.cursor/rules/sdk.mdc:0-0
Timestamp: 2025-07-23T12:40:34.834Z
Learning: Applies to agent-docs/src/agents/**/*.ts : Import types from 'agentuity/sdk'

Applied to files:

• lib/validation/types.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Workers Builds: docs

🔇 Additional comments (10)

app/api/sessions/[sessionId]/messages/route.ts (1)

12-14: LGTM: Centralized validation imports

Adopts shared middleware/types to standardize input handling. Good move.

app/api/sessions/route.ts (2)

30-37: Good: distinguish 404 (no sessions) from other KV errors

Returning an empty payload for 404 avoids failing first‑time users; other errors propagate correctly.

---

82-88: LGTM: centralized validation + ISO timestamp normalization

Validation middleware and uniform timestamp handling reduce downstream parsing issues.

Also applies to: 91-99

app/api/sessions/[sessionId]/route.ts (4)

60-66: LGTM: switch to centralized session validation

Consistent with the new middleware and improves error shaping.

---

67-71: LGTM: explicit sessionId mismatch check

Prevents accidental cross‑session writes.

---

76-85: LGTM: normalize message timestamps on PUT

Ensures consistency when updating full sessions.

---

204-214: LGTM: message validation on POST

Inline validator keeps this route aligned with the messages route.

app/api/tutorials/[id]/steps/[stepNumber]/route.ts (1)

15-23: LGTM: centralized param validators with structured 400s.

Solid move to validateTutorialId/validateStepNumber and createValidationError.

app/api/users/tutorial-state/route.ts (2)

3-9: LGTM: static imports and centralized validation wiring.

Good consolidation of config/KV imports and validator usage.

---

46-52: LGTM: parse-then-validate flow with standardized 400s.

This keeps the handler lean and consistent.