feat: add docker restart mutation#2022
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Docker container restart: a DockerService.restart that calls Docker restart, polls up to 20 times with 500ms sleeps until RUNNING (or errors/warns), a GraphQL nested mutation resolver exposing it with UPDATE_ANY permissions, and unit tests for service and resolver behavior. Also updates generated schema and bumps dev config version. ChangesDocker restart operation
Schema and config
Sequence DiagramsequenceDiagram
participant Resolver
participant DockerService
participant DockerAPI
participant PubSub
Resolver->>DockerService: restart(id)
DockerService->>DockerAPI: restart container (t:10)
loop Poll up to 20 times with 500ms delay
DockerService->>DockerAPI: list containers
DockerAPI-->>DockerService: container state
end
alt Container found and RUNNING
DockerService->>PubSub: publish getAppInfo()
DockerService-->>Resolver: return updated container
else Container not found after restart
DockerService-->>Resolver: throw error
else Container found but not RUNNING
DockerService->>PubSub: publish getAppInfo()
DockerService-->>Resolver: return updated container with warning
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds GraphQL support for restarting Docker containers by introducing a restart operation in the Docker service and exposing it via the Docker mutations resolver.
Changes:
- Implemented
DockerService.restart()with polling to return an updated container state and publish updated app info. - Exposed a new
restartmutation/field inDockerMutationsResolverwith permission checks. - Updated resolver/service test scaffolding and added a resolver unit test for restart.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| api/src/unraid-api/graph/resolvers/docker/docker.service.ts | Adds restart() implementation with polling and pubsub publish. |
| api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts | Extends Docker container mock to include restart. |
| api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.ts | Adds GraphQL restart handler with permissions. |
| api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.spec.ts | Adds resolver unit test for restart. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 0b27126eef
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/docker/docker.service.ts (1)
302-326: ⚡ Quick winAdd warning log if container doesn't reach RUNNING state.
The
restartmethod doesn't log a warning if the container fails to reachRUNNINGstate after polling completes, unlike thestopmethod (lines 268-270) which warns whenEXITEDstate isn't reached. For consistency and debuggability, consider adding similar logging.📝 Suggested enhancement
if (!updatedContainer) { throw new Error(`Container ${id} not found after restarting`); + } else if (updatedContainer.state !== ContainerState.RUNNING) { + this.logger.warn(`Container ${id} did not reach RUNNING state after restart command.`); } const appInfo = await this.getAppInfo();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/src/unraid-api/graph/resolvers/docker/docker.service.ts` around lines 302 - 326, The restart method currently polls for ContainerState.RUNNING but doesn’t emit a warning if the container never reaches RUNNING; update the restart function to log a warning (use this.logger.warn) when updatedContainer exists but its state !== ContainerState.RUNNING after the retry loop (mirror the behavior in stop), and also log a warning when updatedContainer is undefined before throwing the Error; place these warnings just after the polling loop and before the existing throw/pubsub/publish logic to aid debugging (refer to restart, getContainers, ContainerState.RUNNING, and this.logger.warn).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts`:
- Line 41: The test-suite is leaving the restart mock (restart: vi.fn())
uncleared which can leak state between tests; update the beforeEach setup used
for container method mocks (where start, stop, pause, unpause, inspect are
reset) to also reset the restart mock so it’s reinitialized before each test —
locate the beforeEach block that resets start/stop/pause/unpause/inspect and add
the same reset logic for restart to ensure isolation of tests referencing the
restart mock.
---
Nitpick comments:
In `@api/src/unraid-api/graph/resolvers/docker/docker.service.ts`:
- Around line 302-326: The restart method currently polls for
ContainerState.RUNNING but doesn’t emit a warning if the container never reaches
RUNNING; update the restart function to log a warning (use this.logger.warn)
when updatedContainer exists but its state !== ContainerState.RUNNING after the
retry loop (mirror the behavior in stop), and also log a warning when
updatedContainer is undefined before throwing the Error; place these warnings
just after the polling loop and before the existing throw/pubsub/publish logic
to aid debugging (refer to restart, getContainers, ContainerState.RUNNING, and
this.logger.warn).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0c7acaa4-7912-4cf2-9c9b-bf21a27a440c
📒 Files selected for processing (4)
api/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.mutations.resolver.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts (1)
419-427: ⚡ Quick winMatch test name to what is actually asserted.
Line 419 says the test "warns and returns", but it only verifies return value and publish call. Either assert the warning side effect or rename the test to avoid misleading intent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts` around lines 419 - 427, The test titled 'warns and returns the container when it does not reach RUNNING state after retries' is misleading because it only asserts the returned container and that pubsub.publish was called; update the spec to either (A) rename the test to something like 'returns the container and publishes info when it does not reach RUNNING state after retries' to match the current assertions, or (B) keep the title and add an explicit expectation for the warning side-effect (e.g., assert that the logger or warning-publisher was called) by referencing the existing mocks used in this test (mockListContainers, mockContainer.restart, pubsub.publish) so the test name correctly reflects what is asserted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts`:
- Around line 401-427: The tests for service.restart leak calls to the shared
pubsub.publish mock across cases; clear its call history between tests by
resetting or clearing the mock in the test setup (e.g., add
pubsub.publish.mockClear() or mockReset() in the beforeEach/afterEach used for
the Docker tests), ensuring pubsub.publish assertions in the restart tests only
reflect calls made during that test and not earlier ones; target the
pubsub.publish mock used in the restart specs so service.restart behavior is
validated accurately.
---
Nitpick comments:
In `@api/src/unraid-api/graph/resolvers/docker/docker.service.spec.ts`:
- Around line 419-427: The test titled 'warns and returns the container when it
does not reach RUNNING state after retries' is misleading because it only
asserts the returned container and that pubsub.publish was called; update the
spec to either (A) rename the test to something like 'returns the container and
publishes info when it does not reach RUNNING state after retries' to match the
current assertions, or (B) keep the title and add an explicit expectation for
the warning side-effect (e.g., assert that the logger or warning-publisher was
called) by referencing the existing mocks used in this test (mockListContainers,
mockContainer.restart, pubsub.publish) so the test name correctly reflects what
is asserted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 24072dcb-5c70-42d2-986c-4a78d169236e
📒 Files selected for processing (2)
api/src/unraid-api/graph/resolvers/docker/docker.service.spec.tsapi/src/unraid-api/graph/resolvers/docker/docker.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/docker/docker.service.ts
There was a problem hiding this comment.
File was automatically generated via pnpm dev
- Replaced getContainers() poll in stop/pause/restart/unpause with a single container.inspect() poll - Added waitForContainerState() that polls a container via inspect - Added finalizeMutation() so each mutation only calls getContainers() once - All docker tests pass - Verified on personal unraid server via api and in GraphQL sandbox for restart, start, and stop mutations
Restore generated-schema.graphql as it was generated locally on macOS without the connect plugin. Regenerated codegen output from original schema
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2022 +/- ##
==========================================
+ Coverage 52.63% 52.70% +0.07%
==========================================
Files 1035 1035
Lines 72033 72049 +16
Branches 8243 8262 +19
==========================================
+ Hits 37914 37977 +63
+ Misses 33993 33946 -47
Partials 126 126 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds a mutation to restart docker containers.
Work Intent approved by SimonF CLD-411
Summary by CodeRabbit
New Features
Tests
Chores