Adding container application type#380
Conversation
…EDM-2451)\n\n- Add appType to application form model\n- Preserve appType from API and emit on save\n- Add Compose/Quadlet selector in Applications step
WalkthroughIntroduces Mock Service Worker (MSW) infrastructure for offline standalone UI development with mocked API endpoints, adds container-based inline application support with quadlet format conversion, extends types and validation for the new container specification, and creates a new ApplicationContainerForm UI component for managing container settings. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (index.tsx)
participant MSW as MSW Worker
participant Browser as Browser Network
participant Handlers as Mock Handlers
App->>App: Check USE_MSW env var
alt USE_MSW = true
App->>MSW: dynamicImport & worker.start()
MSW->>Browser: Register Service Worker
MSW->>MSW: Initialize with bypassUnhandledRequest
App->>App: Render UI with API_PORT = undefined
Note over App,Handlers: Network requests intercepted by MSW
Browser->>MSW: Fetch request
MSW->>Handlers: Route to mock handler
Handlers->>MSW: Mock response
MSW->>Browser: Return mock response
else USE_MSW = false
App->>App: Render UI with API_PORT = 3001
Browser->>Browser: Connect to backend on :3001
end
sequenceDiagram
participant Form as ApplicationForm
participant Utils as deviceSpecUtils
participant Quadlet as Quadlet Generator
Form->>Form: User selects container format
Form->>Form: Populates container specs (image, ports, volumes, resources)
Form->>Utils: toAPIApplication(containerApp)
Utils->>Quadlet: Generate quadlet from container
Quadlet->>Quadlet: Build systemd unit (PublishPort, Volume, Memory, CPU)
Quadlet->>Utils: Return quadlet manifest content
Utils->>Utils: Return ApplicationProviderSpec with appType=quadlet and inline file
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
liatb-rh
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
274-285: Ensure quadlet-form apps send the correct AppType.If a user picks the “Quadlet” inline format in the UI,
inlineFormatbecomes'quadlet'butapp.appTypestill defaults to compose. This branch then emitsAppTypeCompose, so the backend treats the submission as a compose app even though the payload contains quadlet content. The rendered unit will be malformed or rejected. Please derive the fallback app type frominlineFormatbefore defaulting.- return { - name: (app as InlineAppForm).name, - appType: (app.appType || AppType.AppTypeCompose), - inline: (app as InlineAppForm).files.map( + const inlineApp = app as InlineAppForm; + const derivedAppType = + inlineApp.inlineFormat === 'quadlet' ? AppType.AppTypeQuadlet : AppType.AppTypeCompose; + + return { + name: inlineApp.name, + appType: inlineApp.appType || derivedAppType, + inline: inlineApp.files.map( (file): InlineApplicationFileFixed => ({ path: file.path, content: file.content || '',
🧹 Nitpick comments (8)
libs/ui-components/src/components/Fleet/CreateFleet/CreateFleetWizard.tsx (1)
2-14: Remove trailing whitespace from import list.Line 6 has trailing whitespace after the
Bullseye,import that should be removed to align with standard formatting practices and avoid unnecessary whitespace in the codebase.import { Alert, Breadcrumb, BreadcrumbItem, - Bullseye, + Bullseye, PageSection, PageSectionVariants, Spinner, Title, Wizard, WizardStep, WizardStepType, } from '@patternfly/react-core';README.md (1)
50-61: Clarify scope of mocked endpoints in docsThe new mock-mode section reads well and matches the
npm run dev:mockworkflow. Sincehandlers.tsalso mocks devices and auth/login endpoints, consider mentioning those (or using wording like “fleets, repositories, devices, and auth”) so readers understand how complete the mock coverage is.apps/standalone/src/index.tsx (1)
39-51: Error handling for MSW initialization aligns with MSW best practicesMSW recommends awaiting
worker.start()and handling rejections, making the suggested try/catch/finally approach sound and well-grounded. The current code leaves the app vulnerable to a blank UI if MSW fails to initialize.The refactored approach—catching initialization failures and ensuring
renderruns in afinallyblock—is a recommended practice and follows MSW's own guidance for handling worker-registration failures in dev bootstraps.apps/standalone/src/mocks/handlers.ts (5)
1-8: Scope the ESLint disable andanyusage to keep this file safer to evolveUsing a file-wide disable for all the unsafe/
anyrules is understandable for mocks, but it also removes guardrails if more logic is added here later. Consider either:
- Limiting the disable to the fixture-heavy section (e.g., around
getMockDevicesonly), or- Tightening types so most of the file compiles without
any(reservingas anyfor truly awkward fields).That keeps the MSW handlers themselves type-checked while still letting you be pragmatic with the fixtures.
21-139: Enrich device fixtures and reduceanyto better exercise the new container spec
getMockDevicesprovides good coverage of basic device fields, but:
- Most of
specandstatusis typed asany, so changes in@flightctl/types.Device(especially around applications) won’t be caught here.- Given this PR is adding container/quadlet support, none of the sample
applicationsactually exercise those new fields; they are just{ name, image }.To make the standalone+MSW setup more valuable for the new flows, consider:
- Typing the fixture more narrowly (e.g., a strongly‑typed
const mockDevices: Device[] = [...]or at leastPartial<Device>with feweras anycasts), and- Adding at least one device whose application spec uses the new container/quadlet fields so the ApplicationContainerForm can be wired/tested against realistic data.
Based on learnings
164-219: Consider using MSW path params instead of regex + manual URL parsingThe device handlers currently combine
new RegExp(...)with explicitURLparsing andpathname.match(...). MSW supports Express-style paths with named params, which can make these routes simpler and avoid regex complexity. For example:- http.get(new RegExp(`${apiBase}/devices/([\\w-]+)$`), ({ request }) => { - const url = new URL(request.url); - const nameMatch = url.pathname.match(/\\/devices\\/([\\w-]+)$/); - const name = nameMatch ? nameMatch[1] : ''; + http.get(`${apiBase}/devices/:name`, ({ params }) => { + const name = params.name as string;Similarly, list and other routes could be expressed as:
http.get(`${apiBase}/devices`, /* ... */); http.get(`${apiBase}/devices/:name/lastseen`, /* ... */); http.patch(`${apiBase}/devices/:name`, /* ... */); http.put(`${apiBase}/devices/:name/decommission`, /* ... */); http.delete(`${apiBase}/devices/:name`, /* ... */);This keeps the handlers easier to maintain and naturally supports a wider range of valid names (not just
[\w-]).Please double-check against the MSW version in this repo to confirm the
:parampath syntax is supported as shown.
206-219: Optional: maintain simple in‑memory state for device mutationsThe PATCH, decommission (PUT), and DELETE handlers all return success but don’t adjust any shared device state, so subsequent GETs will still reflect the original fixtures. That’s fine for “happy-path only” mocks, but it can limit how much of the edit/decommission flows you can realistically exercise in the standalone app.
If you find yourself testing those flows end‑to‑end, consider:
- Hoisting
const mockDevices = getMockDevices()to module scope, and- Updating/removing entries in that array in the PATCH/PUT/DELETE handlers before returning.
This remains in-memory and dev-only, but better mirrors real backend behavior.
221-261: Echo more of the POSTed fleet body in the create‑fleet handlerThe create‑fleet endpoint currently does:
const base = basicFleets[0]; const created: Fleet = { ...base, metadata: { ...base.metadata, name: body?.metadata?.name ?? base.metadata.name }, };This means:
- The response ignores most of the POSTed spec/status fields, and
- New fleets are essentially clones of
basicFleets[0]with only the name changed.For standalone dev it can be more useful if the response reflects what the UI just submitted. For example:
- const base = basicFleets[0]; - const created: Fleet = { - ...base, - metadata: { ...base.metadata, name: body?.metadata?.name ?? base.metadata.name }, - }; + const base = basicFleets[0]; + const created: Fleet = { + ...base, + ...body, + metadata: { + ...base.metadata, + ...body.metadata, + }, + };That keeps any required defaults from
basebut lets the POST body drive the returned spec, making it easier to validate new fields in the UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
README.md(1 hunks)apps/standalone/package.json(3 hunks)apps/standalone/public/mockServiceWorker.js(1 hunks)apps/standalone/src/index.tsx(1 hunks)apps/standalone/src/mocks/browser.ts(1 hunks)apps/standalone/src/mocks/handlers.ts(1 hunks)apps/standalone/webpack.config.ts(3 hunks)libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts(3 hunks)libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationContainerForm.tsx(1 hunks)libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationTemplates.tsx(5 hunks)libs/ui-components/src/components/Fleet/CreateFleet/CreateFleetWizard.tsx(1 hunks)libs/ui-components/src/components/form/validations.ts(2 hunks)libs/ui-components/src/types/deviceSpec.ts(3 hunks)package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
📚 Learning: 2025-03-20T12:37:36.986Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 240
File: libs/ui-components/src/types/deviceSpec.ts:0-0
Timestamp: 2025-03-20T12:37:36.986Z
Learning: In the FlightCtl UI project, `name` is required for inline applications but optional for image applications. This is enforced at the type level in the `InlineAppForm` type.
Applied to files:
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationContainerForm.tsxlibs/ui-components/src/components/form/validations.tslibs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationTemplates.tsxlibs/ui-components/src/types/deviceSpec.tslibs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
📚 Learning: 2025-03-19T08:55:03.335Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 240
File: libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx:44-50
Timestamp: 2025-03-19T08:55:03.335Z
Learning: In FlightCtl, the name field is required for Inline applications but optional for Image-based applications. This requirement is enforced by validation logic even though TypeScript type definitions might not explicitly show this distinction.
Applied to files:
libs/ui-components/src/components/form/validations.tslibs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationTemplates.tsxlibs/ui-components/src/types/deviceSpec.ts
📚 Learning: 2025-05-26T11:41:40.796Z
Learnt from: rawagner
Repo: flightctl/flightctl-ui PR: 300
File: libs/ui-components/src/components/form/validations.ts:459-461
Timestamp: 2025-05-26T11:41:40.796Z
Learning: In TypeScript validation contexts, when working with properly typed form data like `BatchForm`, required array fields (without `?` marker) are guaranteed to be defined arrays, making defensive checks for undefined unnecessary in those specific contexts.
Applied to files:
libs/ui-components/src/components/form/validations.ts
📚 Learning: 2025-02-17T08:52:50.747Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 218
File: libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsPage.tsx:109-111
Timestamp: 2025-02-17T08:52:50.747Z
Learning: When wrapping a Button component (like `deleteAction`) inside another Button in React/PatternFly, use `component="a"` on the outer Button to avoid accessibility issues caused by button nesting. Include `tabIndex={0}` to ensure proper keyboard navigation.
Applied to files:
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationTemplates.tsx
📚 Learning: 2025-03-19T08:57:16.233Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 240
File: libs/ui-components/src/components/Device/DeviceDetails/DeviceApplications.tsx:44-50
Timestamp: 2025-03-19T08:57:16.233Z
Learning: In FlightCtl, the name field is required for Inline applications but optional for Image-based applications. This requirement is enforced by both frontend validation logic and backend API constraints, ensuring that no inline application can exist in the system without a name field.
Applied to files:
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationTemplates.tsxlibs/ui-components/src/types/deviceSpec.ts
📚 Learning: 2025-10-29T16:47:29.614Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
Applied to files:
libs/ui-components/src/types/deviceSpec.tslibs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
📚 Learning: 2025-10-10T11:35:04.458Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 0
File: :0-0
Timestamp: 2025-10-10T11:35:04.458Z
Learning: In the flightctl-ui repository, TypeScript types are generated using hey-api/openapi-ts and output to a single index.ts file (previously used openapi-typescript-codegen with one file per type under /libs/types/models).
Applied to files:
libs/ui-components/src/types/deviceSpec.ts
📚 Learning: 2025-03-20T12:34:33.199Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 240
File: libs/types/models/FileSpec.ts:8-8
Timestamp: 2025-03-20T12:34:33.199Z
Learning: Files in the types directory are autogenerated from the OpenAPI spec in the flightctl project and should not be directly modified.
Applied to files:
libs/ui-components/src/types/deviceSpec.ts
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to handle undefined input through its optional parameter definition and consistent use of optional chaining.
Applied to files:
libs/ui-components/src/components/Fleet/CreateFleet/CreateFleetWizard.tsx
📚 Learning: 2025-03-12T09:09:44.139Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 227
File: libs/ui-components/src/components/Device/EditDeviceWizard/EditDeviceWizard.tsx:87-87
Timestamp: 2025-03-12T09:09:44.139Z
Learning: The `getUpdatePolicyValues` function in libs/ui-components/src/components/Fleet/CreateFleet/fleetSpecUtils.ts is designed to safely handle undefined values for the updatePolicy parameter.
Applied to files:
libs/ui-components/src/components/Fleet/CreateFleet/CreateFleetWizard.tsx
🧬 Code graph analysis (7)
apps/standalone/src/mocks/browser.ts (1)
apps/standalone/src/mocks/handlers.ts (1)
handlers(141-262)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationContainerForm.tsx (2)
libs/ui-components/src/types/deviceSpec.ts (1)
InlineAppForm(60-74)libs/ui-components/src/hooks/useTranslation.ts (1)
useTranslation(5-8)
libs/ui-components/src/components/form/validations.ts (1)
libs/ui-components/src/types/deviceSpec.ts (2)
InlineAppForm(60-74)AppForm(97-97)
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationTemplates.tsx (1)
libs/ui-components/src/types/deviceSpec.ts (3)
AppForm(97-97)isInlineAppForm(107-107)isImageAppForm(106-106)
libs/ui-components/src/types/deviceSpec.ts (1)
libs/types/index.ts (1)
AppType(15-15)
apps/standalone/src/index.tsx (1)
apps/standalone/src/mocks/browser.ts (1)
worker(4-4)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
libs/ui-components/src/types/deviceSpec.ts (1)
InlineAppForm(60-74)
🪛 ast-grep (0.40.0)
apps/standalone/src/mocks/handlers.ts
[warning] 164-164: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/devices(\\?.*)?$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 191-191: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/devices/([\\w-]+)$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 201-201: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/devices/([\\w-]+)/lastseen$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 206-206: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/devices/([\\w-]+)$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 211-211: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/devices/([\\w-]+)/decommission$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 216-216: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/devices/([\\w-]+)$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 221-221: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/fleets(\\?.*)?$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 226-226: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/fleets/([\\w-]+)$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 253-253: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${apiBase}/repositories/([\\w-]+)$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (4)
package.json (1)
21-21: Rootdev:mockscript wiring looks goodThe new
dev:mockscript cleanly delegates to the standalone workspace and matches the README instructions; no issues spotted.apps/standalone/src/mocks/browser.ts (1)
1-4: MSW browser worker setup is correct and follows MSW v2 best practicesThe
setupWorker(...handlers)pattern with import from'msw/browser'is confirmed as the recommended approach in MSW v2. Code matches idiomatic usage; no issues identified.apps/standalone/webpack.config.ts (1)
29-37: Fix CopyPlugin to copy the correct MSW worker file instead of the client libraryThe verification confirms your concern: the
CopyPluginat lines 168–177 copiesmsw/browser.js(the client-side library used bysetupWorker) rather than the actual service worker. SincedevServer.staticservesdistbeforepublic, the dist-copied file will shadow the committedpublic/mockServiceWorker.js, causing the browser to load the wrong asset and break MSW functionality.Solution:
Replace the CopyPlugin to copy the real worker from
public:new CopyPlugin({ patterns: [ { - from: path.resolve(__dirname, '../../node_modules/msw/browser.js'), + from: path.resolve(__dirname, 'public/mockServiceWorker.js'), to: 'mockServiceWorker.js', - noErrorOnMissing: true, }, ], }),Alternatively, remove the
CopyPluginentirely if you rely on servingpublic/mockServiceWorker.jsdirectly via the static config.Regarding the
API_PORTdefine (lines 220–231), the suggested refinement is valid for consistency:- defines['window.API_PORT'] = JSON.stringify(process.env.API_PORT) || '3001'; + defines['window.API_PORT'] = JSON.stringify(process.env.API_PORT || '3001');Likely an incorrect or invalid review comment.
apps/standalone/src/mocks/handlers.ts (1)
9-16: Reusable list response helper looks good
buildListResponse<T>cleanly standardizes list payloads (apiVersion/kind/metadata/items) and keeps the handlers DRY where it’s used. No changes needed here.
| "dev:mock": "USE_MSW=true npm run dev", | ||
| "dev:kind": ". ./scripts/setup_env.sh && npm run dev", |
There was a problem hiding this comment.
🧩 Analysis chain
Align msw dependency with generated worker version
The dev:mock script and msw.workerDirectory config look good and line up with the new mock flow.
One thing to double‑check: apps/standalone/public/mockServiceWorker.js declares a PACKAGE_VERSION of 2.12.1, while this package pins msw to ^2.4.1. MSW uses an integrity check between the client library and the worker, so having different versions can cause warnings and, in worst cases, protocol mismatches.
To keep things in sync, consider either:
- Bumping the devDependency to match the worker’s version (for example, to
"msw": "^2.12.x"based on the worker file), and reinstalling; or - Pinning
mswto your desired version and regenerating the worker with the matching CLI (npx msw@<version> init public --save).
That way the client and worker stay version-aligned.
Also applies to: 24-24, 63-68
🏁 Script executed:
#!/bin/bash
# Show the version constant in the worker and the msw devDependency side-by-side.
echo "Worker version constant:"
rg -n "PACKAGE_VERSION" apps/standalone/public/mockServiceWorker.js -n || echo "No PACKAGE_VERSION found"
echo
echo "msw devDependency in apps/standalone/package.json:"
rg -n '"msw"' apps/standalone/package.json -n || echo "No msw devDependency found"Length of output: 196
Align msw dependency version with generated worker
The dev:mock script and msw.workerDirectory config are correctly set up.
However, verification confirms a version mismatch: the worker at apps/standalone/public/mockServiceWorker.js (line 10) declares PACKAGE_VERSION = '2.12.1', while apps/standalone/package.json (line 24) pins "msw": "^2.4.1". MSW's integrity check between client and worker can flag or fail on version mismatches.
Resolve by either:
- Updating the devDependency to
"msw": "^2.12.x"and reinstalling, or - Pinning to your desired version and regenerating the worker:
npx msw@<version> init public --save
🤖 Prompt for AI Agents
In apps/standalone/package.json around lines 14-15 (and note line 24 where "msw"
is pinned) and apps/standalone/public/mockServiceWorker.js (line 10 showing
PACKAGE_VERSION = '2.12.1'), the MSW package version in package.json mismatches
the generated worker; update package.json to use a matching MSW version (e.g.,
set "msw": "^2.12.x") and run a reinstall (npm install/yarn) so lockfile
updates, or alternatively pin package.json to your desired version and
regenerate the worker with npx msw@<version> init public --save; finally commit
the updated package.json, lockfile, and the regenerated
public/mockServiceWorker.js.
| /* eslint-disable */ | ||
| /* tslint:disable */ | ||
|
|
||
| /** | ||
| * Mock Service Worker. | ||
| * @see https://github.com/mswjs/msw | ||
| * - Please do NOT modify this file. | ||
| */ | ||
|
|
||
| const PACKAGE_VERSION = '2.12.1' | ||
| const INTEGRITY_CHECKSUM = '4db4a41e972cec1b64cc569c66952d82' | ||
| const IS_MOCKED_RESPONSE = Symbol('isMockedResponse') | ||
| const activeClientIds = new Set() | ||
|
|
||
| addEventListener('install', function () { | ||
| self.skipWaiting() | ||
| }) | ||
|
|
||
| addEventListener('activate', function (event) { | ||
| event.waitUntil(self.clients.claim()) | ||
| }) | ||
|
|
||
| addEventListener('message', async function (event) { | ||
| const clientId = Reflect.get(event.source || {}, 'id') | ||
|
|
||
| if (!clientId || !self.clients) { | ||
| return | ||
| } | ||
|
|
||
| const client = await self.clients.get(clientId) | ||
|
|
||
| if (!client) { | ||
| return | ||
| } | ||
|
|
||
| const allClients = await self.clients.matchAll({ | ||
| type: 'window', | ||
| }) | ||
|
|
||
| switch (event.data) { | ||
| case 'KEEPALIVE_REQUEST': { | ||
| sendToClient(client, { | ||
| type: 'KEEPALIVE_RESPONSE', | ||
| }) | ||
| break | ||
| } | ||
|
|
||
| case 'INTEGRITY_CHECK_REQUEST': { | ||
| sendToClient(client, { | ||
| type: 'INTEGRITY_CHECK_RESPONSE', | ||
| payload: { | ||
| packageVersion: PACKAGE_VERSION, | ||
| checksum: INTEGRITY_CHECKSUM, | ||
| }, | ||
| }) | ||
| break | ||
| } | ||
|
|
||
| case 'MOCK_ACTIVATE': { | ||
| activeClientIds.add(clientId) | ||
|
|
||
| sendToClient(client, { | ||
| type: 'MOCKING_ENABLED', | ||
| payload: { | ||
| client: { | ||
| id: client.id, | ||
| frameType: client.frameType, | ||
| }, | ||
| }, | ||
| }) | ||
| break | ||
| } | ||
|
|
||
| case 'CLIENT_CLOSED': { | ||
| activeClientIds.delete(clientId) | ||
|
|
||
| const remainingClients = allClients.filter((client) => { | ||
| return client.id !== clientId | ||
| }) | ||
|
|
||
| // Unregister itself when there are no more clients | ||
| if (remainingClients.length === 0) { | ||
| self.registration.unregister() | ||
| } | ||
|
|
||
| break | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| addEventListener('fetch', function (event) { | ||
| const requestInterceptedAt = Date.now() | ||
|
|
||
| // Bypass navigation requests. | ||
| if (event.request.mode === 'navigate') { | ||
| return | ||
| } | ||
|
|
||
| // Opening the DevTools triggers the "only-if-cached" request | ||
| // that cannot be handled by the worker. Bypass such requests. | ||
| if ( | ||
| event.request.cache === 'only-if-cached' && | ||
| event.request.mode !== 'same-origin' | ||
| ) { | ||
| return | ||
| } | ||
|
|
||
| // Bypass all requests when there are no active clients. | ||
| // Prevents the self-unregistered worked from handling requests | ||
| // after it's been terminated (still remains active until the next reload). | ||
| if (activeClientIds.size === 0) { | ||
| return | ||
| } | ||
|
|
||
| const requestId = crypto.randomUUID() | ||
| event.respondWith(handleRequest(event, requestId, requestInterceptedAt)) | ||
| }) | ||
|
|
||
| /** | ||
| * @param {FetchEvent} event | ||
| * @param {string} requestId | ||
| * @param {number} requestInterceptedAt | ||
| */ | ||
| async function handleRequest(event, requestId, requestInterceptedAt) { | ||
| const client = await resolveMainClient(event) | ||
| const requestCloneForEvents = event.request.clone() | ||
| const response = await getResponse( | ||
| event, | ||
| client, | ||
| requestId, | ||
| requestInterceptedAt, | ||
| ) | ||
|
|
||
| // Send back the response clone for the "response:*" life-cycle events. | ||
| // Ensure MSW is active and ready to handle the message, otherwise | ||
| // this message will pend indefinitely. | ||
| if (client && activeClientIds.has(client.id)) { | ||
| const serializedRequest = await serializeRequest(requestCloneForEvents) | ||
|
|
||
| // Clone the response so both the client and the library could consume it. | ||
| const responseClone = response.clone() | ||
|
|
||
| sendToClient( | ||
| client, | ||
| { | ||
| type: 'RESPONSE', | ||
| payload: { | ||
| isMockedResponse: IS_MOCKED_RESPONSE in response, | ||
| request: { | ||
| id: requestId, | ||
| ...serializedRequest, | ||
| }, | ||
| response: { | ||
| type: responseClone.type, | ||
| status: responseClone.status, | ||
| statusText: responseClone.statusText, | ||
| headers: Object.fromEntries(responseClone.headers.entries()), | ||
| body: responseClone.body, | ||
| }, | ||
| }, | ||
| }, | ||
| responseClone.body ? [serializedRequest.body, responseClone.body] : [], | ||
| ) | ||
| } | ||
|
|
||
| return response | ||
| } | ||
|
|
||
| /** | ||
| * Resolve the main client for the given event. | ||
| * Client that issues a request doesn't necessarily equal the client | ||
| * that registered the worker. It's with the latter the worker should | ||
| * communicate with during the response resolving phase. | ||
| * @param {FetchEvent} event | ||
| * @returns {Promise<Client | undefined>} | ||
| */ | ||
| async function resolveMainClient(event) { | ||
| const client = await self.clients.get(event.clientId) | ||
|
|
||
| if (activeClientIds.has(event.clientId)) { | ||
| return client | ||
| } | ||
|
|
||
| if (client?.frameType === 'top-level') { | ||
| return client | ||
| } | ||
|
|
||
| const allClients = await self.clients.matchAll({ | ||
| type: 'window', | ||
| }) | ||
|
|
||
| return allClients | ||
| .filter((client) => { | ||
| // Get only those clients that are currently visible. | ||
| return client.visibilityState === 'visible' | ||
| }) | ||
| .find((client) => { | ||
| // Find the client ID that's recorded in the | ||
| // set of clients that have registered the worker. | ||
| return activeClientIds.has(client.id) | ||
| }) | ||
| } | ||
|
|
||
| /** | ||
| * @param {FetchEvent} event | ||
| * @param {Client | undefined} client | ||
| * @param {string} requestId | ||
| * @param {number} requestInterceptedAt | ||
| * @returns {Promise<Response>} | ||
| */ | ||
| async function getResponse(event, client, requestId, requestInterceptedAt) { | ||
| // Clone the request because it might've been already used | ||
| // (i.e. its body has been read and sent to the client). | ||
| const requestClone = event.request.clone() | ||
|
|
||
| function passthrough() { | ||
| // Cast the request headers to a new Headers instance | ||
| // so the headers can be manipulated with. | ||
| const headers = new Headers(requestClone.headers) | ||
|
|
||
| // Remove the "accept" header value that marked this request as passthrough. | ||
| // This prevents request alteration and also keeps it compliant with the | ||
| // user-defined CORS policies. | ||
| const acceptHeader = headers.get('accept') | ||
| if (acceptHeader) { | ||
| const values = acceptHeader.split(',').map((value) => value.trim()) | ||
| const filteredValues = values.filter( | ||
| (value) => value !== 'msw/passthrough', | ||
| ) | ||
|
|
||
| if (filteredValues.length > 0) { | ||
| headers.set('accept', filteredValues.join(', ')) | ||
| } else { | ||
| headers.delete('accept') | ||
| } | ||
| } | ||
|
|
||
| return fetch(requestClone, { headers }) | ||
| } | ||
|
|
||
| // Bypass mocking when the client is not active. | ||
| if (!client) { | ||
| return passthrough() | ||
| } | ||
|
|
||
| // Bypass initial page load requests (i.e. static assets). | ||
| // The absence of the immediate/parent client in the map of the active clients | ||
| // means that MSW hasn't dispatched the "MOCK_ACTIVATE" event yet | ||
| // and is not ready to handle requests. | ||
| if (!activeClientIds.has(client.id)) { | ||
| return passthrough() | ||
| } | ||
|
|
||
| // Notify the client that a request has been intercepted. | ||
| const serializedRequest = await serializeRequest(event.request) | ||
| const clientMessage = await sendToClient( | ||
| client, | ||
| { | ||
| type: 'REQUEST', | ||
| payload: { | ||
| id: requestId, | ||
| interceptedAt: requestInterceptedAt, | ||
| ...serializedRequest, | ||
| }, | ||
| }, | ||
| [serializedRequest.body], | ||
| ) | ||
|
|
||
| switch (clientMessage.type) { | ||
| case 'MOCK_RESPONSE': { | ||
| return respondWithMock(clientMessage.data) | ||
| } | ||
|
|
||
| case 'PASSTHROUGH': { | ||
| return passthrough() | ||
| } | ||
| } | ||
|
|
||
| return passthrough() | ||
| } | ||
|
|
||
| /** | ||
| * @param {Client} client | ||
| * @param {any} message | ||
| * @param {Array<Transferable>} transferrables | ||
| * @returns {Promise<any>} | ||
| */ | ||
| function sendToClient(client, message, transferrables = []) { | ||
| return new Promise((resolve, reject) => { | ||
| const channel = new MessageChannel() | ||
|
|
||
| channel.port1.onmessage = (event) => { | ||
| if (event.data && event.data.error) { | ||
| return reject(event.data.error) | ||
| } | ||
|
|
||
| resolve(event.data) | ||
| } | ||
|
|
||
| client.postMessage(message, [ | ||
| channel.port2, | ||
| ...transferrables.filter(Boolean), | ||
| ]) | ||
| }) | ||
| } | ||
|
|
||
| /** | ||
| * @param {Response} response | ||
| * @returns {Response} | ||
| */ | ||
| function respondWithMock(response) { | ||
| // Setting response status code to 0 is a no-op. | ||
| // However, when responding with a "Response.error()", the produced Response | ||
| // instance will have status code set to 0. Since it's not possible to create | ||
| // a Response instance with status code 0, handle that use-case separately. | ||
| if (response.status === 0) { | ||
| return Response.error() | ||
| } | ||
|
|
||
| const mockedResponse = new Response(response.body, response) | ||
|
|
||
| Reflect.defineProperty(mockedResponse, IS_MOCKED_RESPONSE, { | ||
| value: true, | ||
| enumerable: true, | ||
| }) | ||
|
|
||
| return mockedResponse | ||
| } | ||
|
|
||
| /** | ||
| * @param {Request} request | ||
| */ | ||
| async function serializeRequest(request) { | ||
| return { | ||
| url: request.url, | ||
| mode: request.mode, | ||
| method: request.method, | ||
| headers: Object.fromEntries(request.headers.entries()), | ||
| cache: request.cache, | ||
| credentials: request.credentials, | ||
| destination: request.destination, | ||
| integrity: request.integrity, | ||
| redirect: request.redirect, | ||
| referrer: request.referrer, | ||
| referrerPolicy: request.referrerPolicy, | ||
| body: await request.arrayBuffer(), | ||
| keepalive: request.keepalive, | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Treat this MSW worker file as the single source of truth
This looks like the standard mockServiceWorker.js generated by the MSW CLI (with integrity checks, message protocol, etc.), and it’s already annotated as “do NOT modify”.
Given that, it’s best to:
- Keep this file entirely generated (regenerate via MSW CLI when upgrading
msw), and - Ensure your bundling config (e.g.,
webpack.config.ts) copies this file when needed, rather than pulling in other MSW artifacts that might diverge from it.
That will keep the client and worker behavior consistent across environments.
🌐 Web query:
Check MSW's documentation on how `mockServiceWorker.js` is generated and kept in sync with the installed `msw` version, and whether it should be committed vs copied from `node_modules`.
💡 Result:
Short answer: MSW provides an init CLI that copies the packaged worker into your public dir (npx msw init <PUBLIC_DIR> --save). The docs recommend committing mockServiceWorker.js to Git so everyone uses the same worker, but also note you may treat it as a generated artifact and generate it automatically (CI/install step) if you prefer. The client accepts any worker within the same major MSW version, but MSW still recommends keeping the copied worker up-to-date with the installed msw and performs an integrity check (you can configure msw.workerDirectory). [1][2][3]
Key doc points:
- How it’s produced: run npx msw init <PUBLIC_DIR> to copy the worker from the msw package into your app. [1][2]
- Commit vs generated: recommended to commit mockServiceWorker.js to Git for team consistency; alternative is to generate it automatically as part of project setup/CI. [1][3]
- Version/sync: the client can use a worker within the same major version, but MSW recommends keeping it in sync and runs an integrity check that may surface an “outdated Service Worker” message; use msw.workerDirectory if needed. [1][3]
Sources:
- MSW “Managing the worker” / generating & updating the worker. [1]
- MSW “Browser integration” / init command. [2]
- MSW FAQ (Should I commit the worker script?). [3]
Align worker file handling with MSW's recommended practice
MSW's recommended approach is to commit mockServiceWorker.js to Git for team consistency, though treating it as a generated artifact and auto-generating it during CI/install is an alternative. The file should not be manually edited (as noted in the existing comment).
For your setup:
- Use
npx msw init <PUBLIC_DIR>to copy the worker from the msw package when updating to a new MSW version, then commit the result. - MSW runs an integrity check and may surface an "outdated Service Worker" message if the worker drifts from the installed msw version, so keep them in sync.
- Ensure your bundling config copies this committed worker as needed rather than pulling from elsewhere.
🤖 Prompt for AI Agents
apps/standalone/public/mockServiceWorker.js lines 1-349: The committed
mockServiceWorker.js is currently treated as a generated artifact but the review
requests aligning with MSW best practices; update your workflow so this file is
produced by running `npx msw init <PUBLIC_DIR>` from the project (or copy the
worker from the msw package) whenever MSW is upgraded, commit the resulting file
to source control, and ensure your bundler/static asset pipeline serves the
committed file from apps/standalone/public (do not modify its contents
manually).
| const quadlet = lines.join('\n'); | ||
| return { | ||
| name, | ||
| appType: AppType.AppTypeQuadlet, |
There was a problem hiding this comment.
This doesn't seem right.
The user knows there are several types of applications in the system.
We let them create a "container" application and we transform it to a quadlet application. So next time they want to edit it etc, they will see it as a quadlet application that maybe they are not familiar with.
And we'd be dealing with possible conversion issues that we're unaware of. We should respect what the user defined and store the application as a single container application.
| name: app.name, | ||
| appType: AppType.AppTypeCompose, | ||
| inline: app.files.map( | ||
| name: (app as InlineAppForm).name, |
There was a problem hiding this comment.
Types need to be adjusted using npm run gen-types.
That might remove the need for some of the castings you're doing here, as there was an "Artifact" application type which was quickly dropped.
| ); | ||
| } | ||
| // ensure appType stays 'quadlet' for container format | ||
| if (app.inlineFormat === 'container' && app.appType !== ('quadlet' as unknown as InlineAppForm['appType'])) { |
There was a problem hiding this comment.
Same as previous comment, we shouldn't be mixing two application types when they exist in the system independent to each other.
| </FormGroup> | ||
| </SplitItem> | ||
| <SplitItem> | ||
| <FormGroup label={t('Protocol')}> |
There was a problem hiding this comment.
Protocols are not defined in the spec flightctl/flightctl#1955
| <TextField name={`${mName}.name`} isDisabled={isReadOnly} /> | ||
| </FormGroup> | ||
| </SplitItem> | ||
| <SplitItem isFilled> |
There was a problem hiding this comment.
There are several types of application volumes, each has a different set of properties.
export type ApplicationVolume = ({
/**
* Unique name of the volume used within the application.
*/
name: string;
} & (ImageVolumeProviderSpec | MountVolumeProviderSpec | ImageMountVolumeProviderSpec));
| )} | ||
| </FieldArray> | ||
|
|
||
| <FormGroup label={t('Memory limit')}> |
There was a problem hiding this comment.
API only defines "cpu" and "memory" limits.
|
Full implementation here #382 |
2 participants
Vibe coding - Adding support for container application type
Summary by CodeRabbit
Release Notes
New Features
dev:mocknpm script for convenient mock-enabled developmentDocumentation