Feature/interactive cli wizard for project bootstrapping#753
Feature/interactive cli wizard for project bootstrapping#753
Conversation
…ackage.json, and page model
…apping of TEMPLATES
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds seven UI blocks to API/frontend, extends many package.json manifests with o2sTemplate/o2sModules metadata, implements a rewritten interactive create-o2s-app CLI (wizard, scaffold, clone/cleanup, transforms, env generation, install), and introduces a full mocked-dxp integration with multilingual mock CMS, articles, and search data. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as create-o2s-app
participant Git as GitHub Repo
participant Wizard
participant Scaffold
participant FS as File System
participant NPM
User->>CLI: run create-o2s-app (flags or interactive)
CLI->>Git: clone repository to temp dir
Git-->>CLI: repo content
CLI->>Wizard: runWizard(tempDir, cli args)
Wizard->>Wizard: discover blocks & integrations
Wizard-->>CLI: WizardAnswers (projectName, template, blocks, integrations, envVars, resolutions)
CLI->>Scaffold: scaffold(tempDir, WizardAnswers)
Scaffold->>FS: move temp to targetDir
Scaffold->>FS: remove unselected blocks/integrations (cleanup)
Scaffold->>FS: transform files (app.module.ts, page.model.ts, renderBlocks.tsx, package.jsons)
Scaffold->>FS: generate .env.local and integration configs
Scaffold->>NPM: installDependencies (optional)
NPM-->>Scaffold: install result
Scaffold-->>CLI: return targetDir, uncoveredModules
CLI->>User: printSummary(targetDir, template, counts, warnings)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (34)
packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.login-page.mapper.ts-12-58 (1)
12-58:⚠️ Potential issue | 🟠 MajorEnsure
errorMessages.idvalues are unique per form field.
usernameandpasswordreuse the same ids (required-1,min-1,matches-1) in each locale (for example, Line 16 vs Line 43). If these ids are used for DOM ids, ARIA references, or keyed rendering, collisions can cause incorrect error binding and accessibility breakage.Proposed fix
- id: 'required-1', + id: 'username-required-1', ... - id: 'matches-1', + id: 'username-matches-1', ... - id: 'min-1', + id: 'username-min-1', ... - id: 'required-1', + id: 'password-required-1', ... - id: 'min-1', + id: 'password-min-1', ... - id: 'matches-1', + id: 'password-matches-1',Apply the same pattern in PL/EN/DE blocks.
Also applies to: 103-149, 215-261
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.login-page.mapper.ts` around lines 12 - 58, The errorMessages.id values for the username and password Input objects are duplicated (e.g., 'required-1', 'min-1', 'matches-1'); update ids inside the username and password objects in cms.login-page.mapper.ts so they are unique per field (for example append the field name: 'required-username-1', 'min-password-1', etc.), and apply the same unique-id pattern to the other locale blocks referenced (lines ~103-149 and ~215-261) to avoid collisions used for DOM/ARIA/keyed rendering.packages/integrations/mocked-dxp/src/modules/cms/mappers/mocks/pages/business-cards.page.ts-36-36 (1)
36-36:⚠️ Potential issue | 🟠 MajorAvoid mutable
mainbranch image URLs in mock fixtures.Referencing
raw.githubusercontent.com/.../main/...makes fixtures non-deterministic over time. Prefer a commit-pinned URL or local mocked asset path.Also applies to: 139-139, 242-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/mocks/pages/business-cards.page.ts` at line 36, Replace the mutable raw.githubusercontent.com ".../main/..." image URLs used in the mock page object with deterministic references: either a commit-pinned GitHub blob URL or a local mocked asset path. Locate the offending `url` properties in the business-cards mock (the image entries in the exported page object in business-cards.page.ts) and update each occurrence (three spots) to use a commit SHA in the GitHub URL or point to a local asset under the mock assets directory so fixtures remain stable.packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.category.mapper.ts-11-19 (1)
11-19:⚠️ Potential issue | 🟠 MajorRemove
__typenamefrom framework-model mock payloads.These objects are typed as
CMS.Model.CategoryBlock.CategoryBlock; embedding GraphQL discriminators incomponentsleaks API-harmonization concerns into normalized framework models and can cause contract drift.Suggested fix
components: [ { - __typename: 'FaqBlock', id: 'faq-1', layout: { variant: 'narrow', spacing: 'medium', background: 'none', }, }, ],Based on learnings: In framework CMS models, explicit
__typenamediscriminators should be added at API harmonization, not in framework-layer model payloads.Also applies to: 44-52, 77-85, 110-118, 144-152, 177-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.category.mapper.ts` around lines 11 - 19, The mock payloads for CategoryBlock objects include GraphQL discriminators (__typename) which must be removed so framework models (CMS.Model.CategoryBlock.CategoryBlock) don’t leak API harmonizers; locate the mock objects in cms.category.mapper.ts (the CategoryBlock/category blocks declared with layout/spacing/background and components arrays) and delete all __typename properties from those objects (and any nested component entries), ensuring the remaining shape matches the CMS.Model.CategoryBlock.CategoryBlock type; apply the same removal to the other occurrences noted (around the other block entries).packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.quick-links.mapper.ts-410-417 (1)
410-417:⚠️ Potential issue | 🟠 MajorHandle unknown
idexplicitly instead of using non-null assertion.Lines 412, 414, and 416 use
.find(...)!with non-null assertions. Ifiddoesn't match any block in the locale-specific array,.find()returnsundefined, causing the non-null assertion to violate the declared return typeCMS.Model.QuickLinksBlock.QuickLinksBlock. This can trigger runtime errors in downstream code.Add an explicit guard after the find operation to throw a
NotFoundExceptionwhen the block is not found:const block = blocks.find((block) => block.id === id); if (!block) { throw new NotFoundException(`Quick links block not found for id: ${id}`); } return block;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.quick-links.mapper.ts` around lines 410 - 417, Replace the non-null assertions in the locale switch inside the function that returns CMS.Model.QuickLinksBlock.QuickLinksBlock by first assigning the result of find(...) from QUICK_LINKS_BLOCKS_DE / QUICK_LINKS_BLOCKS_PL / QUICK_LINKS_BLOCKS_EN to a local const (e.g. block), then check if (!block) and throw a NotFoundException with a clear message like `Quick links block not found for id: ${id}` before returning block; do this for each case instead of using .find(...)! so the function never returns undefined at runtime.packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.bento-grid.mapper.ts-379-387 (1)
379-387:⚠️ Potential issue | 🟠 MajorHandle missing block IDs explicitly instead of relying on non-null assertions.
On lines 381, 383, and 385,
find(...)!can still returnundefinedwhenidis not found in the arrays, but the function contract requires returning a block. This can surface as downstream runtime failures instead of controlled 404 errors. This pattern is repeated across all block mappers in the same directory and should be fixed systematically.Suggested approach:
- Extract locale-to-blocks mapping into a lookup object to eliminate the switch statement.
- Validate the block lookup and throw
NotFoundExceptionwith an explicit error message if not found.- Apply the same fix across all affected mappers (cms.quick-links.mapper.ts, cms.hero-section.mapper.ts, cms.media-section.mapper.ts, cms.feature-section.mapper.ts, cms.pricing-section.mapper.ts, cms.cta-section.mapper.ts, cms.feature-section-grid.mapper.ts).
Proposed fix
+const BENTO_GRID_BLOCKS_BY_LOCALE = { + de: MOCK_BENTO_GRID_BLOCKS_DE, + pl: MOCK_BENTO_GRID_BLOCKS_PL, + en: MOCK_BENTO_GRID_BLOCKS_EN, +} as const; + export const mapBentoGridBlock = ({ locale, id, }: CMS.Request.GetCmsEntryParams): CMS.Model.BentoGridBlock.BentoGridBlock => { - switch (locale) { - case 'de': - return MOCK_BENTO_GRID_BLOCKS_DE.find((block) => block.id === id)!; - case 'pl': - return MOCK_BENTO_GRID_BLOCKS_PL.find((block) => block.id === id)!; - case 'en': - return MOCK_BENTO_GRID_BLOCKS_EN.find((block) => block.id === id)!; - default: - throw new NotFoundException(); - } + const blocks = BENTO_GRID_BLOCKS_BY_LOCALE[locale as keyof typeof BENTO_GRID_BLOCKS_BY_LOCALE]; + if (!blocks) { + throw new NotFoundException(`Unsupported locale: ${locale}`); + } + + const block = blocks.find((candidate) => candidate.id === id); + if (!block) { + throw new NotFoundException(`Bento grid block not found for locale='${locale}', id='${id}'`); + } + + return block; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.bento-grid.mapper.ts` around lines 379 - 387, The switch-based lookup in the mapper uses non-null assertions (e.g., MOCK_BENTO_GRID_BLOCKS_DE.find(...)!) which can return undefined; replace the switch in the function handling locale lookups with a locale->blocks lookup object (map locales 'de','pl','en' to MOCK_BENTO_GRID_BLOCKS_DE/PL/EN), perform a safe find on that array, and if the result is falsy throw a NotFoundException with an explicit message like `Block with id ${id} not found for locale ${locale}` instead of using `!`; apply the same pattern to the other mappers named in the review (cms.quick-links.mapper.ts, cms.hero-section.mapper.ts, cms.media-section.mapper.ts, cms.feature-section.mapper.ts, cms.pricing-section.mapper.ts, cms.cta-section.mapper.ts, cms.feature-section-grid.mapper.ts) to ensure consistent, explicit 404 handling.packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.faq.mapper.ts-200-208 (1)
200-208:⚠️ Potential issue | 🟠 MajorNormalize locale input before routing to language payload.
mapFaqBlockonly matches exact'de'/'pl'. Inputs likede-DE,pl-PL, orDEwill fall through to English (Line 206), causing wrong-language content.Proposed fix
export const mapFaqBlock = (locale: string): CMS.Model.FaqBlock.FaqBlock => { - switch (locale) { + const normalizedLocale = locale.toLowerCase().split('-')[0]; + switch (normalizedLocale) { case 'de': return MOCK_FAQ_LIST_BLOCK_DE; case 'pl': return MOCK_FAQ_LIST_BLOCK_PL; default: return MOCK_FAQ_LIST_BLOCK_EN; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.faq.mapper.ts` around lines 200 - 208, mapFaqBlock currently matches only exact 'de'/'pl' so inputs like 'de-DE', 'pl-PL' or 'DE' resolve to English; normalize the incoming locale in mapFaqBlock by lowercasing and extracting the primary language subtag (e.g. locale = (locale || '').toLowerCase().split('-')[0]) before the switch, then route to MOCK_FAQ_LIST_BLOCK_DE, MOCK_FAQ_LIST_BLOCK_PL or default MOCK_FAQ_LIST_BLOCK_EN accordingly to ensure variants like 'de-DE' and uppercase codes are handled.packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.faq.mapper.ts-3-198 (1)
3-198:⚠️ Potential issue | 🟠 MajorReturn a deep clone of the FAQ block objects to prevent mutations between requests.
The function returns direct references to module-level singleton objects. If any consumer mutates nested fields (items array, banner properties, etc.), subsequent requests will receive contaminated data. Implement cloning before returning to ensure each caller receives independent data.
Suggested implementation
+const cloneFaqBlock = (block: CMS.Model.FaqBlock.FaqBlock): CMS.Model.FaqBlock.FaqBlock => + JSON.parse(JSON.stringify(block)); + export const mapFaqBlock = (locale: string): CMS.Model.FaqBlock.FaqBlock => { switch (locale) { case 'de': - return MOCK_FAQ_LIST_BLOCK_DE; + return cloneFaqBlock(MOCK_FAQ_LIST_BLOCK_DE); case 'pl': - return MOCK_FAQ_LIST_BLOCK_PL; + return cloneFaqBlock(MOCK_FAQ_LIST_BLOCK_PL); default: - return MOCK_FAQ_LIST_BLOCK_EN; + return cloneFaqBlock(MOCK_FAQ_LIST_BLOCK_EN); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.faq.mapper.ts` around lines 3 - 198, The exported mapper currently returns module-level singletons (MOCK_FAQ_LIST_BLOCK_EN, MOCK_FAQ_LIST_BLOCK_DE, MOCK_FAQ_LIST_BLOCK_PL) directly which allows callers to mutate shared state; update the mapper(s) that return these constants to return a deep clone instead (use structuredClone where available or a reliable deep-clone utility like lodash.cloneDeep/JSON parse-stringify fallback) so each caller receives an independent copy of items, banner and nested fields; locate the function(s) that return these symbols in cms.faq.mapper.ts and replace the direct return with a deep-cloned copy of the corresponding MOCK_FAQ_LIST_BLOCK_* constant.packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.hero-section.mapper.ts-1153-1167 (1)
1153-1167:⚠️ Potential issue | 🟠 MajorThrow
NotFoundExceptionwhen a block ID is missing for a supported locale.On Line 1159, Line 1161, and Line 1163,
.find(... )!can still resolve toundefinedat runtime whenidis unknown, so this path can return an invalid value instead of 404.💡 Proposed fix
export const mapHeroSectionBlock = ({ locale, id, }: CMS.Request.GetCmsEntryParams): CMS.Model.HeroSectionBlock.HeroSectionBlock => { + let block: CMS.Model.HeroSectionBlock.HeroSectionBlock | undefined; switch (locale) { case 'de': - return HERO_SECTION_BLOCKS_DE.find((block) => block.id === id)!; + block = HERO_SECTION_BLOCKS_DE.find((item) => item.id === id); + break; case 'pl': - return HERO_SECTION_BLOCKS_PL.find((block) => block.id === id)!; + block = HERO_SECTION_BLOCKS_PL.find((item) => item.id === id); + break; case 'en': - return HERO_SECTION_BLOCKS_EN.find((block) => block.id === id)!; + block = HERO_SECTION_BLOCKS_EN.find((item) => item.id === id); + break; default: - throw new NotFoundException(); + throw new NotFoundException(`Unsupported locale: ${locale}`); } + + if (!block) { + throw new NotFoundException(`Hero section block "${id}" not found for locale "${locale}"`); + } + + return block; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.hero-section.mapper.ts` around lines 1153 - 1167, The mapHeroSectionBlock function currently uses non-null assertions (e.g., HERO_SECTION_BLOCKS_DE.find(... )!) which can return undefined for unknown ids; change each locale case (in mapHeroSectionBlock) to store the result of HERO_SECTION_BLOCKS_DE/PL/EN.find(...) in a local variable (e.g., const block = ...), check if block is undefined and if so throw new NotFoundException(), otherwise return block—this ensures a 404 is thrown when an id is missing instead of returning an invalid value.packages/integrations/mocked-dxp/src/modules/search/mappers/articles.mapper.ts-31-35 (1)
31-35:⚠️ Potential issue | 🟠 MajorReturn total filtered matches, not current page length.
Lines 24 and 28 filter the articles array. Line 35 returns
total: articlesToReturn.length, which is the current page size. For pagination metadata, this should be the total filtered count (articles.length) before slicing, consistent with patterns across other mappers in the codebase (orders, organizations, notifications, invoices).🔧 Suggested fix
const articlesToReturn = articles.slice(offset, offset + limit); return { data: articlesToReturn, - total: articlesToReturn.length, + total: articles.length, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/search/mappers/articles.mapper.ts` around lines 31 - 35, The returned pagination metadata is using the paged array length; change the mapper so total reports the total number of filtered matches before slicing. In the function that builds articlesToReturn (using articles.slice(offset, offset + limit)) and returns { data: articlesToReturn, total: ... }, replace total: articlesToReturn.length with total: articles.length so total reflects the full filtered count prior to paging.packages/integrations/mocked-dxp/src/auth/auth.providers.ts-39-44 (1)
39-44:⚠️ Potential issue | 🟠 MajorDo not collapse all runtime failures into
Authentication error.This catch block currently masks operational failures (e.g., DB issues) as invalid auth, which makes outages harder to detect and debug.
Suggested fix
} catch (error) { if (error instanceof ZodError) { throw new Error('Validation error'); - } else { - throw new Error('Authentication error'); } + if (error instanceof Error && error.message === 'Invalid credentials') { + throw error; + } + throw error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/auth/auth.providers.ts` around lines 39 - 44, The catch in auth.providers.ts currently converts every failure into a generic 'Authentication error'; change it so validation failures keep a clear validation error and all other runtime errors are not masked: in the catch block check `if (error instanceof ZodError)` and throw a Validation/BadRequest error containing validation details, but for non-ZodError cases rethrow the original `error` (or wrap it while preserving the original message/stack) instead of throwing 'Authentication error' so operational failures (DB, network, etc.) remain visible; update any surrounding function (the catch in auth.providers.ts) to preserve error type/stack for non-validation cases.packages/integrations/mocked-dxp/src/auth/auth.jwt.ts-87-87 (1)
87-87:⚠️ Potential issue | 🟠 MajorAdd an explicit guard for missing
AUTH_JWT_SECRET.The non-null assertion hides configuration mistakes and leads to late runtime failures.
Suggested fix
function signUserToken(token: JWT): string { + const secret = process.env.AUTH_JWT_SECRET; + if (!secret) { + throw new Error('AUTH_JWT_SECRET is not configured'); + } return jwt.sign( @@ - process.env.AUTH_JWT_SECRET!, + secret, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/auth/auth.jwt.ts` at line 87, The code uses process.env.AUTH_JWT_SECRET! with a non-null assertion; add an explicit runtime guard instead. Read process.env.AUTH_JWT_SECRET into a local constant (e.g., const AUTH_JWT_SECRET = process.env.AUTH_JWT_SECRET) at module init or inside the function that uses it (where process.env.AUTH_JWT_SECRET! appears), check if it's falsy and throw a clear Error like "Missing AUTH_JWT_SECRET" (or return an error) before proceeding, then use that constant for JWT signing/verification so you fail fast with a helpful message instead of relying on the non-null assertion.packages/integrations/mocked-dxp/src/auth/auth.jwt.ts-35-39 (1)
35-39:⚠️ Potential issue | 🟠 Major
accessTokenExpiresis used but never initialized in this flow.The refresh decision depends on
token.accessTokenExpires, but this callback does not set it before checking expiration.Suggested fix
+const ACCESS_TOKEN_TTL_MS = 15 * 60 * 1000; + export const jwtCallback = async ( @@ - token.accessToken = signUserToken(token); + if (!token.accessTokenExpires) { + token.accessTokenExpires = Date.now() + ACCESS_TOKEN_TTL_MS; + } + token.accessToken = signUserToken(token); if (Date.now() >= token.accessTokenExpires) { return refreshAccessToken(token); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/auth/auth.jwt.ts` around lines 35 - 39, The code checks token.accessTokenExpires before it's ever set; update the flow so accessTokenExpires is initialized immediately after signing: have signUserToken return the access token plus its expiry (or expose a constant expiry duration) and assign token.accessTokenExpires = <returnedExpiry> or Date.now() + <expiryMs> before calling if (Date.now() >= token.accessTokenExpires) and refreshAccessToken(token); adjust signUserToken and the caller (the token assignment block) so the signed token and its expiry are produced/consumed together and ensure refreshAccessToken receives the initialized token.accessTokenExpires.packages/integrations/mocked-dxp/package.json-35-43 (1)
35-43:⚠️ Potential issue | 🟠 Major
preparescript runs database migrations and seeding automatically.The
preparelifecycle hook runs onnpm installin development. Having it executeprisma migrate deployandprisma db seedmeans every install will attempt to modify database state, which can be:
- Unexpected for developers cloning the repo
- Problematic if database connection isn't configured
- Potentially destructive if run against the wrong database
Consider moving migrations and seeding to explicit scripts that developers run manually.
Proposed fix
"scripts": { - "prepare": "npx prisma generate && dotenv -e .env.local -- prisma migrate deploy && dotenv -e .env.local -- prisma db seed", + "prepare": "npx prisma generate", + "db:setup": "dotenv -e .env.local -- prisma migrate deploy && dotenv -e .env.local -- prisma db seed", "postinstall": "npx prisma generate",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/package.json` around lines 35 - 43, Remove destructive DB commands from the "prepare" npm lifecycle script inside the package.json scripts object: stop running "prisma migrate deploy" and "prisma db seed" in the "prepare" entry (leave only safe tasks like "npx prisma generate" or make "prepare" empty), and instead add explicit scripts such as "migrate:deploy" (running "dotenv -e .env.local -- prisma migrate deploy") and "db:seed" or update the existing "seed" script to be the canonical seeding command; also ensure any build-time script ("build") does not implicitly run migrations unless intentionally desired.packages/integrations/mocked-dxp/src/modules/cms/mappers/mocks/pages/business-cards-business-debit.page.ts-119-143 (1)
119-143:⚠️ Potential issue | 🟠 MajorLocalize DE/PL SEO metadata instead of reusing English copy.
Line 119–143 and Line 228–252 duplicate English
seo.title/description/keywordsfor non-English locales. This degrades localized UX and search relevance for/geschaftlich/...and/firma/...pages.Also applies to: 228-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/mocks/pages/business-cards-business-debit.page.ts` around lines 119 - 143, The DE and PL localized page objects currently reuse the English seo.title, seo.description and seo.keywords for the "Business Debit" page; update the seo.title, seo.description and seo.keywords fields for the German (de) and Polish (pl) locale entries with proper localized strings (not the English copy) for the Business Debit page, ensuring the seo.title and seo.description are translated and the seo.keywords array contains relevant localized terms; keep the English values only as a fallback if a localized entry is missing and apply the same change for the other duplicate block that mirrors this page (the second Business Debit locale object).packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.header.mapper.ts-201-204 (1)
201-204:⚠️ Potential issue | 🟠 MajorFix Polish business cards route slug typo.
Line [203] uses
/firma/karten, which is inconsistent with the Polish label (Karty) and likely points to a non-existent route.🔧 Proposed fix
- url: '/firma/karten', + url: '/firma/karty',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.header.mapper.ts` around lines 201 - 204, There is a typo in the mock NavigationItem where label 'Karty' is paired with url '/firma/karten'; update the url to the correct Polish slug '/firma/karty' in the cms.header.mapper.ts NavigationItem entry so the label and route match (search for the NavigationItem object with label 'Karty' to locate the change).packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.app-config.mapper.ts-101-104 (1)
101-104:⚠️ Potential issue | 🟠 MajorFix swapped date translations between DE and PL configs.
The
dates.today/dates.yesterdayvalues are reversed acrossAPP_CONFIG_DEandAPP_CONFIG_PL.💡 Proposed fix
const APP_CONFIG_DE: CMS.Model.AppConfig.AppConfig = { @@ dates: { - today: 'Dzisiaj', - yesterday: 'Wczoraj', + today: 'Heute', + yesterday: 'Gestern', }, @@ const APP_CONFIG_PL: CMS.Model.AppConfig.AppConfig = { @@ dates: { - today: 'Heute', - yesterday: 'Gestern', + today: 'Dzisiaj', + yesterday: 'Wczoraj', },Also applies to: 175-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.app-config.mapper.ts` around lines 101 - 104, The Polish and German date translations are swapped: locate APP_CONFIG_PL and APP_CONFIG_DE in cms.app-config.mapper.ts and ensure the dates object for APP_CONFIG_PL has today: 'Dzisiaj' and yesterday: 'Wczoraj', while APP_CONFIG_DE has today: 'Heute' and yesterday: 'Gestern'; update both occurrences (also the second pair around the other block mentioned) so the keys dates.today and dates.yesterday map to the correct language strings.packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.pricing-section.mapper.ts-1654-1662 (1)
1654-1662:⚠️ Potential issue | 🟠 MajorHandle missing pricing block IDs explicitly.
If
idis not present in the selected locale list, this returnsundefinedinstead of failing fast withNotFoundException.💡 Proposed fix
export const mapPricingSectionBlock = ({ locale, id, }: CMS.Request.GetCmsEntryParams): CMS.Model.PricingSectionBlock.PricingSectionBlock => { - switch (locale) { - case 'de': - return MOCK_PRICING_SECTION_BLOCKS_DE.find((block) => block.id === id)!; - case 'pl': - return MOCK_PRICING_SECTION_BLOCKS_PL.find((block) => block.id === id)!; - case 'en': - return MOCK_PRICING_SECTION_BLOCKS_EN.find((block) => block.id === id)!; - default: - throw new NotFoundException(); - } + const blocksByLocale = { + en: MOCK_PRICING_SECTION_BLOCKS_EN, + de: MOCK_PRICING_SECTION_BLOCKS_DE, + pl: MOCK_PRICING_SECTION_BLOCKS_PL, + } as const; + + const blocks = blocksByLocale[locale as keyof typeof blocksByLocale]; + if (!blocks) throw new NotFoundException(`Unsupported locale: ${locale}`); + + const block = blocks.find((item) => item.id === id); + if (!block) throw new NotFoundException(`PricingSectionBlock not found: ${id} (${locale})`); + + return block; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.pricing-section.mapper.ts` around lines 1654 - 1662, The switch returns the result of .find(...) with a non-null assertion, which can still yield undefined; change each case to assign the found block from MOCK_PRICING_SECTION_BLOCKS_DE / MOCK_PRICING_SECTION_BLOCKS_PL / MOCK_PRICING_SECTION_BLOCKS_EN to a local variable, then if the variable is falsy throw NotFoundException(), otherwise return it—remove the "!" assertions and add an explicit existence check after each find (or perform the check once after selecting the array).packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.feature-section-grid.mapper.ts-146-154 (1)
146-154:⚠️ Potential issue | 🟠 MajorThrow when ID is missing for a supported locale.
For a valid locale and unknown
id, this mapper currently returnsundefinedinstead of aNotFoundException. That creates unpredictable downstream failures.💡 Proposed fix
export const mapFeatureSectionGridBlock = ({ locale, id, }: CMS.Request.GetCmsEntryParams): CMS.Model.FeatureSectionGridBlock.FeatureSectionGridBlock => { - switch (locale) { - case 'de': - return MOCK_FEATURE_SECTION_GRID_BLOCKS_DE.find((block) => block.id === id)!; - case 'pl': - return MOCK_FEATURE_SECTION_GRID_BLOCKS_PL.find((block) => block.id === id)!; - case 'en': - return MOCK_FEATURE_SECTION_GRID_BLOCKS_EN.find((block) => block.id === id)!; - default: - throw new NotFoundException(); - } + const blocksByLocale = { + en: MOCK_FEATURE_SECTION_GRID_BLOCKS_EN, + de: MOCK_FEATURE_SECTION_GRID_BLOCKS_DE, + pl: MOCK_FEATURE_SECTION_GRID_BLOCKS_PL, + } as const; + + const blocks = blocksByLocale[locale as keyof typeof blocksByLocale]; + if (!blocks) throw new NotFoundException(`Unsupported locale: ${locale}`); + + const block = blocks.find((item) => item.id === id); + if (!block) throw new NotFoundException(`FeatureSectionGridBlock not found: ${id} (${locale})`); + + return block; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.feature-section-grid.mapper.ts` around lines 146 - 154, The switch on locale returns the result of .find(...) which can be undefined for a supported locale; change the logic in the locale switch (for 'de', 'pl', 'en') to locate the block from MOCK_FEATURE_SECTION_GRID_BLOCKS_DE / MOCK_FEATURE_SECTION_GRID_BLOCKS_PL / MOCK_FEATURE_SECTION_GRID_BLOCKS_EN respectively, and if the .find(...) result is falsy throw a NotFoundException (instead of returning undefined) so unknown ids for valid locales produce a proper NotFoundException.packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.media-section.mapper.ts-216-224 (1)
216-224:⚠️ Potential issue | 🟠 MajorGuard missing media block IDs with
NotFoundException.For supported locales, an unknown
idcurrently returnsundefinedinstead of a controlled 404-style error.💡 Proposed fix
export const mapMediaSectionBlock = ({ locale, id, }: CMS.Request.GetCmsEntryParams): CMS.Model.MediaSectionBlock.MediaSectionBlock => { - switch (locale) { - case 'de': - return MEDIA_SECTION_BLOCKS_DE.find((block) => block.id === id)!; - case 'pl': - return MEDIA_SECTION_BLOCKS_PL.find((block) => block.id === id)!; - case 'en': - return MEDIA_SECTION_BLOCKS_EN.find((block) => block.id === id)!; - default: - throw new NotFoundException(); - } + const blocksByLocale = { + en: MEDIA_SECTION_BLOCKS_EN, + de: MEDIA_SECTION_BLOCKS_DE, + pl: MEDIA_SECTION_BLOCKS_PL, + } as const; + + const blocks = blocksByLocale[locale as keyof typeof blocksByLocale]; + if (!blocks) throw new NotFoundException(`Unsupported locale: ${locale}`); + + const block = blocks.find((item) => item.id === id); + if (!block) throw new NotFoundException(`MediaSectionBlock not found: ${id} (${locale})`); + + return block; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.media-section.mapper.ts` around lines 216 - 224, The switch currently returns MEDIA_SECTION_BLOCKS_DE/PL/EN.find(...) with a non-null assertion, which returns undefined for unknown ids; update the code in the cms.media-section.mapper switch so each case captures the result of MEDIA_SECTION_BLOCKS_DE/PL/EN.find((block) => block.id === id) into a local variable (e.g., found) and if not found throw a NotFoundException; ensure the default still throws NotFoundException and remove reliance on the non-null assertion to provide a proper 404 for unknown media block ids.packages/cli/create-o2s-app/src/constants.ts-3-4 (1)
3-4:⚠️ Potential issue | 🟠 MajorUpdate
GITHUB_BRANCHbefore merging.The branch is currently set to the feature branch. This should be updated to
main(or the appropriate default branch) before merging to ensure the CLI clones from the correct branch in production.🔧 Proposed fix
export const GITHUB_REPO_URL = 'https://github.com/o2sdev/openselfservice'; -export const GITHUB_BRANCH = 'feature/Interactive-CLI-wizard-for-project-bootstrapping'; +export const GITHUB_BRANCH = 'main';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/constants.ts` around lines 3 - 4, The GITHUB_BRANCH constant is pointing to a feature branch; update the exported constant GITHUB_BRANCH to the repository's default branch (e.g., 'main') so production runs clone the correct branch; locate the GITHUB_BRANCH constant in constants.ts and replace 'feature/Interactive-CLI-wizard-for-project-bootstrapping' with 'main' (or the appropriate default branch name).packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.feature-section.mapper.ts-2058-2072 (1)
2058-2072:⚠️ Potential issue | 🟠 Major
mapFeatureSectionBlockcan returnundefineddespite typed return.The
.find()method returnsundefinedwhen no block matches the givenid, but the non-null assertion (!) suppresses the TypeScript error while still returningundefinedat runtime. This is inconsistent with theNotFoundExceptionthrown for unsupported locales and may cause runtime errors in callers.🐛 Proposed fix to throw when block is not found
export const mapFeatureSectionBlock = ({ locale, id, }: CMS.Request.GetCmsEntryParams): CMS.Model.FeatureSectionBlock.FeatureSectionBlock => { + let blocks: CMS.Model.FeatureSectionBlock.FeatureSectionBlock[]; switch (locale) { case 'de': - return FEATURE_SECTION_BLOCKS_DE.find((block) => block.id === id)!; + blocks = FEATURE_SECTION_BLOCKS_DE; + break; case 'pl': - return FEATURE_SECTION_BLOCKS_PL.find((block) => block.id === id)!; + blocks = FEATURE_SECTION_BLOCKS_PL; + break; case 'en': - return FEATURE_SECTION_BLOCKS_EN.find((block) => block.id === id)!; + blocks = FEATURE_SECTION_BLOCKS_EN; + break; default: throw new NotFoundException(); } + const block = blocks.find((b) => b.id === id); + if (!block) { + throw new NotFoundException(`Feature section block with id '${id}' not found`); + } + return block; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/blocks/cms.feature-section.mapper.ts` around lines 2058 - 2072, mapFeatureSectionBlock uses .find(...) with a non-null assertion which can still return undefined at runtime; instead, find the block from FEATURE_SECTION_BLOCKS_DE / FEATURE_SECTION_BLOCKS_PL / FEATURE_SECTION_BLOCKS_EN inside mapFeatureSectionBlock and if the result is undefined throw NotFoundException (or a specific not-found error) so the function never returns undefined; update the switch cases in mapFeatureSectionBlock to assign the found block to a local variable and explicitly throw NotFoundException when the block is not found.packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.page.mapper.ts-394-399 (1)
394-399:⚠️ Potential issue | 🟠 MajorRegex pattern used as literal string in
replace.The pattern
'(.+)'is treated as a literal string, not a regex. Thereplacecall will look for the literal characters(.+)which won't exist in the slug, so the replacement never occurs. If the intent is to replace the last path segment, use a proper regex.🐛 Proposed fix
.map((page) => { return { ...page, - slug: page.slug.replace('(.+)', slug.match(/(.+)\/(.+)/)?.[2] || ''), + slug: page.slug.replace(/\/[^/]+$/, `/${slug.match(/\/([^/]+)$/)?.[1] || ''}`), }; });Note: The intent of this slug manipulation is unclear. Please verify the expected behavior and adjust the regex accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.page.mapper.ts` around lines 394 - 399, The code is using a string literal '(.+)' in page.slug.replace which does not act as a regex; update the replacement to use a proper regex for the intended behavior: in the map callback that builds slug (the anonymous function using page.slug.replace and slug.match(/(.+)\/(.+)/)), replace the literal with a regex that targets the last path segment (for example use page.slug.replace(/[^/]+$/, slug.match(/(.+)\/(.+)/)?.[2] || '') ) so the last segment is actually replaced with the captured value from slug.match.packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.page.mapper.ts-391-393 (1)
391-393:⚠️ Potential issue | 🟠 MajorNon-null assertion on potentially undefined result.
mapPage(page.slug, locale)can returnundefinedfor unhandled slugs, but the non-null assertion (!) will allowundefinedto pass through, causing errors in the subsequent.map()when accessingpage.slug.🐛 Proposed fix to filter out undefined results
.filter((page) => page.id === id) - .map((page) => { - return mapPage(page.slug, locale)!; - }) + .map((page) => mapPage(page.slug, locale)) + .filter((page): page is CMS.Model.Page.Page => page !== undefined) .map((page) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/mappers/cms.page.mapper.ts` around lines 391 - 393, The current mapping uses mapPage(page.slug, locale)! which force-asserts a non-null result; instead ensure undefined pages are filtered out before downstream use by replacing the forced non-null mapping with a safe map+filter (or flatMap) that calls mapPage(page.slug, locale) and then filters out undefined results; update the expression that currently contains mapPage(page.slug, locale)! to a version that only returns defined pages (e.g. map to the result and then .filter((p): p is NonNullable<typeof p> => p != null) or use flatMap to emit only defined values) so subsequent code never receives undefined values.packages/cli/create-o2s-app/src/constants.ts-31-39 (1)
31-39:⚠️ Potential issue | 🟠 MajorAdd
mockedintegration toINTEGRATION_MODULES.The codebase includes both
mockedandmocked-dxpintegrations. Currently, onlymocked-dxpis mapped inINTEGRATION_MODULES, but the plainmockedintegration provides a comprehensive set of modules (articles, cache, carts, checkout, cms, customers, orders, payments, products, resources, search, tickets) and should also be included in the mapping for consistency.'mocked': ['articles', 'cache', 'carts', 'checkout', 'cms', 'customers', 'orders', 'payments', 'products', 'resources', 'search', 'tickets'],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/constants.ts` around lines 31 - 39, INTEGRATION_MODULES currently maps 'mocked-dxp' but misses the plain 'mocked' integration; update the INTEGRATION_MODULES constant to add a 'mocked' key with the full list of modules (articles, cache, carts, checkout, cms, customers, orders, payments, products, resources, search, tickets) so both mocked and mocked-dxp are represented, by editing the exported INTEGRATION_MODULES object in packages/cli/create-o2s-app/src/constants.ts where INTEGRATION_MODULES is declared.packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts-71-73 (1)
71-73:⚠️ Potential issue | 🟠 MajorAvoid early-returning before cleanup/reporting.
The return on Line 71-73 skips Line 92 dependency cleanup and Line 97-107 uncovered-module detection. This can silently report success when no module mappings exist.
Proposed fix
- if (moduleMap.size === 0) { - return []; - } - const modelsDir = path.join(projectDir, CONFIGS_MODELS_PATH); @@ - for (const [module, integration] of moduleMap.entries()) { - const filePath = path.join(modelsDir, `${module}.ts`); - - if (!(await fs.pathExists(filePath))) continue; - - const content = await fs.readFile(filePath, 'utf-8'); - const updatedContent = content.replace(MOCKED_IMPORT, `@o2s/integrations.${integration}/integration`); - - await fs.writeFile(filePath, updatedContent, 'utf-8'); - } + if (moduleMap.size > 0) { + for (const [module, integration] of moduleMap.entries()) { + const filePath = path.join(modelsDir, `${module}.ts`); + if (!(await fs.pathExists(filePath))) continue; + + const content = await fs.readFile(filePath, 'utf-8'); + const updatedContent = content.replace(MOCKED_IMPORT, `@o2s/integrations.${integration}/integration`); + await fs.writeFile(filePath, updatedContent, 'utf-8'); + } + }Also applies to: 92-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts` around lines 71 - 73, The early return when moduleMap.size === 0 in transformIntegrationConfigs short-circuits the later dependency cleanup and uncovered-module detection logic; remove the immediate return and instead let execution fall through so the existing dependency cleanup block and the uncovered-module detection/reporting block still run (i.e., keep moduleMap check but skip processing modules while still invoking the dependency cleanup routine and the uncovered-module detection/reporting code paths in transformIntegrationConfigs).packages/cli/create-o2s-app/src/wizard/env-prompts.ts-67-76 (1)
67-76:⚠️ Potential issue | 🟠 MajorRequired env vars are currently not enforced.
Line 67 marks fields as required, but Line 75 still stores empty values when user presses Enter. Add validation for required variables at prompt time.
Proposed fix
const { value } = await prompts({ type: 'text', name: 'value', message: `${envVar.key}${requiredLabel}`, hint: envVar.description, + validate: (input: string) => + envVar.required && !input?.trim() ? `${envVar.key} is required` : true, }); - envVars[envVar.key] = value ?? ''; + envVars[envVar.key] = (value ?? '').trim();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/wizard/env-prompts.ts` around lines 67 - 76, The prompt currently accepts empty input even for required env vars; update the prompts call that builds the env var input (the prompts invocation using type: 'text', name: 'value', message: `${envVar.key}${requiredLabel}`, hint: envVar.description) to enforce validation when envVar.required is true by providing a validate function (or loop until non-empty) so that empty strings are rejected and the user is re-prompted; ensure envVars[envVar.key] only receives a value after passing validation and keep the existing fallback for optional vars.packages/cli/create-o2s-app/src/wizard/index.ts-19-27 (1)
19-27:⚠️ Potential issue | 🟠 MajorFull CLI mode bypasses conflict handling for
customtemplate.Line 19-27 returns immediately with empty
conflictResolutions. Forcustomselections with overlapping modules, this silently skips explicit resolution and can produce unintended mappings.Proposed fix
if (cliName && cliTemplate && cliBlocks && cliIntegrations) { + if (cliTemplate === 'custom') { + const conflicts = detectConflicts(cliIntegrations); + if (conflicts.length > 0) { + throw new Error( + `Conflicting integrations in non-interactive mode: ${conflicts + .map((c) => `${c.module}(${c.integrations.join(',')})`) + .join(', ')}. Please resolve conflicts interactively.`, + ); + } + } return { projectName: cliName, template: cliTemplate, selectedBlocks: cliBlocks, selectedIntegrations: cliIntegrations,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/wizard/index.ts` around lines 19 - 27, The early-return when cliName && cliTemplate && cliBlocks && cliIntegrations returns an object with conflictResolutions: [] which bypasses conflict handling for the `custom` template; update the logic in this block so that when cliTemplate === 'custom' you do not return conflictResolutions empty but instead invoke the existing conflict resolution routine (e.g., the function that computes/asks for conflicts — locate and call the module conflict resolver used elsewhere such as resolveConflicts or getConflictResolutions) with cliBlocks and cliIntegrations to populate conflictResolutions before returning; keep the immediate-return behavior for non-custom templates but ensure `cliTemplate`, `cliBlocks`, `cliIntegrations`, and `conflictResolutions` are handled accordingly.packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts-44-45 (1)
44-45:⚠️ Potential issue | 🟠 MajorGuard
pkg.dependenciesbefore iterating and assigning.Line 44 accesses
pkg.dependenciesviaObject.keys()without checking if it exists. If apackage.jsonlacks adependenciesfield,Object.keys(undefined)will throw aTypeError. Lines 49 and 57 have the same unguarded vulnerability.The TypeScript cast
as Record<string, string>at line 44 shows awareness of potentialundefined, but no runtime guard exists.Proposed fix
- const pkg = await fs.readJson(pkgPath); + const pkg = await fs.readJson(pkgPath); + const dependencies: Record<string, string> = { ...(pkg.dependencies ?? {}) }; @@ - for (const key of Object.keys(pkg.dependencies as Record<string, string>)) { + for (const key of Object.keys(dependencies)) { @@ - delete pkg.dependencies[key]; + delete dependencies[key]; @@ - pkg.dependencies[`@o2s/integrations.${integration}`] = '*'; + dependencies[`@o2s/integrations.${integration}`] = '*'; } } + pkg.dependencies = dependencies;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts` around lines 44 - 45, Guard access to pkg.dependencies before calling Object.keys and before iterating/assigning: check that pkg.dependencies is a truthy object (e.g., if (pkg.dependencies)) prior to the for (const key of Object.keys(pkg.dependencies as Record<string,string>)) loops and any subsequent use at the other two spots; if absent, skip the loop or treat as an empty object so the code referencing dependency keys (the loops that check key.startsWith('@o2s/integrations.')) and later assignments do not run on undefined. Ensure the same guard is applied at the two other unguarded usages to avoid TypeError at runtime.packages/integrations/mocked-dxp/src/modules/cms/cms.service.ts-31-37 (1)
31-37:⚠️ Potential issue | 🟠 MajorAvoid returning fake typed payloads from generic CMS methods.
of({} as T)can violate callers’ expected shapes (arrays/paginated models) and fail later at runtime. Prefer fail-fast until proper mapping is implemented.🛠️ Safer interim behavior
override getEntry<T>(_options: CMS.Request.GetCmsEntryParams): Observable<T> { - return of<T>({} as T); + throw new Error('CmsService.getEntry is not implemented for mocked-dxp'); } override getEntries<T>(_options: CMS.Request.GetCmsEntriesParams): Observable<T> { - return of<T>({} as T); + throw new Error('CmsService.getEntries is not implemented for mocked-dxp'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/cms/cms.service.ts` around lines 31 - 37, The current overrides getEntry<T> and getEntries<T> return unsafe fake payloads via of<T>({} as T); replace these with fail-fast Observables so callers don’t receive malformed data — e.g., return throwError(() => new Error('MockedCmsService.getEntry not implemented')) for getEntry and throwError(() => new Error('MockedCmsService.getEntries not implemented')) for getEntries (include the method name in the message); use RxJS throwError(() => ...) so the failure is observable-compatible and update imports accordingly.packages/cli/create-o2s-app/src/scaffold/transform-page-model.ts-7-18 (1)
7-18:⚠️ Potential issue | 🟠 MajorRegex parsing is brittle with CRLF line endings.
Using
split('\n')plus$-anchored regex can miss import matches when lines contain trailing\r, causing no block removals on some environments.🛠️ Proposed fix
-const BLOCK_IMPORT_REGEX = /^import \* as (\w+) from '@o2s\/blocks\.([^/]+)\/api-harmonization';$/; +const BLOCK_IMPORT_REGEX = /^import \* as (\w+) from '@o2s\/blocks\.([^/]+)\/api-harmonization';\r?$/; @@ -const UNION_TYPE_REGEX = /^\s+\| (\w+)\.Model\.\w+\['__typename'\]/; +const UNION_TYPE_REGEX = /^\s+\| (\w+)\.Model\.\w+\['__typename'\];?\r?$/; @@ - const lines = content.split('\n'); + const lines = content.split(/\r?\n/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/transform-page-model.ts` around lines 7 - 18, transformPageModel's line parsing is brittle on CRLF because content is split with '\n' and regexes BLOCK_IMPORT_REGEX and UNION_TYPE_REGEX are anchored and can fail if lines end with '\r'; normalize newlines before splitting (or split with a CRLF-aware pattern) or trim trailing '\r' from each line so the regex matches consistently, then proceed to use the normalized lines variable for import and union matching/removal.packages/cli/create-o2s-app/src/wizard/prompts.ts-68-83 (1)
68-83:⚠️ Potential issue | 🟠 MajorDon’t collapse missing integration answers into a valid empty selection.
selectedIntegrations ?? []masks incomplete prompt outcomes and lets the wizard continue with partial data, unlike the fail-fast behavior in other prompts.🛠️ Proposed fix
export const promptIntegrationSelection = async ( @@ - return selectedIntegrations ?? []; + if (selectedIntegrations === undefined) { + throw new Error('Integration selection was not completed'); + } + + return selectedIntegrations; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/wizard/prompts.ts` around lines 68 - 83, The code currently collapses a missing prompt result into an empty selection by returning selectedIntegrations ?? []; update the behavior to fail-fast instead: detect when the prompts call returns undefined for selectedIntegrations (i.e., the user aborted or the prompt failed) and surface that by throwing or returning an error rather than converting to []; modify the function around the prompts(...) call (referencing selectedIntegrations and the prompts multiselect) to explicitly check for selectedIntegrations === undefined and handle it consistently with other prompts (throw an Error or propagate the undefined) so the wizard stops instead of continuing with a masked empty array.packages/cli/create-o2s-app/src/scaffold/clone.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟠 MajorGITHUB_BRANCH must be updated to a stable branch before merge.
The
GITHUB_BRANCHconstant is currently set to'feature/Interactive-CLI-wizard-for-project-bootstrapping'. After this PR merges, the feature branch will be deleted, causing clone operations to fail. Update the constant to'main'or an appropriate stable branch/release tag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/clone.ts` at line 1, Update the GITHUB_BRANCH constant from the feature branch name to a stable branch or tag (e.g., 'main') so clone operations won't break after the feature branch is deleted; locate the GITHUB_BRANCH definition referenced by the import in clone.ts (symbol: GITHUB_BRANCH) and change its value from 'feature/Interactive-CLI-wizard-for-project-bootstrapping' to 'main' or the appropriate stable release tag.packages/cli/create-o2s-app/src/index.ts-16-16 (1)
16-16:⚠️ Potential issue | 🟠 Major
--directoryis declared but never applied.The option is exposed to users, but scaffolding still uses the wizard/project name path. This makes
--directorymisleading and non-functional.💡 Proposed fix
try { const cliTemplate = parseTemplate(options.template); const cliBlocks = parseCommaSeparated(options.blocks); const cliIntegrations = parseCommaSeparated(options.integrations); + const cliProjectName = + typeof options.directory === 'string' && options.directory.trim().length > 0 + ? options.directory.trim() + : name; @@ - const answers = await runWizard(tempDir, name, cliTemplate, cliBlocks, cliIntegrations); + const answers = await runWizard(tempDir, cliProjectName, cliTemplate, cliBlocks, cliIntegrations);Also applies to: 35-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/index.ts` at line 16, The declared CLI option --directory is never read or applied; update the command setup to read the parsed value (e.g., via program.opts().directory or the options object returned by commander) and use it when computing the output path instead of always using the wizard/project name path (replace occurrences that build the destination from projectName or process.cwd(), e.g., path.join(process.cwd(), projectName) or similar). If the user passes --directory treat it as the destination root (or join it with projectName if that was the intended behavior), normalize with path.resolve, and pass that resulting destination to the scaffolding function(s) that create/write files. Also apply the same read-and-use pattern to the other related options referenced around lines 35-38 so each option is actually consumed.packages/integrations/mocked-dxp/src/modules/articles/articles.mapper.ts-33-40 (1)
33-40:⚠️ Potential issue | 🟠 MajorGuard dynamic sort fields before
localeCompare.
sort.fieldis unconstrained at the type level and lacks upstream validation. If it resolves to a non-string value or an omitted/inherited property,localeComparewill throw at runtime. The current truthy check is insufficient; add an allowlist of sortable fields and verify the value is a string before comparing.💡 Proposed hardening
+const SORTABLE_CATEGORY_FIELDS = ['id', 'title'] as const; + export const mapCategories = ( locale: string, options: Articles.Request.GetCategoryListQuery, ): Articles.Model.Categories => { @@ const sortedCategories = [...categories]; const sort = options.sort; - if (sort) { + if (sort && SORTABLE_CATEGORY_FIELDS.includes(sort.field as (typeof SORTABLE_CATEGORY_FIELDS)[number])) { + const field = sort.field as (typeof SORTABLE_CATEGORY_FIELDS)[number]; sortedCategories.sort((a, b) => { - const fieldA = a[sort.field as keyof Omit<Articles.Model.Category, 'icon' | 'parent'>]; - const fieldB = b[sort.field as keyof Omit<Articles.Model.Category, 'icon' | 'parent'>]; + const fieldA = a[field]; + const fieldB = b[field]; - if (fieldA && fieldB) { + if (typeof fieldA === 'string' && typeof fieldB === 'string') { return sort.order === 'asc' ? fieldA.localeCompare(fieldB) : fieldB.localeCompare(fieldA); } return 0; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integrations/mocked-dxp/src/modules/articles/articles.mapper.ts` around lines 33 - 40, The comparator in sortedCategories.sort uses an unconstrained sort.field and calls localeCompare directly; harden it by defining an allowlist of sortable keys (e.g., ['name', 'slug', 'createdAt'] or other string fields from Articles.Model.Category) and, inside the comparator for function sortedCategories.sort, verify that sort.field is in that allowlist and that both fieldA and fieldB are strings (typeof === 'string') before calling localeCompare; if the checks fail, fall back to returning 0 or another deterministic comparison to avoid runtime exceptions.
…'o2s' instead of 'ssp'
There was a problem hiding this comment.
🧹 Nitpick comments (8)
packages/cli/create-o2s-app/src/utils/logger.ts (2)
6-13: Consider using an options object for better maintainability.The function has 6 parameters, with 2 optional ones. An options object would improve readability at call sites and make future extensions easier.
♻️ Proposed refactor with options object
+interface PrintSummaryOptions { + targetDir: string; + template: TemplateType; + blockCount: number; + integrationCount: number; + uncoveredModules?: string[]; + skipInstall?: boolean; +} + -export const printSummary = ( - targetDir: string, - template: TemplateType, - blockCount: number, - integrationCount: number, - uncoveredModules: string[] = [], - skipInstall = false, -): void => { +export const printSummary = ({ + targetDir, + template, + blockCount, + integrationCount, + uncoveredModules = [], + skipInstall = false, +}: PrintSummaryOptions): void => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/utils/logger.ts` around lines 6 - 13, The printSummary function signature currently takes six positional parameters (targetDir, template: TemplateType, blockCount, integrationCount, uncoveredModules = [], skipInstall = false), which hurts readability and extensibility; refactor it to accept a single options object (e.g., printSummary(options: { targetDir: string; template: TemplateType; blockCount: number; integrationCount: number; uncoveredModules?: string[]; skipInstall?: boolean })) and update all call sites to pass a named object, preserving default values for uncoveredModules and skipInstall and keeping the function body behavior unchanged.
32-34: Hardcoded path may become stale.The path
packages/configs/integrations/src/models/<module>.tsis hardcoded. If the project structure changes, this message will become misleading. Consider extracting this to a constant or deriving it dynamically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/utils/logger.ts` around lines 32 - 34, The three console.log lines in logger.ts currently print a hardcoded path ("packages/configs/integrations/src/models/<module>.ts"); replace that literal with a derived or constant value (e.g., compute the integrations models path from process.cwd() or a shared constant like INTEGRATIONS_MODELS_PATH) and update the message strings that reference it (the console.log calls shown in the diff). Locate the console.log occurrences in packages/cli/create-o2s-app/src/utils/logger.ts and refactor to use the derived constant/path-building (using path.join or a project-config helper) so the message remains correct if the repo layout changes. Ensure the text still instructs editing "<module>.ts" but uses the new variable instead of the hardcoded string.packages/cli/create-o2s-app/src/scaffold/install.ts (2)
20-24: Consider adding a timeout to prevent indefinite hangs.The
execSynccall has no timeout, which could cause the CLI to hang indefinitely on slow networks or if npm encounters issues. Consider adding a reasonable timeout.⏱️ Proposed fix to add timeout
execSync('npm install', { cwd: projectDir, stdio: 'inherit', + timeout: 600000, // 10 minutes });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/install.ts` around lines 20 - 24, The execSync('npm install', { cwd: projectDir, stdio: 'inherit' }) call can hang indefinitely; add a timeout option (e.g. timeout: 300000 for 5 minutes or a configurable value) to the options object passed to execSync in install.ts so the child will be killed after the timeout; update the options to include timeout and ensure any thrown timeout error is propagated or handled consistently with existing error handling around the execSync invocation.
16-18: Consider using the logger utility for consistent output.The file uses direct
console.log/console.errorcalls while the codebase has a dedicated logger utility (packages/cli/create-o2s-app/src/utils/logger.ts). For consistency across the CLI, consider using the logger or at minimumkleurfor colored output to match other modules.Also applies to: 25-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/install.ts` around lines 16 - 18, Replace the direct console.log/console.error calls in install.ts with the project's logger utility: import the logger from packages/cli/create-o2s-app/src/utils/logger.ts and use logger.info(...) for regular messages (e.g., the "Installing dependencies..." and surrounding blank outputs) and logger.error(...) for errors; if you prefer colored output without the logger, use the project's kleur usage pattern instead. Locate the console calls in the install flow (the console.log entries shown and the console.* calls around lines 25–28) and switch them to the appropriate logger methods, ensuring imports are added and any blank-line prints are reproduced via logger.info('') or equivalent. Ensure consistent message text and severity levels when replacing console methods.packages/cli/create-o2s-app/src/index.ts (2)
55-58: Fragile error detection for user cancellation.Detecting user cancellation by checking if the error message includes
'is required'is fragile—it depends on the exact wording of prompt library errors and could match unrelated errors. Consider using a more explicit cancellation signal or checking for a specific error type/code.🔧 Proposed improvement
Consider creating a custom
UserCancellationErrorclass or checking for the specific error type from the prompts library:// Option 1: Check for specific prompt library error if (error instanceof Error && error.name === 'ExitPromptError') { console.log(); console.log('Setup cancelled.'); } // Option 2: Use a custom error class thrown from wizard class UserCancellationError extends Error { constructor() { super('User cancelled setup'); this.name = 'UserCancellationError'; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/index.ts` around lines 55 - 58, Replace the fragile message substring check when handling prompt cancellation in the error handler with an explicit cancellation signal: detect the prompt library's specific error (e.g., check error.name === 'ExitPromptError' or the library-specific class) or define and throw a custom UserCancellationError from the wizard code and check for error instanceof UserCancellationError in the catch block where the current code uses error.message.includes('is required'); then log the same cancellation messages (console.log(); console.log('Setup cancelled.')) for that explicit case and let other errors fall through to normal error handling.
67-67: Move helper functions beforeprogram.parse()to avoid potential issues.
parseTemplateandparseCommaSeparatedare defined afterprogram.parse()is called. While Commander's action handler is an async callback that may execute after the module finishes parsing, placing these utility functions after the code that registers them as dependencies is unconventional and could cause confusion. For clarity and to follow standard patterns, define these helpers before they're referenced.♻️ Proposed reorganization
Move the helper functions to before the
programdefinition:import { Command } from 'commander'; +const parseTemplate = (value: unknown): TemplateType | undefined => { + if (!value || typeof value !== 'string') return undefined; + const valid: TemplateType[] = ['o2s', 'dxp', 'custom']; + if (valid.includes(value as TemplateType)) { + return value as TemplateType; + } + console.warn(`Warning: Unknown template "${value}". Valid options: o2s, dxp, custom`); + return undefined; +}; + +const parseCommaSeparated = (value: unknown): string[] | undefined => { + if (!value || typeof value !== 'string') return undefined; + return value + .split(',') + .map((s) => s.trim()) + .filter((s) => s.length > 0); +}; + const program = new Command(); // ... rest of the code program.parse(process.argv); - -const parseTemplate = ... -const parseCommaSeparated = ...Also applies to: 69-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/index.ts` at line 67, Move the helper functions parseTemplate and parseCommaSeparated so they are declared before they are referenced by the Commander setup and before program.parse(process.argv); — locate the parseTemplate and parseCommaSeparated function definitions in this file and cut/paste them above the program definition/option setup (or at least above program.parse), ensuring the action handlers reference already-declared functions and no other code changes are required.packages/cli/create-o2s-app/src/wizard/templates.ts (1)
34-61: Consider extracting shared filtering logic.
filterBlocksByTemplateandfilterIntegrationsByTemplatehave identical implementation logic, differing only in the input type. A generic helper could reduce duplication.♻️ Optional: Extract shared filter logic
+type Categorizable = { name: string; category: string[] }; + +const filterByTemplate = <T extends Categorizable>( + template: TemplateType, + items: T[], +): string[] => { + const templateDef = TEMPLATES[template]; + const { category } = templateDef; + if (!category) return []; + return items.filter((item) => item.category.includes(category)).map((item) => item.name); +}; + // Filter discovered blocks by template category -export const filterBlocksByTemplate = (template: TemplateType, allBlocks: BlockInfo[]): string[] => { - const templateDef = TEMPLATES[template]; - - const { category } = templateDef; - - if (!category) { - return []; // Custom template - user selects manually - } - - // Include blocks that declare support for this template category - return allBlocks.filter((block) => block.category.includes(category)).map((block) => block.name); -}; +export const filterBlocksByTemplate = (template: TemplateType, allBlocks: BlockInfo[]): string[] => + filterByTemplate(template, allBlocks); // Filter discovered integrations by template category -// For O2S/DXP demo templates, only mocked integrations are pre-selected. -export const filterIntegrationsByTemplate = (template: TemplateType, allIntegrations: IntegrationInfo[]): string[] => { - const templateDef = TEMPLATES[template]; - - const { category } = templateDef; - - if (!category) { - return []; // Custom template - user selects manually - } - - return allIntegrations - .filter((integration) => integration.category.includes(category)) - .map((integration) => integration.name); -}; +export const filterIntegrationsByTemplate = (template: TemplateType, allIntegrations: IntegrationInfo[]): string[] => + filterByTemplate(template, allIntegrations);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/wizard/templates.ts` around lines 34 - 61, Both functions filterBlocksByTemplate and filterIntegrationsByTemplate duplicate the same logic; extract a single generic helper (e.g., filterByTemplate<T extends { category?: string[]; name: string }>) that takes (template: TemplateType, items: T[]) and returns string[] by looking up TEMPLATES[template].category, returning [] if no category, otherwise filtering items whose category includes that category and mapping to name; then implement filterBlocksByTemplate and filterIntegrationsByTemplate as thin wrappers that call this helper with BlockInfo[] and IntegrationInfo[] respectively (keep references to TemplateType, TEMPLATES, category and name).packages/cli/create-o2s-app/src/scaffold/index.ts (1)
56-59: RedundantremovePackageLockcall.
removePackageLock(targetDir)is called here on line 57, butinstallDependencies(targetDir)(line 59) also callsremovePackageLockinternally at the start of its execution. This results in checking/removing the lock file twice whenskipInstallis false.♻️ Proposed fix to remove redundancy
// Step 6: Remove stale package-lock.json (always) and install dependencies - await removePackageLock(targetDir); if (!skipInstall) { await installDependencies(targetDir); + } else { + // Only remove lock file when skipping install + await removePackageLock(targetDir); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/index.ts` around lines 56 - 59, The code currently always calls removePackageLock(targetDir) and then calls installDependencies(targetDir) which itself calls removePackageLock, causing duplicate work; change the logic so you only call removePackageLock when skipping installation and rely on installDependencies to remove the lock when running installs: replace the unconditional removePackageLock call with a conditional branch that calls removePackageLock(targetDir) only when skipInstall is true, otherwise call installDependencies(targetDir); reference the symbols removePackageLock and installDependencies in index.ts to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/cli/create-o2s-app/src/index.ts`:
- Around line 55-58: Replace the fragile message substring check when handling
prompt cancellation in the error handler with an explicit cancellation signal:
detect the prompt library's specific error (e.g., check error.name ===
'ExitPromptError' or the library-specific class) or define and throw a custom
UserCancellationError from the wizard code and check for error instanceof
UserCancellationError in the catch block where the current code uses
error.message.includes('is required'); then log the same cancellation messages
(console.log(); console.log('Setup cancelled.')) for that explicit case and let
other errors fall through to normal error handling.
- Line 67: Move the helper functions parseTemplate and parseCommaSeparated so
they are declared before they are referenced by the Commander setup and before
program.parse(process.argv); — locate the parseTemplate and parseCommaSeparated
function definitions in this file and cut/paste them above the program
definition/option setup (or at least above program.parse), ensuring the action
handlers reference already-declared functions and no other code changes are
required.
In `@packages/cli/create-o2s-app/src/scaffold/index.ts`:
- Around line 56-59: The code currently always calls
removePackageLock(targetDir) and then calls installDependencies(targetDir) which
itself calls removePackageLock, causing duplicate work; change the logic so you
only call removePackageLock when skipping installation and rely on
installDependencies to remove the lock when running installs: replace the
unconditional removePackageLock call with a conditional branch that calls
removePackageLock(targetDir) only when skipInstall is true, otherwise call
installDependencies(targetDir); reference the symbols removePackageLock and
installDependencies in index.ts to locate and update the logic.
In `@packages/cli/create-o2s-app/src/scaffold/install.ts`:
- Around line 20-24: The execSync('npm install', { cwd: projectDir, stdio:
'inherit' }) call can hang indefinitely; add a timeout option (e.g. timeout:
300000 for 5 minutes or a configurable value) to the options object passed to
execSync in install.ts so the child will be killed after the timeout; update the
options to include timeout and ensure any thrown timeout error is propagated or
handled consistently with existing error handling around the execSync
invocation.
- Around line 16-18: Replace the direct console.log/console.error calls in
install.ts with the project's logger utility: import the logger from
packages/cli/create-o2s-app/src/utils/logger.ts and use logger.info(...) for
regular messages (e.g., the "Installing dependencies..." and surrounding blank
outputs) and logger.error(...) for errors; if you prefer colored output without
the logger, use the project's kleur usage pattern instead. Locate the console
calls in the install flow (the console.log entries shown and the console.* calls
around lines 25–28) and switch them to the appropriate logger methods, ensuring
imports are added and any blank-line prints are reproduced via logger.info('')
or equivalent. Ensure consistent message text and severity levels when replacing
console methods.
In `@packages/cli/create-o2s-app/src/utils/logger.ts`:
- Around line 6-13: The printSummary function signature currently takes six
positional parameters (targetDir, template: TemplateType, blockCount,
integrationCount, uncoveredModules = [], skipInstall = false), which hurts
readability and extensibility; refactor it to accept a single options object
(e.g., printSummary(options: { targetDir: string; template: TemplateType;
blockCount: number; integrationCount: number; uncoveredModules?: string[];
skipInstall?: boolean })) and update all call sites to pass a named object,
preserving default values for uncoveredModules and skipInstall and keeping the
function body behavior unchanged.
- Around line 32-34: The three console.log lines in logger.ts currently print a
hardcoded path ("packages/configs/integrations/src/models/<module>.ts"); replace
that literal with a derived or constant value (e.g., compute the integrations
models path from process.cwd() or a shared constant like
INTEGRATIONS_MODELS_PATH) and update the message strings that reference it (the
console.log calls shown in the diff). Locate the console.log occurrences in
packages/cli/create-o2s-app/src/utils/logger.ts and refactor to use the derived
constant/path-building (using path.join or a project-config helper) so the
message remains correct if the repo layout changes. Ensure the text still
instructs editing "<module>.ts" but uses the new variable instead of the
hardcoded string.
In `@packages/cli/create-o2s-app/src/wizard/templates.ts`:
- Around line 34-61: Both functions filterBlocksByTemplate and
filterIntegrationsByTemplate duplicate the same logic; extract a single generic
helper (e.g., filterByTemplate<T extends { category?: string[]; name: string }>)
that takes (template: TemplateType, items: T[]) and returns string[] by looking
up TEMPLATES[template].category, returning [] if no category, otherwise
filtering items whose category includes that category and mapping to name; then
implement filterBlocksByTemplate and filterIntegrationsByTemplate as thin
wrappers that call this helper with BlockInfo[] and IntegrationInfo[]
respectively (keep references to TemplateType, TEMPLATES, category and name).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
packages/blocks/article-list/package.jsonpackages/blocks/article-search/package.jsonpackages/blocks/article/package.jsonpackages/blocks/category-list/package.jsonpackages/blocks/category/package.jsonpackages/blocks/document-list/package.jsonpackages/blocks/faq/package.jsonpackages/blocks/featured-service-list/package.jsonpackages/blocks/invoice-list/package.jsonpackages/blocks/notification-details/package.jsonpackages/blocks/notification-list/package.jsonpackages/blocks/notification-summary/package.jsonpackages/blocks/order-details/package.jsonpackages/blocks/order-list/package.jsonpackages/blocks/orders-summary/package.jsonpackages/blocks/payments-history/package.jsonpackages/blocks/payments-summary/package.jsonpackages/blocks/product-details/package.jsonpackages/blocks/product-list/package.jsonpackages/blocks/quick-links/package.jsonpackages/blocks/recommended-products/package.jsonpackages/blocks/service-details/package.jsonpackages/blocks/service-list/package.jsonpackages/blocks/surveyjs-form/package.jsonpackages/blocks/ticket-details/package.jsonpackages/blocks/ticket-list/package.jsonpackages/blocks/ticket-recent/package.jsonpackages/blocks/ticket-summary/package.jsonpackages/blocks/user-account/package.jsonpackages/cli/create-o2s-app/src/index.tspackages/cli/create-o2s-app/src/scaffold/index.tspackages/cli/create-o2s-app/src/scaffold/install.tspackages/cli/create-o2s-app/src/types.tspackages/cli/create-o2s-app/src/utils/logger.tspackages/cli/create-o2s-app/src/wizard/templates.tspackages/integrations/mocked/package.json
✅ Files skipped from review due to trivial changes (1)
- packages/blocks/order-details/package.json
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/blocks/service-list/package.json
- packages/blocks/ticket-details/package.json
- packages/blocks/product-list/package.json
- packages/blocks/notification-details/package.json
- packages/blocks/ticket-list/package.json
- packages/blocks/notification-list/package.json
- packages/blocks/surveyjs-form/package.json
- packages/blocks/orders-summary/package.json
- packages/blocks/payments-summary/package.json
- packages/blocks/payments-history/package.json
- packages/blocks/article-search/package.json
- packages/blocks/service-details/package.json
- packages/blocks/ticket-recent/package.json
- packages/blocks/recommended-products/package.json
- packages/blocks/product-details/package.json
- packages/blocks/category/package.json
…d-dxp integration
… related types and functions
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/cli/create-o2s-app/src/wizard/index.ts (2)
14-14: Consider adding error handling for discovery failures.If either
discoverBlocksordiscoverIntegrationsthrows, the entire wizard fails without a user-friendly message. Wrapping in try/catch would allow graceful degradation or clearer error messaging.🛡️ Suggested improvement
- const [allBlocks, allIntegrations] = await Promise.all([discoverBlocks(repoDir), discoverIntegrations(repoDir)]); + let allBlocks, allIntegrations; + try { + [allBlocks, allIntegrations] = await Promise.all([ + discoverBlocks(repoDir), + discoverIntegrations(repoDir), + ]); + } catch (error) { + console.error('Failed to discover blocks or integrations:', error); + throw new Error('Unable to initialize wizard: discovery failed'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/wizard/index.ts` at line 14, Wrap the Promise.all call that invokes discoverBlocks(repoDir) and discoverIntegrations(repoDir) in a try/catch inside the wizard initialization (the code that currently calls Promise.all). On catch, log or surface a clear user-friendly error (e.g., via the CLI logger or prompt) and set sensible fallbacks (e.g., allBlocks = [] and allIntegrations = []) so the wizard can continue or exit gracefully; include the caught error details in the log for debugging while keeping the user message simple.
21-31: Conflict detection bypassed when CLI arguments are fully specified.When all CLI parameters are provided, the function returns immediately without calling
detectConflicts. This means integrations with conflicting modules won't be flagged in non-interactive mode. If this is intentional (e.g., assuming CLI users know what they're doing), consider documenting this behavior. Otherwise, you may want to run conflict detection here as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/wizard/index.ts` around lines 21 - 31, The early return when cliName, cliTemplate, cliBlocks and cliIntegrations are all provided skips running detectConflicts, so non-interactive runs miss conflicting integration modules; before returning the assembled object, call detectConflicts with the selected integrations/blocks and integrationModules (e.g., detectConflicts(selectedIntegrations, selectedBlocks, integrationModules)), populate the returned conflictResolutions field with its result, and handle the case where conflicts exist (either by throwing/returning an error or returning the object with conflictResolutions populated so the caller can handle them) instead of bypassing conflict detection.packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts (1)
96-109: Consider extracting 'mocked' to a named constant.The hardcoded
'mocked'string is a magic value. Extracting it to a constant (e.g.,FULL_MOCKED_INTEGRATION = 'mocked') would improve clarity and maintainability.♻️ Suggested improvement
const CONFIGS_MODELS_PATH = 'packages/configs/integrations/src/models'; const CONFIGS_PACKAGE_JSON_PATH = 'packages/configs/integrations/package.json'; const MOCKED_IMPORT = `@o2s/integrations.mocked/integration`; +const FULL_MOCKED_INTEGRATION = 'mocked';Then at line 100:
- if (!selectedIntegrations.includes('mocked')) { + if (!selectedIntegrations.includes(FULL_MOCKED_INTEGRATION)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts` around lines 96 - 109, Replace the magic string 'mocked' with a named constant and use it where needed: add a descriptive constant (e.g., FULL_MOCKED_INTEGRATION = 'mocked') near the top of this module, then update the conditional that checks selectedIntegrations (selectedIntegrations.includes(...)), the comment mentioning 'mocked', and any related logic (e.g., the uncoveredModules detection block referencing modelsDir and MOCKED_IMPORT) to reference FULL_MOCKED_INTEGRATION instead of the raw string so the intent is explicit and maintainable.packages/cli/create-o2s-app/src/utils/integration-discovery.ts (1)
36-39: Preserve the original error for debugging.Wrapping the error loses the original stack trace and context. Consider preserving the cause for better debuggability.
♻️ Suggested improvement
} catch (error) { console.error('Error fetching the integration list:', error); - throw new Error('Failed to fetch the integration list.'); + throw new Error('Failed to discover the integration list.', { cause: error }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/create-o2s-app/src/utils/integration-discovery.ts` around lines 36 - 39, The catch block in packages/cli/create-o2s-app/src/utils/integration-discovery.ts currently logs and throws a new Error which discards the original error and its stack; instead preserve the original error as the cause (or rethrow) so callers can debug: capture the caught error in the catch(error) and throw a new Error('Failed to fetch the integration list.', { cause: error }) or simply rethrow the original error to retain stack trace; update the catch block where the integration list is fetched to use one of these approaches and keep the existing console.error logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts`:
- Around line 45-53: The loop that removes unselected integration deps accesses
pkg.dependencies without guarding for its absence; update the code around the
for (const key of Object.keys(pkg.dependencies as Record<string,string>)) { ...
} to handle missing or non-object dependencies (e.g. use
Object.keys(pkg.dependencies || {}) or check typeof pkg.dependencies ===
'object' before iterating) so the routine that prunes keys for integrations
(using selectedIntegrations and deleting from pkg.dependencies) does not throw
when dependencies is undefined.
- Around line 88-91: The current replacement uses String.replace which only
replaces the first occurrence; update the transformation in
transform-integration-configs.ts to use content.replaceAll(MOCKED_IMPORT,
`@o2s/integrations.${integration}/integration`) (or a global RegExp) so every
occurrence of MOCKED_IMPORT in the file is replaced before writing back to
filePath; ensure you still read via fs.readFile and write via fs.writeFile using
the new updatedContent variable.
---
Nitpick comments:
In `@packages/cli/create-o2s-app/src/scaffold/transform-integration-configs.ts`:
- Around line 96-109: Replace the magic string 'mocked' with a named constant
and use it where needed: add a descriptive constant (e.g.,
FULL_MOCKED_INTEGRATION = 'mocked') near the top of this module, then update the
conditional that checks selectedIntegrations
(selectedIntegrations.includes(...)), the comment mentioning 'mocked', and any
related logic (e.g., the uncoveredModules detection block referencing modelsDir
and MOCKED_IMPORT) to reference FULL_MOCKED_INTEGRATION instead of the raw
string so the intent is explicit and maintainable.
In `@packages/cli/create-o2s-app/src/utils/integration-discovery.ts`:
- Around line 36-39: The catch block in
packages/cli/create-o2s-app/src/utils/integration-discovery.ts currently logs
and throws a new Error which discards the original error and its stack; instead
preserve the original error as the cause (or rethrow) so callers can debug:
capture the caught error in the catch(error) and throw a new Error('Failed to
fetch the integration list.', { cause: error }) or simply rethrow the original
error to retain stack trace; update the catch block where the integration list
is fetched to use one of these approaches and keep the existing console.error
logging.
In `@packages/cli/create-o2s-app/src/wizard/index.ts`:
- Line 14: Wrap the Promise.all call that invokes discoverBlocks(repoDir) and
discoverIntegrations(repoDir) in a try/catch inside the wizard initialization
(the code that currently calls Promise.all). On catch, log or surface a clear
user-friendly error (e.g., via the CLI logger or prompt) and set sensible
fallbacks (e.g., allBlocks = [] and allIntegrations = []) so the wizard can
continue or exit gracefully; include the caught error details in the log for
debugging while keeping the user message simple.
- Around line 21-31: The early return when cliName, cliTemplate, cliBlocks and
cliIntegrations are all provided skips running detectConflicts, so
non-interactive runs miss conflicting integration modules; before returning the
assembled object, call detectConflicts with the selected integrations/blocks and
integrationModules (e.g., detectConflicts(selectedIntegrations, selectedBlocks,
integrationModules)), populate the returned conflictResolutions field with its
result, and handle the case where conflicts exist (either by throwing/returning
an error or returning the object with conflictResolutions populated so the
caller can handle them) instead of bypassing conflict detection.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.gitignorepackages/cli/create-o2s-app/src/constants.tspackages/cli/create-o2s-app/src/scaffold/index.tspackages/cli/create-o2s-app/src/scaffold/transform-integration-configs.tspackages/cli/create-o2s-app/src/types.tspackages/cli/create-o2s-app/src/utils/integration-discovery.tspackages/cli/create-o2s-app/src/wizard/env-prompts.tspackages/cli/create-o2s-app/src/wizard/index.tspackages/cli/create-o2s-app/src/wizard/prompts.tspackages/integrations/algolia/package.jsonpackages/integrations/contentful-cms/package.jsonpackages/integrations/medusajs/package.jsonpackages/integrations/mocked-dxp/package.jsonpackages/integrations/redis/package.jsonpackages/integrations/strapi-cms/package.jsonpackages/integrations/zendesk/package.json
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/cli/create-o2s-app/src/scaffold/index.ts
- packages/cli/create-o2s-app/src/constants.ts
- packages/cli/create-o2s-app/src/wizard/prompts.ts
- packages/cli/create-o2s-app/src/wizard/env-prompts.ts
- packages/integrations/mocked-dxp/package.json
- packages/cli/create-o2s-app/src/types.ts
…roject scaffolding
Summary
Implements an interactive CLI wizard (
create-o2s-app) for bootstrapping O2S projects — similar tocreate-next-app. The wizard guides users through project setup with template selection, block/integration picking, and environment variable configuration.o2sTemplatefield from each package'spackage.jsonto categorize blocks and integrations per templateapp.module.ts,package.json, page models, and.env.localgenerationkleurWizard Flow
.env.localKey Technical Changes
o2sTemplatefield topackage.jsonof all blocks and integrations for template categorizationSTORYBOOK_URLandDOCS_URLconstants for CLI outputRelated Issue
Closes #675
Test Plan
npx create-o2s-appand verify the wizard flow for each template (O2S, DXP, Custom).env.localis generated with correct variables when promptednpm run buildnpm run devnpm run lintandnpm run buildon the monorepo to confirm no regressions