Skip to content

feat(server): connect a cloud account from the conversation - #36

Open
JeroenSoeters wants to merge 22 commits into
mcp-onboardingfrom
mcp-connect
Open

feat(server): connect a cloud account from the conversation#36
JeroenSoeters wants to merge 22 commits into
mcp-onboardingfrom
mcp-connect

Conversation

@JeroenSoeters

Copy link
Copy Markdown
Contributor

Summary

Three tools and a skill for connecting a cloud account to a hosted formae installation, plus the setup step that decides whether to offer it.

  • connect_cloud_account computes the CloudFormation console link that creates the role the installation assumes.
  • register_cloud_role records the role the applied stack produced.
  • list_cloud_connections reports which accounts are registered, and whether the listing was read in full.
  • /formae:connect drives the first two; /formae:setup gains a step that uses the third.

Neither of the first two mutates a cloud. The stack is applied by the user, in their own browser, under their own admin session, which is why computing the link carries no destructive hint. Reporting that an account was already connected with the same role is success: it is the idempotent case, and it is what makes re-running safe.

Draft, and why

list_cloud_connections drives formae connect list, which does not exist on any released formae. It is gated on a new FeatureCloudConnectionList floor pinned to a placeholder 0.90.0 that must be corrected against the real release tag once the CLI change ships. Merging before then would ship a tool that cannot work and a floor that is wrong.

The failure envelope was being discarded

runConnect called cmd.Output() and, on a non-zero exit, returned only formae connect failed (exit N) while dropping the captured stdout that carries the machine failure envelope. Every actionable failure reached the model as an exit code, and a model with nothing to act on improvises.

It now decodes the envelope and renders our own text from the declared code, modelled on the resolve oracle's decoder: the producer's own message is decoded so it is visibly accounted for and never read, because it is built from a plugin error string and a Pkl failure quotes profile source lines that can hold an inline password. One document only, proven by requiring EOF after it.

The setup step distinguishes three outcomes, not two

A reachable agent is not the finish line: a hosted installation with no registered cloud account cannot manage anything. But treating every refusal as "cannot determine" would dead-end the skill for two whole populations, since the tool refuses on a classic profile and on a formae below the floor.

So the step separates a check that does not apply (skip silently, close the journey normally) from a listing that ran and came back incomplete (the genuine cannot-determine case, where offering to provision on an unreadable answer is exactly the mistake to avoid) from a listing that succeeded.

Nothing here claims a registration is reachable. There is no verified state to read, and a revoked role leaves the row untouched, so every message says registered.

Two tools and a skill. connect_cloud_account computes the CloudFormation
console link that creates the role a hosted installation assumes, and
register_cloud_role records the role the applied stack produced. Neither
mutates a cloud: the stack is applied by the user, in their own browser, under
their own admin session, which is why computing the link carries no destructive
hint.

Reporting that the account was already connected with the same role is success.
It is the idempotent case, and it is what makes re-running the flow safe.

The tool descriptions and the skill both say to show the user the link and wait,
and never to run a formae command instead. A model handed a command in an error
or a hint will run it, in a terminal it cannot drive.
Step 6 of skills/setup/SKILL.md ran list_cloud_connections unconditionally
and treated any failure as the same cannot-determine case: stop and say so.
That collapsed three outcomes into one. Every user on a released formae hits
the 0.90.0 feature gate today, and every self-hosted user reaches step 6
through steps 2/4/5 without ever volunteering that they self-host, so
'formae connect list' refusing a classic profile was a permanent dead end.
Step 6 now tells apart a step that does not apply (classic profile, or
formae too old for the check) from a listing that genuinely could not
complete, skips silently in the former case, and keeps the closing line
reachable whenever nothing is actually wrong.

Also fixed three smaller findings from the same review pass:

TestListCloudConnections_SaysWhenItCannotTell asserted output that lacked
'no cloud account is registered' and contained 'could not' or 'cannot', but
never asserted the result was not an error. errUnreadableConnections also
reads 'could not read', so a future change that made decodeConnectionsDoc
reject complete:false documents outright would leave this test green while
reporting a decode failure instead of the incomplete-listing case. Verified
by making decodeConnectionsDoc reject complete:false, confirming the test
now fails, then reverting. The test now asserts isError is false and matches
the exact rendered sentence; the comment claiming the wording appears only
via the incomplete branch was also false and is corrected.

describeConnectFailure's default arm in connect.go was unreachable:
decodeConnectFailure already rejects any code outside the declared set
before calling it. Replaced the code-set map plus switch with a single
code-to-description map, so there is no second place to keep a closed
namespace in sync and no dead branch to carry.

Corrected a stale comment on TestListCloudConnections_RefusedBelowTheVersionFloor:
featuregate's detectFn is package-level state, and an earlier test's
withFakeVersion cleanup leaves it forced to return 0.0.0 rather than
restoring the real CLI detector, so in a full-package run loginStub's own
--version reply is never consulted. The assertion still holds either way;
only the comment was wrong.
…ships it

The floor was a placeholder of 0.90.0, which is a whole minor above the line
the feature actually lands in: version.semver reads 0.88.0 and the current dev
tags are 0.89.0-dev.N, so the next tag after the CLI change merges is
0.89.0-dev.9. A 0.90.0 floor would have refused the exact build carrying the
command, and because the setup skill now treats a version refusal as "this
check does not apply", it would have skipped silently rather than failing
loudly. Nobody would have noticed until someone asked why the step never ran.

The test now uses a real dev tag rather than a round number, which also pins
the behaviour the floor depends on: parseParts splits on "." and discards what
it cannot read, so a prerelease suffix is dropped instead of sorting below the
release the way semver would.
Adds the faster, local-credentials path for connecting an AWS account
alongside the existing console-link flow (connect_cloud_account /
register_cloud_role):

- list_aws_profiles drives 'formae connect aws profiles' and shows
  each local AWS profile with the account it resolves to, or the
  reason it could not resolve. Neither case is an error, and an empty
  list is a normal 'quick-create is the only path' result.
- provision_cloud_role drives 'formae connect aws --account ... --profile-aws
  ...', which creates the role with the chosen profile's credentials and
  registers it in one invocation. Unlike connect_cloud_account this
  mutates immediately with no console step, so it carries
  DestructiveHint and its description says so plainly.

Both are gated on featuregate.FeatureCloudConnectionList (0.89.0),
reusing the constant list_cloud_connections already gates on rather
than adding a third. provision_cloud_role reuses the existing
registered-document decode and render (factored out of
handleRegisterCloudRole as decodeRegisteredDoc) instead of a second
decoder for the same shape.
`formae-connect` previously documented only the console-link flow and
stated AWS as the sole option in a way that dead-ended the question.
It now opens by asking which cloud (AWS, Azure, GCP — offered together
so the shape of the product is visible, even though only AWS works and
the other two get no promised date), then for AWS calls
`list_aws_profiles` first: each resolved profile is shown with the
account it reaches, each unavailable one with its reason (e.g. an
expired SSO session), and a "none of these" option sits alongside them.
Picking a profile provisions and registers the role directly via
`provision_cloud_role`, using the account already shown beside the
profile rather than asking the user to retype it. The skill states
before that call, not after, that it mutates immediately: a real IAM
role, and possibly an account-global OIDC provider, with no console
step in between. "None of these," or no profiles at all, falls back to
the existing console-link flow unchanged. Both paths report the result
as registered, never verified or working.

`setup`'s step 6 empty-connections branch now hands off into that
richer flow instead of assuming the console link is the only route;
the other three branches and their gating are untouched, and the step
numbering across the file still holds.

CHANGELOG's Unreleased entry for `list_aws_profiles` /
`provision_cloud_role` and for `/formae:connect` now describes the
user-visible flow (profile list with account shown, immediate
provisioning, fallback to the console link) rather than just naming
the tools.
…nded

The account parameter's description told the model to ask the user for it,
copied from the console-link tool where that is right. On this path it is
wrong twice over: the account came from list_aws_profiles, shown beside the
profile the user picked, and seeing it there is the whole reason the pick was
informed. Asking again undoes that, and invites a typo the credentials would
then reject as a mismatch.

A harness that loads the tools without the skill reads only this string, so
the contradiction had to be fixed here rather than in the skill alone.
…emitted

Every other test in this file feeds the decoder a document this repository
wrote, which proves the decoder agrees with itself. This one was captured from
a live hosted installation with dev.10 and redacted, so it proves the decoder
agrees with the producer instead.

It caught nothing, which is the useful outcome: the contract matches. Two
details were confirmed rather than assumed, and both would have been easy to
get wrong in the other direction. The warnings key is absent entirely when
there are none rather than present as an empty array, and roleArn rides on an
AWS row while being omitted where a cloud has no role.
The decoder demanded schemaVersion 2 on the failure envelope because the
success documents carry 2. They version independently: printer declares its own
failureSchemaVersion, and it is 1. So every real failure was rejected as
unreadable and reported as an exit status, which is exactly the behaviour
decoding the envelope was built to replace. Every test missed it because every
test fed the decoder an envelope this repository invented.

The code map was also nine of the nineteen printer declares. It now covers the
set, but the more useful change is that a code missing from it is no longer
discarded: the namespace is closed but it grows, this map will always lag it,
and a code is our own vocabulary so it is safe to name. An unmapped failure now
says which code it was instead of which exit status a process returned. Only
the producer's message is still withheld.

Both fixtures are captured from live runs rather than written here.
Registering a cloud account grants trust but leaves the agent with
nothing to discover or manage until a target exists. Adds step 6,
which offers to create one after either registration path converges:
asks for region and label (AWS only; Azure/GCP say not built yet),
asks for a project directory (git-init style, refuses to merge into
an existing PklProject), writes PklProject/vars.pkl/targets.pkl, and
applies targets.pkl. Calls out the two silent failure modes: the
discoverable flag defaulting to false, and the role ARN coming from
the registration rather than the user.

The former closing step is renumbered to step 7 and, when a target
was created, now tells the user what to try next with one example
prompt for discovery and one for creating infrastructure.
The step described what to do without saying why, which left it reading like a
workaround for a missing API. It is the opposite: in formae everything is
created by declaring it and applying, targets no differently from buckets, and
the absence of a create-target verb is the model rather than a gap in it.

Worth a sentence because this is often a user's first forma, and because the
apologetic reading invites someone to close the supposed gap later. A second
way to bring a target into existence is exactly what the model prevents.
Step 6 called apply_forma directly while /formae:apply requires simulating
first. The inconsistency mattered here more than anywhere: this is the first
apply a new user ever runs, and it is where they learn what an apply looks
like.

Reconcile destroys deployed resources the file does not declare, so a forma
holding only a target is a claim about everything in scope rather than about
the target alone. On a fresh installation there is nothing to destroy and the
simulation says so in a line, which is the point: the shape is learned while
the stakes are zero.
The setup skill fell back to "run `formae login --hosted` and share the
output" whenever a step could not explain itself, in three places. That
makes the user the transport between two components that are already
connected, and it dead-ends the journey on the one path where the user
has least context to act.

It also fails in a way that reads as a broken install: the MCP resolves
formae for its own process, so a shell that never had that directory on
PATH answers `command not found`, and the model reports the plugin as
missing a binary that is sitting right there.

The connect skill mentions `formae connect aws` as something the user
may do themselves. That stays, but it now says plainly it is not for the
assistant to run: it is interactive and provisions real cloud trust, and
a model that runs it from a harness burns state and leaves a
half-finished flow behind.
The self-hosted close still sent the user to `formae profile edit`. It is
the same delegation the rest of this skill just stopped doing, and it has
the same two problems: the user becomes the transport for something the
MCP can already do, and on a machine where formae is not on their PATH
the command they were handed does not run at all.

write_profile is the tool for this, and offering to do it is strictly
more useful than naming a command and stopping.
…pt it

Setup's cloud-account step described the connect flow it hands off to,
including which clouds are implemented. A model that has just read that
only one of them works does not then ask the question - it offers that
cloud directly, and the choice the connect skill exists to present is
never made.

Observed: after a clean setup the assistant closed with "would you like
me to connect an AWS account now?" rather than asking which cloud.

The step now offers a cloud account without naming one, and says why,
so the parenthetical does not get helpfully restored later.
@JeroenSoeters
JeroenSoeters marked this pull request as ready for review August 28, 2026 22:15
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.

1 participant