Update project files - #14
Open
khulnasoft-bot wants to merge 4 commits into
Open
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
- 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
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 |
Reviewer's GuideAdds first-class Vercel serverless deployment support, including a dedicated Express handler, build script, Postgres URL resolution helper, and deployment docs, while tightening server behavior in serverless environments and standardizing the Taskcore icon usage in the UI. Sequence diagram for Vercel serverless request handlingsequenceDiagram
actor User
participant Vercel
participant api_index as api_index_handler
participant vercel as taskcoreVercelHandler
participant bootFn as boot
participant configFn as loadConfig
participant pgEnv as resolvePostgresUrlFromEnv
participant dbFn as createDb
participant appFn as createAppForServerless
User->>Vercel: HTTP request
Vercel->>api_index: invoke default export
api_index->>vercel: taskcoreVercelHandler(req, res)
vercel->>bootFn: boot() [on first request]
bootFn->>configFn: loadConfig()
configFn-->>bootFn: Config
bootFn->>pgEnv: resolvePostgresUrlFromEnv()
pgEnv-->>bootFn: databaseUrl
bootFn->>dbFn: createDb(databaseUrl, { max, prepare })
dbFn-->>bootFn: Db
bootFn->>appFn: createAppForServerless(Config, Db)
appFn-->>bootFn: ExpressApp
bootFn-->>vercel: ExpressApp
vercel->>appFn: ExpressApp(req, res)
appFn-->>User: HTTP response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 3 issues, and left some high level feedback:
- The Hermes adapter registry currently forces
hermesListSkills/hermesSyncSkillsthroughas unknown ascasts; consider updating the Hermes implementations or theServerAdapterModuletypes so they align without unsafe casting. - Serverless/Vercel runtime detection is implemented in multiple places (
isServerlessRuntimeinlogger.ts,isVercelRuntimeinvercel.ts) with similar env checks; it may be worth centralizing this into a shared helper to avoid drift in the conditions over time. - In
server/src/vercel.ts, theprepareDisabledflag andTASKCORE_PG_PREPAREsemantics are inverted (setting the env totruedisables prepared statements); consider renaming the env/variable or flipping the boolean logic to make the behavior more intuitive.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Hermes adapter registry currently forces `hermesListSkills`/`hermesSyncSkills` through `as unknown as` casts; consider updating the Hermes implementations or the `ServerAdapterModule` types so they align without unsafe casting.
- Serverless/Vercel runtime detection is implemented in multiple places (`isServerlessRuntime` in `logger.ts`, `isVercelRuntime` in `vercel.ts`) with similar env checks; it may be worth centralizing this into a shared helper to avoid drift in the conditions over time.
- In `server/src/vercel.ts`, the `prepareDisabled` flag and `TASKCORE_PG_PREPARE` semantics are inverted (setting the env to `true` disables prepared statements); consider renaming the env/variable or flipping the boolean logic to make the behavior more intuitive.
## Individual Comments
### Comment 1
<location path="server/src/middleware/logger.ts" line_range="8-9" />
<code_context>
import { resolveDefaultLogsDir, resolveHomeAwarePath } from "../home-paths.js";
import { shouldSilenceHttpSuccessLog } from "./http-log-policy.js";
+function isServerlessRuntime(): boolean {
+ return process.env.VERCEL === "1" || process.env.NOW === "1";
+}
+
</code_context>
<issue_to_address>
**suggestion:** Serverless runtime detection is duplicated and could be centralized.
We now have two helpers for the same Vercel runtime check: `isServerlessRuntime()` here and `isVercelRuntime()` in `server/src/vercel.ts`. Please consolidate this into a single shared utility so runtime detection logic stays consistent across logger configuration and serverless boot code.
Suggested implementation:
```typescript
import path from "node:path";
import pino from "pino";
import { pinoHttp } from "pino-http";
import { readConfigFile } from "../config-file.js";
import { resolveDefaultLogsDir, resolveHomeAwarePath } from "../home-paths.js";
import { shouldSilenceHttpSuccessLog } from "./http-log-policy.js";
import { isVercelRuntime } from "../vercel.js";
function resolveServerLogDir(): string {
const envOverride = process.env.TASKCORE_LOG_DIR?.trim();
if (envOverride) return resolveHomeAwarePath(envOverride);
}
const logDir = resolveServerLogDir();
```
1. Replace any usages of `isServerlessRuntime()` in `server/src/middleware/logger.ts` with `isVercelRuntime()` so the shared utility is used consistently.
2. If `isServerlessRuntime` was intended to be exported and used elsewhere, either:
- Update those call sites to import and use `isVercelRuntime` directly from `server/src/vercel.ts`, or
- Re-export `isVercelRuntime` under the `isServerlessRuntime` name from a shared module, while keeping the implementation centralized in `vercel.ts`.
</issue_to_address>
### Comment 2
<location path="server/src/vercel.ts" line_range="132-133" />
<code_context>
+ assertVercelConfig(config);
+
+ const maxConnections = Math.max(1, Number(process.env.TASKCORE_PG_MAX_CONNECTIONS) || 10);
+ const prepareDisabled = process.env.TASKCORE_PG_PREPARE !== undefined
+ ? process.env.TASKCORE_PG_PREPARE === "true"
+ : isVercelRuntime();
+ const db = createDb(config.databaseUrl, {
</code_context>
<issue_to_address>
**issue (bug_risk):** `TASKCORE_PG_PREPARE` semantics appear inverted relative to the variable name.
`prepareDisabled` is set to `true` when `TASKCORE_PG_PREPARE === "true"`, and then `{ prepare: false }` is passed to `createDb`. So setting `TASKCORE_PG_PREPARE="true"` actually disables prepared statements. To avoid confusing or misconfiguring deployments, either invert the logic so `"true"` enables prepared statements, or rename the env var (e.g. `TASKCORE_PG_PREPARE_DISABLED`) to match the current behavior.
</issue_to_address>
### Comment 3
<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>
**suggestion (bug_risk):** Double `unknown` cast hides type mismatches between Hermes skill APIs and `ServerAdapterModule`.
The `unknown` → `ServerAdapterModule` casts bypass TypeScript’s structural checks, so any mismatch between the Hermes helpers and the expected signatures will only fail at runtime. Instead, update the Hermes helper types (or `ServerAdapterModule` if Hermes is canonical) so they are directly compatible without unsafe casts.
Suggested implementation:
```typescript
sessionCodec: hermesSessionCodec,
listSkills: hermesListSkills,
syncSkills: hermesSyncSkills,
models: hermesModels,
```
To fully implement the suggestion (and surface any real type mismatches instead of hiding them), you should also:
1. Ensure the object this snippet belongs to is explicitly typed as `ServerAdapterModule`, e.g. `const hermesAdapter: ServerAdapterModule = { ... }`. This will make TypeScript check that `hermesListSkills` and `hermesSyncSkills` match the required signatures.
2. Update the type signatures of `hermesListSkills` and `hermesSyncSkills` in their respective modules so that they are structurally compatible with `ServerAdapterModule["listSkills"]` and `ServerAdapterModule["syncSkills"]` (parameters, return types, and async/Promise shape).
3. If Hermes is the canonical API, adjust the `ServerAdapterModule` interface instead so its `listSkills` and `syncSkills` definitions match the Hermes helpers, then let this registry file rely on standard type inference without casts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+8
to
+9
| function isServerlessRuntime(): boolean { | ||
| return process.env.VERCEL === "1" || process.env.NOW === "1"; |
There was a problem hiding this comment.
suggestion: Serverless runtime detection is duplicated and could be centralized.
We now have two helpers for the same Vercel runtime check: isServerlessRuntime() here and isVercelRuntime() in server/src/vercel.ts. Please consolidate this into a single shared utility so runtime detection logic stays consistent across logger configuration and serverless boot code.
Suggested implementation:
import path from "node:path";
import pino from "pino";
import { pinoHttp } from "pino-http";
import { readConfigFile } from "../config-file.js";
import { resolveDefaultLogsDir, resolveHomeAwarePath } from "../home-paths.js";
import { shouldSilenceHttpSuccessLog } from "./http-log-policy.js";
import { isVercelRuntime } from "../vercel.js";
function resolveServerLogDir(): string {
const envOverride = process.env.TASKCORE_LOG_DIR?.trim();
if (envOverride) return resolveHomeAwarePath(envOverride);
}
const logDir = resolveServerLogDir();- Replace any usages of
isServerlessRuntime()inserver/src/middleware/logger.tswithisVercelRuntime()so the shared utility is used consistently. - If
isServerlessRuntimewas intended to be exported and used elsewhere, either:- Update those call sites to import and use
isVercelRuntimedirectly fromserver/src/vercel.ts, or - Re-export
isVercelRuntimeunder theisServerlessRuntimename from a shared module, while keeping the implementation centralized invercel.ts.
- Update those call sites to import and use
Comment on lines
+132
to
+133
| const prepareDisabled = process.env.TASKCORE_PG_PREPARE !== undefined | ||
| ? process.env.TASKCORE_PG_PREPARE === "true" |
There was a problem hiding this comment.
issue (bug_risk): TASKCORE_PG_PREPARE semantics appear inverted relative to the variable name.
prepareDisabled is set to true when TASKCORE_PG_PREPARE === "true", and then { prepare: false } is passed to createDb. So setting TASKCORE_PG_PREPARE="true" actually disables prepared statements. To avoid confusing or misconfiguring deployments, either invert the logic so "true" enables prepared statements, or rename the env var (e.g. TASKCORE_PG_PREPARE_DISABLED) to match the current behavior.
Comment on lines
+188
to
+189
| listSkills: hermesListSkills as unknown as ServerAdapterModule["listSkills"], | ||
| syncSkills: hermesSyncSkills as unknown as ServerAdapterModule["syncSkills"], |
There was a problem hiding this comment.
suggestion (bug_risk): Double unknown cast hides type mismatches between Hermes skill APIs and ServerAdapterModule.
The unknown → ServerAdapterModule casts bypass TypeScript’s structural checks, so any mismatch between the Hermes helpers and the expected signatures will only fail at runtime. Instead, update the Hermes helper types (or ServerAdapterModule if Hermes is canonical) so they are directly compatible without unsafe casts.
Suggested implementation:
sessionCodec: hermesSessionCodec,
listSkills: hermesListSkills,
syncSkills: hermesSyncSkills,
models: hermesModels,To fully implement the suggestion (and surface any real type mismatches instead of hiding them), you should also:
- Ensure the object this snippet belongs to is explicitly typed as
ServerAdapterModule, e.g.const hermesAdapter: ServerAdapterModule = { ... }. This will make TypeScript check thathermesListSkillsandhermesSyncSkillsmatch the required signatures. - Update the type signatures of
hermesListSkillsandhermesSyncSkillsin their respective modules so that they are structurally compatible withServerAdapterModule["listSkills"]andServerAdapterModule["syncSkills"](parameters, return types, and async/Promise shape). - If Hermes is the canonical API, adjust the
ServerAdapterModuleinterface instead so itslistSkillsandsyncSkillsdefinitions match the Hermes helpers, then let this registry file rely on standard type inference without casts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generated by v0
v0 Session
Summary by Sourcery
Add first-class Vercel serverless deployment support and tighten environment-driven configuration for logging, database, auth, and plugins.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation: