feat(docker): add real-time container update progress streaming #1869
Conversation
Implement GraphQL subscription for streaming Docker container update progress in real-time. This replaces the static spinner during container updates with detailed progress information including per-layer download progress. Backend: - Add DockerUpdateProgress GraphQL type with layer progress details - Add DockerUpdateProgressService to parse docker pull output and stream events - Add dockerUpdateProgress subscription to DockerResolver - Integrate progress tracking into DockerService.updateContainer() Frontend: - Add useDockerUpdateProgress composable for subscription management - Add DockerUpdateProgressModal component showing update progress - Integrate progress modal into DockerContainersTable - Add onUpdateStart/onUpdateComplete callbacks to useDockerUpdateActions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughThis pull request adds real-time Docker container update progress tracking. It introduces GraphQL types for update events, a backend service to stream per-layer progress via external script execution, a pubsub channel for event delivery, frontend composables for state management, and UI components to visualize ongoing updates with per-layer progress details. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend UI
participant Comp as Update Composable
participant Sub as GraphQL Subscription
participant API as Backend API
participant Svc as Update Service
participant Script as Update Script
participant PubSub as PubSub Channel
UI->>Comp: startTracking(containerId, containerName)
Comp->>Sub: subscribe(DOCKER_UPDATE_PROGRESS_SUBSCRIPTION)
Sub->>API: connect WebSocket
API->>Svc: updateContainerWithProgress(containerId, containerName)
Svc->>Script: spawn update script with DOCKER_CLI_FORMAT=json
Script-->>Svc: stdout JSON progress events & logs
Script-->>Svc: stderr (captured & logged)
loop Each progress line
Svc->>Svc: parseDockerJson() or processLayerStatus()
Svc->>Svc: updateLayerProgress & calculateOverallProgress()
Svc->>PubSub: publishProgress(DockerUpdateProgress)
PubSub->>Sub: emit progress event
Sub->>Comp: receive updated progress
Comp->>Comp: updateContainerState(containerId, progress)
Comp->>UI: reactively update modal (progress, layers, status)
end
Script-->>Svc: process exits
Svc->>PubSub: publishProgress(type: COMPLETE or ERROR)
PubSub->>Sub: emit final event
Sub->>Comp: mark container as complete
Comp->>UI: update modal visibility & clear on demand
Svc->>Svc: cleanupActiveUpdate(containerId)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 764ff0341e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @Field(() => Int, { nullable: true, description: 'Bytes downloaded/processed' }) | ||
| current?: number; | ||
|
|
||
| @Field(() => Int, { nullable: true, description: 'Total bytes for this layer' }) | ||
| total?: number; |
There was a problem hiding this comment.
The current and total fields are exposed as GraphQL Int, which is limited to 32‑bit signed values. Docker layer sizes can exceed 2,147,483,647 bytes for large images, and when that happens GraphQL serialization throws ("Int cannot represent non 32-bit signed integer"), causing the subscription to error and progress streaming to break for those updates. Consider switching these fields to GraphQLBigInt (as used elsewhere for byte sizes) or another 64‑bit friendly scalar.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts:
- Around line 277-299: In calculateOverallProgress, the final average
incorrectly divides totalProgress by layers.length; change the denominator to
countedLayers so the average is computed only over layers that contributed (keep
the early return when countedLayers === 0); update the return to use
Math.round(totalProgress / countedLayers) instead of Math.round(totalProgress /
layers.length) and keep existing logic for accumulating totalProgress and
countedLayers in the for loop.
In @web/src/components/Docker/DockerUpdateProgressModal.vue:
- Around line 48-57: The sorting treats unknown statuses as -1 (from findIndex)
which sorts them before known ones; in the computed sortedLayers you should
coerce aIndex and bIndex to place unknowns after known statuses (e.g., if
findIndex returns -1, set index = statusOrder.length) before comparing; update
the code in the sortedLayers computed (statusOrder, aIndex, bIndex) to use this
adjusted index and keep the tie-breaker of a.layerId.localeCompare(b.layerId).
In @web/src/composables/useDockerUpdateProgress.ts:
- Around line 1-11: The imports of local modules in useDockerUpdateProgress.ts
are missing the required .js extension; update the import paths for
DOCKER_UPDATE_PROGRESS_SUBSCRIPTION and the generated GraphQL types to include
the .js extension (e.g. change import from
'@/components/Docker/docker-update-progress.subscription' to
'@/components/Docker/docker-update-progress.subscription.js' and
'@/composables/gql/graphql' to '@/composables/gql/graphql.js') so the .ts file
follows the repo guideline for local imports.
- Around line 42-70: The new per-container state is created as a plain object
and then mutated, which bypasses Vue reactivity; wrap the newly-created state
with Vue's reactive() before mutating or storing it so subsequent property/Map
changes are tracked. Update the code in useDockerUpdateProgress.ts to import
reactive from 'vue' and create/set state = reactive({ ... }) (or call reactive
on the object immediately after creation and before any pushes/sets) so
containerUpdates.set(containerId, state) stores a reactive object and UI updates
will occur.
- Around line 28-86: The subscription result is being cast manually; update the
useSubscription call to pass the generated generics
DockerUpdateProgressSubscription and DockerUpdateProgressSubscriptionVariables
so result.data is correctly typed; then remove the unnecessary casts of
result.data?.dockerUpdateProgress and of type (the DockerUpdateEventType cast)
in the onProgressResult handler and rely on the typed dockerUpdateProgress and
its .type to avoid the as casts.
🧹 Nitpick comments (3)
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts (1)
63-77: Consider closing the readline interface explicitly on error or process termination.The readline interface is created on
updateProcess.stdoutbut not explicitly closed. While it should auto-close when the stream ends, explicitly closing it in error scenarios ensures proper resource cleanup.♻️ Optional: Explicit cleanup
try { if (updateProcess.stdout) { const rl = createInterface({ input: updateProcess.stdout, crlfDelay: Infinity, }); rl.on('line', (line) => { this.processOutputLine(activeUpdate, line); }); rl.on('error', (err) => { this.logger.error(`Error reading update output for ${containerName}`, err); }); + + rl.on('close', () => { + this.logger.debug(`Readline closed for ${containerName}`); + }); }web/src/components/Docker/DockerUpdateProgressModal.vue (1)
4-5: (Optional) Align local TS import to the repo’s “.js extension” convention.This file is
.vue(so the guideline may or may not be enforced here), but if you’re standardizing ESM imports, consider:Proposed change
-import type { ContainerUpdateState } from '@/composables/useDockerUpdateProgress'; +import type { ContainerUpdateState } from '@/composables/useDockerUpdateProgress.js';web/src/composables/useDockerUpdateProgress.ts (1)
56-57: (Optional)eventsgrows unbounded.If updates are long-running/noisy,
state.events.push(progress)can retain a lot of data; consider keeping only the latest N events (or removingeventsif not used by UI).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
api/dev/configs/api.jsonapi/generated-schema.graphqlapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsweb/src/components/Docker/DockerContainersTable.vueweb/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/docker-update-progress.subscription.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
packages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsweb/src/components/Docker/docker-update-progress.subscription.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
packages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsweb/src/components/Docker/docker-update-progress.subscription.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
packages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/generated-schema.graphqlapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsapi/dev/configs/api.jsonweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsweb/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/docker-update-progress.subscription.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsweb/src/components/Docker/DockerContainersTable.vue
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/generated-schema.graphqlapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/docker-update-progress.subscription.tsweb/src/components/Docker/DockerContainersTable.vue
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/components/Docker/docker-update-progress.subscription.ts
🧠 Learnings (19)
📚 Learning: 2025-03-14T16:10:38.562Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the UsePermissions decorator that requires UPDATE permission on the CONFIG resource.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store getter dependencies are properly mocked
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use factory functions for module mocks in Pinia store tests to avoid hoisting issues
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-07-24T18:48:43.590Z
Learnt from: mdatelle
Repo: unraid/api PR: 1533
File: web/components/Docker/Console.vue:22-29
Timestamp: 2025-07-24T18:48:43.590Z
Learning: The Docker Console component in web/components/Docker/Console.vue is temporary/dummy code with placeholder terminal responses, used for building out the UI layout while developing the real components in PR #1533.
Applied to files:
web/src/composables/useDockerUpdateProgress.tsweb/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/DockerContainersTable.vue
📚 Learning: 2025-08-29T19:48:45.517Z
Learnt from: elibosley
Repo: unraid/api PR: 1618
File: web/components/Logs/SingleLogViewer.vue:248-299
Timestamp: 2025-08-29T19:48:45.517Z
Learning: In Vue Apollo's composition API, subscribeToMore automatically handles subscription cleanup when the related query is stopped or the component unmounts, unlike React Apollo where manual cleanup is required. Manual unsubscribe is only needed when changing subscription variables, not for lifecycle management.
Applied to files:
web/src/composables/useDockerUpdateProgress.ts
📚 Learning: 2025-06-11T14:14:30.348Z
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Applied to files:
api/dev/configs/api.json
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in `.query.ts` files
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-09-02T19:35:14.427Z
Learnt from: elibosley
Repo: unraid/api PR: 1639
File: web/components/standalone-mount.ts:38-54
Timestamp: 2025-09-02T19:35:14.427Z
Learning: In the unraid/api project, exposing Apollo client and GraphQL utilities (apolloClient, gql, graphqlParse) on the global window object is intentional to allow developers to interact with GraphQL from the browser console without needing to be inside Vue components. Developer experience is prioritized over minimizing global exposure in this context.
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-01-31T22:01:22.708Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Applied to files:
web/src/composables/gql/graphql.ts
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.ts
📚 Learning: 2025-07-24T18:48:44.035Z
Learnt from: mdatelle
Repo: unraid/api PR: 1533
File: web/components/Docker/Edit.vue:16-32
Timestamp: 2025-07-24T18:48:44.035Z
Learning: In web/components/Docker/Edit.vue, the hardcoded configuration values in the config ref are intentional temporary/dummy data used during initial UI development phase while building out the real Docker components, as clarified by mdatelle in PR #1533.
Applied to files:
web/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/DockerContainersTable.vue
📚 Learning: 2025-02-21T18:59:47.977Z
Learnt from: mdatelle
Repo: unraid/api PR: 1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Applied to files:
web/src/components/Docker/DockerUpdateProgressModal.vue
🧬 Code graph analysis (5)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (4)
api/src/unraid-api/decorators/use-feature-flag.decorator.ts (1)
UseFeatureFlag(20-22)packages/unraid-shared/src/use-permissions.directive.ts (1)
UsePermissions(81-128)web/src/composables/gql/graphql.ts (2)
Subscription(2337-2354)DockerUpdateProgress(992-1010)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts (2)
web/src/composables/gql/graphql.ts (2)
DockerLayerProgress(861-873)DockerUpdateProgress(992-1010)api/src/core/pubsub.ts (1)
pubsub(12-12)
web/src/composables/useDockerUpdateProgress.ts (2)
web/src/composables/gql/graphql.ts (2)
DockerLayerProgress(861-873)DockerUpdateProgress(992-1010)web/src/components/Docker/docker-update-progress.subscription.ts (1)
DOCKER_UPDATE_PROGRESS_SUBSCRIPTION(3-22)
web/src/components/Docker/docker-update-progress.subscription.ts (1)
api/src/unraid-api/cli/generated/gql.ts (1)
gql(96-98)
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts (1)
web/src/composables/gql/graphql.ts (2)
DockerLayerProgress(861-873)DockerUpdateProgress(992-1010)
🔇 Additional comments (22)
api/dev/configs/api.json (1)
9-10: LGTM! Formatting-only change.The addition of a trailing newline is a standard formatting convention with no functional impact.
packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
21-21: LGTM! Clean enum extension.The new
DOCKER_UPDATE_PROGRESSchannel is properly added to support real-time Docker update progress events.api/src/unraid-api/graph/resolvers/docker/docker.module.ts (1)
17-17: LGTM! Proper NestJS module wiring.The
DockerUpdateProgressServiceis correctly imported with the.jsextension and registered as a provider for dependency injection.Also applies to: 41-41
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts (2)
15-15: LGTM! Proper test mock setup.The
DockerUpdateProgressServicemock is correctly structured with all necessary methods (updateContainerWithProgress,isUpdating,getActiveUpdates) and follows the project's testing patterns.Also applies to: 171-176
220-221: LGTM! Correct mock lifecycle and DI registration.The mock is properly reset in
beforeEachand correctly registered as a test provider usinguseValue, ensuring clean test isolation.Also applies to: 254-257
web/src/components/Docker/docker-update-progress.subscription.ts (1)
1-22: LGTM! Well-structured GraphQL subscription.The subscription document correctly queries all necessary fields for Docker update progress tracking. Verification confirms that
pnpm codegenhas been executed in the web directory and the corresponding TypeScript types for this subscription are properly generated inweb/src/composables/gql/graphql.ts, including theDockerUpdateProgressSubscriptiontype with all fields correctly typed.api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
389-401: LGTM!The new
dockerUpdateProgresssubscription correctly follows the established patterns in this resolver:
- Feature flag guard via
@UseFeatureFlag- Proper
READ_ANYpermission onDOCKERresource- Consistent use of
subscriptionHelper.createTrackedSubscription- Payload resolution matching other subscriptions
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (2)
40-42: LGTM!The circular dependency between
DockerServiceandDockerUpdateProgressServiceis correctly handled usingforwardRef. The injection pattern is appropriate for this use case.
340-345: Clean delegation to progress service.The update flow now delegates to
DockerUpdateProgressService.updateContainerWithProgress()while preserving the existing error handling and logging. The method signature remains unchanged, maintaining backward compatibility.web/src/components/Docker/DockerContainersTable.vue (3)
208-213: LGTM!The callbacks are correctly wired. The comment on line 212 accurately explains that
onUpdateCompleteis intentionally a no-op since the subscription handles completion events, but the callback is still useful for the actions composable to call.
708-716: Good UX improvement with clickable progress indicator.The update progress indicator is now interactive, allowing users to click for details. The styling uses appropriate Tailwind classes and the "Click for details" hint provides clear affordance.
790-798: Modal integration looks correct.The
DockerUpdateProgressModalis properly wired with two-way bindings foropenandactive-container-id, and the@clear-completedevent is correctly handled.api/generated-schema.graphql (2)
1358-1414: Generated schema matches model definitions.The auto-generated types
DockerLayerProgress,DockerUpdateProgress, and enumDockerUpdateEventTypecorrectly reflect the NestJS model definitions. Field types and nullability are consistent.
2959-2961: Subscription schema correctly added.The
dockerUpdateProgresssubscription returnsDockerUpdateProgress!(non-nullable) with appropriate description.web/src/composables/useDockerUpdateActions.ts (2)
17-18: LGTM!The new optional callbacks provide clean extension points for tracking update lifecycle events. The typing is correct with both callbacks being optional.
134-138: Consider: onUpdateComplete is called for all rows even on partial failure.In bulk operations,
onUpdateCompleteis called for every row in thefinallyblock regardless of whether individual containers succeeded or failed. This is acceptable given that:
- The GraphQL subscription provides per-container completion/error events
- The callbacks are primarily used to trigger UI updates (start/stop tracking)
The current approach keeps the UI in sync even when errors occur.
Also applies to: 157-161
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts (2)
26-29: Good guard against concurrent updates.The check for existing updates on the same container prevents race conditions when rapid update requests are made.
40-52: Path follows established pattern and is consistent with other Unraid plugin script references in the codebase.The hardcoded path
/usr/local/emhttp/plugins/dynamix.docker.manager/scripts/update_containeraligns with how other dynamix.docker.manager resources are referenced throughout the codebase (e.g.,DockerUpdate.phpandDockerContainers.php), and matches the project's standard pattern for Unraid plugin script references. No action needed.api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts (2)
5-20: Well-structured enum with comprehensive event types.The
DockerUpdateEventTypeenum covers the full lifecycle of a Docker update operation including per-layer progress states. The GraphQL registration with description is correct.
22-71: LGTM!Both
DockerLayerProgressandDockerUpdateProgressclasses are properly decorated with GraphQL@Fielddecorators, appropriate nullability, and descriptive documentation. The types align with the generated schema and client-side TypeScript types.web/src/composables/gql/gql.ts (1)
53-54: Ensurepnpm codegenwas run to regenerate this file.The GraphQL mappings for
DockerUpdateProgressandUpdateDockerViewPreferencesare consistently added across the type map (lines 53-54), runtime map (lines 124-125), and function overloads (lines 320+), matching the expected structure from GraphQL CodeGen.web/src/composables/gql/graphql.ts (1)
860-873: LGTM! Auto-generated GraphQL types look correct.The GraphQL code generation has properly added the Docker update progress types and subscription:
DockerLayerProgressfor per-layer trackingDockerUpdateEventTypeenum with comprehensive event statesDockerUpdateProgressmain type with flexible optional fields- Subscription integration is correctly structured
As per coding guidelines, ensure
pnpm codegenwas run in the web directory to generate this file.Also applies to: 978-1010, 2341-2342, 3168-3171, 3422-3422
| private calculateOverallProgress(update: ActiveUpdate): number { | ||
| const layers = Array.from(update.layers.values()); | ||
| if (layers.length === 0) return 0; | ||
|
|
||
| let totalProgress = 0; | ||
| let countedLayers = 0; | ||
|
|
||
| for (const layer of layers) { | ||
| if (layer.progress !== undefined) { | ||
| totalProgress += layer.progress; | ||
| countedLayers++; | ||
| } else if ( | ||
| layer.status.toLowerCase().includes('complete') || | ||
| layer.status.toLowerCase().includes('already exists') | ||
| ) { | ||
| totalProgress += 100; | ||
| countedLayers++; | ||
| } | ||
| } | ||
|
|
||
| if (countedLayers === 0) return 0; | ||
| return Math.round(totalProgress / layers.length); | ||
| } |
There was a problem hiding this comment.
Bug: Progress calculation divides by total layers instead of counted layers.
The denominator uses layers.length but only layers with defined progress or completed status contribute to totalProgress. This can underestimate overall progress.
For example: If 2 of 4 layers are complete (100 each) and 2 have undefined progress, the result is 200/4=50% instead of the more accurate 200/2=100% for the layers we have data for.
🐛 Proposed fix
private calculateOverallProgress(update: ActiveUpdate): number {
const layers = Array.from(update.layers.values());
if (layers.length === 0) return 0;
let totalProgress = 0;
let countedLayers = 0;
for (const layer of layers) {
if (layer.progress !== undefined) {
totalProgress += layer.progress;
countedLayers++;
} else if (
layer.status.toLowerCase().includes('complete') ||
layer.status.toLowerCase().includes('already exists')
) {
totalProgress += 100;
countedLayers++;
}
}
if (countedLayers === 0) return 0;
- return Math.round(totalProgress / layers.length);
+ return Math.round(totalProgress / countedLayers);
}🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts
around lines 277 - 299, In calculateOverallProgress, the final average
incorrectly divides totalProgress by layers.length; change the denominator to
countedLayers so the average is computed only over layers that contributed (keep
the early return when countedLayers === 0); update the return to use
Math.round(totalProgress / countedLayers) instead of Math.round(totalProgress /
layers.length) and keep existing logic for accumulating totalProgress and
countedLayers in the for loop.
| const sortedLayers = computed(() => { | ||
| if (!props.activeState) return []; | ||
| return Array.from(props.activeState.layers.values()).sort((a, b) => { | ||
| const statusOrder = ['Downloading', 'Extracting', 'Pull complete', 'Already exists']; | ||
| const aIndex = statusOrder.findIndex((s) => a.status.toLowerCase().includes(s.toLowerCase())); | ||
| const bIndex = statusOrder.findIndex((s) => b.status.toLowerCase().includes(s.toLowerCase())); | ||
| if (aIndex !== bIndex) return aIndex - bIndex; | ||
| return a.layerId.localeCompare(b.layerId); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Fix layer sorting: unknown statuses currently sort before known ones.
findIndex() returns -1 for unknown statuses, so aIndex - bIndex can float unknowns to the top.
Proposed fix
return Array.from(props.activeState.layers.values()).sort((a, b) => {
const statusOrder = ['Downloading', 'Extracting', 'Pull complete', 'Already exists'];
- const aIndex = statusOrder.findIndex((s) => a.status.toLowerCase().includes(s.toLowerCase()));
- const bIndex = statusOrder.findIndex((s) => b.status.toLowerCase().includes(s.toLowerCase()));
- if (aIndex !== bIndex) return aIndex - bIndex;
+ const aIndexRaw = statusOrder.findIndex((s) => a.status.toLowerCase().includes(s.toLowerCase()));
+ const bIndexRaw = statusOrder.findIndex((s) => b.status.toLowerCase().includes(s.toLowerCase()));
+ const aIndex = aIndexRaw === -1 ? Number.POSITIVE_INFINITY : aIndexRaw;
+ const bIndex = bIndexRaw === -1 ? Number.POSITIVE_INFINITY : bIndexRaw;
+ if (aIndex !== bIndex) return aIndex - bIndex;
return a.layerId.localeCompare(b.layerId);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sortedLayers = computed(() => { | |
| if (!props.activeState) return []; | |
| return Array.from(props.activeState.layers.values()).sort((a, b) => { | |
| const statusOrder = ['Downloading', 'Extracting', 'Pull complete', 'Already exists']; | |
| const aIndex = statusOrder.findIndex((s) => a.status.toLowerCase().includes(s.toLowerCase())); | |
| const bIndex = statusOrder.findIndex((s) => b.status.toLowerCase().includes(s.toLowerCase())); | |
| if (aIndex !== bIndex) return aIndex - bIndex; | |
| return a.layerId.localeCompare(b.layerId); | |
| }); | |
| }); | |
| const sortedLayers = computed(() => { | |
| if (!props.activeState) return []; | |
| return Array.from(props.activeState.layers.values()).sort((a, b) => { | |
| const statusOrder = ['Downloading', 'Extracting', 'Pull complete', 'Already exists']; | |
| const aIndexRaw = statusOrder.findIndex((s) => a.status.toLowerCase().includes(s.toLowerCase())); | |
| const bIndexRaw = statusOrder.findIndex((s) => b.status.toLowerCase().includes(s.toLowerCase())); | |
| const aIndex = aIndexRaw === -1 ? Number.POSITIVE_INFINITY : aIndexRaw; | |
| const bIndex = bIndexRaw === -1 ? Number.POSITIVE_INFINITY : bIndexRaw; | |
| if (aIndex !== bIndex) return aIndex - bIndex; | |
| return a.layerId.localeCompare(b.layerId); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In @web/src/components/Docker/DockerUpdateProgressModal.vue around lines 48 -
57, The sorting treats unknown statuses as -1 (from findIndex) which sorts them
before known ones; in the computed sortedLayers you should coerce aIndex and
bIndex to place unknowns after known statuses (e.g., if findIndex returns -1,
set index = statusOrder.length) before comparing; update the code in the
sortedLayers computed (statusOrder, aIndex, bIndex) to use this adjusted index
and keep the tie-breaker of a.layerId.localeCompare(b.layerId).
| import { computed, reactive, ref } from 'vue'; | ||
| import { useSubscription } from '@vue/apollo-composable'; | ||
|
|
||
| import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription'; | ||
|
|
||
| import type { | ||
| DockerLayerProgress, | ||
| DockerUpdateEventType, | ||
| DockerUpdateProgress, | ||
| } from '@/composables/gql/graphql'; | ||
|
|
There was a problem hiding this comment.
Use .js extensions for local imports in .ts files (repo guideline).
Proposed change
import { computed, reactive, ref } from 'vue';
import { useSubscription } from '@vue/apollo-composable';
-import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription';
+import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription.js';
import type {
DockerLayerProgress,
DockerUpdateEventType,
DockerUpdateProgress,
-} from '@/composables/gql/graphql';
+} from '@/composables/gql/graphql.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { computed, reactive, ref } from 'vue'; | |
| import { useSubscription } from '@vue/apollo-composable'; | |
| import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription'; | |
| import type { | |
| DockerLayerProgress, | |
| DockerUpdateEventType, | |
| DockerUpdateProgress, | |
| } from '@/composables/gql/graphql'; | |
| import { computed, reactive, ref } from 'vue'; | |
| import { useSubscription } from '@vue/apollo-composable'; | |
| import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription.js'; | |
| import type { | |
| DockerLayerProgress, | |
| DockerUpdateEventType, | |
| DockerUpdateProgress, | |
| } from '@/composables/gql/graphql.js'; |
🤖 Prompt for AI Agents
In @web/src/composables/useDockerUpdateProgress.ts around lines 1 - 11, The
imports of local modules in useDockerUpdateProgress.ts are missing the required
.js extension; update the import paths for DOCKER_UPDATE_PROGRESS_SUBSCRIPTION
and the generated GraphQL types to include the .js extension (e.g. change import
from '@/components/Docker/docker-update-progress.subscription' to
'@/components/Docker/docker-update-progress.subscription.js' and
'@/composables/gql/graphql' to '@/composables/gql/graphql.js') so the .ts file
follows the repo guideline for local imports.
| const { onResult: onProgressResult, onError: onProgressError } = useSubscription( | ||
| DOCKER_UPDATE_PROGRESS_SUBSCRIPTION, | ||
| null, | ||
| () => ({ | ||
| fetchPolicy: 'network-only', | ||
| }) | ||
| ); | ||
|
|
||
| onProgressResult((result) => { | ||
| const progress = result.data?.dockerUpdateProgress as DockerUpdateProgress | undefined; | ||
| if (!progress || !progress.containerId) return; | ||
|
|
||
| const { containerId, containerName, type, message, overallProgress, error, layers } = progress; | ||
|
|
||
| let state = containerUpdates.get(containerId); | ||
| if (!state) { | ||
| state = { | ||
| containerId, | ||
| containerName, | ||
| status: 'pending', | ||
| overallProgress: 0, | ||
| message: '', | ||
| layers: new Map(), | ||
| events: [], | ||
| }; | ||
| containerUpdates.set(containerId, state); | ||
| } | ||
|
|
||
| state.events.push(progress); | ||
|
|
||
| if (message) { | ||
| state.message = message; | ||
| } | ||
|
|
||
| if (overallProgress !== undefined && overallProgress !== null) { | ||
| state.overallProgress = overallProgress; | ||
| } | ||
|
|
||
| if (layers) { | ||
| for (const layer of layers) { | ||
| state.layers.set(layer.layerId, layer); | ||
| } | ||
| } | ||
|
|
||
| const eventType = type as DockerUpdateEventType; | ||
| switch (eventType) { | ||
| case 'STARTED': | ||
| state.status = 'in_progress'; | ||
| break; | ||
| case 'COMPLETE': | ||
| state.status = 'complete'; | ||
| state.overallProgress = 100; | ||
| break; | ||
| case 'ERROR': | ||
| state.status = 'error'; | ||
| state.error = error ?? 'Unknown error'; | ||
| break; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, find the file and examine it
cd web/src/composables && wc -l useDockerUpdateProgress.tsRepository: unraid/api
Length of output: 84
🏁 Script executed:
# Look for the GraphQL operation definition and generated types
find . -name "*docker*" -o -name "*DOCKER*" | head -20Repository: unraid/api
Length of output: 1246
🏁 Script executed:
# Check how useSubscription is imported and typed in the codebase
rg "useSubscription" --type ts --type tsx -A 3 | head -50Repository: unraid/api
Length of output: 81
🏁 Script executed:
# Look for the DOCKER_UPDATE_PROGRESS_SUBSCRIPTION definition
rg "DOCKER_UPDATE_PROGRESS_SUBSCRIPTION" -A 5Repository: unraid/api
Length of output: 1492
🏁 Script executed:
# Check the graphql.ts file for generated subscription types
rg "DockerUpdateProgress" --type ts -B 2 -A 5 | head -60Repository: unraid/api
Length of output: 11272
🏁 Script executed:
# Look at package.json to see Vue Apollo version
cat package.json | grep -A 2 "apollo"Repository: unraid/api
Length of output: 128
🏁 Script executed:
# Look at the graphql.ts file to see DockerUpdateProgress type definition
rg "export type DockerUpdateProgress" -A 15 web/src/composables/gql/graphql.tsRepository: unraid/api
Length of output: 2776
🏁 Script executed:
# Check the graphql.ts file size and look for subscription result types
wc -l web/src/composables/gql/graphql.tsRepository: unraid/api
Length of output: 93
🏁 Script executed:
# Look at the imported types and how DockerUpdateProgress is defined
sed -n '1,100p' web/src/composables/gql/graphql.ts | head -60Repository: unraid/api
Length of output: 3106
🏁 Script executed:
# Check other useSubscription usages to see if they use proper typing
rg "useSubscription" -B 2 -A 10 web/src/ | head -100Repository: unraid/api
Length of output: 8271
🏁 Script executed:
# Look at Vue Apollo types to understand useSubscription signature
find . -name "*.d.ts" -o -name "*.ts" | xargs grep -l "useSubscription" | grep -v node_modules | head -5Repository: unraid/api
Length of output: 151
🏁 Script executed:
# Check Vue Apollo package.json to confirm version
find . -path "*/node_modules/@vue/apollo-composable/package.json" -exec cat {} \; 2>/dev/null | head -20Repository: unraid/api
Length of output: 36
🏁 Script executed:
# Look at the result type structure by checking Vue Apollo types
find . -path "*/node_modules/@vue/apollo-composable/dist/*" -name "*.d.ts" 2>/dev/null | head -5Repository: unraid/api
Length of output: 36
🏁 Script executed:
# Check if there are other subscription usages with proper typing patterns
rg "useSubscription.*<" web/src/ --type ts 2>/dev/null || echo "No generic typed subscriptions found"Repository: unraid/api
Length of output: 90
🏁 Script executed:
# Check the actual useDockerUpdateProgress.ts file fully to see the context
cat -n web/src/composables/useDockerUpdateProgress.ts | head -100Repository: unraid/api
Length of output: 3522
🏁 Script executed:
# Check for how result.data is typed in vue apollo composable
rg "result\.data\?" web/src/ --type ts -A 1 -B 1 | head -80Repository: unraid/api
Length of output: 660
🏁 Script executed:
# Check if DockerUpdateProgressSubscription type is generated (which would have the correct typing)
rg "DockerUpdateProgressSubscription" web/src/composables/gql/ -A 2Repository: unraid/api
Length of output: 6218
🏁 Script executed:
# Check how Vue Apollo types useSubscription - look at the actual exported type
rg "export.*useSubscription" web/src/ -B 2 -A 5 2>/dev/null || echo "Not exported locally"Repository: unraid/api
Length of output: 74
🏁 Script executed:
# Check if there's a .js import that shows the Vue Apollo types being used
rg "from '@vue/apollo-composable'" web/src/ | head -5Repository: unraid/api
Length of output: 472
🏁 Script executed:
# Look at how the Document is cast - checking the full context of DocumentNode typing
rg "DocumentNode<DockerUpdateProgressSubscription" web/src/composables/gql/ -B 2 -A 2Repository: unraid/api
Length of output: 8092
🏁 Script executed:
# Check actual Vue Apollo documentation or types for useSubscription generics
# Search online or check what parameters useSubscription accepts with typing
web_search
Vue Apollo v4 useSubscription TypeScript generic typing result.dataRepository: unraid/api
Length of output: 146
🌐 Web query:
Vue Apollo 4 useSubscription TypeScript generic typing result data
💡 Result:
Short answer — use the same generic pattern as Apollo hooks: call useSubscription with your result and variables types, e.g.
- Signature (concept): useSubscription<TData, TVariables>(DOCUMENT, variables?, options?)
- Example:
import { useSubscription } from '@vue/apollo-composable'
import { gql } from 'graphql-tag'
type MessageSub = { messageAdded: { id: string; text: string } }
type Vars = { roomId: string }
const SUB = gqlsubscription onMessageAdded($roomId: ID!) { messageAdded { id text } }
// use generics to type the returned data
const { result, data, onResult } = useSubscription<MessageSub, Vars>(
SUB,
() => ({ roomId: 'room-1' })
)
// result is a Ref<MessageSub | null | undefined>
console.log(result.value?.messageAdded?.text)
// onResult handler receives typed payload
onResult(({ data: payload }) => {
console.log(payload?.messageAdded.id) // typed
})
References: Vue Apollo useSubscription docs and API (shows composable return values) and Apollo hook typing examples for the generic pattern. [1][2]
Sources:
[1] Vue Apollo — Subscriptions / useSubscription (guide + API).
[2] Apollo docs — useSubscription TypeScript generic pattern.
🏁 Script executed:
# Confirm that DockerUpdateProgressDocument is properly typed
rg "DockerUpdateProgressDocument.*=" web/src/composables/gql/graphql.ts | head -1Repository: unraid/api
Length of output: 1347
🏁 Script executed:
# Check if other similar composables use the generic typing pattern
rg "useSubscription<" web/src/ --type ts || echo "No generically-typed subscriptions found in codebase"Repository: unraid/api
Length of output: 106
Type useSubscription with generics to eliminate unnecessary as casts.
Pass DockerUpdateProgressSubscription and DockerUpdateProgressSubscriptionVariables as type parameters to useSubscription():
const { onResult: onProgressResult, onError: onProgressError } = useSubscription<
DockerUpdateProgressSubscription,
DockerUpdateProgressSubscriptionVariables
>(
DOCKER_UPDATE_PROGRESS_SUBSCRIPTION,
null,
() => ({
fetchPolicy: 'network-only',
})
);This properly types result.data and eliminates the casts on lines 37 and 72. The generated DockerUpdateProgressSubscription type already provides correct typing for dockerUpdateProgress and its nested type property.
🤖 Prompt for AI Agents
In @web/src/composables/useDockerUpdateProgress.ts around lines 28 - 86, The
subscription result is being cast manually; update the useSubscription call to
pass the generated generics DockerUpdateProgressSubscription and
DockerUpdateProgressSubscriptionVariables so result.data is correctly typed;
then remove the unnecessary casts of result.data?.dockerUpdateProgress and of
type (the DockerUpdateEventType cast) in the onProgressResult handler and rely
on the typed dockerUpdateProgress and its .type to avoid the as casts.
| let state = containerUpdates.get(containerId); | ||
| if (!state) { | ||
| state = { | ||
| containerId, | ||
| containerName, | ||
| status: 'pending', | ||
| overallProgress: 0, | ||
| message: '', | ||
| layers: new Map(), | ||
| events: [], | ||
| }; | ||
| containerUpdates.set(containerId, state); | ||
| } | ||
|
|
||
| state.events.push(progress); | ||
|
|
||
| if (message) { | ||
| state.message = message; | ||
| } | ||
|
|
||
| if (overallProgress !== undefined && overallProgress !== null) { | ||
| state.overallProgress = overallProgress; | ||
| } | ||
|
|
||
| if (layers) { | ||
| for (const layer of layers) { | ||
| state.layers.set(layer.layerId, layer); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reactivity bug: newly-created state is mutated via raw reference (may not update UI).
When state is missing, you create a plain object, set() it into the reactive Map, then mutate state (raw) afterwards. Those mutations can bypass Vue proxy tracking.
Proposed fix: make `state` reactive before mutating
let state = containerUpdates.get(containerId);
if (!state) {
- state = {
+ state = reactive<ContainerUpdateState>({
containerId,
containerName,
status: 'pending',
overallProgress: 0,
message: '',
layers: new Map(),
events: [],
- };
+ });
containerUpdates.set(containerId, state);
}
state.events.push(progress);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @web/src/composables/useDockerUpdateProgress.ts around lines 42 - 70, The new
per-container state is created as a plain object and then mutated, which
bypasses Vue reactivity; wrap the newly-created state with Vue's reactive()
before mutating or storing it so subsequent property/Map changes are tracked.
Update the code in useDockerUpdateProgress.ts to import reactive from 'vue' and
create/set state = reactive({ ... }) (or call reactive on the object immediately
after creation and before any pushes/sets) so containerUpdates.set(containerId,
state) stores a reactive object and UI updates will occur.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1869 +/- ##
==========================================
- Coverage 46.40% 46.01% -0.40%
==========================================
Files 954 959 +5
Lines 59791 60453 +662
Branches 5538 5559 +21
==========================================
+ Hits 27749 27818 +69
- Misses 31923 32514 +591
- Partials 119 121 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts:
- Around line 22-38: The DockerLayerProgress model uses GraphQL Int for byte
counters which can overflow; update the file to import GraphQLBigInt from
'graphql-scalars', remove the Int import, and change the @Field type for the
current and total properties to use GraphQLBigInt (keeping nullable and
descriptions), and adjust the TypeScript property types accordingly (e.g.,
BigInt or string as used elsewhere) for the current and total fields in the
DockerLayerProgress class.
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts:
- Around line 301-320: parseBytes can produce values that exceed GraphQL Int
limits (2^31-1) causing serialization errors for Docker layer sizes; update the
GraphQL schema type for progress.current/progress.total from Int to BigInt (and
update the corresponding TypeScript DockerUpdateProgress/interface types and any
resolver return types) so large byte counts serialize correctly, or
alternatively clamp values in parseBytes or right before publishProgress to
Number.MAX_SAFE_INTEGER (or GraphQL Int max) and document that overflowed values
are clamped/omitted; adjust parseBytes, the DockerUpdateProgress type, and
publishProgress to enforce the chosen approach consistently.
- Around line 277-299: In calculateOverallProgress, you're summing progress into
totalProgress and tracking countedLayers but mistakenly divide by layers.length;
change the final average to use countedLayers as the denominator (i.e., return
Math.round(totalProgress / countedLayers)) so the average only considers layers
that contributed to totalProgress (keep the existing countedLayers === 0 guard).
- Around line 40-52: The updateProcess invocation is URL-encoding containerName
and using shell: 'bash'; change it to pass the raw containerName (remove
encodeURIComponent) and stop using shell mode—invoke bash directly by making the
command 'bash' and passing the script path and the raw containerName as args (or
use execa.node/execFile style) so arguments are passed safely; update the execa
call at the updateProcess creation to use the raw containerName and remove the
shell option.
- Around line 63-110: The readline created by createInterface (rl) is never
closed and only stdout is parsed, which can leak listeners and miss stderr
progress; fix by creating and tracking readline interfaces for both
updateProcess.stdout and updateProcess.stderr (or use the combined "all" stream
if available), route each line into processOutputLine (or debug/stderr handling)
as you do for stdout, and ensure rl.close() is called and event listeners
('line', 'error') removed when the update finishes or in the finally block that
deletes activeUpdates; reference symbols: createInterface, rl, updateProcess,
processOutputLine, publishProgress, activeUpdates.
In @web/src/components/Docker/docker-update-progress.subscription.ts:
- Around line 1-22: Replace the raw @apollo/client gql usage with the generated
typed helper: import and call graphql() to define
DOCKER_UPDATE_PROGRESS_SUBSCRIPTION so the subscription uses the generated
types; update the top-level import to use the graphql() factory and wrap the
subscription document with graphql`...` while keeping the exported const name
DOCKER_UPDATE_PROGRESS_SUBSCRIPTION and the same GraphQL selection set
(containerId, containerName, type, message, layerId, overallProgress, error,
layers { layerId, status, progress, current, total }) so the rest of the code
stays compatible but now benefits from type safety.
In @web/src/composables/gql/graphql.ts:
- Around line 860-873: DockerLayerProgress currently types current and total as
Scalars['Int'] which risks overflow for layers >2,147,483,647 bytes; change
those fields in the GraphQL schema/type definition from Int to a BigInt (or
custom scalar like BigInt) so DockerLayerProgress.current and
DockerLayerProgress.total use the BigInt scalar, update any scalar mappings in
your codegen config so generated types reflect the new scalar, and re-run code
generation to propagate the change.
In @web/src/composables/useDockerUpdateProgress.ts:
- Around line 1-10: Update the local/alias ESM imports to include .js
extensions: change the alias import DOCKER_UPDATE_PROGRESS_SUBSCRIPTION from
'@/components/Docker/docker-update-progress.subscription' to
'@/components/Docker/docker-update-progress.subscription.js' and change the
types import from '@/composables/gql/graphql' to '@/composables/gql/graphql.js';
do not alter bare imports from 'vue' or '@vue/apollo-composable'.
- Around line 24-86: When creating new entries in the reactive containerUpdates
Map inside the onProgressResult handler, wrap the new state object with Vue's
reactive() so mutations are tracked (e.g., make state = reactive({ containerId,
containerName, status: 'pending', overallProgress: 0, message: '', layers:
reactive(new Map()), events: [] }) and then containerUpdates.set(containerId,
state)); also ensure layers is created as reactive(new Map()) so subsequent
state.layers.set(...) calls are reactive; update references to state and
state.layers in the rest of the onProgressResult callback accordingly.
🧹 Nitpick comments (12)
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts (1)
14-15: Type the service mock (and clear vi mocks) to prevent drift/flaky tests.Right now
mockDockerUpdateProgressServiceis an untyped object ofvi.fn()s; a signature change inDockerUpdateProgressServicecould silently compile but fail at runtime. Also, adding morevi.fn()state increases the value ofvi.clearAllMocks()between tests.Possible refactor
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { beforeEach } from 'vitest'; @@ -const mockDockerUpdateProgressService = { +const mockDockerUpdateProgressService: Pick< + DockerUpdateProgressService, + 'updateContainerWithProgress' | 'isUpdating' | 'getActiveUpdates' +> = { updateContainerWithProgress: vi.fn().mockResolvedValue(undefined), isUpdating: vi.fn().mockReturnValue(false), getActiveUpdates: vi.fn().mockReturnValue([]), }; @@ describe.runIf(dockerAvailable)('DockerService Integration', () => { @@ + beforeEach(() => { + vi.clearAllMocks(); + });Also applies to: 32-37, 86-88
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
390-401: Consider server-side filtering (e.g., bycontainerId) to avoid broadcasting all updates to every subscriber.
Right now, any client withREAD_ANYreceives progress events for every update operation; adding an optional@Args('containerId')(or similar) + filtering would reduce noise and potential information leakage.api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts (1)
15-16: Type the mock + reset all its methods to avoid drift and cross-test leakage.
Consider typing asPick<DockerUpdateProgressService, 'updateContainerWithProgress' | 'isUpdating' | 'getActiveUpdates'>(or usingvi.mocked(...)) and resettingisUpdating/getActiveUpdatescall history inbeforeEachas well.Also applies to: 171-176, 220-222, 254-257
web/src/composables/useDockerUpdateActions.ts (2)
12-19: Callback API semantics:onUpdateCompletecurrently means “ended”, not “succeeded”.Because
onUpdateCompleteis invoked fromfinally(including error paths), consider either (a) renaming toonUpdateEnd, or (b) adding a success flag / error param so callers can distinguish success vs failure.Also applies to: 21-28
95-120: Avoid duplicate callback invocations in bulk/update-all flows.
onUpdateStart/onUpdateCompleteare called per-row; ifrowscan contain duplicates (or multiple rows mapping to the samecontainerId), callbacks may fire multiple times for the same container. Consider de-duping bycontainerIdbefore invoking callbacks (mirroring the de-dupe used forcontainerIds).Also applies to: 122-163, 165-207
web/src/components/Docker/DockerUpdateProgressModal.vue (2)
48-57:sortedLayersordering: treat “unknown” statuses as last, not first.
findIndexreturns-1, which currently sorts ahead of known statuses. Consider normalizing-1to a large value (e.g.,Number.POSITIVE_INFINITY) before comparing.
112-172: Multi-container UX: consider auto-selecting the first container when none is active.When
containerStates.length > 1andactiveContainerIdis null, users get an empty details panel until they pick a container. IfuseDockerUpdateProgressdoesn’t already handle this, consider defaultingcurrentContainerIdto the first option when opening the modal.web/src/components/Docker/DockerContainersTable.vue (2)
203-214: Drop the no-oponUpdateComplete(and its comment) instead of passing an empty handler.Since
onUpdateCompleteis optional, you can omit it entirely and keep the call-site noise/comment out of the table component.
708-716: Addtype="button"to prevent accidental form submission.This is a plain
<button>in a toolbar; addingtype="button"avoids unintended submits if the table ever ends up inside a<form>.Proposed diff
- <button + <button + type="button" v-if="isUpdatingContainers && activeUpdateSummary" class="border-primary/30 bg-primary/5 text-primary hover:bg-primary/10 my-2 flex w-full cursor-pointer items-center gap-2 rounded border px-3 py-2 text-left text-sm transition-colors" @click="updateProgress.openModal()" >web/src/composables/useDockerUpdateProgress.ts (3)
118-131:clearCompleted()deletes from a Map while iterating—works, but easy to make safer.Deleting during iteration is usually okay for
Map, but collecting keys first avoids edge-case surprises and keeps intent clearer.Proposed diff
function clearCompleted() { - for (const [id, state] of containerUpdates.entries()) { - if (state.status === 'complete' || state.status === 'error') { - containerUpdates.delete(id); - } - } + const toDelete: string[] = []; + for (const [id, state] of containerUpdates.entries()) { + if (state.status === 'complete' || state.status === 'error') toDelete.push(id); + } + for (const id of toDelete) containerUpdates.delete(id); if (activeContainerId.value && !containerUpdates.has(activeContainerId.value)) { const remaining = Array.from(containerUpdates.keys()); activeContainerId.value = remaining.length > 0 ? remaining[0] : null; } if (containerUpdates.size === 0) { isModalOpen.value = false; } }
56-70: Unboundedeventsgrowth could leak memory during long sessions.If the UI doesn’t need the full history, consider capping
state.events(e.g., last N events) or storing only the latest event per layer + latest message.
28-40: Use TypedDocumentNode to eliminate casts and prefer undefined over null for variables.The
fetchPolicy: 'network-only'is correct for subscriptions. However, when a subscription has no variables, passundefined(or omit the parameter) instead ofnull—this aligns with GraphQL semantics where omitted variables are distinct from null values. More importantly, use a TypedDocumentNode for the subscription query so Vue Apollo automatically typesresult.datawithout requiring casts. This will eliminate theas DockerUpdateProgress | undefinedcast and provide full type safety.Applies to lines 28-40 and 72-85.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
api/dev/configs/api.jsonapi/generated-schema.graphqlapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tspackages/unraid-shared/src/pubsub/graphql.pubsub.tsweb/src/components/Docker/DockerContainersTable.vueweb/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/docker-update-progress.subscription.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
packages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsweb/src/components/Docker/docker-update-progress.subscription.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
packages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsweb/src/components/Docker/docker-update-progress.subscription.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
packages/unraid-shared/src/pubsub/graphql.pubsub.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsweb/src/components/Docker/docker-update-progress.subscription.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsweb/src/components/Docker/DockerUpdateProgressModal.vueapi/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/docker/docker.module.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsweb/src/composables/gql/gql.tsweb/src/components/Docker/DockerContainersTable.vueweb/src/composables/gql/graphql.tsapi/generated-schema.graphql
api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer adding new files to the NestJS repo located at
api/src/unraid-api/instead of the legacy code
Files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/generated-schema.graphql
api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
cache-manager v7 expects TTL values in milliseconds, not seconds (e.g., 600000 for 10 minutes, not 600)
Files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.module.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsapi/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/Docker/docker-update-progress.subscription.tsweb/src/components/Docker/DockerUpdateProgressModal.vueweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsweb/src/composables/gql/gql.tsweb/src/components/Docker/DockerContainersTable.vueweb/src/composables/gql/graphql.ts
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/components/Docker/docker-update-progress.subscription.tsweb/src/composables/useDockerUpdateActions.tsweb/src/composables/useDockerUpdateProgress.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.ts
🧠 Learnings (19)
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Mock external services and API calls
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in Vue component tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:51:46.348Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-11-24T17:51:46.348Z
Learning: Applies to api/**/*.test.{ts,tsx} : Prefer to not mock simple dependencies
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Mock external dependencies appropriately in Pinia store tests
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Use `vi.mock()` for module-level mocks
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-08-09T01:03:29.676Z
Learnt from: elibosley
Repo: unraid/api PR: 1575
File: packages/unraid-shared/src/services/socket-config.service.spec.ts:10-13
Timestamp: 2025-08-09T01:03:29.676Z
Learning: Vitest is used for all testing across all repositories in the unraid organization, not Jest. Always use `vi` for mocking utilities, not `jest`.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.ts
📚 Learning: 2025-11-24T17:52:26.907Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.907Z
Learning: Applies to **/*.test.ts : Reset mocks between tests with `vi.clearAllMocks()`
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.integration.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-07-24T18:48:43.590Z
Learnt from: mdatelle
Repo: unraid/api PR: 1533
File: web/components/Docker/Console.vue:22-29
Timestamp: 2025-07-24T18:48:43.590Z
Learning: The Docker Console component in web/components/Docker/Console.vue is temporary/dummy code with placeholder terminal responses, used for building out the UI layout while developing the real components in PR #1533.
Applied to files:
web/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/DockerContainersTable.vue
📚 Learning: 2025-07-24T18:48:44.035Z
Learnt from: mdatelle
Repo: unraid/api PR: 1533
File: web/components/Docker/Edit.vue:16-32
Timestamp: 2025-07-24T18:48:44.035Z
Learning: In web/components/Docker/Edit.vue, the hardcoded configuration values in the config ref are intentional temporary/dummy data used during initial UI development phase while building out the real Docker components, as clarified by mdatelle in PR #1533.
Applied to files:
web/src/components/Docker/DockerUpdateProgressModal.vueweb/src/components/Docker/DockerContainersTable.vue
📚 Learning: 2025-02-21T18:59:47.977Z
Learnt from: mdatelle
Repo: unraid/api PR: 1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Applied to files:
web/src/components/Docker/DockerUpdateProgressModal.vue
📚 Learning: 2025-06-11T14:14:30.348Z
Learnt from: pujitm
Repo: unraid/api PR: 1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
Applied to files:
api/dev/configs/api.json
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
Repo: unraid/api PR: 942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.module.ts
📚 Learning: 2025-08-29T19:48:45.517Z
Learnt from: elibosley
Repo: unraid/api PR: 1618
File: web/components/Logs/SingleLogViewer.vue:248-299
Timestamp: 2025-08-29T19:48:45.517Z
Learning: In Vue Apollo's composition API, subscribeToMore automatically handles subscription cleanup when the related query is stopped or the component unmounts, unlike React Apollo where manual cleanup is required. Manual unsubscribe is only needed when changing subscription variables, not for lifecycle management.
Applied to files:
web/src/composables/useDockerUpdateProgress.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
Repo: unraid/api PR: 1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsweb/src/composables/gql/gql.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Test Pinia store getter dependencies are properly mocked
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:52:26.908Z
Learnt from: CR
Repo: unraid/api PR: 0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-11-24T17:52:26.908Z
Learning: Applies to **/__test__/store/**/*.ts : Use factory functions for module mocks in Pinia store tests to avoid hoisting issues
Applied to files:
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts
📚 Learning: 2025-11-24T17:51:37.915Z
Learnt from: CR
Repo: unraid/api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:51:37.915Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in `.query.ts` files
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-09-02T19:35:14.427Z
Learnt from: elibosley
Repo: unraid/api PR: 1639
File: web/components/standalone-mount.ts:38-54
Timestamp: 2025-09-02T19:35:14.427Z
Learning: In the unraid/api project, exposing Apollo client and GraphQL utilities (apolloClient, gql, graphqlParse) on the global window object is intentional to allow developers to interact with GraphQL from the browser console without needing to be inside Vue components. Developer experience is prioritized over minimizing global exposure in this context.
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-01-31T22:01:22.708Z
Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Applied to files:
web/src/composables/gql/graphql.ts
🧬 Code graph analysis (5)
web/src/components/Docker/docker-update-progress.subscription.ts (1)
api/src/unraid-api/cli/generated/gql.ts (1)
gql(96-98)
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (3)
packages/unraid-shared/src/use-permissions.directive.ts (1)
UsePermissions(81-128)web/src/composables/gql/graphql.ts (2)
Subscription(2337-2354)DockerUpdateProgress(992-1010)web/composables/gql/graphql.ts (1)
Subscription(2032-2044)
web/src/composables/useDockerUpdateProgress.ts (2)
web/src/composables/gql/graphql.ts (2)
DockerLayerProgress(861-873)DockerUpdateProgress(992-1010)web/src/components/Docker/docker-update-progress.subscription.ts (1)
DOCKER_UPDATE_PROGRESS_SUBSCRIPTION(3-22)
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts (1)
web/src/composables/gql/graphql.ts (2)
DockerLayerProgress(861-873)DockerUpdateProgress(992-1010)
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts (2)
web/src/composables/gql/graphql.ts (2)
DockerLayerProgress(861-873)DockerUpdateProgress(992-1010)api/src/core/pubsub.ts (1)
pubsub(12-12)
🔇 Additional comments (12)
api/dev/configs/api.json (1)
9-10: Formatting-only change (trailing newline) looks good.api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts (1)
5-20: Enum + registration look correct.packages/unraid-shared/src/pubsub/graphql.pubsub.ts (1)
5-24: New pubsub channel addition is clean and low-risk.api/src/unraid-api/graph/resolvers/docker/docker.module.ts (1)
17-18: Provider wiring for DockerUpdateProgressService looks correct.Also applies to: 41-42
api/src/unraid-api/graph/resolvers/docker/docker.resolver.ts (1)
26-26: Import looks correct and consistent with ESM.jsimports.api/generated-schema.graphql (1)
2959-2962: Verify expected client behavior whenENABLE_NEXT_DOCKER_RELEASEis off but the subscription field still appears in schema.
If this is intentional, all good; otherwise consider schema-level hiding or clearer error semantics.api/src/unraid-api/graph/resolvers/docker/docker.service.ts (2)
326-358: Verify terminal progress semantics (COMPLETE/ERROR + cleanup) so UIs don’t hang on failures.
updateContainer()now delegates the update; ensureDockerUpdateProgressService.updateContainerWithProgress(...)reliably publishes a terminal event and clears any “active update” state in both success and exception paths.
1-2: Remove the unnecessaryforwardRef— there is no circular dependency here.
DockerUpdateProgressServicehas no dependencies and does not importDockerService. The dependency is one-way:DockerService→DockerUpdateProgressService. Simply inject it directly without@Inject(forwardRef()).private readonly dockerUpdateProgressService: DockerUpdateProgressServiceOr with explicit injection if needed:
@Inject(DockerUpdateProgressService) private readonly dockerUpdateProgressService: DockerUpdateProgressServiceLikely an incorrect or invalid review comment.
web/src/composables/gql/graphql.ts (1)
860-873: GraphQL codegen artifacts confirmed present and properly generated.The
graphql.tsfile contains all expected generated types and subscription artifacts:
DockerLayerProgress,DockerUpdateEventType, andDockerUpdateProgresstypes are completeDockerUpdateProgressSubscriptionand subscription variables are properly typedDockerUpdateProgressDocumentcontains the full subscription definitionThe
web/codegen.tsconfiguration is in place with the client preset and proper schema reference. All generated code is correctly formatted with the eslint-disable header as configured.web/src/components/Docker/DockerContainersTable.vue (1)
24-25: Vue component imports are correct as-is; no.jsextension changes needed.Lines 24-25 show Vue component imports (
.vuefiles) which correctly preserve the.vueextension. Lines 40-41 are TypeScript composable imports that follow the established pattern in this codebase, which consistently omits.jsextensions for all TypeScript module imports throughoutweb/src/. While CLAUDE.md documents that "TypeScript imports use.jsextensions," this pattern is not reflected in the actual codebase implementation—Vite/Nuxt handles module resolution without requiring explicit.jsextensions in source files. The imports are correct and consistent with the repository's current patterns.Likely an incorrect or invalid review comment.
api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts (1)
4-6: No action needed —ResultPromiseis the correct export in execa v9.6.0.The
ResultPromisetype exists and is the correct type returned byexeca(). It represents both the spawned subprocess and the Promise resolving to the command result.web/src/composables/gql/gql.ts (1)
53-54: Ensurepnpm codegenruns after GraphQL subscription changes.The
Documentstype entry,documentsmapping, andgraphql()overload forDockerUpdateProgressare all consistent. Since this file is CodeGen-managed and intentionally maintains duplicate strings across these three locations, avoid manual edits—regenerate viapnpm codegenwhenever the subscription definition changes.Also applies to: 124-125, 317-320
| """Progress information for a single image layer""" | ||
| type DockerLayerProgress { | ||
| """Layer ID (short hash)""" | ||
| layerId: String! | ||
|
|
||
| """Current status of the layer""" | ||
| status: String! | ||
|
|
||
| """Download/extract progress percentage (0-100)""" | ||
| progress: Float | ||
|
|
||
| """Bytes downloaded/processed""" | ||
| current: Int | ||
|
|
||
| """Total bytes for this layer""" | ||
| total: Int | ||
| } | ||
|
|
||
| """Real-time progress update for a Docker container update operation""" | ||
| type DockerUpdateProgress { | ||
| """Container ID being updated""" | ||
| containerId: PrefixedID! | ||
|
|
||
| """Container name being updated""" | ||
| containerName: String! | ||
|
|
||
| """Type of progress event""" | ||
| type: DockerUpdateEventType! | ||
|
|
||
| """Human-readable message or log line""" | ||
| message: String | ||
|
|
||
| """Layer ID for layer-specific events""" | ||
| layerId: String | ||
|
|
||
| """Overall progress percentage (0-100) for the current operation""" | ||
| overallProgress: Float | ||
|
|
||
| """Per-layer progress details""" | ||
| layers: [DockerLayerProgress!] | ||
|
|
||
| """Error message if type is ERROR""" | ||
| error: String | ||
| } | ||
|
|
||
| """Type of Docker update progress event""" | ||
| enum DockerUpdateEventType { | ||
| STARTED | ||
| LAYER_DOWNLOADING | ||
| LAYER_EXTRACTING | ||
| LAYER_COMPLETE | ||
| LAYER_ALREADY_EXISTS | ||
| PULLING | ||
| LOG | ||
| COMPLETE | ||
| ERROR | ||
| } |
There was a problem hiding this comment.
current/total should not be GraphQL Int (32-bit) for byte counts; use BigInt (or Float) instead.
Large layers can exceed Int limits and break the subscription payload.
| @ObjectType({ description: 'Progress information for a single image layer' }) | ||
| export class DockerLayerProgress { | ||
| @Field(() => String, { description: 'Layer ID (short hash)' }) | ||
| layerId!: string; | ||
|
|
||
| @Field(() => String, { description: 'Current status of the layer' }) | ||
| status!: string; | ||
|
|
||
| @Field(() => Float, { nullable: true, description: 'Download/extract progress percentage (0-100)' }) | ||
| progress?: number; | ||
|
|
||
| @Field(() => Int, { nullable: true, description: 'Bytes downloaded/processed' }) | ||
| current?: number; | ||
|
|
||
| @Field(() => Int, { nullable: true, description: 'Total bytes for this layer' }) | ||
| total?: number; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for 64-bit scalar registrations (Long/BigInt)
rg -n --hidden -S "registerScalarType\(|GraphQLBigInt|BigIntResolver|LongResolver|GraphQLLong|class .*Long|scalar\s+Long|scalar\s+BigInt" .Repository: unraid/api
Length of output: 3432
🏁 Script executed:
# List files in the GraphQL resolvers directory
find api/src/unraid-api/graph -type f -name "*.ts" | head -20Repository: unraid/api
Length of output: 1247
🏁 Script executed:
# Check the docker-update-progress.model.ts file
cat -n api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.tsRepository: unraid/api
Length of output: 2961
🏁 Script executed:
# Search for other byte counter patterns in the codebase
rg -n "bytes|Bytes" api/src --type ts -A 2 | head -50Repository: unraid/api
Length of output: 3544
Use GraphQLBigInt for byte counters to prevent 32-bit overflow.
The current and total fields (lines 33, 36) represent byte counts but use GraphQL Int (32-bit signed), which overflows beyond ~2.1 GB. The repo already uses GraphQLBigInt from 'graphql-scalars' for byte counters in memory.model.ts, docker.model.ts, and array.model.ts. Update the imports and field types:
Required changes
-import { Field, Float, Int, ObjectType, registerEnumType } from '@nestjs/graphql';
+import { Field, Float, ObjectType, registerEnumType } from '@nestjs/graphql';
+import { GraphQLBigInt } from 'graphql-scalars';
import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js';
@ObjectType({ description: 'Progress information for a single image layer' })
export class DockerLayerProgress {
@Field(() => String, { description: 'Layer ID (short hash)' })
layerId!: string;
@Field(() => String, { description: 'Current status of the layer' })
status!: string;
@Field(() => Float, { nullable: true, description: 'Download/extract progress percentage (0-100)' })
progress?: number;
- @Field(() => Int, { nullable: true, description: 'Bytes downloaded/processed' })
+ @Field(() => GraphQLBigInt, { nullable: true, description: 'Bytes downloaded/processed' })
current?: number;
- @Field(() => Int, { nullable: true, description: 'Total bytes for this layer' })
+ @Field(() => GraphQLBigInt, { nullable: true, description: 'Total bytes for this layer' })
total?: number;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.model.ts
around lines 22 - 38, The DockerLayerProgress model uses GraphQL Int for byte
counters which can overflow; update the file to import GraphQLBigInt from
'graphql-scalars', remove the Int import, and change the @Field type for the
current and total properties to use GraphQLBigInt (keeping nullable and
descriptions), and adjust the TypeScript property types accordingly (e.g.,
BigInt or string as used elsewhere) for the current and total fields in the
DockerLayerProgress class.
| const updateProcess = execa( | ||
| '/usr/local/emhttp/plugins/dynamix.docker.manager/scripts/update_container', | ||
| [encodeURIComponent(containerName)], | ||
| { | ||
| shell: 'bash', | ||
| all: true, | ||
| reject: false, | ||
| env: { | ||
| ...process.env, | ||
| DOCKER_CLI_FORMAT: 'json', | ||
| }, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Don’t URL-encode containerName, and avoid shell: 'bash' for command execution.
encodeURIComponent(containerName) will change values like /mycontainer → %2Fmycontainer, which is very likely not what update_container expects. Also, using shell expands the attack surface; prefer executing bash directly.
Proposed diff
- const updateProcess = execa(
- '/usr/local/emhttp/plugins/dynamix.docker.manager/scripts/update_container',
- [encodeURIComponent(containerName)],
- {
- shell: 'bash',
- all: true,
- reject: false,
- env: {
- ...process.env,
- DOCKER_CLI_FORMAT: 'json',
- },
- }
- );
+ const scriptPath = '/usr/local/emhttp/plugins/dynamix.docker.manager/scripts/update_container';
+ const updateProcess = execa(
+ 'bash',
+ [scriptPath, containerName],
+ {
+ all: true,
+ reject: false,
+ env: {
+ ...process.env,
+ DOCKER_CLI_FORMAT: 'json',
+ },
+ }
+ );🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts
around lines 40 - 52, The updateProcess invocation is URL-encoding containerName
and using shell: 'bash'; change it to pass the raw containerName (remove
encodeURIComponent) and stop using shell mode—invoke bash directly by making the
command 'bash' and passing the script path and the raw containerName as args (or
use execa.node/execFile style) so arguments are passed safely; update the execa
call at the updateProcess creation to use the raw containerName and remove the
shell option.
| try { | ||
| if (updateProcess.stdout) { | ||
| const rl = createInterface({ | ||
| input: updateProcess.stdout, | ||
| crlfDelay: Infinity, | ||
| }); | ||
|
|
||
| rl.on('line', (line) => { | ||
| this.processOutputLine(activeUpdate, line); | ||
| }); | ||
|
|
||
| rl.on('error', (err) => { | ||
| this.logger.error(`Error reading update output for ${containerName}`, err); | ||
| }); | ||
| } | ||
|
|
||
| if (updateProcess.stderr) { | ||
| updateProcess.stderr.on('data', (data: Buffer) => { | ||
| const message = data.toString().trim(); | ||
| if (message) { | ||
| this.logger.debug(`Update stderr for ${containerName}: ${message}`); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| const result = await updateProcess; | ||
|
|
||
| if (result.failed) { | ||
| this.publishProgress({ | ||
| containerId, | ||
| containerName, | ||
| type: DockerUpdateEventType.ERROR, | ||
| message: `Update failed for ${containerName}`, | ||
| error: result.stderr || result.shortMessage || 'Unknown error', | ||
| }); | ||
| throw new Error(`Failed to update container ${containerName}: ${result.shortMessage}`); | ||
| } | ||
|
|
||
| this.publishProgress({ | ||
| containerId, | ||
| containerName, | ||
| type: DockerUpdateEventType.COMPLETE, | ||
| message: `Successfully updated ${containerName}`, | ||
| overallProgress: 100, | ||
| }); | ||
| } finally { | ||
| this.activeUpdates.delete(containerId); | ||
| } |
There was a problem hiding this comment.
Close the readline interface (and consider listening to the combined all stream).
createInterface is never closed explicitly; rely-on-GC can leak listeners in long-lived services. Also, if the script writes progress to stderr, you’ll miss it (you currently only parse stdout).
Proposed diff (minimal cleanup)
- if (updateProcess.stdout) {
+ let rl: ReturnType<typeof createInterface> | null = null;
+ if (updateProcess.stdout) {
const rl = createInterface({
input: updateProcess.stdout,
crlfDelay: Infinity,
});
+ rl = rl;
rl.on('line', (line) => {
this.processOutputLine(activeUpdate, line);
});
rl.on('error', (err) => {
this.logger.error(`Error reading update output for ${containerName}`, err);
});
}
@@
- const result = await updateProcess;
+ const result = await updateProcess;
@@
} finally {
+ rl?.close();
this.activeUpdates.delete(containerId);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts
around lines 63 - 110, The readline created by createInterface (rl) is never
closed and only stdout is parsed, which can leak listeners and miss stderr
progress; fix by creating and tracking readline interfaces for both
updateProcess.stdout and updateProcess.stderr (or use the combined "all" stream
if available), route each line into processOutputLine (or debug/stderr handling)
as you do for stdout, and ensure rl.close() is called and event listeners
('line', 'error') removed when the update finishes or in the finally block that
deletes activeUpdates; reference symbols: createInterface, rl, updateProcess,
processOutputLine, publishProgress, activeUpdates.
| private processDockerJson(update: ActiveUpdate, json: Record<string, unknown>): void { | ||
| const { containerId, containerName } = update; | ||
|
|
||
| if (json.status && typeof json.id === 'string') { | ||
| const layerId = json.id as string; | ||
| const status = json.status as string; | ||
|
|
||
| const progressDetail = json.progressDetail as | ||
| | { current?: number; total?: number } | ||
| | undefined; | ||
|
|
||
| const layerProgress: DockerLayerProgress = { | ||
| layerId, | ||
| status, | ||
| current: progressDetail?.current, | ||
| total: progressDetail?.total, | ||
| progress: | ||
| progressDetail?.current && progressDetail?.total | ||
| ? Math.round((progressDetail.current / progressDetail.total) * 100) | ||
| : undefined, | ||
| }; | ||
|
|
||
| update.layers.set(layerId, layerProgress); | ||
|
|
||
| let eventType: DockerUpdateEventType; | ||
| if (status.toLowerCase().includes('downloading')) { | ||
| eventType = DockerUpdateEventType.LAYER_DOWNLOADING; | ||
| } else if (status.toLowerCase().includes('extracting')) { | ||
| eventType = DockerUpdateEventType.LAYER_EXTRACTING; | ||
| } else if (status.toLowerCase().includes('pull complete')) { | ||
| eventType = DockerUpdateEventType.LAYER_COMPLETE; | ||
| } else if (status.toLowerCase().includes('already exists')) { | ||
| eventType = DockerUpdateEventType.LAYER_ALREADY_EXISTS; | ||
| } else { | ||
| eventType = DockerUpdateEventType.LOG; | ||
| } | ||
|
|
||
| const overallProgress = this.calculateOverallProgress(update); | ||
|
|
||
| this.publishProgress({ | ||
| containerId, | ||
| containerName, | ||
| type: eventType, | ||
| layerId, | ||
| message: json.progress | ||
| ? `${layerId}: ${status} ${json.progress}` | ||
| : `${layerId}: ${status}`, | ||
| overallProgress, | ||
| layers: Array.from(update.layers.values()), | ||
| }); | ||
| } else if (json.status) { | ||
| this.publishProgress({ | ||
| containerId, | ||
| containerName, | ||
| type: DockerUpdateEventType.LOG, | ||
| message: json.status as string, | ||
| }); | ||
| } else if (json.error) { | ||
| this.publishProgress({ | ||
| containerId, | ||
| containerName, | ||
| type: DockerUpdateEventType.ERROR, | ||
| error: json.error as string, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix 0% progress handling and tighten JSON field typing.
progressDetail?.current && progressDetail?.total treats current: 0 as “missing”, so you’ll drop valid updates and skew overall progress. Also json.status should be validated as a string before use.
Proposed diff
- if (json.status && typeof json.id === 'string') {
- const layerId = json.id as string;
- const status = json.status as string;
+ if (typeof json.status === 'string' && typeof json.id === 'string') {
+ const layerId = json.id;
+ const status = json.status;
@@
- const layerProgress: DockerLayerProgress = {
+ const computedProgress =
+ progressDetail?.current != null && progressDetail?.total != null && progressDetail.total > 0
+ ? Math.round((progressDetail.current / progressDetail.total) * 100)
+ : undefined;
+
+ const layerProgress: DockerLayerProgress = {
layerId,
status,
current: progressDetail?.current,
total: progressDetail?.total,
- progress:
- progressDetail?.current && progressDetail?.total
- ? Math.round((progressDetail.current / progressDetail.total) * 100)
- : undefined,
+ progress: computedProgress,
};| private parseBytes(value: string, unit: string): number { | ||
| const num = parseFloat(value); | ||
| const multipliers: Record<string, number> = { | ||
| B: 1, | ||
| KB: 1024, | ||
| MB: 1024 * 1024, | ||
| GB: 1024 * 1024 * 1024, | ||
| TB: 1024 * 1024 * 1024 * 1024, | ||
| }; | ||
| return num * (multipliers[unit.toUpperCase()] || 1); | ||
| } | ||
|
|
||
| private publishProgress(progress: DockerUpdateProgress): void { | ||
| this.logger.debug( | ||
| `Update progress for ${progress.containerName}: ${progress.type} - ${progress.message ?? ''}` | ||
| ); | ||
| pubsub.publish(PUBSUB_CHANNEL.DOCKER_UPDATE_PROGRESS, { | ||
| dockerUpdateProgress: progress, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bytes fields may overflow GraphQL Int; consider BigInt in the schema and/or clamping.
parseBytes can yield values > 2^31-1, but the GraphQL layer progress types appear to expose current/total as Int. This can break serialization for large layers. Consider moving current/total to BigInt in the GraphQL schema (and matching TS types), or clamp/omit when out of range.
🤖 Prompt for AI Agents
In @api/src/unraid-api/graph/resolvers/docker/docker-update-progress.service.ts
around lines 301 - 320, parseBytes can produce values that exceed GraphQL Int
limits (2^31-1) causing serialization errors for Docker layer sizes; update the
GraphQL schema type for progress.current/progress.total from Int to BigInt (and
update the corresponding TypeScript DockerUpdateProgress/interface types and any
resolver return types) so large byte counts serialize correctly, or
alternatively clamp values in parseBytes or right before publishProgress to
Number.MAX_SAFE_INTEGER (or GraphQL Int max) and document that overflowed values
are clamped/omitted; adjust parseBytes, the DockerUpdateProgress type, and
publishProgress to enforce the chosen approach consistently.
| import { gql } from '@apollo/client'; | ||
|
|
||
| export const DOCKER_UPDATE_PROGRESS_SUBSCRIPTION = gql` | ||
| subscription DockerUpdateProgress { | ||
| dockerUpdateProgress { | ||
| containerId | ||
| containerName | ||
| type | ||
| message | ||
| layerId | ||
| overallProgress | ||
| error | ||
| layers { | ||
| layerId | ||
| status | ||
| progress | ||
| current | ||
| total | ||
| } | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find gql helper files in the composables directory
find web/src/composables -type f -name "*gql*" 2>/dev/null | head -20Repository: unraid/api
Length of output: 84
🏁 Script executed:
# Search for other subscription files in the codebase
find web -type f -name "*.subscription.ts" 2>/dev/nullRepository: unraid/api
Length of output: 287
🏁 Script executed:
# Check if there's a gql helper in web/src/composables/gql/
ls -la web/src/composables/gql/ 2>/dev/null || echo "Directory not found"Repository: unraid/api
Length of output: 484
🏁 Script executed:
# Search for how gql is used in other subscription or GraphQL files
rg "subscription\s+" web/src --type ts -A 3 | head -50Repository: unraid/api
Length of output: 12869
🏁 Script executed:
# Check for GraphQL CodeGen configuration
find . -name "codegen.ts" -o -name "codegen.yml" -o -name "codegen.yaml" 2>/dev/null | grep -v node_modulesRepository: unraid/api
Length of output: 135
🏁 Script executed:
# Check how existing subscription files import their documents
cat web/src/components/Docker/docker-stats.subscription.tsRepository: unraid/api
Length of output: 300
🏁 Script executed:
# Check another subscription file
cat web/src/components/Logs/log.subscription.tsRepository: unraid/api
Length of output: 300
🏁 Script executed:
# Check notification subscription
cat web/src/components/Notifications/graphql/notification.subscription.tsRepository: unraid/api
Length of output: 757
🏁 Script executed:
# Check the generated gql.ts file header to understand the pattern
head -100 web/src/composables/gql/gql.tsRepository: unraid/api
Length of output: 25294
Use the generated typed GraphQL helper for consistency with newer subscription patterns in the codebase.
web/src/composables/gql/gql.ts exports a graphql() function that provides full type safety for GraphQL documents. Other subscriptions like log.subscription.ts and notification.subscription.ts already use this pattern. Replace the direct @apollo/client gql import with the typed graphql() function to align with the codebase's generated type system and ensure the subscription is properly typed.
🤖 Prompt for AI Agents
In @web/src/components/Docker/docker-update-progress.subscription.ts around
lines 1 - 22, Replace the raw @apollo/client gql usage with the generated typed
helper: import and call graphql() to define DOCKER_UPDATE_PROGRESS_SUBSCRIPTION
so the subscription uses the generated types; update the top-level import to use
the graphql() factory and wrap the subscription document with graphql`...` while
keeping the exported const name DOCKER_UPDATE_PROGRESS_SUBSCRIPTION and the same
GraphQL selection set (containerId, containerName, type, message, layerId,
overallProgress, error, layers { layerId, status, progress, current, total }) so
the rest of the code stays compatible but now benefits from type safety.
| /** Progress information for a single image layer */ | ||
| export type DockerLayerProgress = { | ||
| __typename?: 'DockerLayerProgress'; | ||
| /** Bytes downloaded/processed */ | ||
| current?: Maybe<Scalars['Int']['output']>; | ||
| /** Layer ID (short hash) */ | ||
| layerId: Scalars['String']['output']; | ||
| /** Download/extract progress percentage (0-100) */ | ||
| progress?: Maybe<Scalars['Float']['output']>; | ||
| /** Current status of the layer */ | ||
| status: Scalars['String']['output']; | ||
| /** Total bytes for this layer */ | ||
| total?: Maybe<Scalars['Int']['output']>; | ||
| }; |
There was a problem hiding this comment.
Bytes as Int: overflow risk for current/total.
DockerLayerProgress.current/total are Int; if images/layers exceed 2,147,483,647 bytes, this can overflow (or be rejected by GraphQL tooling). Consider switching schema to BigInt for these fields (and codegen will follow).
🤖 Prompt for AI Agents
In @web/src/composables/gql/graphql.ts around lines 860 - 873,
DockerLayerProgress currently types current and total as Scalars['Int'] which
risks overflow for layers >2,147,483,647 bytes; change those fields in the
GraphQL schema/type definition from Int to a BigInt (or custom scalar like
BigInt) so DockerLayerProgress.current and DockerLayerProgress.total use the
BigInt scalar, update any scalar mappings in your codegen config so generated
types reflect the new scalar, and re-run code generation to propagate the
change.
| import { computed, reactive, ref } from 'vue'; | ||
| import { useSubscription } from '@vue/apollo-composable'; | ||
|
|
||
| import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription'; | ||
|
|
||
| import type { | ||
| DockerLayerProgress, | ||
| DockerUpdateEventType, | ||
| DockerUpdateProgress, | ||
| } from '@/composables/gql/graphql'; |
There was a problem hiding this comment.
Use .js import specifiers for ESM consistency (per guidelines).
This file’s imports don’t include .js extensions (including the alias import). As per coding guidelines, switch to .js specifiers.
Proposed diff
-import { computed, reactive, ref } from 'vue';
-import { useSubscription } from '@vue/apollo-composable';
+import { computed, reactive, ref } from 'vue';
+import { useSubscription } from '@vue/apollo-composable';
-import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription';
+import { DOCKER_UPDATE_PROGRESS_SUBSCRIPTION } from '@/components/Docker/docker-update-progress.subscription.js';
import type {
DockerLayerProgress,
DockerUpdateEventType,
DockerUpdateProgress,
-} from '@/composables/gql/graphql';
+} from '@/composables/gql/graphql.js';🤖 Prompt for AI Agents
In @web/src/composables/useDockerUpdateProgress.ts around lines 1 - 10, Update
the local/alias ESM imports to include .js extensions: change the alias import
DOCKER_UPDATE_PROGRESS_SUBSCRIPTION from
'@/components/Docker/docker-update-progress.subscription' to
'@/components/Docker/docker-update-progress.subscription.js' and change the
types import from '@/composables/gql/graphql' to '@/composables/gql/graphql.js';
do not alter bare imports from 'vue' or '@vue/apollo-composable'.
| const containerUpdates = reactive(new Map<string, ContainerUpdateState>()); | ||
| const isModalOpen = ref(false); | ||
| const activeContainerId = ref<string | null>(null); | ||
|
|
||
| const { onResult: onProgressResult, onError: onProgressError } = useSubscription( | ||
| DOCKER_UPDATE_PROGRESS_SUBSCRIPTION, | ||
| null, | ||
| () => ({ | ||
| fetchPolicy: 'network-only', | ||
| }) | ||
| ); | ||
|
|
||
| onProgressResult((result) => { | ||
| const progress = result.data?.dockerUpdateProgress as DockerUpdateProgress | undefined; | ||
| if (!progress || !progress.containerId) return; | ||
|
|
||
| const { containerId, containerName, type, message, overallProgress, error, layers } = progress; | ||
|
|
||
| let state = containerUpdates.get(containerId); | ||
| if (!state) { | ||
| state = { | ||
| containerId, | ||
| containerName, | ||
| status: 'pending', | ||
| overallProgress: 0, | ||
| message: '', | ||
| layers: new Map(), | ||
| events: [], | ||
| }; | ||
| containerUpdates.set(containerId, state); | ||
| } | ||
|
|
||
| state.events.push(progress); | ||
|
|
||
| if (message) { | ||
| state.message = message; | ||
| } | ||
|
|
||
| if (overallProgress !== undefined && overallProgress !== null) { | ||
| state.overallProgress = overallProgress; | ||
| } | ||
|
|
||
| if (layers) { | ||
| for (const layer of layers) { | ||
| state.layers.set(layer.layerId, layer); | ||
| } | ||
| } | ||
|
|
||
| const eventType = type as DockerUpdateEventType; | ||
| switch (eventType) { | ||
| case 'STARTED': | ||
| state.status = 'in_progress'; | ||
| break; | ||
| case 'COMPLETE': | ||
| state.status = 'complete'; | ||
| state.overallProgress = 100; | ||
| break; | ||
| case 'ERROR': | ||
| state.status = 'error'; | ||
| state.error = error ?? 'Unknown error'; | ||
| break; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fix potential non-reactive mutations for newly created state (reactive Map value pitfall).
When state is first created (Line 43-54), it’s a plain object; subsequent mutations in the same callback can be invisible to Vue reactivity in some cases. Safer: make the stored value reactive up-front (and make layers a reactive Map too), then mutate that proxy.
Proposed diff
onProgressResult((result) => {
- const progress = result.data?.dockerUpdateProgress as DockerUpdateProgress | undefined;
+ const progress = result.data?.dockerUpdateProgress as DockerUpdateProgress | undefined;
if (!progress || !progress.containerId) return;
const { containerId, containerName, type, message, overallProgress, error, layers } = progress;
let state = containerUpdates.get(containerId);
if (!state) {
- state = {
+ state = reactive<ContainerUpdateState>({
containerId,
containerName,
status: 'pending',
overallProgress: 0,
message: '',
- layers: new Map(),
+ layers: reactive(new Map()),
events: [],
- };
+ });
containerUpdates.set(containerId, state);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const containerUpdates = reactive(new Map<string, ContainerUpdateState>()); | |
| const isModalOpen = ref(false); | |
| const activeContainerId = ref<string | null>(null); | |
| const { onResult: onProgressResult, onError: onProgressError } = useSubscription( | |
| DOCKER_UPDATE_PROGRESS_SUBSCRIPTION, | |
| null, | |
| () => ({ | |
| fetchPolicy: 'network-only', | |
| }) | |
| ); | |
| onProgressResult((result) => { | |
| const progress = result.data?.dockerUpdateProgress as DockerUpdateProgress | undefined; | |
| if (!progress || !progress.containerId) return; | |
| const { containerId, containerName, type, message, overallProgress, error, layers } = progress; | |
| let state = containerUpdates.get(containerId); | |
| if (!state) { | |
| state = { | |
| containerId, | |
| containerName, | |
| status: 'pending', | |
| overallProgress: 0, | |
| message: '', | |
| layers: new Map(), | |
| events: [], | |
| }; | |
| containerUpdates.set(containerId, state); | |
| } | |
| state.events.push(progress); | |
| if (message) { | |
| state.message = message; | |
| } | |
| if (overallProgress !== undefined && overallProgress !== null) { | |
| state.overallProgress = overallProgress; | |
| } | |
| if (layers) { | |
| for (const layer of layers) { | |
| state.layers.set(layer.layerId, layer); | |
| } | |
| } | |
| const eventType = type as DockerUpdateEventType; | |
| switch (eventType) { | |
| case 'STARTED': | |
| state.status = 'in_progress'; | |
| break; | |
| case 'COMPLETE': | |
| state.status = 'complete'; | |
| state.overallProgress = 100; | |
| break; | |
| case 'ERROR': | |
| state.status = 'error'; | |
| state.error = error ?? 'Unknown error'; | |
| break; | |
| } | |
| }); | |
| const containerUpdates = reactive(new Map<string, ContainerUpdateState>()); | |
| const isModalOpen = ref(false); | |
| const activeContainerId = ref<string | null>(null); | |
| const { onResult: onProgressResult, onError: onProgressError } = useSubscription( | |
| DOCKER_UPDATE_PROGRESS_SUBSCRIPTION, | |
| null, | |
| () => ({ | |
| fetchPolicy: 'network-only', | |
| }) | |
| ); | |
| onProgressResult((result) => { | |
| const progress = result.data?.dockerUpdateProgress as DockerUpdateProgress | undefined; | |
| if (!progress || !progress.containerId) return; | |
| const { containerId, containerName, type, message, overallProgress, error, layers } = progress; | |
| let state = containerUpdates.get(containerId); | |
| if (!state) { | |
| state = reactive<ContainerUpdateState>({ | |
| containerId, | |
| containerName, | |
| status: 'pending', | |
| overallProgress: 0, | |
| message: '', | |
| layers: reactive(new Map()), | |
| events: [], | |
| }); | |
| containerUpdates.set(containerId, state); | |
| } | |
| state.events.push(progress); | |
| if (message) { | |
| state.message = message; | |
| } | |
| if (overallProgress !== undefined && overallProgress !== null) { | |
| state.overallProgress = overallProgress; | |
| } | |
| if (layers) { | |
| for (const layer of layers) { | |
| state.layers.set(layer.layerId, layer); | |
| } | |
| } | |
| const eventType = type as DockerUpdateEventType; | |
| switch (eventType) { | |
| case 'STARTED': | |
| state.status = 'in_progress'; | |
| break; | |
| case 'COMPLETE': | |
| state.status = 'complete'; | |
| state.overallProgress = 100; | |
| break; | |
| case 'ERROR': | |
| state.status = 'error'; | |
| state.error = error ?? 'Unknown error'; | |
| break; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In @web/src/composables/useDockerUpdateProgress.ts around lines 24 - 86, When
creating new entries in the reactive containerUpdates Map inside the
onProgressResult handler, wrap the new state object with Vue's reactive() so
mutations are tracked (e.g., make state = reactive({ containerId, containerName,
status: 'pending', overallProgress: 0, message: '', layers: reactive(new Map()),
events: [] }) and then containerUpdates.set(containerId, state)); also ensure
layers is created as reactive(new Map()) so subsequent state.layers.set(...)
calls are reactive; update references to state and state.layers in the rest of
the onProgressResult callback accordingly.
Summary
What Changed
Backend
DockerUpdateProgressGraphQL type with layer progress details (layerId, status, progress %, current/total bytes)DockerUpdateProgressServicethat parses docker pull JSON output and streams events via PubSubdockerUpdateProgresssubscription toDockerResolverDockerService.updateContainer()Frontend
useDockerUpdateProgresscomposable for subscription management and state trackingDockerUpdateProgressModalcomponent showing overall and per-layer progressDockerContainersTable(click "Updating..." to see progress)onUpdateStart/onUpdateCompletecallbacks touseDockerUpdateActionsTesting Instructions
To test this feature without waiting for a real update, create a container from an older image tag:
Then:
After testing:
# Clean up docker rm update-test-container docker rmi alpine:3.18Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.