Add one-click deploy CLI orchestration#143
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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.
Code Review
This pull request introduces a new --one-click deployment option to the agentworkforce deploy CLI command, allowing users to deploy an agent manifest directly into the shared cloud platform. It includes parsing, planning, and execution logic for these deployments, along with corresponding unit tests. The review feedback highlights two key areas for improvement: validating that all required inputs are provided before proceeding with a deployment to prevent failures, and ensuring that temporary directories created during testing are properly cleaned up to avoid resource leaks.
| if (parsed.dryRun) { | ||
| process.stdout.write('\nok: one-click dry-run\n'); | ||
| process.exit(0); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The CLI currently renders the plan with missing required inputs but proceeds to call deploy anyway when not in dry-run mode. This can lead to broken deployments or failures on the cloud side. Consider validating that all required inputs are provided and aborting the deployment if any are missing.
if (parsed.dryRun) {
process.stdout.write('\nok: one-click dry-run\n');
process.exit(0);
return;
}
const missingInputs = requirements.inputs.filter((input) => !(input.name in inputs));
if (missingInputs.length > 0) {
die(`deploy --one-click: missing required inputs: ${missingInputs.map((i) => i.name).join(', ')}`);
}| @@ -1,9 +1,12 @@ | |||
| import test from 'node:test'; | |||
| import assert from 'node:assert/strict'; | |||
| import { mkdtemp, writeFile } from 'node:fs/promises'; | |||
There was a problem hiding this comment.
The temporary directories created by mkdtemp in writeOneClickFixture are never cleaned up, which can clutter the system's temporary directory over time. Consider importing rm from node:fs/promises and cleaning up the created directory in a finally block within each test.
| import { mkdtemp, writeFile } from 'node:fs/promises'; | |
| import { mkdtemp, writeFile, rm } from 'node:fs/promises'; |
|
Superseded by #146. This was the manifest-based one-click CLI; its base #142 (AgentManifest schema) is closed, and #145/#146 reworked the command to read the persona file directly as the single source of truth. The reusable platform-secret derivation was already salvaged from the #142 lineage into #146 (Layer B). Closing. |
Summary
Validation
Hold
Stacked on #142 (feat/one-click-manifest). Parked; do not merge until schema PR lands and final e2e gate passes.