feat(cli): implement up, down, status, logs, doctor, and sync#22
Conversation
wax911
left a comment
There was a problem hiding this comment.
Review notes against #6:
-
The PR body describes the sync pipeline as
config -> discover -> generate -> render -> deploy. That is wrong forsync.syncmust only generate into a temporary directory and compare against committed canonical stack files. It must not render and must never deploy. -
The command surface diverges from the accepted plan.
uplists flags such as--follow-logs,--detach, and--prune, while the issue required compatibility with--no-logs,--dry-run,--skip-generate,--allow-unrendered,--stacks, and--profile. New flags are fine later, but compatibility flags must be present and tested first. -
doctorneeds to validate the render path anddocker compose configfor rendered files. Do not reduce it to only Docker availability/Swarm checks. -
logsmust support default configured services when no service names are passed and prefix output per service. Verify this behavior is present and covered by tests. -
External command execution needs to stay fully behind the process runner with argument arrays only. No shell-concatenated user input should appear anywhere in Docker command paths.
Please correct sync first. A deploy-capable sync command is a hard safety regression.
wax911
left a comment
There was a problem hiding this comment.
Follow-up review after the push:
Good: this PR is now draft, which is appropriate while the lower layers are still moving.
Still unresolved:
-
The PR still targets
mainand includes a large cumulative diff. Retarget it to the preceding implementation branch once the stack order is finalized. -
The PR body still describes
syncasconfig -> discover -> generate -> render -> deploy. That is still unsafe.syncmust beconfig -> generate into temp -> diff against committed stacks -> exit with drift status. No render, no deploy. -
The command flag list in the PR body still diverges from the compatibility contract. Keep
--no-logs,--dry-run,--skip-generate,--allow-unrendered,--stacks, and--profileas first-class compatibility flags before adding extras such as--detachand--prune.
Leave this draft until the body and implementation are corrected. The sync/deploy wording alone is enough to block readiness.
2df3881 to
900b4e2
Compare
Closes #6