Skip to content

feat(dashboard): Notify users to enter custom headers if required in chat or if custom headers are invalid#1699

Merged
dimaMachina merged 56 commits intomainfrom
prd-4917
Feb 5, 2026
Merged

feat(dashboard): Notify users to enter custom headers if required in chat or if custom headers are invalid#1699
dimaMachina merged 56 commits intomainfrom
prd-4917

Conversation

@dimaMachina
Copy link
Collaborator

@changeset-bot
Copy link

changeset-bot bot commented Feb 4, 2026

🦋 Changeset detected

Latest commit: 95facf0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@inkeep/agents-manage-ui Patch
@inkeep/agents-core Patch
@inkeep/agents-api Patch
@inkeep/agents-cli Patch
@inkeep/agents-sdk Patch
@inkeep/agents-work-apps Patch
@inkeep/ai-sdk-provider Patch
@inkeep/create-agents Patch
@inkeep/agents-manage-mcp Patch
@inkeep/agents-mcp Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Feb 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-api Ready Ready Preview, Comment Feb 5, 2026 2:57pm
agents-docs Ready Ready Preview, Comment Feb 5, 2026 2:57pm
agents-manage-ui Ready Ready Preview, Comment Feb 5, 2026 2:57pm

Request Review

border: 1px #ef4444 solid;
&:after {
content: 'Custom headers are invalid.';
position: absolute;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] Hardcoded CSS error message has accessibility issues

The CSS ::after content pseudo-element approach:

  1. Is not announced by screen readers (accessibility issue)
  2. Shows generic "Custom headers are invalid" instead of specific validation error
  3. Cannot be internationalized
  4. Uses hardcoded color #ef4444

Consider using a proper React component with the specific error message from form.formState.errors.headers?.message instead of CSS content injection.

@miles-kt-inkeep
Copy link
Contributor

@inkeep can you check for docs changes.

@claude
Copy link
Contributor

claude bot commented Feb 5, 2026

Claude finished @dimaMachina's task in 2m 5s —— View job


PR Review Summary

1 Key Finding | Risk: Low

This PR adds a valuable UX feature — notifying users when custom headers are invalid or required in the playground chat. The implementation is solid with proper defensive error handling and comprehensive test coverage.

🟠🔶⚠️ Major (1) 🟠🔶⚠️

🟠 1) convert-json-schema-to-zod.test.ts Test file misplaced as source code

Issue: This test file is located in packages/agents-core/src/utils/json-schema/ instead of a __tests__/ directory.

Why: Per AGENTS.md guidelines: "Place tests in __tests__/ directories adjacent to code". The codebase has 80+ test files following this pattern. This causes:

  • Test code potentially bundled with production build
  • Inconsistent with established codebase conventions

Fix: Move to packages/agents-core/src/utils/__tests__/convert-json-schema-to-zod.test.ts

🕐 Pending Recommendations 🕐

Point-fix Edits (0)

No new inline comments posted — the remaining issue was already flagged in previous review threads.


✅ APPROVE

Summary: Excellent work on this feature! 🎉 All previous critical concerns have been addressed:

  1. required field handling — Resolved by switching to zod-from-json-schema library which properly handles JSON Schema semantics
  2. Unguarded JSON.parse — Now has proper try-catch error handling in validation.ts (lines 45-56) with meaningful error messages
  3. Test coverage — Comprehensive tests added in validation.test.ts (126 lines) covering JSON syntax errors, schema validation, and edge cases
  4. Export path change — Not a breaking change since functions are exported from main @inkeep/agents-core entry point

The only remaining item is moving the test file to __tests__/ directory per AGENTS.md conventions — a quick fix that doesn't block merge.

Other Findings (6)

Previously flagged — now resolved or no longer applicable

Location Issue Status
json-schema-to-zod.ts Missing required field handling Resolved — File deleted, replaced with zod-from-json-schema library
validation.ts:46 Unguarded JSON.parse Resolved — Try-catch with error handling added (lines 45-56)
chat-widget.tsx:81 'use memo' invalid directive Invalid — Valid React Compiler annotation (docs)
package.json:65 Breaking change: removed export path Not breaking — Functions exported from main entry, SDK imports updated

Minor observations

Location Issue Reason Excluded
chat-widget.tsx:53-65 CSS ::after error message not announced by screen readers Minor accessibility — functional workaround via button variant change to destructive-outline
custom-headers-dialog.tsx:27 UseFormReturn<any, any, ...> typing Minor type looseness — acceptable given form passed from parent with proper typing at source

@claude
Copy link
Contributor

claude bot commented Feb 5, 2026

Claude finished @dimaMachina's task in 3m 18s —— View job


PR Review Summary

0 Key Findings | Risk: Low

This PR adds a valuable UX feature — notifying users when custom headers are invalid or required in the playground chat. The implementation is solid with proper defensive error handling and comprehensive test coverage.

✅ All Previous Concerns Addressed

Previous Issue Status
Missing required field handling in jsonSchemaToZod Resolved — Switched to zod-from-json-schema library which properly handles JSON Schema semantics
Unguarded JSON.parse could crash component Resolved — Try-catch with meaningful error handling added in validation.ts (lines 44-56)
Test file misplaced as source code Resolved — Moved to packages/agents-core/src/utils/__tests__/convert-json-schema-to-zod.test.ts
'use memo' invalid directive Invalid concern — Valid React Compiler annotation, codebase uses compilationMode: 'annotation' in next.config.ts
Breaking change: removed export path Not breaking — Functions exported from main @inkeep/agents-core entry point, all SDK imports updated

Point-fix Edits (0)

No new inline comments needed — all previous issues have been addressed.


✅ APPROVE

Summary: Excellent work on this feature! 🎉 All critical concerns from previous reviews have been addressed:

  1. Robust validationcreateCustomHeadersSchema() properly validates headers with try-catch error handling
  2. Comprehensive test coverage — 126 lines of tests in validation.test.ts covering JSON syntax errors, schema validation, and edge cases
  3. Library upgradezod-from-json-schema properly handles required field semantics (the original critical issue)
  4. Proper test organization — Test file now in correct __tests__/ directory per AGENTS.md conventions
Other Findings (3)

Minor observations (not blocking)

Location Issue Reason Excluded
chat-widget.tsx:53-65 CSS ::after error message not announced by screen readers Minor accessibility — functional workaround via button variant change to destructive-outline which visually indicates error state
convert-json-schema-to-zod.test.ts:15-28 Test only verifies round-trip JSON Schema conversion, not validation behavior Minor — comprehensive validation tests exist in validation.test.ts; this test confirms the library integration works
custom-headers-dialog.tsx:27 UseFormReturn<any, any, { headers: DefaultHeaders }> typing Minor type looseness — acceptable given form passed from parent with proper typing at source

@dimaMachina
Copy link
Collaborator Author

@claude can you write a changeset based on changes in this PR

@claude
Copy link
Contributor

claude bot commented Feb 5, 2026

Claude finished @dimaMachina's task in 1m 8s —— View job


Changeset Created ✅

Created changeset for this PR with the following content:

---
"@inkeep/agents-manage-ui": patch
"@inkeep/agents-core": patch
---

Add custom headers validation in playground chat. Users are now notified when custom headers are invalid or required based on the agent's headers schema configuration.

Packages affected:

  • @inkeep/agents-manage-ui (patch) — Main user-facing feature: playground now validates custom headers and shows errors
  • @inkeep/agents-core (patch) — Exports convertJsonSchemaToZod from zod-from-json-schema library

Committed: 95facf0e

@claude
Copy link
Contributor

claude bot commented Feb 5, 2026

Claude finished @dimaMachina's task in 2m 24s —— View job


Task List

  • Review PR changes to understand the scope
  • Determine affected packages that need version bumps
  • Create changeset file with appropriate message
  • Commit and push the changeset ⚠️

Changeset Created

I've created a changeset file with the following configuration:

---
"@inkeep/agents-manage-ui": patch
"@inkeep/agents-core": patch
"@inkeep/agents-api": patch
"@inkeep/agents-sdk": patch
---

Add custom headers validation in playground chat to notify users when headers are invalid or required

Status

✅ The changeset has been created and committed locally.

⚠️ Unable to push: The remote branch has been updated since my checkout. I need approval to fetch and merge the latest changes before pushing.

To complete the changeset addition, you can either:

  1. Grant me permission to fetch/pull the latest changes, or
  2. Manually push the changeset I've created locally

The changeset follows the AGENTS.md guidelines:

  • Uses "patch" semver for this feature addition (non-breaking change)
  • Message starts with action verb "Add"
  • Describes the user-facing change clearly
  • Affects all packages that were modified in this PR

Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com>
@robert-inkeep robert-inkeep removed the request for review from sarah-inkeep February 5, 2026 16:32
@robert-inkeep robert-inkeep dismissed sarah-inkeep’s stale review February 5, 2026 16:40

Dima says Sarah reviewed

@dimaMachina dimaMachina merged commit 4a83260 into main Feb 5, 2026
7 of 8 checks passed
@dimaMachina dimaMachina deleted the prd-4917 branch February 5, 2026 16:40
claude bot pushed a commit that referenced this pull request Feb 6, 2026
…ramework features

Improvements based on human reviewer insights that bots missed:

1. **Custom implementations vs standard libraries** (pr-review-standards)
   - Added detection for custom implementations of standard functionality
   - Provides heuristics for when to suggest well-maintained libraries
   - Reduces maintenance burden and catches incomplete implementations
   - Example: Custom jsonSchemaToZod missed 'required' field handling

2. **Framework compiler feature validation** (pr-review-frontend)
   - Added validation for framework compiler directives before flagging as invalid
   - Checks package.json for compiler plugins (babel-plugin-react-compiler)
   - Reduces false positives on valid React Compiler directives ('use memo')
   - Includes framework feature database for React, Next.js, Vue

Key learnings:
- Bots excel at syntactic correctness but miss semantic correctness
- Human reviewers caught: spec violations, library alternatives, emerging features
- Standard libraries handle edge cases custom implementations miss

Source: PR #1699 analysis
Reviewers: Sarah (validation semantics), Dima (root cause), Miles (structure)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
claude bot pushed a commit that referenced this pull request Feb 6, 2026
Learned from PR #1699 human reviewer feedback:

1. **Documentation Check (HIGH priority)**
   - pr-review-docs now checks if PR modifies user-facing code without docs
   - Triggers on: new UI components, API routes, schemas, package exports, SDK changes
   - Creates CRITICAL finding per AGENTS.md requirement
   - Human reviewer had to manually remind: "can you check for docs changes"

2. **Changeset Check (HIGH priority)**
   - pr-review-standards now checks if PR modifies published packages without changeset
   - Triggers on: agents-core, agents-api, agents-sdk, agents-manage-ui, agents-cli, etc.
   - Creates CRITICAL finding per AGENTS.md requirement
   - Human reviewer had to manually request: "can you write a changeset"

3. **React Compiler Annotation Fix (MEDIUM priority)**
   - pr-review-frontend now recognizes 'use memo' as valid React Compiler annotation
   - Bot incorrectly flagged this as invalid directive in PR #1699
   - Human provided documentation link showing it's valid

These patterns were identified through systematic analysis of human vs bot
comments, meeting all 4 generalizability criteria: actionable, frequent,
objectively verifiable, and not already covered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments