Conversation
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Fixes #119. The PR registers the existing backend option by name (addOptionFlagsByName) on every engine-using command that isn't in backendFreeActions: ps, status, stop, attach, exec, update, theia/theia-next, cleanup, info, auth import/export, img import, and the engine-using network subcommands. That way a session started with --backend podman can still be managed when Docker is the saved default. The CLI value flows through normal option precedence and resolveBackend, so the podman-shim mapping and docker.SetBinary apply unchanged. Engine-free commands (network print/diff, tools, features, ...) still reject the flag.
The second part is validation. Run now calls a shared validateBackendName after resolveBackend for every engine-using action, and selectBackend no longer maps "" to docker. An explicit --backend= or an invalid name now fails before dispatch on every command, not just the run-like ones.
I checked this locally. go test ./internal/cli ./internal/app passes. ps --backend= and ps --backend=foo exit 1 with the unsupported-backend error, and network print --backend podman is rejected as an unknown flag. I didn't test against a live engine.
Points for the maintainer:
- The flag list per command is hand-maintained against
backendFreeActionsininternal/app/actions.go. The parse tests pin the current set, but a new engine-using command won't be caught automatically. - The empty-backend handling is now split:
RunandselectBackendreject"", butValidateOptionsstill maps it to docker. Only tests depend on that fallback (inline comment). ps --jsonrows don't say which engine they came from. Consumers that query both engines have to track that themselves. That's fine for this PR, but relevant for the integration contract.
Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>
f25be1d to
b0cb5b1
Compare
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Small follow-up since my last pass: only internal/app/validation.go (comment reword) and a new test in internal/app/app_test.go changed. Both of my earlier threads are handled by these, no new findings.
These previous comments can be resolved as they are now handled:
- comment now documents why
ValidateOptionsstill falls back to docker for test fixtures TestRunRejectsEmptyBackendBeforeDispatchcoversRunrejecting an empty--backendbefore dispatch
I can't resolve them myself as I would need write permission on this repository.
What it does
Fixes #119.
Adds
--backendto lifecycle and other engine-using management commands, so sessions started with Podman remain manageable when Docker is the saved default. CLI overrides take precedence over config; invalid or empty backend names fail before dispatch. Updates docs and parsing tests.How to test
enclave run --backend podman --tool codex --background --name backend-review.ps --backend podman --jsonlists it, then useattach --backend podman backend-reviewandstop --backend podman backend-review. Confirm the saved backend remains unchanged. Repeat with Docker.ps --backend=invalidandps --backend=fail, andnetwork print --backend podmanrejects the flag.Build and lint passed. The test suite failed only in terminal-tint tests with inherited
NO_COLOR=1; that package passed with color-suppression variables unset. Live engine testing remains for the reviewer.Follow-ups
None.
Breaking changes
Review checklist