Update project files - #13
Conversation
- Add missing server/src/types/express.d.ts (Request.actor augmentation) - Cast hermes listSkills/syncSkills at the adapter-utils type boundary - Replace nonexistent lucide Taskcore icon with local TaskcoreIcon component - Un-ignore server/src/types/express.d.ts so clones can build
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideRefactors UI to use a new custom TaskcoreIcon component instead of the lucide-react Taskcore icon, tightens server adapter typing for Hermes skill listing/syncing, and adds explicit Express Request actor type augmentation for better request-scoped identity typing. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
TaskcoreIconcomponent only exposes aclassNameprop; consider extendingReact.SVGProps<SVGSVGElement>so it can be used more flexibly (e.g., witharia-*,role,onClick, etc.) like other icon components. - The
as unknown as ServerAdapterModule[...]casts forhermesListSkills/hermesSyncSkillsinregistry.tsobscure potential type mismatches; it would be safer to align the underlying function signatures withServerAdapterModuleinstead of using double assertions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `TaskcoreIcon` component only exposes a `className` prop; consider extending `React.SVGProps<SVGSVGElement>` so it can be used more flexibly (e.g., with `aria-*`, `role`, `onClick`, etc.) like other icon components.
- The `as unknown as ServerAdapterModule[...]` casts for `hermesListSkills`/`hermesSyncSkills` in `registry.ts` obscure potential type mismatches; it would be safer to align the underlying function signatures with `ServerAdapterModule` instead of using double assertions.
## Individual Comments
### Comment 1
<location path="server/src/adapters/registry.ts" line_range="188-189" />
<code_context>
sessionCodec: hermesSessionCodec,
- listSkills: hermesListSkills,
- syncSkills: hermesSyncSkills,
+ listSkills: hermesListSkills as unknown as ServerAdapterModule["listSkills"],
+ syncSkills: hermesSyncSkills as unknown as ServerAdapterModule["syncSkills"],
models: hermesModels,
supportsLocalAgentJwt: true,
</code_context>
<issue_to_address>
**issue (bug_risk):** The double `unknown` cast on `listSkills`/`syncSkills` hides type mismatches that could surface as runtime bugs.
These `unknown` casts disable type checking and could let signature drift between the `hermes*` functions and `ServerAdapterModule` go unnoticed until runtime. Prefer updating the `hermesListSkills`/`hermesSyncSkills` types to match `ServerAdapterModule`, or introduce small adapter functions that explicitly map arguments/return types so TypeScript can enforce the contract without unsafe casts.
</issue_to_address>
### Comment 2
<location path="ui/src/components/TaskcoreIcon.tsx" line_range="3-7" />
<code_context>
+import { cn } from "../lib/utils";
+
+interface TaskcoreIconProps {
+ className?: string;
+}
+
+export function TaskcoreIcon({ className }: TaskcoreIconProps) {
+ return (
+ <svg
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The icon component only accepts `className`, which limits compatibility with consumers expecting standard SVG icon props.
`TaskcoreIconProps` only exposes `className` and doesn’t spread extra props to `<svg>`, so any existing usages passing things like `color`, `strokeWidth`, `aria-*`, or `onClick` will stop working without errors. Consider using `React.SVGProps<SVGSVGElement>` (or extending it) and spreading the rest props onto `<svg>` so this can act as a drop‑in replacement for the previous lucide icon component.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| listSkills: hermesListSkills as unknown as ServerAdapterModule["listSkills"], | ||
| syncSkills: hermesSyncSkills as unknown as ServerAdapterModule["syncSkills"], |
There was a problem hiding this comment.
issue (bug_risk): The double unknown cast on listSkills/syncSkills hides type mismatches that could surface as runtime bugs.
These unknown casts disable type checking and could let signature drift between the hermes* functions and ServerAdapterModule go unnoticed until runtime. Prefer updating the hermesListSkills/hermesSyncSkills types to match ServerAdapterModule, or introduce small adapter functions that explicitly map arguments/return types so TypeScript can enforce the contract without unsafe casts.
| interface TaskcoreIconProps { | ||
| className?: string; | ||
| } | ||
|
|
||
| export function TaskcoreIcon({ className }: TaskcoreIconProps) { |
There was a problem hiding this comment.
suggestion (bug_risk): The icon component only accepts className, which limits compatibility with consumers expecting standard SVG icon props.
TaskcoreIconProps only exposes className and doesn’t spread extra props to <svg>, so any existing usages passing things like color, strokeWidth, aria-*, or onClick will stop working without errors. Consider using React.SVGProps<SVGSVGElement> (or extending it) and spreading the rest props onto <svg> so this can act as a drop‑in replacement for the previous lucide icon component.
- add serverless entry (server/src/vercel.ts) with Vercel runtime defaults and config guards - bundle server into a single ESM function via scripts/build-vercel-function.mjs - support Vercel Postgres/RDS env conventions (POSTGRES_URL, PGHOST/...) - stdout-only logging and disable plugins/background jobs in serverless runtime - allow overriding DB pool options (max, prepare) for bounded serverless connections
2 participants
Generated by v0
v0 Session
Summary by Sourcery
Introduce a shared Taskcore SVG icon component and use it across the UI, while tightening server typing around request actors and Hermes adapter skill methods.
Enhancements: