Skip to content

Add MCP tools to delete an instance and roll back a hosted instance - #8693

Merged
andypalmi merged 6 commits into
mainfrom
feat/mcp-delete-instance
Oct 2, 2026
Merged

andypalmi merged 6 commits into
mainfrom
feat/mcp-delete-instance

Conversation

@cstns

@cstns cstns commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Closes #7686

Adds platform_delete_instance (hosted or remote, like the other shared instance tools) and platform_rollback_hosted_instance.

Both are marked destructive, so they're served as delete tools and read-only tokens can't reach them. Rollback isn't a delete, but it overwrites what's running, so it goes behind the same gate.

What I found when I tried deleting against the real routes:

  • A hosted instance takes ALL of its snapshots with it (the FK cascades). Its assigned devices get unassigned, lose their target and stop running Node-RED. Pipeline stages that pointed at it stay around with no target.
  • A remote instance has its credentials revoked, so the physical device can't talk to the platform anymore. Its snapshots stay in the database but can't be opened.
  • The description asks the agent to list snapshots and devices first, offer to export snapshots, and confirm with the user.

For rollback: it replaces flows, credentials, settings, env vars (replaced, not merged) and modules. If the instance is running it restarts the flows before replying. A snapshot that isn't this instance's gets a 400 invalid_snapshot. The current state isn't saved anywhere first, so the description suggests taking a snapshot before rolling back. The input is called snapshotId to match the other snapshot tools, where the issue draft had snapshot.

Heads up: this touches the same spots in instances.js, shared-instances-devices.js and their specs as #8692, so whichever lands second will need a quick rebase.

@cstns cstns self-assigned this Sep 29, 2026
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.87%. Comparing base (871bda3) to head (54f47bd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8693      +/-   ##
==========================================
+ Coverage   77.86%   77.87%   +0.01%     
==========================================
  Files         474      474              
  Lines       25582    25594      +12     
  Branches     6810     6814       +4     
==========================================
+ Hits        19920    19932      +12     
  Misses       5662     5662              
Flag Coverage Δ
backend 77.87% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…t the rollback description

The instance id went straight into the inject URL, which resolves dot
segments, so an id like ../applications/<id> deleted an application.
The handler now takes only a hosted instance UUID or a device hashid
matching instanceType.

Rollback replies before the triggered restart finishes, and skips
settings the template locks. Devices of a deleted hosted instance are
left without an application.
@cstns
cstns marked this pull request as ready for review September 30, 2026 12:47

@andypalmi andypalmi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good. Delete goes through the real project and device routes, so container removal, device notifications, billing and audit logging are all handled there. Rollback matches the route's snapshot ownership check.

A heads up on merge order: this and #8692 both add tools at the end of instances.js and shared-instances-devices.js (and their specs). Whichever merges second will conflict there, and keeping both sides should be enough. Both PRs also add the same hosted-UUID-or-device-hashid check, so once both are in, it might be worth pulling that into a shared helper in schemas.js.

Comment thread forge/ee/lib/mcp/tools/instances.js Outdated
Co-authored-by: Andrea Palmieri <76187074+andypalmi@users.noreply.github.com>
@cstns
cstns requested a review from andypalmi October 2, 2026 11:55
@cstns
cstns deployed to staging October 2, 2026 11:59 — with GitHub Actions Active
# Conflicts:
#	forge/ee/lib/mcp/tools/instances.js
#	forge/ee/lib/mcp/tools/shared-instances-devices.js
#	test/unit/forge/ee/lib/mcp/tools/instances_spec.js
#	test/unit/forge/ee/lib/mcp/tools/shared-instances-devices_spec.js
@andypalmi
andypalmi enabled auto-merge (squash) October 2, 2026 14:05
@andypalmi
andypalmi merged commit 87c22ae into main Oct 2, 2026
29 checks passed
@andypalmi
andypalmi deleted the feat/mcp-delete-instance branch October 2, 2026 14:25

This branch was successfully deployed

1 active deployment
staging — 54f47bde Deployed Oct 2, 2026 by andypalmi via Remove application #12108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5.1-c Delete and destructive tools (phase 2)

2 participants