Test pr no deploy migration#1058
Conversation
…st-frontmatter
- Fix run: key indentation (was 12+ spaces, now 8 spaces) - Add mkdir -p to prevent directory exists errors - Standardize YAML formatting throughout workflow - Ensure proper alignment of all step fields
…haned resource cleanup - Filter deployments by health status (availableReplicas > 0) - Auto-cleanup failed deployments older than 10 minutes - Remove orphaned services and ingresses without deployments - Enhanced logging and status reporting - Prevent stuck/unhealthy deployments from blocking new ones
- Remove -O flag from curl commands (was using both -O and -o) - Keep only -sL with -o kubectl for single download - Fixes all 4 kubectl setup occurrences in workflow - Prevents silent failures and resource waste from duplicate downloads
- Remove problematic hapi stub type packages that conflict with built-in types - Fix Elasticsearch imports to use correct estypes export from main package - Update tsconfig-test.json to exclude conflicting stub packages - All tests now pass TypeScript compilation
WalkthroughAdds PR preview infrastructure and wildcard ingress, enables registry-backed Docker layer caching and a build validation script, refactors Dockerfile for Puppeteer/Chromium and dependency caching, adds Supabase client and share-image services (Puppeteer + SVG fallback) with OpenAPI docs, updates Prisma User fields/migration, adds MyST transformer and tests, and various ES/TypeScript/IPFS fixes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant ECR as AWS ECR
participant EKS as AWS EKS
participant ALB as AWS ALB
participant Vault as Vault
Dev->>GH: Open/Sync PR
GH->>ECR: docker login / pull cache (branch-cache)
GH->>ECR: build with inline cache, push image + cache
GH->>EKS: kubectl apply Deployment/Service/Ingress (PR-labeled)
GH->>Vault: Inject secrets via annotations
EKS->>ALB: Route via wildcard ingress/preview-router
GH-->>Dev: Comment with preview URL and status
Dev->>GH: Close PR or scheduled cleanup
GH->>EKS: Cleanup PR deployments/services/ingress older/closed
sequenceDiagram
autonumber
actor Client
participant API as /v1/services/generate-share-image
participant SB as Supabase
participant PP as Puppeteer/Chromium
participant SVG as SVG Generator (Sharp fallback)
Client->>API: GET with id/text/answer/refs/citations
alt id provided
API->>SB: Fetch record by id
SB-->>API: Data or 404
end
API->>PP: Render HTML and screenshot
alt Puppeteer succeeds
PP-->>API: PNG buffer
API-->>Client: 200 PNG
else Puppeteer fails/unavailable
API->>SVG: Generate SVG/PNG via Sharp
SVG-->>API: PNG buffer
API-->>Client: 200 PNG (fallback)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🧹 Nitpick comments (18)
desci-server/README.md (1)
3-4: Consider removing the test-only HTML comment before merging to main
<!-- PR Preview Deployment Test -->is useful for workflow experimentation but is invisible in the rendered README and may confuse future readers. If the comment’s only purpose is to trigger the preview-deployment test, please strip it (or replace it with a visible explanatory sentence) before merging.desci-models/package.json (1)
25-26: New runtime depjs-yaml– confirm tree-shaking & size impact
js-yamlis ~60 kB minified; if this library is only used during build-time parsing, consider moving it todevDependencies. Otherwise, all good..env.example (2)
71-81: Re-enable placeholders instead of fully commenting out AWS/SQS variables
.env.exampleserves as documentation; commenting the keys out removes them from generated.envfiles and new developers may miss the required variables.
Prefer leaving the keys present with empty values:-# AWS_S3_BUCKET_NAME= +# AWS_S3_BUCKET_NAME= # … -# AWS_SQS_QUEUE_URL= +# AWS_SQS_QUEUE_URL=
200-203: Fix key order and ensure trailing newlineStatic-analysis flags two minor style issues:
SUPABASE_SERVICE_KEYshould appear immediately beforeSUPABASE_URLto keep alphabetical order.- Add a blank line after the last entry.
-#MIXPANEL_TOKEN= -SUPABASE_URL= -SUPABASE_SERVICE_KEY= +#MIXPANEL_TOKEN= +SUPABASE_SERVICE_KEY= +SUPABASE_URL= +.dockerignore (1)
13-18: Deduplicate ignore patterns for cleaner maintenance
**/.git,**/.vscode, and markdown patterns appear twice (Lines 14–18 and 40–44).
Removing duplicates reduces noise and the risk of diverging edits.Also applies to: 40-44
desci-server/scripts/be-node-dev.sh (1)
21-24: Reconsider disabling seeding during local startupCommenting out
npx prisma db seedandnpm run script:seed-social-dataspeeds boot-up but leaves a blank database, breaking API endpoints that rely on reference data.If accidental, re-enable or gate behind an env flag (e.g.
SKIP_SEED=1).desci-server/tsconfig.json (1)
25-25:noResolveproperty is redundant
noResolvedefaults tofalse; keeping an explicitfalseadds noise without functional benefit.- "noResolve": false,desci-server/src/types/express/index.d.ts (1)
3-13: Duplicated Express augmentations – consolidate to a single fileThese
RequestandResponseextensions now also live insrc/types/express-augmentations.d.ts.
Maintaining two identical augmentations invites drift and can confuse future readers. Prefer keeping them in one dedicated file and removing the duplicate.-// Just augment the Express namespace without changing module structure -... entire file ...desci-server/src/routes/v1/admin/communities/index.ts (1)
68-68: Avoidanycast arounduploadHandlerCasting
req/restoanyforfeits the type-safety gained from your new Express augmentations.
Consider defining:const uploadHandler: RequestHandler = upload.fields([...]);and then calling it without casts:
uploadHandler(req, res, (err) => { ... });This keeps strict types and still satisfies
multer’s signature.desci-server/src/docs/services.ts (2)
4-14: Consider adding validation for mutually exclusive parameter combinationsThe schema allows all parameters to be optional, but based on the implementation logic, either
idortextshould be required. Consider using Zod'srefineorunionto express this constraint at the schema level.-export const generateShareImageQuerySchema = z.object({ - query: z.object({ - text: z.string().describe('The search question text to display on the image').optional(), - id: z - .string() - .describe('ID of a stored search result to generate image from (alternative to other parameters)') - .optional(), - answer: z.string().describe('The answer text to preview on the image').optional(), - refs: z.string().describe('Number of references/citations').optional(), - citations: z.string().describe('JSON string containing citation data').optional(), - }), +export const generateShareImageQuerySchema = z.object({ + query: z + .object({ + text: z.string().describe('The search question text to display on the image').optional(), + id: z + .string() + .describe('ID of a stored search result to generate image from (alternative to other parameters)') + .optional(), + answer: z.string().describe('The answer text to preview on the image').optional(), + refs: z.string().describe('Number of references/citations').optional(), + citations: z.string().describe('JSON string containing citation data').optional(), + }) + .refine((data) => data.id || data.text, { + message: 'Either id or text parameter is required', + }), });
94-95: Remove hardcoded timestamp commentThe comment with a hardcoded timestamp should be removed as it provides no value and will become outdated.
-// Documentation updated for generateShareImage API Mon Jul 28 18:30:40 WITA 2025 -Dockerfile (1)
64-65: Document the package.json transformation rationaleThe sed command replacing "link:" with "file:" appears to be a workaround. Consider adding a comment explaining why this transformation is necessary for better maintainability.
# Install dependencies after desci-models is built (better cache invalidation) +# Replace "link:" with "file:" to avoid symlink issues in Docker context RUN mv package.json package.json.old && \ sed 's/link:/file:/' package.json.old > package.jsondesci-server/src/controllers/services/shareImage.ts (1)
114-185: Consider breaking down the large controller functionThe
generateShareImagefunction handles multiple responsibilities (validation, database fetching, parameter parsing). Consider extracting these into separate functions for better testability and maintainability.Consider extracting:
- Database fetching logic into a separate
fetchSearchDataByIdfunction- Parameter validation and parsing into a
parseShareImageParamsfunction- This would make the controller more modular and easier to test
desci-server/src/controllers/services/shareImagePuppeteer.ts (1)
353-591: Consider extracting HTML template to a separate fileThe HTML template is quite large (200+ lines). Consider extracting it to a separate template file for better maintainability and separation of concerns.
Consider:
- Moving the HTML template to a separate file (e.g.,
shareImageTemplate.html)- Using a template engine like Handlebars or EJS for better template management
- This would make the code more maintainable and allow designers to work on templates independently
desci-server/kubernetes/wildcard_preview_ingress.yaml (1)
58-58: Remove unnecessary empty line.- - +desci-models/src/transformers/MystTransformer.ts (1)
678-750: Code duplication between frontmatter generation methods.There's significant duplication between
generateComprehensiveFrontmatterandgenerateBasicFrontmatterDatamethods. The logic for processing authors, references, and other fields is repeated.Consider refactoring to have
generateComprehensiveFrontmattercallgenerateBasicFrontmatterDataand then add the additional fields:private generateComprehensiveFrontmatter(researchObject: ResearchObjectV1): string { - const frontmatterData: any = {}; - // ... duplicated code ... + const frontmatterData = this.generateBasicFrontmatterData(researchObject); + + // Add additional comprehensive fields not in basic + frontmatterData.subject = 'Research Article'; + frontmatterData.open_access = true; + // ... other comprehensive-specific fields ... return this.generateYamlFrontmatter(frontmatterData); }.github/workflows/pr-preview.yaml (2)
59-59: Remove trailing spaces throughout the file.YAMLlint detected trailing spaces on multiple lines which should be removed for cleaner code.
Remove trailing spaces from the indicated lines. Most editors can be configured to automatically trim trailing whitespace on save.
Also applies to: 62-62, 67-67, 69-69, 144-144, 147-147, 155-155, 159-159, 164-164, 176-176, 191-191, 209-209, 267-267, 313-313, 473-473, 476-476
635-635: Consider making deployment timeout configurable.The hardcoded 300s timeout for deployment rollout might be insufficient for larger deployments or during high cluster load.
- kubectl rollout status deployment/desci-server-pr-${PR_NUMBER} --timeout=300s + # Make timeout configurable with a reasonable default + DEPLOYMENT_TIMEOUT="${DEPLOYMENT_TIMEOUT:-300s}" + kubectl rollout status deployment/desci-server-pr-${PR_NUMBER} --timeout=${DEPLOYMENT_TIMEOUT}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
desci-models/yarn.lockis excluded by!**/yarn.lock,!**/*.lockdesci-server/package-lock.jsonis excluded by!**/package-lock.jsondesci-server/public/ai-share-blank.pngis excluded by!**/*.pngdesci-server/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
.dockerignore(1 hunks).env.example(2 hunks).github/workflows/build-server.yaml(1 hunks).github/workflows/pr-preview.yaml(1 hunks)Dockerfile(1 hunks)desci-models/package.json(2 hunks)desci-models/src/ResearchObject.ts(2 hunks)desci-models/src/transformers/MystTransformer.ts(1 hunks)desci-models/src/transformers/index.ts(1 hunks)desci-models/tests/transformers/MystTransformer.test.ts(1 hunks)desci-server/Makefile(1 hunks)desci-server/README.md(1 hunks)desci-server/kubernetes/deployment_demo.yaml(1 hunks)desci-server/kubernetes/deployment_dev.yaml(2 hunks)desci-server/kubernetes/deployment_prod.yaml(2 hunks)desci-server/kubernetes/deployment_staging.yaml(1 hunks)desci-server/kubernetes/wildcard_preview_ingress.yaml(1 hunks)desci-server/package.json(4 hunks)desci-server/prisma/migrations/20250806170333_add_first_last_name_user/migration.sql(1 hunks)desci-server/prisma/schema.prisma(1 hunks)desci-server/prisma/seed.ts(1 hunks)desci-server/scripts/be-node-dev.sh(1 hunks)desci-server/src/controllers/search/query.ts(2 hunks)desci-server/src/controllers/search/types.ts(1 hunks)desci-server/src/controllers/services/shareImage.ts(1 hunks)desci-server/src/controllers/services/shareImagePuppeteer.ts(1 hunks)desci-server/src/docs/openapi.ts(6 hunks)desci-server/src/docs/services.ts(1 hunks)desci-server/src/lib/supabase.ts(1 hunks)desci-server/src/middleware/checkJwt.ts(1 hunks)desci-server/src/routes/v1/admin/communities/index.ts(1 hunks)desci-server/src/routes/v1/services.ts(2 hunks)desci-server/src/server.ts(2 hunks)desci-server/src/services/ElasticSearchService.ts(23 hunks)desci-server/src/services/ipfs.ts(7 hunks)desci-server/src/swagger.ts(1 hunks)desci-server/src/types/express-augmentations.d.ts(1 hunks)desci-server/src/types/express/index.d.ts(1 hunks)desci-server/src/utils/dagConcat.ts(1 hunks)desci-server/src/utils/diffUtils.ts(1 hunks)desci-server/src/utils/draftTreeUtils.ts(1 hunks)desci-server/tsconfig-test.json(2 hunks)desci-server/tsconfig.json(2 hunks)test-optimizations.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
desci-server/src/docs/**/*
📄 CodeRabbit Inference Engine (.cursorrules)
When asked to create API documentation or update API documentation for an API route/controller/service, follow the pattern already established in the desci-server/src/docs folder
Files:
desci-server/src/docs/services.tsdesci-server/src/docs/openapi.ts
🧠 Learnings (1)
📚 Learning: applies to desci-server/src/docs/**/* : when asked to create api documentation or update api documen...
Learnt from: CR
PR: desci-labs/nodes#0
File: .cursorrules:0-0
Timestamp: 2025-07-18T15:23:44.865Z
Learning: Applies to desci-server/src/docs/**/* : When asked to create API documentation or update API documentation for an API route/controller/service, follow the pattern already established in the desci-server/src/docs folder
Applied to files:
desci-server/src/routes/v1/services.tsdesci-server/src/docs/openapi.tsdesci-server/src/swagger.tsdesci-server/src/server.ts
🧬 Code Graph Analysis (6)
desci-server/src/utils/draftTreeUtils.ts (1)
desci-server/src/services/ipfs.ts (1)
client(36-36)
desci-server/src/types/express-augmentations.d.ts (1)
desci-server/src/types/express/index.d.ts (2)
Request(6-9)Response(10-12)
desci-server/src/docs/openapi.ts (1)
desci-server/src/docs/services.ts (1)
servicesPaths(89-93)
desci-server/prisma/seed.ts (1)
desci-server/src/client.ts (1)
prisma(3-7)
desci-server/src/controllers/services/shareImagePuppeteer.ts (2)
desci-server/src/lib/supabase.ts (1)
supabase(10-10)desci-server/src/controllers/services/shareImage.ts (1)
generateShareImage(114-185)
desci-models/src/transformers/MystTransformer.ts (1)
desci-models/src/ResearchObject.ts (4)
ResearchObjectV1(18-56)ResearchObject(1-3)ResearchObjectV1Component(80-97)ResearchObjectV1Author(102-123)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 68-68: [QuoteCharacter] The value has quote characters (', ")
[warning] 203-203: [EndingBlankLine] No blank line at the end of the file
[warning] 203-203: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
🪛 checkmake (0.2.2)
desci-server/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 Biome (2.1.2)
desci-server/tsconfig-test.json
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-34: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 ast-grep (0.38.6)
desci-server/src/controllers/services/shareImage.ts
[warning] 439-439: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\[${citation.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\], 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Gitleaks (8.27.2)
desci-server/prisma/seed.ts
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 actionlint (1.7.7)
.github/workflows/build-server.yaml
135-135: shellcheck reported issue in this script: SC2086:info:2:37: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:2:104: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:2:128: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:5:50: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:6:45: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:12:13: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:17:16: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:18:6: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:19:6: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:19:30: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:19:64: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:20:6: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:24:13: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:24:37: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:24:71: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:25:13: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:2:37: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:2:104: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:2:128: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:5:50: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:6:45: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:12:13: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:17:16: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:18:6: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:19:6: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:19:30: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:19:64: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:20:6: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:24:13: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:24:37: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:24:71: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:25:13: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:2:37: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:2:104: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:2:128: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:5:50: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:6:45: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:12:13: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:17:16: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:18:6: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:19:6: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:19:30: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:19:64: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:20:6: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:24:13: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:24:37: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:24:71: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:25:13: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/pr-preview.yaml
29-29: label "blacksmith-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
32-32: shellcheck reported issue in this script: SC2086:info:12:10: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:13:58: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2162:info:21:7: read without -r will mangle backslashes
(shellcheck)
32-32: shellcheck reported issue in this script: SC2236:style:28:8: Use -n instead of ! -z
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:29:97: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:32:31: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:33:44: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:34:44: Double quote to prevent globbing and word splitting
(shellcheck)
76-76: label "blacksmith-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
79-79: shellcheck reported issue in this script: SC2086:info:12:10: Double quote to prevent globbing and word splitting
(shellcheck)
79-79: shellcheck reported issue in this script: SC2086:info:13:58: Double quote to prevent globbing and word splitting
(shellcheck)
112-112: label "blacksmith-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
120-120: shellcheck reported issue in this script: SC2086:info:12:10: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:13:58: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2162:info:22:7: read without -r will mangle backslashes
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:31:44: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:41:33: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:42:46: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:43:46: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2162:info:54:7: read without -r will mangle backslashes
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:58:47: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:60:28: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2162:info:69:7: read without -r will mangle backslashes
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:73:47: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2086:info:75:28: Double quote to prevent globbing and word splitting
(shellcheck)
120-120: shellcheck reported issue in this script: SC2162:info:87:7: read without -r will mangle backslashes
(shellcheck)
120-120: shellcheck reported issue in this script: SC2126:style:100:17: Consider using 'grep -c' instead of 'grep|wc -l'
(shellcheck)
253-253: shellcheck reported issue in this script: SC2086:info:12:37: Double quote to prevent globbing and word splitting
(shellcheck)
253-253: shellcheck reported issue in this script: SC2086:info:16:30: Double quote to prevent globbing and word splitting
(shellcheck)
253-253: shellcheck reported issue in this script: SC2086:info:20:17: Double quote to prevent globbing and word splitting
(shellcheck)
253-253: shellcheck reported issue in this script: SC2086:info:22:38: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:11:31: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:19:37: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:19:104: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:19:128: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:31:13: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:38:18: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:39:8: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:39:28: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:40:8: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:40:32: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:40:66: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:40:86: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:41:8: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:47:10: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:57:13: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:57:37: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:57:71: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:57:91: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:58:13: Double quote to prevent globbing and word splitting
(shellcheck)
347-347: shellcheck reported issue in this script: SC2086:info:61:32: Double quote to prevent globbing and word splitting
(shellcheck)
412-412: shellcheck reported issue in this script: SC2086:info:12:10: Double quote to prevent globbing and word splitting
(shellcheck)
412-412: shellcheck reported issue in this script: SC2086:info:13:58: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 Shellcheck (0.10.0)
test-optimizations.sh
[error] 36-36: < is for string comparisons. Use -lt instead.
(SC2071)
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yaml
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 69-69: trailing spaces
(trailing-spaces)
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
[error] 155-155: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 191-191: trailing spaces
(trailing-spaces)
[error] 209-209: trailing spaces
(trailing-spaces)
[error] 267-267: trailing spaces
(trailing-spaces)
[error] 313-313: trailing spaces
(trailing-spaces)
[error] 473-473: trailing spaces
(trailing-spaces)
[error] 476-476: trailing spaces
(trailing-spaces)
🔇 Additional comments (61)
desci-server/prisma/schema.prisma (1)
182-183: Verify downstream code & indexes for newly addedfirstName/lastNamecolumnsThe new optional fields look innocuous, but please double-check:
- API DTOs / GraphQL types / serializers include these properties (otherwise writes will silently drop).
- Any full-text or autocomplete queries that key off
namemight need to include the split fields.- No uniqueness or length constraints were required—confirm that is intentional.
If everything is covered, no action needed.
desci-server/prisma/migrations/20250806170333_add_first_last_name_user/migration.sql (1)
1-3: LGTM – migration matches the schema changeAdds two nullable columns; safe for irreversible-down scenarios.
desci-models/package.json (2)
3-3: Semver bump looks fine but ensure rebuilt artefacts are publishedVersion moves to
0.2.27; remember to run the build & publish job so consumers receive the compiled output that includes the new transformer.
31-32: Good call adding the corresponding type definitionsNo further action.
.dockerignore (1)
44-46: Double-check ignoringDockerfile*does not exclude the build DockerfileThe pattern
**/Dockerfile*matches the root-levelDockerfile; Docker will still send the build file, but the pattern produces a warning and may hide auxiliary Dockerfiles needed by CI.
Consider restringing it to sub-directories:-**/Dockerfile* +*/**/Dockerfile*desci-server/kubernetes/deployment_demo.yaml (1)
75-75: Verify new entry-point path./dist/src/index.jsThe build previously emitted
dist/index.js; switching todist/src/index.jswill crash the pod if the file does not exist.
Confirm that:
yarn buildoutputs that exact path.- All other environment manifests are updated consistently (they appear to be).
desci-server/scripts/be-node-dev.sh (1)
16-19: Good addition: ensure log directories exist
No issues found with themkdir -pguard.desci-server/kubernetes/deployment_staging.yaml (1)
134-134: Same path change as demo – please validate build artefactsAs with the demo deployment, ensure
dist/src/index.jsexists in the production image; otherwise, rolling updates will fail.desci-server/tsconfig.json (1)
39-46: Include list looks correct – no action needed
Additional paths ensure new seed script and Express augmentations are compiled.desci-server/kubernetes/deployment_dev.yaml (2)
117-118: Secrets must contain new Supabase keysEnsure the Vault secret
secrets/desci-server/dev/appreally definesSUPABASE_URLandSUPABASE_SERVICE_KEY; otherwise pods will start with empty env vars.
130-130: Verify new entry-point path
node ./dist/src/index.jsassumes the build output preserves thesrc/folder insidedist.
Double-check thattscproduces this path (or adjust the Docker build) to prevent container crash-loops.desci-models/src/transformers/index.ts (1)
4-4: Export added correctly
MystTransformeris now publicly exposed; change looks good.desci-server/src/utils/dagConcat.ts (1)
1-3: Please verify TypeScript setup and implicit-any count in desci-serverBefore removing
// @ts-nocheck, ensure your local TypeScript toolchain and config are in place and that the TODO’s “20 implicit any” count is accurate:• Check that desci-server/package.json lists
"typescript"(likely as a devDependency) and that desci-server/tsconfig.json enables--strictand--noImplicitAny.
• From the desci-server folder, run:npx tsc --noEmit --strict --noImplicitAny src/utils/dagConcat.ts 2>&1 | grep -i "any" | wc -lto confirm the number of implicit-any errors.
• If the result isn’t ~20, update the TODO to reflect the true scope (and/or add missing type annotations now).desci-server/kubernetes/deployment_prod.yaml (1)
116-117: Supabase integration verified – please confirm Vault secrets
- desci-server/src/lib/supabase.ts correctly reads
SUPABASE_URLandSUPABASE_SERVICE_KEYand warns if they’re missing- Both
deployment_prod.yamlanddeployment_dev.yamlinject these environment variables- No inconsistencies found in code or deployments
Ensure that the corresponding Vault secrets (
SUPABASE_URLandSUPABASE_SERVICE_KEY) are configured in your production (and any other) Vault namespaces to prevent runtime failures.desci-server/src/controllers/search/types.ts (1)
1-3: LGTM! Improved type import consistency.The refactoring to use the
estypesnamespace for Elasticsearch types improves consistency and maintainability while preserving backward compatibility through the type alias.desci-server/src/utils/draftTreeUtils.ts (1)
124-130: LGTM! Excellent CID format standardization.This change ensures consistent CID format handling by converting from the kubo-rpc-client specific format to the standard multiformats CID. This improves compatibility and consistency across IPFS operations throughout the codebase.
The explicit await and format conversion prevents potential issues with different CID representations between client implementations.
desci-server/src/routes/v1/services.ts (1)
6-18: Documentation Verified for generate-share-image EndpointThe API docs in desci-server/src/docs/services.ts include the new
'/v1/services/generate-share-image'path and a matching
generateShareImageOperationwith operationId, summary, description, tags,
requestParams, and responses—consistent with existing documentation patterns.No further changes needed.
desci-models/src/ResearchObject.ts (2)
51-52: LGTM! Clean addition of date field.The optional
dateproperty is well-documented with the expected format (YYYY-MM-DD) and maintains backward compatibility. This supports enhanced metadata capture for research objects.
109-110: LGTM! Author email field addition is appropriate.The optional
desci-server/Makefile (4)
1-1: LGTM! Makefile improvements support better build workflows.The addition of
buildto the phony targets is correct and supports the enhanced CI/CD pipeline mentioned in the summary.
5-5: LGTM! Building after installation streamlines the workflow.Running
yarn buildafter dependency installation ensures the project is ready to use immediately after the install target completes, which aligns with the improved build and deployment processes.
7-8: LGTM! Dedicated build target provides flexibility.The standalone
buildtarget allows for independent build execution, supporting both CI/CD workflows and developer convenience.
11-11: LGTM! Enhanced clean target is more thorough.Removing both
node_modulesanddistdirectories ensures a complete cleanup, which is appropriate sincedistcontains build artifacts that should be regenerated.desci-server/package.json (5)
88-88: LGTM! Supabase client dependency supports new database integration.The
@supabase/supabase-jsdependency enables the Supabase client functionality implemented insrc/lib/supabase.ts.
90-90: LGTM! Markdown processing dependencies support MystTransformer.The
markedlibrary and its TypeScript definitions enable markdown parsing functionality for the new MystTransformer feature.Also applies to: 122-122
137-137: LGTM! Puppeteer enables headless browser automation for image generation.The Puppeteer dependency supports the share image generation services mentioned in the summary.
145-145: LGTM! Sharp provides efficient image processing capabilities.Sharp is a high-performance image processing library that complements Puppeteer for the image generation services.
157-157: LGTM! XSS library enhances security posture.The
xsslibrary provides input sanitization to prevent cross-site scripting attacks, improving the application's security.desci-server/src/lib/supabase.ts (1)
1-11: LGTM! Clean Supabase client initialization with proper error handling.The implementation follows best practices:
- Proper environment variable configuration
- Graceful degradation with informative warning when credentials are missing
- Conditional client creation prevents runtime errors in misconfigured environments
- Clean exports support the image generation services mentioned in the summary
The approach aligns well with the Kubernetes deployment configurations that inject these environment variables via Vault secrets.
desci-server/tsconfig-test.json (4)
12-12: LGTM - Configuration consolidation looks good.The consolidation of the "lib" array into a single line improves readability while maintaining the same functionality.
21-24: LGTM - Appropriate compiler options added.The new compiler options are well-chosen:
skipDefaultLibCheck: trueimproves build performancenoResolve: falseallows proper module resolutionnoImplicitAny: falseprovides flexibility for test scenariostypes: []gives explicit control over included types
28-28: Good improvement - explicit typeRoots configuration.Replacing the Express path mapping with explicit
typeRootsconfiguration is more maintainable and aligns with the Express type augmentations being introduced.
31-32: LGTM - Proper inclusion of type augmentations.Including
src/types/**/*.d.tsand the specific Express augmentation file ensures proper type resolution across the test environment.desci-server/src/controllers/search/query.ts (2)
151-162: LGTM - Well-implemented cache-first strategy.The caching implementation follows best practices:
- Proper error handling that doesn't interrupt normal flow
- Descriptive logging for cache hits
- Graceful degradation when cache fails
190-207: Excellent caching implementation with proper error handling.The response construction and caching logic is well-structured:
- Response object is properly typed
- 1-hour TTL (3600s) is appropriate for monthly aggregations
- Error handling ensures the API continues working even if caching fails
- Consistent logging for debugging
desci-server/src/server.ts (2)
23-23: Good correction - proper naming for OpenAPI documentation.The import rename from 'openai' to 'openapi' corrects the naming and aligns with the actual OpenAPI documentation structure.
221-221: Appropriate type casting for swagger-ui-express compatibility.The middleware setup with type casting addresses TypeScript compatibility issues with swagger-ui-express. The spread operator and type assertions are a common workaround for this library's typing issues.
desci-server/prisma/seed.ts (6)
2-7: Good refactoring - improved file loading approach.Replacing the JSON import with synchronous file read using
fs.readFileSyncis more reliable and avoids potential module loading issues with JSON assertions.
10-16: Excellent addition - comprehensive logging and connection management.The enhanced logging and explicit Prisma connection management will greatly improve debugging capabilities:
- Environment variable logging helps with configuration issues
- Explicit connection testing catches connectivity problems early
- Structured approach to connection management
17-46: LGTM - Proper indentation after try-catch wrapping.The core seeding logic remains unchanged with appropriate indentation adjustments for the new error handling structure.
101-105: Well-implemented error handling structure.The try-catch block provides proper error handling with detailed logging, including stack traces for debugging.
115-122: Robust disconnect handling with proper error management.The Prisma disconnect logic is properly wrapped in try-catch to handle potential disconnection errors gracefully.
34-34: Static analysis false positive - test data, not secrets.The static analysis tool flagged these as API keys, but they appear to be base64-encoded hashes used for test/seed data (
yMcm5OwIUcmh98cmpDhCZArwRV+8Q14XIOs2LhKQ6fY=). The memo indicates this is for "nodes-lib tests", confirming it's test data rather than production secrets.Also applies to: 38-38
desci-server/src/swagger.ts (1)
6-171: Comprehensive API documentation implementation looks excellent.The OpenAPI documentation follows the established patterns from the
desci-server/src/docsfolder as specified in the coding guidelines. The schema definitions are well-structured with clear descriptions, examples, and proper type specifications. The response schemas correctly define content types and error handling scenarios.desci-server/src/services/ipfs.ts (6)
40-40: LGTM - Proper HTTPS agent configuration.The addition of the HTTPS agent with SSL certificate for the public IPFS client is correctly implemented.
623-623: LGTM - Consistent IPFS client method usage.The
addAllmethod call correctly uses the IPFS client with proper options.
719-719: LGTM - Consistent return type handling.The migration function correctly handles the result type.
733-733: LGTM - Proper CID parsing in pin operations.The
pin.addmethod correctly usesCID.parse()for type safety.
762-762: LGTM - Consistent CID parsing in block operations.The
block.statmethod correctly usesCID.parse().
811-811: LGTM - Proper CID parsing in block removal.The
block.rmmethod correctly usesCID.parse()for type safety.desci-server/src/docs/openapi.ts (4)
75-112: Dynamic server detection implementation is well-designed.The
getServers()function properly handles different environments and includes intelligent PR preview detection with regex pattern matching. The URL deduplication logic prevents duplicate entries when the current environment matches existing servers.
24-24: Proper integration of services paths.The import and spreading of
servicesPathscorrectly integrates the new share image generation API endpoints into the OpenAPI documentation, following the established pattern.Also applies to: 162-162
114-114: Correct naming convention fix.The export name change from
openaiDocumentationtoopenApiDocumentationfixes the typo and uses the proper OpenAPI terminology.
137-137: Proper tag integration for services.The addition of the "Services" tag aligns with the new service endpoints and follows the established documentation pattern.
desci-models/tests/transformers/MystTransformer.test.ts (1)
1-704: Comprehensive and well-structured test suite.This test file provides excellent coverage of the MystTransformer functionality with:
- Proper use of ts-interface-checker for validation
- Comprehensive roundtrip testing to ensure data integrity
- Edge case handling (missing frontmatter, complex YAML)
- Clear test organization and descriptive test names
- Thorough assertion coverage for all major features
The tests validate the transformer's ability to handle complex academic metadata, author information, references, and various YAML parsing scenarios. The roundtrip tests are particularly valuable for ensuring data fidelity.
desci-server/src/controllers/services/shareImage.ts (1)
439-441: Regex construction is safe - no action neededThe static analysis warning about regex construction from variable input is a false positive. The citation text is properly escaped using
replace(/[.*+?^${}()|[\]\\]/g, '\\$&')before being used in the RegExp constructor, which prevents ReDoS attacks.desci-server/src/controllers/services/shareImagePuppeteer.ts (2)
17-29: Good implementation of safe string replacementThe
replaceAllSafefunction properly avoids regex-based replacement, preventing potential ReDoS vulnerabilities. This is a good security practice.
838-845: Excellent error handling and cleanupThe browser cleanup in the finally block ensures resources are properly released even if an error occurs. The custom error class for fallback handling is also well-designed.
desci-server/src/services/ElasticSearchService.ts (1)
1-1: Good type import consolidation.The change to use namespace import for Elasticsearch types improves code organization and makes it clear which types come from the Elasticsearch library.
.github/workflows/pr-preview.yaml (2)
251-278: Migration check successfully implements PR objective!The schema change detection correctly blocks deployment when migrations are detected, which fulfills the main PR objective of preventing deployments with pending migrations.
547-548: New Supabase environment variables added as mentioned in PR summary.The addition of
SUPABASE_URLandSUPABASE_SERVICE_KEYenvironment variables aligns with the infrastructure enhancements mentioned in the PR summary.
|
|
||
| # A map for email types to sendgrid template IDs, mainly used for sending journal emails. | ||
| SENDGRID_TEMPLATE_ID_MAP= | ||
| SENDGRID_TEMPLATE_ID_MAP="{}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove embedded quotes from SENDGRID_TEMPLATE_ID_MAP to avoid parsing issues
Most dotenv parsers treat the value verbatim, so the surrounding quotes ("{}") will be included in the string.
Unless the application explicitly strips them, every reference will read the literal value "{}" instead of {} and JSON parsing will fail.
-SENDGRID_TEMPLATE_ID_MAP="{}"
+SENDGRID_TEMPLATE_ID_MAP={}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SENDGRID_TEMPLATE_ID_MAP="{}" | |
| SENDGRID_TEMPLATE_ID_MAP={} |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 68-68: [QuoteCharacter] The value has quote characters (', ")
🤖 Prompt for AI Agents
In .env.example at line 68, remove the surrounding double quotes from the
SENDGRID_TEMPLATE_ID_MAP value so it reads SENDGRID_TEMPLATE_ID_MAP={} instead
of SENDGRID_TEMPLATE_ID_MAP="{}". This prevents the quotes from being included
in the string and avoids JSON parsing errors.
| - name: Build and push image (DEV) - With Registry Caching | ||
| if: github.ref == 'refs/heads/develop' | ||
| run: | | ||
| # Build and tag the image | ||
| docker build \ | ||
| -t $CONTAINER_IMAGE-dev:${{ github.sha }} \ | ||
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev \ | ||
| . | ||
| # ECR login | ||
| aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker login --username AWS --password-stdin $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com | ||
|
|
||
| - name: Build and tag the image (DEMO) | ||
| if: github.ref == 'refs/heads/demo' | ||
| run: | | ||
| # Build and tag the image | ||
| docker build \ | ||
| -t $CONTAINER_IMAGE-demo:${{ github.sha }} \ | ||
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-demo \ | ||
| . | ||
| # Create cache repository if it doesn't exist | ||
| aws ecr describe-repositories --repository-names $CONTAINER_IMAGE-dev-cache 2>/dev/null || \ | ||
| aws ecr create-repository --repository-name $CONTAINER_IMAGE-dev-cache | ||
|
|
||
| - name: Build and tag the image (PROD) | ||
| if: github.ref == 'refs/heads/main' | ||
| run: | | ||
| # Build and tag the image | ||
| # Define cache image | ||
| CACHE_IMAGE=$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev-cache:latest | ||
|
|
||
| # Pull cache image (ignore failures for first builds) | ||
| docker pull $CACHE_IMAGE || echo "No cache found, building from scratch" | ||
|
|
||
| # Build with cache-from and inline cache | ||
| docker build \ | ||
| -t $CONTAINER_IMAGE:${{ github.sha }} \ | ||
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE \ | ||
| . | ||
| --build-arg BUILDKIT_INLINE_CACHE=1 \ | ||
| --cache-from $CACHE_IMAGE \ | ||
| -t $CONTAINER_IMAGE-dev:${{ github.sha }} \ | ||
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }} \ | ||
| -t $CACHE_IMAGE \ | ||
| . | ||
|
|
||
| - name: Push (DEV) | ||
| if: github.ref == 'refs/heads/develop' | ||
| run: | | ||
| # Push image to AWS ECR | ||
| aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker login --username AWS --password-stdin $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com | ||
| docker tag $CONTAINER_IMAGE-dev:${{ github.sha }} $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }} | ||
| # Push both main image and cache | ||
| docker push $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }} | ||
| docker push $CACHE_IMAGE |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix shell script variable quoting issues.
The registry caching implementation is well-designed, but there are multiple shellcheck warnings about unquoted variables that could lead to issues with globbing and word splitting.
Apply these fixes to properly quote variables:
run: |
# ECR login
- aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker login --username AWS --password-stdin $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com
+ aws ecr get-login-password --region "$AWS_DEFAULT_REGION" | docker login --username AWS --password-stdin "$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com"
# Create cache repository if it doesn't exist
- aws ecr describe-repositories --repository-names $CONTAINER_IMAGE-dev-cache 2>/dev/null || \
- aws ecr create-repository --repository-name $CONTAINER_IMAGE-dev-cache
+ aws ecr describe-repositories --repository-names "$CONTAINER_IMAGE-dev-cache" 2>/dev/null || \
+ aws ecr create-repository --repository-name "$CONTAINER_IMAGE-dev-cache"
# Define cache image
- CACHE_IMAGE=$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev-cache:latest
+ CACHE_IMAGE="$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev-cache:latest"
# Pull cache image (ignore failures for first builds)
- docker pull $CACHE_IMAGE || echo "No cache found, building from scratch"
+ docker pull "$CACHE_IMAGE" || echo "No cache found, building from scratch"
# Build with cache-from and inline cache
docker build \
--build-arg BUILDKIT_INLINE_CACHE=1 \
- --cache-from $CACHE_IMAGE \
- -t $CONTAINER_IMAGE-dev:${{ github.sha }} \
- -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }} \
- -t $CACHE_IMAGE \
+ --cache-from "$CACHE_IMAGE" \
+ -t "$CONTAINER_IMAGE-dev:${{ github.sha }}" \
+ -t "$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }}" \
+ -t "$CACHE_IMAGE" \
.
# Push both main image and cache
- docker push $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }}
- docker push $CACHE_IMAGE
+ docker push "$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }}"
+ docker push "$CACHE_IMAGE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build and push image (DEV) - With Registry Caching | |
| if: github.ref == 'refs/heads/develop' | |
| run: | | |
| # Build and tag the image | |
| docker build \ | |
| -t $CONTAINER_IMAGE-dev:${{ github.sha }} \ | |
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev \ | |
| . | |
| # ECR login | |
| aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker login --username AWS --password-stdin $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com | |
| - name: Build and tag the image (DEMO) | |
| if: github.ref == 'refs/heads/demo' | |
| run: | | |
| # Build and tag the image | |
| docker build \ | |
| -t $CONTAINER_IMAGE-demo:${{ github.sha }} \ | |
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-demo \ | |
| . | |
| # Create cache repository if it doesn't exist | |
| aws ecr describe-repositories --repository-names $CONTAINER_IMAGE-dev-cache 2>/dev/null || \ | |
| aws ecr create-repository --repository-name $CONTAINER_IMAGE-dev-cache | |
| - name: Build and tag the image (PROD) | |
| if: github.ref == 'refs/heads/main' | |
| run: | | |
| # Build and tag the image | |
| # Define cache image | |
| CACHE_IMAGE=$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev-cache:latest | |
| # Pull cache image (ignore failures for first builds) | |
| docker pull $CACHE_IMAGE || echo "No cache found, building from scratch" | |
| # Build with cache-from and inline cache | |
| docker build \ | |
| -t $CONTAINER_IMAGE:${{ github.sha }} \ | |
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE \ | |
| . | |
| --build-arg BUILDKIT_INLINE_CACHE=1 \ | |
| --cache-from $CACHE_IMAGE \ | |
| -t $CONTAINER_IMAGE-dev:${{ github.sha }} \ | |
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }} \ | |
| -t $CACHE_IMAGE \ | |
| . | |
| - name: Push (DEV) | |
| if: github.ref == 'refs/heads/develop' | |
| run: | | |
| # Push image to AWS ECR | |
| aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker login --username AWS --password-stdin $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com | |
| docker tag $CONTAINER_IMAGE-dev:${{ github.sha }} $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }} | |
| # Push both main image and cache | |
| docker push $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }} | |
| docker push $CACHE_IMAGE | |
| - name: Build and push image (DEV) - With Registry Caching | |
| if: github.ref == 'refs/heads/develop' | |
| run: | | |
| # ECR login | |
| aws ecr get-login-password --region "$AWS_DEFAULT_REGION" | docker login --username AWS --password-stdin "$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com" | |
| # Create cache repository if it doesn't exist | |
| aws ecr describe-repositories --repository-names "$CONTAINER_IMAGE-dev-cache" 2>/dev/null || \ | |
| aws ecr create-repository --repository-name "$CONTAINER_IMAGE-dev-cache" | |
| # Define cache image | |
| CACHE_IMAGE="$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev-cache:latest" | |
| # Pull cache image (ignore failures for first builds) | |
| docker pull "$CACHE_IMAGE" || echo "No cache found, building from scratch" | |
| # Build with cache-from and inline cache | |
| docker build \ | |
| --build-arg BUILDKIT_INLINE_CACHE=1 \ | |
| --cache-from "$CACHE_IMAGE" \ | |
| -t "$CONTAINER_IMAGE-dev:${{ github.sha }}" \ | |
| -t "$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }}" \ | |
| -t "$CACHE_IMAGE" \ | |
| . | |
| # Push both main image and cache | |
| docker push "$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-dev:${{ github.sha }}" | |
| docker push "$CACHE_IMAGE" |
🧰 Tools
🪛 actionlint (1.7.7)
135-135: shellcheck reported issue in this script: SC2086:info:2:37: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:2:104: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:2:128: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:5:50: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:6:45: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:12:13: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:17:16: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:18:6: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:19:6: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:19:30: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:19:64: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:20:6: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:24:13: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:24:37: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:24:71: Double quote to prevent globbing and word splitting
(shellcheck)
135-135: shellcheck reported issue in this script: SC2086:info:25:13: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
In .github/workflows/build-server.yaml between lines 133 and 160, the shell
script uses unquoted variables which can cause issues with word splitting and
globbing. To fix this, enclose all variable references like $AWS_DEFAULT_REGION,
$AWS_ACCOUNT_ID, $CONTAINER_IMAGE, and $CACHE_IMAGE in double quotes throughout
the script, especially in commands like aws ecr get-login-password, docker
login, aws ecr describe-repositories, docker pull, docker build, and docker push
to ensure safe and predictable variable expansion.
| - name: Build and push image (DEMO) - With Registry Caching | ||
| if: github.ref == 'refs/heads/demo' | ||
| run: | | ||
| # Push image to AWS ECR | ||
| # ECR login | ||
| aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker login --username AWS --password-stdin $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com | ||
| docker tag $CONTAINER_IMAGE-demo:${{ github.sha }} $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-demo:${{ github.sha }} | ||
|
|
||
| # Create cache repository if it doesn't exist | ||
| aws ecr describe-repositories --repository-names $CONTAINER_IMAGE-demo-cache 2>/dev/null || \ | ||
| aws ecr create-repository --repository-name $CONTAINER_IMAGE-demo-cache | ||
|
|
||
| # Define cache image | ||
| CACHE_IMAGE=$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-demo-cache:latest | ||
|
|
||
| # Pull cache image (ignore failures for first builds) | ||
| docker pull $CACHE_IMAGE || echo "No cache found, building from scratch" | ||
|
|
||
| # Build with cache-from and inline cache | ||
| docker build \ | ||
| --build-arg BUILDKIT_INLINE_CACHE=1 \ | ||
| --cache-from $CACHE_IMAGE \ | ||
| -t $CONTAINER_IMAGE-demo:${{ github.sha }} \ | ||
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-demo:${{ github.sha }} \ | ||
| -t $CACHE_IMAGE \ | ||
| . | ||
|
|
||
| # Push both main image and cache | ||
| docker push $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-demo:${{ github.sha }} | ||
| docker push $CACHE_IMAGE |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same shell script fixes to the DEMO build step.
The DEMO build step has identical shellcheck issues that need the same variable quoting fixes as the DEV step.
🧰 Tools
🪛 actionlint (1.7.7)
164-164: shellcheck reported issue in this script: SC2086:info:2:37: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:2:104: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:2:128: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:5:50: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:6:45: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:12:13: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:17:16: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:18:6: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:19:6: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:19:30: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:19:64: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:20:6: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:24:13: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:24:37: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:24:71: Double quote to prevent globbing and word splitting
(shellcheck)
164-164: shellcheck reported issue in this script: SC2086:info:25:13: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
In .github/workflows/build-server.yaml between lines 162 and 189, the shell
script for the DEMO build step lacks proper quoting of variables, causing
shellcheck issues. Fix this by adding double quotes around all variable
references in the script, such as "$AWS_DEFAULT_REGION", "$AWS_ACCOUNT_ID",
"$CONTAINER_IMAGE-demo-cache", and others, to ensure safe and correct shell
execution.
| - name: Build and push image (PROD) - With Registry Caching | ||
| if: github.ref == 'refs/heads/main' | ||
| run: | | ||
| # Push image to AWS ECR | ||
| # ECR login | ||
| aws ecr get-login-password --region $AWS_DEFAULT_REGION | docker login --username AWS --password-stdin $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com | ||
| docker tag $CONTAINER_IMAGE:${{ github.sha }} $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE:${{ github.sha }} | ||
|
|
||
| # Create cache repository if it doesn't exist | ||
| aws ecr describe-repositories --repository-names $CONTAINER_IMAGE-cache 2>/dev/null || \ | ||
| aws ecr create-repository --repository-name $CONTAINER_IMAGE-cache | ||
|
|
||
| # Define cache image | ||
| CACHE_IMAGE=$AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE-cache:latest | ||
|
|
||
| # Pull cache image (ignore failures for first builds) | ||
| docker pull $CACHE_IMAGE || echo "No cache found, building from scratch" | ||
|
|
||
| # Build with cache-from and inline cache | ||
| docker build \ | ||
| --build-arg BUILDKIT_INLINE_CACHE=1 \ | ||
| --cache-from $CACHE_IMAGE \ | ||
| -t $CONTAINER_IMAGE:${{ github.sha }} \ | ||
| -t $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE:${{ github.sha }} \ | ||
| -t $CACHE_IMAGE \ | ||
| . | ||
|
|
||
| # Push both main image and cache | ||
| docker push $AWS_ACCOUNT_ID.dkr.ecr.$AWS_DEFAULT_REGION.amazonaws.com/$CONTAINER_IMAGE:${{ github.sha }} | ||
| docker push $CACHE_IMAGE |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same shell script fixes to the PROD build step.
The PROD build step has identical shellcheck issues that need the same variable quoting fixes as the DEV and DEMO steps.
🧰 Tools
🪛 actionlint (1.7.7)
193-193: shellcheck reported issue in this script: SC2086:info:2:37: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:2:104: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:2:128: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:5:50: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:6:45: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:12:13: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:17:16: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:18:6: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:19:6: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:19:30: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:19:64: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:20:6: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:24:13: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:24:37: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:24:71: Double quote to prevent globbing and word splitting
(shellcheck)
193-193: shellcheck reported issue in this script: SC2086:info:25:13: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
In .github/workflows/build-server.yaml from lines 191 to 218, the shell script
in the PROD build step lacks proper quoting of variables, causing shellcheck
issues. Fix this by adding double quotes around all variable references such as
"$AWS_DEFAULT_REGION", "$AWS_ACCOUNT_ID", "$CONTAINER_IMAGE", and "$CACHE_IMAGE"
throughout the script to ensure safe and correct shell execution.
| runs-on: blacksmith-4vcpu-ubuntu-2204 | ||
| steps: | ||
| - name: Cleanup expired PR deployments (15 days TTL) | ||
| run: | |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Shell script improvements needed for robustness.
Multiple shellcheck warnings indicate potential issues with unquoted variables and read commands:
- Unquoted variables can cause word splitting issues
readwithout-rwill mangle backslashes- Use
-ninstead of! -zfor better readability
Example fixes for the most critical issues:
# Line 53 (in cleanup-expired-deployments)
- while read deployment_name created_at pr_number; do
+ while IFS=' ' read -r deployment_name created_at pr_number; do
# Line 142 (in check concurrent deployment)
- while read deployment_name pr_num desired_replicas available_replicas ready_replicas; do
+ while IFS=' ' read -r deployment_name pr_num desired_replicas available_replicas ready_replicas; do
# Line 264
- if git diff --name-only origin/$TARGET_BRANCH...HEAD | grep -q "desci-server/prisma/schema.prisma"; then
+ if git diff --name-only "origin/$TARGET_BRANCH...HEAD" | grep -q "desci-server/prisma/schema.prisma"; thenAlso applies to: 120-120, 253-253, 347-347, 412-412
🧰 Tools
🪛 actionlint (1.7.7)
32-32: shellcheck reported issue in this script: SC2086:info:12:10: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:13:58: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2162:info:21:7: read without -r will mangle backslashes
(shellcheck)
32-32: shellcheck reported issue in this script: SC2236:style:28:8: Use -n instead of ! -z
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:29:97: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:32:31: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:33:44: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:34:44: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
In .github/workflows/pr-preview.yaml at line 32 and also lines 120, 253, 347,
and 412, the shell script has unquoted variables, uses read without the -r flag,
and uses ! -z instead of -n for string tests. To fix, quote all variable
expansions to prevent word splitting, add the -r flag to all read commands to
avoid backslash mangling, and replace tests like ! -z "$var" with the more
readable -n "$var". Apply these changes consistently at all mentioned lines.
| import { JwtPayload } from '../JwtPayload'; | ||
|
|
||
| // Re-export all Express types from the original module | ||
| export * from 'express'; | ||
|
|
||
| // Just augment the Express namespace without changing module structure | ||
| declare global { | ||
| namespace Express { | ||
| export interface Request { | ||
| jwtPayload: JwtPayload; | ||
| // language: Language; | ||
| } | ||
| export interface Response { | ||
| customSuccess(httpStatusCode: number, message: string, data?: any): Response; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Well-implemented Express type augmentation with potential duplication concern.
The global namespace augmentation follows TypeScript best practices and provides proper type safety for JWT payloads and custom response methods.
However, there appears to be potential duplication with src/types/express/index.d.ts which defines identical Request and Response interfaces. Consider consolidating these type definitions to avoid maintenance overhead.
Let me check for potential duplication in the Express type definitions:
🏁 Script executed:
#!/bin/bash
# Description: Check for duplicate Express type definitions
# Expected: Find if there are multiple files defining similar Express augmentations
echo "=== Searching for Express type augmentations ==="
fd -e ts -e d.ts | xargs rg -l "interface Request|interface Response" | grep -E "(express|Express)"
echo -e "\n=== Checking content of Express-related type files ==="
fd -e ts -e d.ts | xargs rg -A 5 -B 2 "namespace Express|interface Request.*jwtPayload|interface Response.*customSuccess"Length of output: 2272
Consolidate Duplicate Express Type Augmentations
The Express augmentation is correctly implemented, but identical Request and Response extensions are declared in multiple places. To reduce maintenance overhead and prevent drift, please consolidate these definitions.
• desci-server/src/types/express-augmentations.d.ts
• desci-server/src/types/express/index.d.ts
• (Optional) desci-server/src/middleware/checkJwt.ts also contains a local augmentation—consider relying on the centralized type file instead of redefining here.
🤖 Prompt for AI Agents
In desci-server/src/types/express-augmentations.d.ts lines 1 to 17, consolidate
the Express Request and Response interface augmentations currently duplicated
across this file, desci-server/src/types/express/index.d.ts, and optionally
desci-server/src/middleware/checkJwt.ts. Remove the duplicate augmentations from
the other files and keep a single centralized augmentation in this file to
reduce maintenance overhead and prevent type definition drift.
| // @ts-nocheck | ||
| // TODO: Remove @ts-nocheck and fix 15 implicit any type errors | ||
| // This file needs proper typing for diff operations and object mapping |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid leaving // @ts-nocheck in perpetuity
Disabling type-checking hides real errors (the file already has 15 implicit-anys). Schedule a follow-up task to add typings and remove the directive; otherwise technical debt will accumulate quickly.
🤖 Prompt for AI Agents
In desci-server/src/utils/diffUtils.ts at the beginning of the file (lines 1 to
3), the use of // @ts-nocheck disables TypeScript checking and hides 15 implicit
any type errors. To fix this, remove the // @ts-nocheck directive and add proper
TypeScript typings for all variables, function parameters, and return types in
the file. Schedule a follow-up task to fully address these type errors and
ensure the file is correctly typed to prevent accumulating technical debt.
| # Create directories and set permissions in a single layer for better caching | ||
| RUN mkdir /app && \ | ||
| chown -R node:node /app && \ | ||
| chown -R node:node /root && \ | ||
| mkdir -p /tmp/ipfs/src /tmp/ipfs/ipfs && \ | ||
| chown -R node:node /tmp/ipfs && \ | ||
| mkdir -p /root/.yarn/v6 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Security concern: Consider running as non-root user
The container appears to run as root user. For better security, consider switching to a non-root user after setting up the required permissions.
Add after line 42:
+# Switch to non-root user for security
+USER node📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Create directories and set permissions in a single layer for better caching | |
| RUN mkdir /app && \ | |
| chown -R node:node /app && \ | |
| chown -R node:node /root && \ | |
| mkdir -p /tmp/ipfs/src /tmp/ipfs/ipfs && \ | |
| chown -R node:node /tmp/ipfs && \ | |
| mkdir -p /root/.yarn/v6 | |
| # Create directories and set permissions in a single layer for better caching | |
| RUN mkdir /app && \ | |
| chown -R node:node /app && \ | |
| chown -R node:node /root && \ | |
| mkdir -p /tmp/ipfs/src /tmp/ipfs/ipfs && \ | |
| chown -R node:node /tmp/ipfs && \ | |
| mkdir -p /root/.yarn/v6 | |
| # Switch to non-root user for security | |
| USER node |
🤖 Prompt for AI Agents
In Dockerfile around lines 36 to 42, the container is currently running as root,
which poses a security risk. After setting up the necessary directories and
permissions, add a USER instruction to switch to the non-root user 'node' to run
the container processes securely. This change should be placed immediately after
line 42 to ensure all setup steps run as root but the application runs with
limited privileges.
| echo -e "${GREEN}✓${NC} Context size: $size" | ||
|
|
||
| # Check if size seems reasonable (less than 200MB) | ||
| if [[ $context_line =~ ([0-9.]+)MB && ${BASH_REMATCH[1]} < 200 ]] || [[ $context_line =~ ([0-9.]+)[KB] ]]; then |
There was a problem hiding this comment.
Fix numeric comparison in shell script
Using < for numeric comparison in bash is incorrect. Use -lt for numeric comparisons.
- if [[ $context_line =~ ([0-9.]+)MB && ${BASH_REMATCH[1]} < 200 ]] || [[ $context_line =~ ([0-9.]+)[KB] ]]; then
+ if [[ $context_line =~ ([0-9.]+)MB ]] && (( $(echo "${BASH_REMATCH[1]} < 200" | bc -l) )) || [[ $context_line =~ ([0-9.]+)[KB] ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ $context_line =~ ([0-9.]+)MB && ${BASH_REMATCH[1]} < 200 ]] || [[ $context_line =~ ([0-9.]+)[KB] ]]; then | |
| if [[ $context_line =~ ([0-9.]+)MB ]] && (( $(echo "${BASH_REMATCH[1]} < 200" | bc -l) )) || [[ $context_line =~ ([0-9.]+)[KB] ]]; then |
🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 36-36: < is for string comparisons. Use -lt instead.
(SC2071)
🤖 Prompt for AI Agents
In test-optimizations.sh at line 36, the script uses the '<' operator for
numeric comparison, which is invalid in bash. Replace the '<' operator with
'-lt' to correctly perform numeric comparison in the if condition.
| echo "// optimization test $(date)" >> desci-server/src/index.ts | ||
| echo "Building second time (should use cached layers)..." | ||
| time3=$(date +%s) | ||
| docker build -q -t desci-server:test-v2 . >/dev/null 2>&1 | ||
| time4=$(date +%s) | ||
| second_build=$((time4-time3)) | ||
|
|
||
| # Clean up the test change | ||
| git checkout -- desci-server/src/index.ts 2>/dev/null || true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure safe file modification and restoration
The script modifies a source file but the cleanup might fail if the script is interrupted. Consider using a trap to ensure cleanup.
+# Set up cleanup trap
+cleanup() {
+ git checkout -- desci-server/src/index.ts 2>/dev/null || true
+}
+trap cleanup EXIT
+
# Make a small change to test incremental builds
echo "// optimization test $(date)" >> desci-server/src/index.ts
echo "Building second time (should use cached layers)..."
time3=$(date +%s)
docker build -q -t desci-server:test-v2 . >/dev/null 2>&1
time4=$(date +%s)
second_build=$((time4-time3))
-# Clean up the test change
-git checkout -- desci-server/src/index.ts 2>/dev/null || trueCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In test-optimizations.sh around lines 54 to 62, the script modifies
desci-server/src/index.ts but only attempts cleanup at the end, which may fail
if interrupted. To fix this, add a trap command at the start of the script or
before the modification that restores the original file using git checkout on
script exit or interruption signals. This ensures the file is always restored
even if the script is terminated prematurely.
|
🚀 PR Preview Deployment Your PR has been deployed and is available at: 📋 Deployment Details:
🧹 This deployment will be automatically cleaned up when:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
.github/workflows/pr-preview.yaml (7)
51-58: Repeat of previous shell lint issues – still reading into vars unsafely
while read deployment_name created_at pr_number; do
– still lacks a safeIFSdefinition and the-rflag. Untrusted values (e.g. deployment names containing spaces or back-slashes) will be split/mangled, producing false clean-ups.Same comment was already raised on earlier commits (SC2162 / SC2086).
Please apply the earlier fix everywhere:-while read deployment_name created_at pr_number; do +while IFS=' ' read -r deployment_name created_at pr_number; do…and quote all variable expansions inside the loop.
60-61: String-test style still non-idiomatic
[ ! -z "$created_at" ]⇒ use the positive form[ -n "$created_at" ]for clarity and to avoid!precedence gotchas.
Issue already raised previously (SC2236).-if [ ! -z "$created_at" ] && [ "$created_at" -lt "$CUTOFF_DATE" ]; then +if [ -n "$created_at" ] && [ "$created_at" -lt "$CUTOFF_DATE" ]; then
140-148: Unquoted, unsafereadloop recursSame hazards as earlier:
while read deployment_name pr_num desired_replicas available_replicas ready_replicas; doAdd
IFS=... read -r …and quote all vars. The prior review already pointed this out.
173-183: Orphan-services loop repeats the unsafe patternUpdate to the hardened form:
-while read service_name pr_num; do +while IFS=' ' read -r service_name pr_num; do…plus quoting as noted earlier.
187-198: Same unsafereadusage for ingress cleanupApply the same
IFS=' ' read -r …refactor and quote expansions.
206-214: File-based loop uses plainreadEven when reading from a tmp-file, keep the
-rflag to avoid back-slash processing:-while read pr_num available_replicas; do +while read -r pr_num available_replicas; do
260-265: Still missing the protectivegit fetchfrom previous reviewIf the workflow runs on a fresh runner where
origin/$TARGET_BRANCHisn’t present,git diffwill fail. The earlier review suggested an explicitgit fetch, but the change here still relies on the conditional fetch inside the sub-shell. A simpler, deterministic pattern is:git fetch --no-tags --depth=1 origin "$TARGET_BRANCH"before the diff logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr-preview.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr-preview.yaml
29-29: label "blacksmith-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
76-76: label "blacksmith-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
112-112: label "blacksmith-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 YAMLlint (1.37.1)
.github/workflows/pr-preview.yaml
[error] 59-59: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 67-67: trailing spaces
(trailing-spaces)
[error] 69-69: trailing spaces
(trailing-spaces)
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
[error] 155-155: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 191-191: trailing spaces
(trailing-spaces)
[error] 209-209: trailing spaces
(trailing-spaces)
[error] 276-276: trailing spaces
(trailing-spaces)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 482-482: trailing spaces
(trailing-spaces)
[error] 485-485: trailing spaces
(trailing-spaces)
|
|
||
| # Setup Sentry | ||
| npm install -g @sentry/cli | ||
| sentry-cli login --auth-token $SENTRY_AUTH_TOKEN | ||
| echo -e "\nSENTRY_AUTH_TOKEN=$SENTRY_AUTH_TOKEN" >> desci-server/.env | ||
|
|
||
| # Build and tag image with retry logic and registry caching |
There was a problem hiding this comment.
Secret baked into container image
echo -e "\nSENTRY_AUTH_TOKEN=$SENTRY_AUTH_TOKEN" >> desci-server/.env
persists the production token inside the built Docker layer, so anyone with image pull access can read it. Consider:
- Injecting the token at runtime via Kubernetes secret or Vault rather than hard-coding.
- At minimum, marking the ENV as build-time only (
--build-arg) and not committing it to the final image layer.
🤖 Prompt for AI Agents
In .github/workflows/pr-preview.yaml around lines 364 to 370, the current
approach appends the SENTRY_AUTH_TOKEN directly into the desci-server/.env file
during build, which bakes the secret into the Docker image layers. To fix this,
remove the line that writes the token into the .env file during build and
instead configure the token to be injected at runtime using Kubernetes secrets
or Vault. If runtime injection is not possible, pass the token as a build
argument with --build-arg and ensure it is not persisted in the final image
layers.
|
This PR contains changes to the Prisma schema that may require database migrations affecting the shared dev database. To prevent accidental data loss or corruption, the preview deployment has been skipped. 📋 Schema Changes Detected: 🔧 Next Steps:
🔄 This comment will be updated if you push new commits to this PR. |
…st-pr-mo-deploy-migration
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.env.example (1)
68-68: Remove quotes around JSON map to avoid literal quotes in valuedotenv will keep the quotes; JSON.parse would fail on '"{}"'.
-SENDGRID_TEMPLATE_ID_MAP="{}" +SENDGRID_TEMPLATE_ID_MAP={}
🧹 Nitpick comments (2)
.env.example (2)
209-209: Remove duplicate MIXPANEL token placeholderThere’s a commented MIXPANEL entry and an active one below; keep a single definition to avoid confusion.
-#MIXPANEL_TOKEN=
211-212: Swap SUPABASE env ordering to satisfy linter and keep grouping consistentdotenv-linter flagged ordering; put SERVICE_KEY before URL (and keep values empty in example).
-SUPABASE_URL= -SUPABASE_SERVICE_KEY= +SUPABASE_SERVICE_KEY= +SUPABASE_URL=
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
desci-server/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.env.example(2 hunks)desci-server/kubernetes/deployment_dev.yaml(2 hunks)desci-server/kubernetes/deployment_prod.yaml(2 hunks)desci-server/kubernetes/deployment_staging.yaml(1 hunks)desci-server/package.json(4 hunks)desci-server/prisma/schema.prisma(1 hunks)desci-server/src/routes/v1/services/index.ts(2 hunks)desci-server/src/server.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- desci-server/prisma/schema.prisma
- desci-server/kubernetes/deployment_staging.yaml
- desci-server/kubernetes/deployment_dev.yaml
- desci-server/src/server.ts
- desci-server/kubernetes/deployment_prod.yaml
- desci-server/package.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
desci-server/src/routes/v1/services/index.ts (1)
desci-server/src/controllers/services/shareImagePuppeteer.ts (1)
generateShareImagePuppeteer(596-689)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 68-68: [QuoteCharacter] The value has quote characters (', ")
[warning] 77-77: [KeyWithoutValue] The <<<<<<< HEAD key should be with a value or have an equal sign
[warning] 77-77: [LeadingCharacter] Invalid leading character detected
[warning] 82-82: [LeadingCharacter] Invalid leading character detected
[warning] 82-82: [UnorderedKey] The key should go before the <<<<<<< HEAD key
[warning] 85-85: [UnorderedKey] The AWS_SQS_REGION key should go before the AWS_SQS_SECRET_ACCESS_KEY key
[warning] 87-87: [UnorderedKey] The AWS_SQS_DATA_MIGRATION_QUEUE_URL key should go before the AWS_SQS_REGION key
[warning] 88-88: [UnorderedKey] The AWS_SQS_ML_TOOL_QUEUE_URL key should go before the AWS_SQS_REGION key
[warning] 89-89: [KeyWithoutValue] The >>>>>>> 9455d09 key should be with a value or have an equal sign
[warning] 89-89: [LeadingCharacter] Invalid leading character detected
[warning] 89-89: [LowercaseKey] The >>>>>>> 9455d09 key should be in uppercase
[warning] 89-89: [UnorderedKey] The >>>>>>> 9455d09 key should go before the AWS_SQS_ACCESS_KEY_ID key
[warning] 212-212: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
🔇 Additional comments (2)
desci-server/src/routes/v1/services/index.ts (1)
5-9: Imports update looks correctESM-style .js extensions from TS are consistent with NodeNext; no issues spotted with the moved paths.
.env.example (1)
71-74: Commenting S3 placeholders is fineNo functional impact in example env; this reduces accidental partial configuration.
| // Share image generation endpoint - now using Puppeteer for better layout and markdown support | ||
| router.get('/generate-share-image', [], generateShareImagePuppeteer); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add basic abuse protection and validation to Puppeteer-powered GET endpoint
Puppeteer is CPU/memory heavy. Recommend adding a rate limiter to this route and validating input size to prevent abuse.
You can wire a simple limiter like below and attach it to this route:
- router.get('/generate-share-image', [], generateShareImagePuppeteer);
+ router.get('/generate-share-image', [shareImageRateLimiter], generateShareImagePuppeteer);Additions outside the selected lines:
import rateLimit from 'express-rate-limit';
const shareImageRateLimiter = rateLimit({
windowMs: 60_000, // 1 minute
limit: 30, // tune per env
standardHeaders: true,
legacyHeaders: false,
});Also consider max-length validation of query params (text, citations JSON) in the controller or with a request-schema middleware.
To ensure docs and route are consistent, search for the OpenAPI path:
/v1/services/generate-share-image should exist in the docs.
🏁 Script executed:
#!/bin/bash
rg -n "/v1/services/generate-share-image" desci-server/srcLength of output: 380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== Show route file (first 240 lines) ===="
sed -n '1,240p' desci-server/src/routes/v1/services/index.ts || true
echo
echo "==== Search for generateShareImagePuppeteer definition/usage ===="
rg -n "generateShareImagePuppeteer" || true
echo
echo "==== Search repository for rate limiter usage (express-rate-limit, rateLimit) ===="
rg -n "express-rate-limit|rateLimit|rate_limiter|rate-limit|rateLimiter" || true
echo
echo "==== Search for common validation libs referenced in repo (express-validator, celebrate, joi, zod) ===="
rg -n "express-validator|celebrate|joi|zod|class-validator|@hapi/joi" || true
echo
echo "==== Search for app-level middleware that might apply rate limiting (app.use or server setup) ===="
rg -n "app.use|server.ts|app.ts|index.ts" || true
echo
echo "==== Show swagger and docs hits for context (lines around matches) ===="
sed -n '1,200p' desci-server/src/swagger.ts || true
sed -n '1,140p' desci-server/src/docs/services.ts || trueLength of output: 56380
I can’t share my internal chain-of-thought, but I ran targeted repository checks and verified the facts below. Summary of findings and an actionable rewritten review comment follow.
Findings (concise):
- Endpoint exists: desci-server/src/routes/v1/services/index.ts — router.get('/generate-share-image', [], generateShareImagePuppeteer);
- Controller location: desci-server/src/controllers/services/shareImagePuppeteer.ts (exported generateShareImagePuppeteer).
- OpenAPI / Zod query schema exists: desci-server/src/docs/services.ts (generateShareImageQuerySchema) and swagger.ts definitions.
- No express-rate-limit usage found in the repository.
- The route currently has no per-route rate limiter or validator middleware attached.
Suggested rewritten review comment (concise + actionable):
Add per-route rate limiting and request validation to Puppeteer share-image endpoint
Short explanation:
- Puppeteer generation is resource-heavy. The route currently has no rate-limiter or validation attached even though a Zod query schema exists in docs. Add a per-route rate limiter and apply the existing Zod query schema (via the repo’s validator middleware) to protect the server from abuse and large inputs.
Pinpointed locations to change:
- desci-server/src/routes/v1/services/index.ts — attach limiter + validator to the route
- desci-server/src/routes/v1/services/index.ts (add imports and limiter definition)
- Optionally: desci-server/src/controllers/services/shareImagePuppeteer.ts — add defensive checks for input lengths if desired
Suggested code diffs (apply to desci-server/src/routes/v1/services/index.ts):
- Add imports and limiter near the top (example):
+ import rateLimit from 'express-rate-limit';
+ import { validate } from '../../../middleware/validator.js'; // adjust relative path if necessary
+ import { generateShareImageQuerySchema } from '../../../docs/services.js';
+
+ const shareImageRateLimiter = rateLimit({
+ windowMs: 60_000, // 1 minute - tune per environment
+ max: 30, // allow up to 30 requests per minute per IP (tune as needed)
+ standardHeaders: true,
+ legacyHeaders: false,
+ });- Attach limiter + validator to the route:
- router.get('/generate-share-image', [], generateShareImagePuppeteer);
+ router.get(
+ '/generate-share-image',
+ [shareImageRateLimiter, validate(generateShareImageQuerySchema)],
+ generateShareImagePuppeteer
+ );Notes and recommendations:
- tune windowMs/max according to deployment env and expected traffic; consider lower limits for public endpoints.
- The repository already defines generateShareImageQuerySchema (Zod) in desci-server/src/docs/services.ts — reuse it via the existing validator middleware (desci-server/src/middleware/validator.ts). If the validator API differs, adapt the above validate(...) call to match (e.g., validateQuery or similar).
- Add explicit max-length checks for heavy query params (text, answer, citations JSON) either in the controller or via a Zod refinement, e.g. z.string().max(2000) / schema refinements for citations. For citations, limit JSON string length and/or number of items parsed.
- Consider applying a global or per-host concurrency limit for Puppeteer (single shared browser pool with queueing) rather than allowing many concurrent Puppeteer launches.
- Add monitoring/alerts for rate-limiter hits and for long-running Puppeteer runs.
Classification:
🤖 Prompt for AI Agents
In desci-server/src/routes/v1/services/index.ts around lines 20-22, the GET
/generate-share-image route uses generateShareImagePuppeteer but has no
per-route rate limiter or request validation attached; add imports for the
express-rate-limit factory used in the repo and the validator middleware plus
the generateShareImageQuerySchema, define a sensible per-route limiter
(windowMs/max) near the top, and then attach both the limiter and the validator
to the route signature (e.g., router.get('/generate-share-image', [limiter,
validate(generateShareImageQuerySchema)], generateShareImagePuppeteer));
optionally add guards in the controller for max input lengths or Zod refinements
for large text/citations.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.env.example (1)
68-68: Remove quotes around JSON to avoid parsing issuesMost dotenv loaders take values verbatim; the quotes will be included, breaking JSON.parse or similar consumers.
-SENDGRID_TEMPLATE_ID_MAP="{}" +SENDGRID_TEMPLATE_ID_MAP={}
🧹 Nitpick comments (2)
.env.example (2)
71-74: Clarify S3 placeholders’ intentSince these are now commented out, add a guiding note so devs know when to enable them.
-# S3 Bucket +# S3 Bucket +# Uncomment these if using S3-backed storage (uploads, static assets, etc.) # AWS_S3_BUCKET_NAME= # AWS_S3_BUCKET_REGION= # AWS_ACCESS_KEY_ID= # AWS_SECRET_ACCESS_KEY=
202-206: Deduplicate MIXPANEL_TOKEN and satisfy dotenv-linter ordering for Supabase
- Keep a single MIXPANEL_TOKEN placeholder (uncommented, empty) for consistency with the rest of the file.
- dotenv-linter warns that SUPABASE_SERVICE_KEY should go before SUPABASE_URL. Reorder to silence the warning.
-#MIXPANEL_TOKEN= - -SUPABASE_URL= -SUPABASE_SERVICE_KEY= -MIXPANEL_TOKEN= +SUPABASE_SERVICE_KEY= +SUPABASE_URL= +MIXPANEL_TOKEN=
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env.example(2 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 68-68: [QuoteCharacter] The value has quote characters (', ")
[warning] 205-205: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
desci-server/src/routes/v1/services/index.ts(2 hunks)desci-server/src/utils/upload.ts(0 hunks)desci-server/tsconfig-test.json(2 hunks)
💤 Files with no reviewable changes (1)
- desci-server/src/utils/upload.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- desci-server/src/routes/v1/services/index.ts
🧰 Additional context used
🪛 Biome (2.1.2)
desci-server/tsconfig-test.json
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-34: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
⏰ 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). (2)
- GitHub Check: build-and-deploy-pr
- GitHub Check: build-and-test
🔇 Additional comments (1)
desci-server/tsconfig-test.json (1)
31-32: Include/Exclude selections look good for isolating tests and custom type augmentations.Including the Express augmentation and excluding scripts/one-off utilities is appropriate.
| "esnext", | ||
| "dom" | ||
| ], | ||
| "lib": ["esnext", "dom"], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix Biome parse errors: tsconfig contains comments (JSONC) but Biome expects strict JSON.
Biome 2.1.2 is erroring with “End of file expected” on multiple lines because this file contains comments (//, /* */). Options:
- Option A (preferred): Strip all comments from tsconfig to keep Biome happy.
- Option B: Configure Biome to ignore tsconfig files or treat them as JSON-with-comments if supported in your setup.
Option A: remove comments (example patch for the commented sections shown in this file):
- /**
- * Implied by "module: nodenext":
- * "moduleResolution": "nodenext",
- * "esModuleInterop": true,
- */
+
- // Recommended by docs, but a lot of errors
- // "verbatimModuleSyntax": true,
+
- // dagConcat.cjs requires this
+ Option B: ignore tsconfig in Biome (.biome.json):
{
"$schema": "https://biomejs.dev/schemas/1.6.0/schema.json",
"files": {
"ignore": [
"desci-server/tsconfig*.json"
]
}
}Alternatively, use an override to disable the linter just for tsconfig files:
{
"overrides": [
{
"include": ["**/tsconfig*.json"],
"linter": { "enabled": false },
"formatter": { "enabled": true }
}
]
}🧰 Tools
🪛 Biome (2.1.2)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🤖 Prompt for AI Agents
In desci-server/tsconfig-test.json around line 12, the file contains JSONC
comments which cause Biome 2.1.2 parse errors ("End of file expected"); remove
all comments (both // and /* */) so the file is strict JSON (option A preferred)
and ensure the JSON remains syntactically valid after removal (no dangling
commas or stray commas where comments were); alternatively, if you choose option
B, add a Biome config to ignore or treat tsconfig*.json as JSON-with-comments
(e.g., add ignore or overrides in .biome.json) but default fix is to strip all
comments from tsconfig-test.json.
🛠️ Refactor suggestion
Reconsider adding "dom" to lib; it can pollute Node test types and cause conflicts.
In a Node/Express test environment, including "dom" frequently introduces conflicting globals (e.g., Request, Event) and can mask real issues. Unless tests truly rely on browser APIs, drop "dom".
Apply this diff to keep the surface lean:
- "lib": ["esnext", "dom"],
+ "lib": ["esnext"],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lib": ["esnext", "dom"], | |
| "lib": ["esnext"], |
🧰 Tools
🪛 Biome (2.1.2)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🤖 Prompt for AI Agents
In desci-server/tsconfig-test.json around line 12, the "lib" array currently
includes "dom" which can pollute Node test types; remove "dom" from the array so
it reads only the needed Node/ECMAScript libs (e.g., keep "esnext" or change to
an appropriate ES target), and if any browser-specific tests truly need DOM
types, create a separate tsconfig (or use per-test /// <reference lib="dom" />
or a specific tsconfig.extend) for those tests instead of adding "dom" globally.
| "skipDefaultLibCheck": true, | ||
| "noResolve": false, | ||
| "noImplicitAny": false, | ||
| "types": [], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
"types": [] disables all ambient types; remove it or explicitly include your test runner + node. Also prune redundant flags.
- "types": [] turns off auto-inclusion of all @types packages. This often breaks test globals (describe/it) and Node types.
- "skipDefaultLibCheck" is redundant when "skipLibCheck" is true.
- "noResolve": false and "noImplicitAny": false are defaults; omit to reduce noise.
Suggested minimal cleanup (if you don’t need explicit test types):
- "skipDefaultLibCheck": true,
- "noResolve": false,
- "noImplicitAny": false,
- "types": [],
+ // Keep defaults; don't restrict ambient typesIf you rely on a specific test framework, explicitly include types to prevent ambiguity:
- Mocha:
- "types": [],
+ "types": ["node", "mocha"],- Jest:
- "types": [],
+ "types": ["node", "jest"],- Vitest:
- "types": [],
+ "types": ["node", "vitest"],Run this script to detect your test framework and whether globals are used:
🏁 Script executed:
#!/bin/bash
# Detect common test frameworks and usage of global test functions
echo "Searching for test frameworks in package.json..."
rg -n '"(jest|mocha|vitest)"' -S --glob package.json -A 3 || true
echo
echo "Checking for global test functions in test files..."
rg -n -e '\b(describe|it|test|before|after)\b' --glob 'test/**/*.ts' -A 2 || trueLength of output: 1924
🏁 Script executed:
#!/bin/bash
# Check if skipLibCheck is set in tsconfig-test.json
rg -n '"skipLibCheck"' -n desci-server/tsconfig-test.json -A2 -B2 || trueLength of output: 236
🏁 Script executed:
#!/bin/bash
# Verify presence of test files under desci-server
echo "Checking for test globals in desci-server/test/**/*.ts..."
rg -n -e '\b(describe|it|test|before|after)\b' --glob 'desci-server/test/**/*.ts' -A2 || trueLength of output: 250658
Clean up desci-server/tsconfig-test.json to restore ambient types and drop redundant flags
Please update desci-server/tsconfig-test.json as follows:
• Remove redundant options
- Drop
"skipDefaultLibCheck": true(you already have"skipLibCheck": true) - Omit
"noResolve": falseand"noImplicitAny": false(those are compiler defaults)
• Re-enable ambient Mocha and Node types
Replace the empty types array with the specific test runner and Node definitions you’re using:
--- a/desci-server/tsconfig-test.json
@@
- "skipDefaultLibCheck": true,
- "noResolve": false,
- "noImplicitAny": false,
- "types": [],
+ // skipLibCheck covers library checks; noResolve/noImplicitAny can be omitted
+ "types": ["node", "mocha"],This ensures that your Mocha globals (describe/it) and Node built-ins are available in your test build.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "skipDefaultLibCheck": true, | |
| "noResolve": false, | |
| "noImplicitAny": false, | |
| "types": [], | |
| // skipLibCheck covers library checks; noResolve/noImplicitAny can be omitted | |
| "types": ["node", "mocha"], |
🧰 Tools
🪛 Biome (2.1.2)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🤖 Prompt for AI Agents
In desci-server/tsconfig-test.json around lines 21 to 24, remove the redundant
compiler flags "skipDefaultLibCheck": true, "noResolve": false and
"noImplicitAny": false, and replace the empty "types": [] entry with the ambient
test and node types you use (e.g. ["mocha","node"]) so Mocha globals and Node
built-ins are available to the test build.
| "./src/types/express" | ||
| ] | ||
| }, | ||
| "typeRoots": ["./node_modules/@types"], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coding typeRoots may break hoisted/monorepo type resolution; prefer defaults.
Setting typeRoots to "./node_modules/@types" prevents TypeScript from searching ancestor node_modules in a workspace. This can break type resolution under pnpm/yarn workspaces.
Apply this diff to rely on TS defaults (it already searches node_modules/@types up the tree):
- "typeRoots": ["./node_modules/@types"],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "typeRoots": ["./node_modules/@types"], |
🧰 Tools
🪛 Biome (2.1.2)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🤖 Prompt for AI Agents
In desci-server/tsconfig-test.json around line 28, the tsconfig hard-codes
"typeRoots": ["./node_modules/@types"], which prevents TypeScript from searching
ancestor node_modules in monorepos/workspaces; remove the "typeRoots" entry (or
comment it out) so the config relies on TypeScript's defaults and allows
hoisted/ancestor node_modules/@types to be resolved correctly.
1 participant
ensure pr with migration doesn't deploy to pr-preview
Summary by CodeRabbit
New Features
Performance
Documentation
Chores
Tests