Extract db, models, ui, and provider-github into dedicated packages - #89
Extract db, models, ui, and provider-github into dedicated packages#89devarshishimpi wants to merge 9 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…r models extraction
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![codra-app-personal[bot]](https://avatars.githubusercontent.com/in/3347243?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codra Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a691753727
ℹ️ About Codra in GitHub
Your team has set up Codra to review pull requests in this repo. Reviews are triggered when you:
- Open a pull request for review
- Mark a draft as ready
- Comment "@codra-app review"
If Codra has suggestions, it will comment; otherwise it will react with 👍.
Codra can also answer questions or update the PR. Try commenting "@codra-app address that feedback".
| import { runBestEffortJobMaintenance } from '@server/core/job-recovery'; | ||
| import { runWithDb } from '@server/db/client'; | ||
| import type { AppBindings } from '../env'; | ||
| import type { WorkflowStep } from 'cloudflare:workers'; |
There was a problem hiding this comment.
The class constructor uses the Workflow type, but it is not imported from cloudflare:workers. This will lead to a compilation error as Workflow is not available globally.
| import type { WorkflowStep } from 'cloudflare:workers'; | |
| import type { Workflow, WorkflowStep } from 'cloudflare:workers'; |
| } | ||
|
|
||
| async createSession(session: DashboardSessionUser): Promise<string> { | ||
| const token = Math.random().toString(36).substring(2); |
There was a problem hiding this comment.
The application uses Math.random() to generate session tokens. This PRNG is not cryptographically secure and is predictable, which allows attackers to potentially guess active session tokens and hijack user sessions. Use crypto.randomUUID() or crypto.getRandomValues() to generate tokens instead.
| private kv = new InMemoryKV(); | ||
|
|
||
| async createSession(session: DashboardSessionUser): Promise<string> { | ||
| const token = Math.random().toString(36).substring(2); |
There was a problem hiding this comment.
Math.random() is not cryptographically secure and should not be used to generate session tokens. It is predictable and vulnerable to attacks, even if used in a mock implementation.
| "test": "vitest run" | ||
| }, | ||
| "dependencies": { | ||
| "@codra/schema": "*", |
There was a problem hiding this comment.
Using '' for monorepo internal dependencies is non-deterministic and can cause resolution issues in workspace environments. Use 'workspace:' to ensure the local workspace version is always used.
| "@codra/schema": "*", | |
| "@codra/schema": "workspace:*" |
| "composite": false, | ||
| "declaration": false, | ||
| "emitDeclarationOnly": false, | ||
| "noEmit": true |
There was a problem hiding this comment.
The 'noEmit: true' setting prevents the package from generating type definitions (.d.ts files). If this package is intended to be consumed by other workspace packages as a library, it will fail to provide type safety to consumers unless they import source files directly.
2 participants
Description
Continues the workspace restructuring started in #87, splitting more of
src/serverandsrc/clientout of the monolithic app into standalone packages/apps:src/server/db/*modules (accounts, jobs, settings, repo-configs, migrations, repository classes) become a standalone package with its ownpackage.json/tsconfig/vitest.config.src/server/models.src/client.constants.tsfiles added topackages/core,packages/db, andpackages/schemato replace scattered default configs.Part of #66
Closes #70 #71 #72 #73 #75
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist: