Skip to content

Make deploy enforce approval: manual (S-008-1, 3, 5..10, 15, 16) - #178

Merged
shipmill-romamo[bot] merged 1 commit into
mainfrom
feat/163-enforce-manual-approval
Oct 31, 2026
Merged

shipmill-romamo[bot] merged 1 commit into
mainfrom
feat/163-enforce-manual-approval

Conversation

@shipmill-romamo

Copy link
Copy Markdown
Contributor

Plan

  • Serves: G-2
  • Files: sdk/src/cloudfall/release_approval.py, sdk/src/cloudfall/lifecycle.py, sdk/src/cloudfall/app.py, sdk/src/cloudfall/commands.py, sdk/src/cloudfall/agent_server.py, sdk/tests/test_deploy_approval.py, sdk/tests/test_agent_server.py, docs/ci-deploy-guide.md, docs/operator-guide.md, changelog.d/106-enforce-manual-approval.md, changelog.d/160-release-approve.md
  • Order: read and classify approval records in release_approval.py (counting rules, reasons, RECORD_INVALID on a broken file); deploy checks them for a manual application after verifying the artifact and before any engine step, pinning the approved digest; the plan and result carry approval; MCP binds approvals and says a person approves; then the changelog fragment and the CI deploy guide
  • Proof: S-008-1, S-008-3, S-008-5, S-008-6, S-008-7, S-008-8, S-008-9, S-008-10, S-008-15, S-008-16, each in a test naming its id

Summary

cloudfall deploy now enforces an application's approval: manual with the record cloudfall release approve writes (#172), as spec S-008 says:

  • Which record counts. release_approval.check_approval reads every *.json at the top of --approvals (default decisions/releases). A record about the same application, component and release counts when it names the SHA-256 verify_artifact computes now, exactly the component's declared servers (compared as a set), and an expiresAt later than now; the latest approvedAt wins. Every other record for that release is listed with digest_mismatch, servers_changed or expired. A file that isn't JSON, isn't an object, or doesn't match release-approval.schema.json stops a manual deploy (plan or --yes) with RECORD_INVALID naming it. An automatic application never reads the folder
  • deploy --yes without one exits 4 PRECONDITION, context.code: deployment_approval_missing, with application, component, release, command (cloudfall release approve <component> --release <R>) and notCounting. It refuses after the artifact check and before the inventory render, so no playbook runs and no receipt is written
  • With one, it deploys and the result carries approval (policy, path, approver, approved_at). The approved digest is passed to lifecycle.deploy (DeployOptions.approved_sha256), which refuses an archive swapped between the check and the deploy. A failed health gate leaves the record alone; a retry uses it again
  • The plan gains approval: {"policy": "automatic"}, or for manual the status (approved, missing), the counting record's path, approver and expires_at, not_counting, and command when none counts
  • MCP. cloudfall mcp serve gains --approvals and binds it, so a deploy call can't name its own folder (ARG_ERROR); release approve stays off the tool list, and the project instructions say a person runs it
  • Docs. docs/ci-deploy-guide.md "Approval: who presses the button" now says deploy enforces manual, and shows the build job, the person's release approve, and the deploy job that downloads the same artifact; docs/operator-guide.md lists release approve with the commands no tool stands in for
  • Changelog. changelog.d/106-enforce-manual-approval.md has the ### Breaking entry with both ways out; the Add cloudfall release approve and its approval record (S-008-2, S-008-4, S-008-11, S-008-12) #160 fragment no longer says deploy doesn't read the record

Out of scope here, in #165: migrate's deploy: step (it calls lifecycle.deploy directly and is not gated yet) and rollback's plan.

Test notes

  • New sdk/tests/test_deploy_approval.py (21 tests) runs the real CLI against a stand-in engine whose deploy playbook only leaves a marker and a receipt on the controller: S-008-1, 3, 6 (digest, servers, expiry, servers as a set), 7, 8 (before/after, latest of several), 9 (plan and --yes, with a junk file), 10 (not JSON, not the schema, not an object; plan and --yes), 15 (a failing playbook then a retry), 16 (fragment and guide), the swapped-archive guard, and the output schema
  • sdk/tests/test_agent_server.py: S-008-5 over the real stdio transport (plan shows missing, a call naming approvals is ARG_ERROR, yes: true is PRECONDITION / deployment_approval_missing, nothing rendered), and the project instructions
  • Ran: uv run ruff check ., uv run mypy, the touched and neighbouring SDK test files (364 passed), and uv run pytest engine/tests (232 passed, 1 skipped) since commands.py is imported by the engine. The full suite is left to CI

Fixes #163

🤖 Generated with Claude Code

A manual application's deploy --yes now deploys a release only when a release approve record counts for it: the same archive SHA-256, exactly the component's declared servers, and not expired. Without one it exits PRECONDITION deployment_approval_missing before any playbook runs, naming the command a person runs and each record that doesn't count with its reason. The plan and the result carry approval, a broken approvals file stops a manual deploy with RECORD_INVALID, an automatic application never reads the folder, and mcp serve binds the approvals folder so a deploy call can't name its own (spec S-008).

Fixes #163

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@shipmill-romamo

Copy link
Copy Markdown
Contributor Author

@romamo Decided without waiting: deploy's output gains two fields the spec doesn't name: an automatic application's --yes result carries approval: {"policy": "automatic"} too, and the records that didn't count are listed with their reason (notCounting in the error context, not_counting in the plan).

Why: both are additions every existing reader keeps working with; S-008-9 already puts approval in the plan, and S-008-6 asks the refusal to list each record with digest_mismatch, servers_changed or expired.
Not taken: leaving the result without approval for automatic applications, which the output adapter would have written as {}.

Reply with a comment starting with veto before 2026-10-31 15:55 UTC to reverse it, adding what you want instead if you like. A later veto still counts and gets a follow-up.

@shipmill-romamo shipmill-romamo Bot added the decided An agent decided at notify; the maintainer may veto label Oct 30, 2026
@shipmill-romamo

Copy link
Copy Markdown
Contributor Author

@romamo Decision needed: may this PR (and #182 stacked on it) land with its ### Breaking changelog entry, which makes the next release a major one?

Spec S-008 (S-008-16) asks for this entry: deploy --yes and migrate --yes on an approval: manual application now refuse until a person runs cloudfall release approve, which hits every project that declares manual today (the example crm.yaml and every Render import). .github/shipmill.toml maps ### Breaking to a major bump, and the changelog has never had one, so the release lane would cut the next version as a major release rather than the next 0.8.0 candidate (v0.8.0rc11 is the newest tag).

  1. Land it with ### Breaking as the spec says (recommended): users upgrading see the break plainly, and the version says so; Make deploy enforce approval: manual (S-008-1, 3, 5..10, 15, 16) #178 and Pause migrate for a release approval and let rollback run without one (S-008-13, S-008-14) #182 merge back to back so migrate --yes is gated in the same release
  2. Land it, but file the entry under ### Changed: the 0.8.0 line continues, but the changelog no longer flags the break, which departs from S-008-16
  3. Hold both until you choose when the major release goes out

Review: no way around the approval was found; CI is green on both. Reply here with a number or your own answer; shipmill takes this up on the tick after your reply. Nothing on #178 or #182 merges until then.

@shipmill-romamo shipmill-romamo Bot added the needs-decision A shipmill session asked a question here; waits for a reply label Oct 30, 2026
@shipmill-romamo

Copy link
Copy Markdown
Contributor Author

Decision by @romamo, relayed by Claude Code

  1. Land it with ### Breaking as the spec says.

@shipmill-romamo shipmill-romamo Bot removed the needs-decision A shipmill session asked a question here; waits for a reply label Oct 31, 2026
@shipmill-romamo
shipmill-romamo Bot merged commit 653d731 into main Oct 31, 2026
1 check passed
@shipmill-romamo
shipmill-romamo Bot deleted the feat/163-enforce-manual-approval branch October 31, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

decided An agent decided at notify; the maintainer may veto

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make deploy enforce approval: manual (S-008-1, 3, 5..10, 15, 16)

0 participants