Repository navigation
Add MCP tools for pipeline creation, stages and deploys - #8500
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8500 +/- ##
==========================================
+ Coverage 77.13% 77.22% +0.08%
==========================================
Files 466 466
Lines 25004 25097 +93
Branches 6664 6684 +20
==========================================
+ Hits 19287 19380 +93
Misses 5717 5717
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Route behavior all checks out, and the tricky bits are handled well: the nested pipeline.name wrap, the blank deviceGroupId to reach the git rebind branch, and the deploy status semantics. Tests cover them nicely.
One optional cleanup, mostly on the two stage tools since they overlap a lot:
- The descriptions restate logic that already lives in the args (
action, the "resend git settings as a set / reset to empty" note,source/linked-list). Could those move fully into the args that own them, keeping the description tool-level? Same idea we landed on for the update-application tool. I'm talking about this comment #8496 (comment) platform_add_pipeline_stageandplatform_update_pipeline_stageduplicate the whole git field block, where the only real difference is the "reset to empty" note on update. Could it be a shared fragment inschemas.js(next toteamId/applicationId), spread into both? Something like:
// schemas.js
function gitStageFields ({ resetNote = false } = {}) {
const reset = resetNote ? '. Resent on every git update; reset to empty when omitted' : ''
return {
url: z.string().optional().describe('Git repository URL' + reset),
branch: z.string().optional().describe('Git branch to push to' + reset),
pullBranch: z.string().optional().describe('Git branch to pull from' + reset),
pushPath: z.string().optional().describe('Repository path to push to' + reset),
pullPath: z.string().optional().describe('Repository path to pull from' + reset),
credentialSecret: z.string().optional().describe('Secret used to encrypt flow credentials pushed to the repository' + (resetNote ? '. Keeps its stored value when omitted' : ''))
}
}- The handler payload key list is duplicated between the two as well, so it could be a shared const.
None of this is blocking, happy for it to be a follow-up if you would rather keep this PR focused.
Deploying replaces the flows running on the next stage's target rather than adding to them, which is the same reasoning that puts platform_set_instance_device_target behind destructive access, so this belongs there too. "A stage added without source is not linked into the chain" undersold it. Adding a source-less stage to a pipeline that already has stages leaves two unlinked heads, and the pipeline then lists only the new one while the existing stages stop appearing, so a single missing argument reads as if it wiped the pipeline (#8582). Say that, on the description and on the field.
|
Tested against a running platform with a throwaway application, two hosted instances, a device group and a git token. The route reading behind this PR holds up well, and both handler workarounds turned out to be load-bearing rather than defensive:
Deploy behaves as described throughout: Two changes in 9415a58: Deploy is now delete-class. It replaces the flows running on the next stage's target rather than adding to them, which is the same reasoning that made The
One small correction to the PR description: it says the ordering rules are spelled out "since the errors are generic Everything above was exercised at the route level with payloads matching exactly what each handler sends; the tool-level pass is pending a gateway catalog refresh. |
|
The destructive reclassification and the sharper source warning both read well, and the ordering-error correction is fair. Coming back to the two cleanups from the last pass, since this is the same shape as what we landed on #8497:
Can you fold these in before this one merges, the same way the shared |
…ptions The add and update stage tools carried the same seven git fields twice, so they move to a gitStageFields fragment in schemas.js alongside the other shared pieces. The one real difference between the two, that an update applies the git settings as a set and empties anything omitted while credentialSecret keeps its value, is what the forUpdate flag carries, so the caveat still lives on the arguments that own it rather than in prose. action and deployToDevices were duplicated too and are now declared once, and both handlers build their payload from one shared key list. The descriptions kept restating logic the arguments already carry: the action modes, the git resend rule, the source linked-list rule, and the partial-update note. Those move onto their arguments, leaving the descriptions to say what each tool does plus the ordering rules, which are cross-field and belong to neither. No behaviour change. The published JSON Schema for both tools is identical before and after, same keys in the same order, same types, same required sets; only the descriptions differ.
|
Both folded in, 44304c5. Shared git block.
Descriptions. Moved onto the arguments that own them: the One thing worth confirming: the No behaviour change. I diffed the generated JSON Schema for both tools before and after: identical keys in the same order, same types, same |
andypalmi
left a comment
There was a problem hiding this comment.
Cleanups look good. gitStageFields in schemas.js with the forUpdate flag is the right shape, and the shared action / deployToDevices / stagePayloadKeys deduplication matches what we did on #8497. Confirmed the generated schema is unchanged bar descriptions.
On the source warning: keep it in both places. The redundancy is deliberate given how the failure reads (a missing source looks like it wiped the pipeline, #8582), so I'd rather it stay slightly over-stated than have someone miss it.
Only thing left is the merge conflict with main in schemas.js (both this and #8497's snapshotComponents fragment landed in the same spot, purely additive). Resolve that against main and this is good to merge.
Closes #7697
Adds the five pipeline write tools:
platform_create_pipeline,platform_update_pipeline,platform_add_pipeline_stage,platform_update_pipeline_stageandplatform_deploy_pipeline_stage. With this, agents can finally run the deploy step of a pipeline.A few route realities shaped the handlers and descriptions:
PUT /pipelines/:iddeclares a flat{ name }body but its handler readsrequest.body.pipeline.name, so the tool keeps a flat agent-facing input and wraps it into the nested shape (the route's own tests use the nested shape too). Might be worth a follow-up fix on the route itself.instanceId/deviceId/deviceGroupIdbeing present in the body. The frontend gets this by always sending a nulldeviceGroupId(AJV coerces it to''); the tool handler mirrors that by adding a blankdeviceGroupIdwhen only git fields are sent, so a git-only update actually lands.addGitReporesets omitted git fields to empty strings on update, exceptcredentialSecretwhich is kept. The field descriptions tell agents to resend the full git settings on every git update.{ status: "importing" }as soon as the deploy has started and finishes in the background, so the description tells agents to poll the target status instead of treating the 200 as completion. The protected-instance Owner gate (403protected_instance), the prompt-actionsourceSnapshotId, and the action-none no-op are also spelled out.sourceis how a new stage is attached after an existing one, and the API's ordering rules (no device group first, no instance/device after a device group) are in the description since the errors are genericinvalid_input.